Page MenuHomeFreeBSD

netinet6: Fix ND link-layer address option layout for IPoIB (stable/15)
Needs ReviewPublic

Authored by vinicius_ferrao.net.br on Thu, Jul 9, 11:14 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jul 17, 8:33 PM
Unknown Object (File)
Fri, Jul 17, 1:15 PM
Unknown Object (File)
Fri, Jul 17, 8:55 AM
Unknown Object (File)
Fri, Jul 17, 8:14 AM
Unknown Object (File)
Fri, Jul 17, 8:09 AM
Unknown Object (File)
Fri, Jul 17, 1:39 AM
Unknown Object (File)
Thu, Jul 16, 6:33 PM
Unknown Object (File)
Thu, Jul 16, 6:24 PM

Details

Reviewers
pouria
adrian
cperciva
Group Reviewers
network
Summary

RFC 4391 (IP over InfiniBand), section 9.3, lays the ND source/target
link-layer address option out as type, length (3), two reserved zero octets,
then the 20-octet IPoIB link-layer address. The ND code assumed the Ethernet
layout (RFC 4861, section 4.6.1) everywhere and read/wrote the address directly
after the option header, i.e. two octets early. The option-length sanity check
computes 24 for both layouts for a 20-octet address, so the mismatch was silent.

Against a conformant peer (e.g. Linux) this broke IPv6 over IPoIB in both
directions: the address parsed from a received NS/NA option was shifted by two
bytes, so the cached link-layer address embedded a GID that does not exist on
the fabric and replies were sent nowhere, and the options we built carried our
address at the wrong offset for the peer. IPv4/ARP was unaffected. This blocked
everything IPv6 over IPoIB against Linux, including rdma_cm's IP-to-GID
resolution and therefore NFS-over-RDMA-over-InfiniBand with IPv6.

Add nd6_lladdr_opt_pad(), the analog of Linux's ndisc_addr_option_pad(): the
number of reserved octets between the option header and the address (2 for
IFT_INFINIBAND and IFT_INFINIBANDLAG, 0 otherwise). Apply it at the option parse
sites (nd6_ns_input(), nd6_na_input(), nd6_rs_input(), nd6_ra_input(),
icmp6_redirect_input()) and the option build sites (nd6_ns_output_fib(),
nd6_na_output_fib(), icmp6_redirect_output()). Zero the redirect target option
before filling it; previously its trailing pad bytes were left uninitialized on
links with 20-octet addresses. No functional change for link types without
reserved octets.

This is a direct commit to stable/15, equivalent to main commit
bac4760524a2a15ce75e35251f0c9cdf31732f3f (D58096). It could not be cherry-picked
cleanly: the neighbor-discovery code diverged on main (the router advertisement
parse was refactored into helpers), so the same fix is reimplemented against the
stable/15 sources.

PR: 296585
Sponsored by: VersatusHPC

Test Plan

Built GENERIC on stable/15 and run the same validation test
defined on the CURRENT fix.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

pouria added inline comments.
sys/netinet6/icmp6.c
2514

Please use memset here

This revision is now accepted and ready to land.Fri, Jul 10, 9:59 AM

Done @pouria

There's pre-existing bzero in other parts, but I could update them for CURRENT, since you're pushing this quality change.

This revision now requires review to proceed.Fri, Jul 10, 2:51 PM

@adrian Hi, could you please MFC this one into stable/15?
I'm not sure how to apply this patch from phabricator while cherry-picking it from CURRENT..

@adrian Hi, could you please MFC this one into stable/15?
I'm not sure how to apply this patch from phabricator while cherry-picking it from CURRENT..

The changes is almost the identical with D58096, so MFC from main should be OK ( despite some conflicts ). See my comments from D58096 .

@adrian Hi, could you please MFC this one into stable/15?
I'm not sure how to apply this patch from phabricator while cherry-picking it from CURRENT..

The changes is almost the identical with D58096, so MFC from main should be OK ( despite some conflicts ). See my comments from D58096 .

Should we close this one? Your call @zlei