Page MenuHomeFreeBSD
Differential D54636

rtadvd: add multi pref64 support
Needs ReviewPublic
Actions

Authored by pouria on Sat, Jan 10, 5:09 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jan 10, 11:48 PM
Unknown Object (File)
Sat, Jan 10, 10:56 PM
Unknown Object (File)
Sat, Jan 10, 8:01 PM
Unknown Object (File)
Sat, Jan 10, 6:31 PM
Subscribers
bz
imp
ziaee

Details

Reviewers
glebius
zlei
markj
ivy
hrs
bz
Group Reviewers
network
Summary

Add support for multi pref64 in rtadvd and rtadvctl

Test Plan

To test, you need to configure
your rtadvd.conf with a pref64 prefix:

bridge0:\
	:raflags="oal":rltime#600:\
	:addr="2a01:e140:cafe::":prefixlen#64:\
        :pref640="2a01:e140:dead:ff::":\
        :pref641="3fff::":pref64len1#64:

then simply check the rtadvctl show output.
Also verified with tcpdump.

Diff Detail

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

Event Timeline

pouria created this revision.Sat, Jan 10, 5:09 PM
Herald added a subscriber: imp. · View Herald TranscriptSat, Jan 10, 5:09 PM
pouria requested review of this revision.Sat, Jan 10, 5:09 PM
Harbormaster completed remote builds in B69784: Diff 169451.Sat, Jan 10, 5:09 PM
pouria added a reviewer: markj.Sun, Jan 11, 2:55 PM
zlei added reviewers: ivy, hrs.EditedMon, Jan 12, 7:58 AM

Added reviewers @ivy who introduced PREF64 (RFC 8781) support for rtadvd(8), and @hrs who wrote rtadvctl(8).

zlei added inline comments.Mon, Jan 12, 8:08 AM
usr.sbin/rtadvctl/rtadvctl.c
914

I'd prefer to convert this into an assertion, given this static function is invoked only by function action_show() and follows the condition rai->rai_pref64.p64_enabled.

923

Then no need to return a zero.

pouria updated this revision to Diff 169519.Mon, Jan 12, 2:42 PM

Convert action_show_pref64 from int to void and use assertion to address @zlei comment.

Harbormaster completed remote builds in B69823: Diff 169519.Mon, Jan 12, 2:42 PM
zlei accepted this revision as: zlei.Tue, Jan 13, 9:43 AM

Looks good to me.

This revision is now accepted and ready to land.Tue, Jan 13, 9:43 AM
bz requested changes to this revision.Tue, Jan 13, 5:39 PM
bz added a subscriber: bz.
bz added inline comments.
usr.sbin/rtadvctl/rtadvctl.c
630

RFC 8781 states in section 5: "This option may appear more than once in an RA"

Looking at rtadvd.h it is a list but as I much as I understand here, we would always only print a single prefix?

631

What's the point of a separate printf here compared to doing that in action_show_pref64()? File style?

This revision now requires changes to proceed.Tue, Jan 13, 5:39 PM
glebius added inline comments.Tue, Jan 13, 5:54 PM
usr.sbin/rtadvctl/rtadvctl.c
631

I agree moving the printf inside action_show_pref64() is better.

916

Can you please write down a comment that would explain the arbitrary numbers.

pouria added inline comments.Wed, Jan 14, 11:14 AM
usr.sbin/rtadvctl/rtadvctl.c
630

Unfortunately, the current implementation does not work that way.
We did not use TAILQ_HEAD inside the struct rainfo the way we have done for other options, such as prefix, rdnss, or dnssl.
Given that our current implementation doesn't provide the ability to use multiple pref64 options at the same time, I will probably have to abandon this revision and start implementing it the standard way, as stated in RFC 8781 section 5, in rtadvd.
I'll try to implement as you said.

916

The thing that comes to my mind is:

/* RFC 8781 Section 4: Map PLC values to prefix lengths */

The RFC 6052 uses a table to describe the values, as there isn't a straightforward algorithm to implement the mapping.
I have mapped the PLC 0 to 96, and since the other values have a fixed spacing (8-bit), I was able to calculate the rest.

glebius added inline comments.Wed, Jan 14, 5:07 PM
usr.sbin/rtadvctl/rtadvctl.c
916

Yes, referencing the RFC that would explain where the values come from should be good enough.

pouria updated this revision to Diff 169747.Wed, Jan 14, 9:38 PM

Change revision to add support for multiple PREF64 options.

Herald added a subscriber: ziaee. · View Herald TranscriptWed, Jan 14, 9:38 PM
Harbormaster completed remote builds in B69895: Diff 169747.Wed, Jan 14, 9:38 PM
pouria retitled this revision from rtadvctl: add pref64 support to rtadvd: add multi pref64 support.Wed, Jan 14, 9:39 PM
pouria edited the summary of this revision. (Show Details)
pouria edited the test plan for this revision. (Show Details)
pouria marked 7 inline comments as done.Wed, Jan 14, 10:18 PM

Here is the output sample of rtadvctl:

% mdo rtadvctl -v show
bridge0: flags=<UP,TRANSITIVE,PERSIST> status=<RA_SEND> mtu 1500
DefaultLifetime: 10m
MinAdvInterval/MaxAdvInterval: 3m20s/10m
AdvLinkMTU: <none>, Flags: MO, Preference: low
ReachableTime: 0s, RetransTimer: 0s, CurHopLimit: 64
AdvIfPrefixes: yes
Next RA send: Thu Jan 15 00:58:25 2026
Last RA send: Thu Jan 15 00:58:06 2026
Prefixes (1):
2a01:e140:1234:5678::/64 (CONFIG, vltime=30d, pltime=7d, flags=LA)
DNSSL entries:
spmzt.net (ltime=15m)
PREF64:
2a01:e140:cafe:ff::/96 (ltime: 3m45s)
2a01:e140:dead:ff::/64 (ltime: 3m45s)

@bz: I also tested it and I can confirm that it works using wireshark.
@zlei: I had to rollback the assert part, as there's no need for p64_enabled anymore. so I removed it.
@glebius should these minor changes to the manual be approved by the manpages group?

I also found a bug related to flag inconsistency. However, I will fix it in another revision.

Revision Contents
Changeset List

PathSize
usr.sbin/
rtadvctl/
24 lines

Diff 169519

View Options

usr.sbin/rtadvctl/rtadvctl.c

Loading...