ndp: Simplify and breakdown nd6_ra_input()

[pouria]

Summary

I want to fix PR263982 by implementing RFC4191.
I faced two issues:

1. struct nd_opts is not scalable for including newer options.
2. nd6_ra_input() is too complex and would become more so

if we wanted to add more options to it.

For this patch, I simplied nd6_ra_input() to make it easier
to add additional options.
Also, since we don't expect to receive many IPv6 RAs,
I don't expect any performance issues with this change.

Test Plan

apply this patch and if DAD (nonce), SLAAC (defrouter),
and PI (prefix_info) works, it should be fine.

[bms]

In my original code drop for ILNP -- which I am currently retrieving from St Andrews remotely due to catastrophic loss of my original copies of my own artefacts -- the ILCC at a site border router (SBR) learns of available network locators by listening to its own IPv6 RAs.

I do not know if Gregor Haywood's most recent code drop for ILNP, which is not based on my work, does this for the host-side implementation, for which I originally added an EVENTHANDLER. So I would be wary of assumptions made that other kernel entities will not consume the prefix list here.

[bms]

I should also point out that in my final experiment chapter of my PhD thesis, I outlined a scheme for distributed site border router (dSBR) operation in FreeBSD, which implements a hot-standby scheme for ILNP on-path SBRs, using a dedicated ilnpsync interface driver very loosely modeled on pfsync.

However, this standby scheme cannot deal with the case where the first router to acquire an ILNP flow (and responsibility for forwarding it) fails. This requires extensions to ND along the lines of RFC 7048, where NUD is extended to include an UNREACHABLE state in the ND state machine, as it applies to candidate default routers.

I would plan to prototype changes following this, unless someone is reading here and beats me to it; it's not like I don't have plenty to do already.

[pouria]

In D55267#1263957, @bms wrote:

I would plan to prototype changes following this

I'm working on refactoring union nd_opts.
The current data structure doesn't scale well with new options.
Considering the type number of ILNPv6 Locator Update Message, we would end up with a BIG data structure. For example:

union nd_opts {
    struct nd_opt_hdr *nd_opt_array[156];    /* max = ILNPv6 */
    struct {
        struct nd_opt_hdr *zero;
        struct nd_opt_hdr *src_lladdr;
        struct nd_opt_hdr *tgt_lladdr;
        struct nd_opt_prefix_info *pi_beg; /* multiple opts, start */
        struct nd_opt_rd_hdr *rh;
        struct nd_opt_mtu *mtu;
        struct nd_opt_hdr *__res6;
        struct nd_opt_hdr *__res7;
        struct nd_opt_hdr *__res8;
        struct nd_opt_hdr *__res9;
        struct nd_opt_hdr *__res10;
        struct nd_opt_hdr *__res11;
        struct nd_opt_hdr *__res12;
        struct nd_opt_hdr *__res13;
        struct nd_opt_nonce *nonce;
        struct nd_opt_hdr *__res15;
        struct nd_opt_hdr *__res16;
        struct nd_opt_hdr *__res17;
        struct nd_opt_hdr *__res18;
        struct nd_opt_hdr *__res19;
        struct nd_opt_hdr *__res20;
        struct nd_opt_hdr *__res21;
        struct nd_opt_hdr *__res22;
        struct nd_opt_hdr *__res23;
        .
        .
        .
        struct nd_opt_hdr *__res155;
        struct nd_opt_hdr *ILNPv6;
        struct nd_opt_hdr *search;      /* multiple opts */
        struct nd_opt_hdr *last;        /* multiple opts */
        int done;
        struct nd_opt_prefix_info *pi_end;/* multiple opts, end */
    } nd_opt_each;
};

@bms, I would appreciate any input/review you have on these changes.

[bms]

In D55267#1264064, @pouria wrote:

I'm working on refactoring union nd_opts.
The current data structure doesn't scale well with new options.
Considering the type number of ILNPv6 Locator Update Message, we would end up with a BIG data structure. For example:

...

@bms, I would appreciate any input/review you have on these changes.

I don't actually see a problem here. ILNPv6 Locator Updates use their own ICMPv6 type which is distinct from ND messages, and were not processed by ND in the historic code drops.

ILNPv6 site border routers were implemented to listen to their own Router Advertisements (RA) and cache prefixes learned this way in the ILCC (effectively ILNP's "routing table") which again is also handled separately. So I'm a little confused why you believe the members should be embedded in nd_opt_hdr{} at all.

[pouria]

In D55267#1264066, @bms wrote:

I don't actually see a problem here. ILNPv6 Locator Updates use their own ICMPv6 type which is distinct from ND messages, and were not processed by ND in the historic code drops.

ILNPv6 site border routers were implemented to listen to their own Router Advertisements (RA) and cache prefixes learned this way in the ILCC (effectively ILNP's "routing table") which again is also handled separately. So I'm a little confused why you believe the members should be embedded in nd_opt_hdr{} at all.

You're right! ILNPv6 has its own ICMPv6 type (156), I mistakenly thought its an ND extension.
I'm not familiar with ILNP. I remember Gregor Haywood’s EuroBSDcon 2024 talk on ILNP.
I'd be glad to see it in the real world.

However, the problem with union nd_opts is still valid, but probably not related to ILNP.

[pouria]

Add more specific comments referenced from RFC 4861
Remove old unclear comments from RFC 2461

[glebius]

I'd suggest to commit all comments and whitespace changes that can be committed without breaking down nd6_ra_input() separately. That will make meaningful diff smaller. btw, it is great putting references to RFC before important actions and checks!

sys/netinet6/nd6_rtr.c

396–409

Later, with a separate change, you may statically initialize pr here, so no call to bzero(9) is needed. Not insisting, just suggesting.

[pouria]

sys/netinet6/nd6_rtr.c

396–409

Sure, I'll do it in a separate review.

[zarychtam_plan-b.pwste.edu.pl]

I have been testing this patch on a couple of computers for over a week now, and everything seems to work fine (at least I see no regression).