Page MenuHomeFreeBSD

if_stf: add 6rd support
ClosedPublic

Authored by kp on Nov 17 2021, 5:03 PM.
Tags
None
Referenced Files
F81572141: D33037.diff
Thu, Apr 18, 7:15 AM
Unknown Object (File)
Sat, Apr 13, 9:04 PM
Unknown Object (File)
Tue, Apr 9, 6:49 AM
Unknown Object (File)
Jan 21 2024, 8:07 PM
Unknown Object (File)
Jan 13 2024, 12:33 AM
Unknown Object (File)
Jan 11 2024, 12:36 PM
Unknown Object (File)
Jan 11 2024, 12:36 PM
Unknown Object (File)
Dec 24 2023, 3:21 AM

Details

Summary

Implement IPv6 Rapid Deployment (RFC5969) on top of the existing 6to4
(RFC3056) if_stf code.

PR: 253328
Obtained from: pfSense
Sponsored by: Rubicon Communications, LLC ("Netgate")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kp requested review of this revision.Nov 17 2021, 5:03 PM
  • Please update stf(4) manual page, too.
  • I would suggest disabling 6to4 functionality when a braddr is configured. A mixed configuration of 6to4 and 6rd is quite confusing when seeing a result of ifconfig(8) command-line output because the braddr is a parameter for the interface, not a single 6rd domain configuration. I believe no one (except for me) uses the both at the same time.
sbin/ifconfig/Makefile
39

This should be under .if ${MK_INET6_SUPPORT} != "no".

sbin/ifconfig/ifstf.c
94

ifconfig(8) generally resolves a host name wherever an address can be specified. I am not sure if it must be supported here because a 6rd configuration is address-family specific, but this should be noted in the manual page at least.

120

This can be removed safely because strtonum() should also check whether it is a number or not.

123

I think either 0 or 32 must be explicity rejected because 0 is silently converted to 32 in the kernel.

126

"prefix length" is better than "width", I think.

132

memcpy()?

sys/net/if_stf.c
433–442

I think there is no need to define ia6 after fixing the following two lines.

434
  • LHS should be simply addr6.
  • RHS can be simplified by using IFA_IN6(is).
  • memcpy() or bcopy() might be preferred. The line 438 uses it even for a 32-bit long address, for example. And by the way, L.392 rewrote bcopy() with memcpy(). Consistency in this file might be good.
435
  • IA6_MASKIN6(ia)
516

in_nullhost()?

820

This should be inside the next if-conditional.

823

These two lines are confusing. How about v4prefix = ntohl(sc->srcv4_addr & (0xffffffffU << v4suffixlen))?

824

(sc->v4prefixlen > 32) must be rejected and return with NULL. It never happens if SIOCSDRVSPEC guarantees it is between 1 to 32, but having a sanity check is useful.

833

(plen > 64) must be rejected and return with NULL before this. It is still possible to happen.

876

sizeof(args)

881

bcopy or memcpy.

882

The value must be checked to guarantee that it is within 1 to 32.

904

Consistency. SIOCSDRVSPEC checks ifd_cmd first, and this checks ifd_len first.

kp marked 17 inline comments as done.Nov 19 2021, 6:19 PM
In D33037#746495, @hrs wrote:
  • Please update stf(4) manual page, too.

There's D33042 for that.

  • I would suggest disabling 6to4 functionality when a braddr is configured. A mixed configuration of 6to4 and 6rd is quite confusing when seeing a result of ifconfig(8) command-line output because the braddr is a parameter for the interface, not a single 6rd domain configuration. I believe no one (except for me) uses the both at the same time.

Thank you for the extensive review!

sbin/ifconfig/ifstf.c
94

All examples of 6rd configuration I can find uses the IP address for the BR. The pfSense implementation also only supports using an IP address.

All together I'd be inclined to keep this the way it is. It'll be easy to extend later if we do encounter users who want to use a DNS name.

The if_stf.4 changes for 6rd describe the configuration command as "The border router IPv4 address is configured with the
.Xr ifconfig 8
.Cm stfv4br
command."

sys/net/if_stf.c
516

That expects a struct in_addr, and we use in_addr_t here.

kp marked an inline comment as done.

Review remarks

Looks good to me. Thanks for your hard work!

This revision is now accepted and ready to land.Nov 19 2021, 8:15 PM
This revision was automatically updated to reflect the committed changes.