Page MenuHomeFreeBSD

nd6: Add support for route information (RFC 4191)
Needs ReviewPublic

Authored by pouria on Feb 22 2026, 9:47 PM.
Tags
None
Referenced Files
F151027587: D55449.id.diff
Sun, Apr 5, 1:10 PM
Unknown Object (File)
Sun, Mar 29, 12:52 PM
Unknown Object (File)
Fri, Mar 27, 7:23 AM
Unknown Object (File)
Thu, Mar 19, 12:52 AM
Unknown Object (File)
Thu, Mar 19, 12:52 AM
Unknown Object (File)
Tue, Mar 17, 12:55 PM
Unknown Object (File)
Tue, Mar 17, 12:50 PM
Unknown Object (File)
Tue, Mar 17, 7:10 AM

Details

Reviewers
glebius
zlei
markj
madpilot
melifaro
bz
Group Reviewers
network
Summary

This revision implements RFC 4191,
it also resolves PR263982.
notes for reviewers:

  1. I've avoided using separate callout and, as this aligns with what

RFC 4191 intends by directly using the routing table instead of separate data
structure. (like defrouter)

  1. I tried to avoid locking the RIB inside the nd6_rtr.c. However, I couldn't

obtain an exact match for my prefix from other public functions without
locking the RIB.

  1. I could reuse rib_add_redirect(), but it only works for host routes (plen 128).
  2. I will create rib_add_temproute(), similar to rib_add_redirect()

but tailored for the route info option in another review if necessary.
Despite nhop_create_from_info() being a public function, I prefer not to use it
inside nd6_rtr.c, consider defrouter_addreq already utilizes nhop
facilities, I've used it for now anyway.

  1. For updating nhop lifetime, I can't use nhop_set_expire() directly, because it linked.

I solved this problem by calling rib_change_route() and making
nhop_create_from_nhop behave like nhop_create_from_info (as intended).

This review depends on D55267.

Test Plan

See my other revisions on route expire arguments
of route(8) and netstat(1).

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 71906
Build 68789: arc lint + arc unit

Event Timeline

Some other notes for reviewers: (see description above too please)

sys/netinet6/nd6_rtr.c
81

This can be in nd6.h, but there is no need for it to being public.
So I made it private.

497

XXX: our dr lifetime is 16 bit. the rti_lifetime is 32 bit. it will overflow if I override it.
So I commented it for now. Any idea?
This should not block this revision however, I can style(9) it by using #ifdef notyet like others.

2465

We also need to prioritize routes from RAs with higher preference values.
That's why I saved the ndrt_flags.
I will do it in another revision/review after this one gets committed.

style commented to use #ifdef without functional change.

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).

sys/netinet6/nd6_rtr.c
479

Please use (char *) in the new code where you want 1 byte pointer arithmetic. Use (void *) where you need a pointer to something (a core address :) ).

See https://grok.com/share/c2hhcmQtMg_de591786-ba8b-4215-a4a3-d665de24d727

Convert caddr_t to char * for pointer arithmetic. (@glebius)

sys/netinet6/nd6_rtr.c
479

Please don't use char * in networking code, please use uint8_t *.

5 people just went and got popcorn

sys/netinet6/nd6_rtr.c
479

I'm available to gladly address all the comments here immediately.
But at this point. I'm not sure what to do :)

sys/netinet6/nd6_rtr.c
479

If Bjoern insists, use uint8_t. Just please don't create new code with caddr_t.

I prefer char * for the following reasons:

  1. It is a native C type and doesn't require the header to be included.
  2. Our goal is not increase by certain number of bits, but by certain number of bytes. A type that represent a byte is more natural than a type that represents a 8-bit unsigned integer.
  3. It is shorter.

I want to remove V_nd6_defrouter and handover selecting default router responsibility to the routing stack.
I need this patch for that.
I'd appreciate if people take look at this.

+1 for doing this, but I have not reviewed code (yet).

kind ping: @bz @glebius @markj
I'm not in rush, however, I just can't continue my work on ndp without this anymore due to number of conflicts.

Do you have any tests for this feature?

Overall it looks sane to me. Some more explanation in the commit log message would be nice, i.e., some direct description of the functionality which is introduced.

sys/netinet6/nd6.h
295

This comment is wrong now.

sys/netinet6/nd6_rtr.c
483

This fits on one line.

501

Don't you need some lock to update these fields?

741

Extra newline.

2507

Just return (rib_change_route(...))?

2531

Should we collect the return value from rt_routemsg()?

pouria marked 7 inline comments as done.

Address @markj comments.
Thank you so much for your review.

Do you have any tests for this feature?

I've one but didn't create a revision for it since it was too customized.
I'll rebase my test on top of the D56128 and will let you know.

sys/netinet6/nd6.h
295

Interesting, it was update in my local branch!

sys/netinet6/nd6_rtr.c
501

I probably should, But, since dr itself is depends on RTI value.
I simplify it and move the initial defrtrlist_update to after RTI processing.

2531

Due to the nature of rt_routemsg, we don't care about its output here.
But to answer your question, I did a bit research on it, seems like nobody cares about rt_routemsg output.

fgrep -r rt_routemsg sys/

Enable not yet part again.

ignore rti default route with lifetimer bigger than UINT16_MAX until I update its structure in another review.

pouria retitled this revision from ndp: Add support for route information (RFC 4191) to nd6: Add support for route information (RFC 4191).Wed, Apr 1, 4:49 PM