On Fri, May 24, 2024 at 11:16:36AM +0200, Florian Obser wrote:
> On 2024-05-23 13:20 -07, Ryan Vogt wrote:
> > On Mon, May 20, 2024 at 12:16:33PM +0000, Florian Obser wrote:
> >> Prefixes delegated via DHCPv6 have a lifetime, honour it.
> >> 
> >> The "auto prefix" feature derives the prefix to announce from a
> >> configured IPv6 address. If that address has a vltime / pltime use
> >> that value in router advertisements instead of statically configured
> >> values.
> >> 
> >> We also need to count down the vltime / pltime as time progresses.
> >> 
> >> This is on top of "rad(8): reduce calls to getifaddrs(3)" and also needs
> >> "send RTM_CHGADDRATTR for vltime / pltime changes" to work correctly.
> >
> > Hi, I have a question about the intended behaviour of this patch in a
> > few cases, which I've outlined below.

[snip]

> > To set up our situation, I have rad(8) running on igc1. On igc1, I
> > have a static prefix, and a prefix delegated from DHCPv6 (dhcpcd
> > running on igc0):
> >
> > 	$ ifconfig igc1
> > 	[...]
> > 	inet6 fe80::bbbb:bbbb:bbbb:bbbb%igc1 prefixlen 64 scopeid 0x2
> > 	inet6 fdaa:aaaa:aaaa:1::1 prefixlen 64
> > 	inet6 2001:db80:eeee:eeee::1 prefixlen 64 pltime 103481 \
> > 	    vltime 103481
> >
> > The fdaa:... prefix is static, and the 2001:... prefix is delegated by
> > my ISP.

[snip]

> > 4) The fourth case is an extension of the third case, with a more-
> > explicit rad.conf. Here's an rad.conf that someone with an
> > essentially-static delegation from their ISP could use (I'm not so
> > lucky):
> >
> > 	interface igc1 {
> > 		prefix fdaa:aaaa:aaaa:1:: {
> > 			preferred lifetime 2000
> > 			valid lifetime 3000
> > 		}
> > 		prefix 2001:db80:eeee:eeee:: {
> > 			preferred lifetime 4000
> > 			valid lifetime 5000
> > 		}
> > 	}
> >
> > The behaviour with the new patch is that clients receive the following
> > prefix info options from rad:
> >
> > 	- fdaa:aaaa:aaaa:1::/64 with valid time 3000, pref. time 2000
> > 	- 2001:db80:eeee:eeee::/64 with valid time 103481,
> > 	    pref. time 103481
> >
> 
> I don't think this actually happens, if you are not configuring an auto
> prefix rad(8) does not look at prefixes, does it?
> 
>                 if (ra_iface_conf->autoprefix)
>                         get_interface_prefixes(ra_iface,
>                             ra_iface_conf->autoprefix, ifap);
> 
> I think you should get this:
>  	- fdaa:aaaa:aaaa:1::/64 with valid time 3000, pref. time 2000
>  	- 2001:db80:eeee:eeee::/64 with valid time 5000, pref. time 4000

Sorry that I took a while to get back to you -- I wanted to double-
check my testing and investigate a little more.

I'm not sure how much it matters if you're planning on changing the
the rad(8) patch anyway, but I've confirmed that the patch behaves as
I reported in my previous message.

To shed a little light on the situation, here are two slightly
different rad.conf(5) files. The first produces the behaviour I
reported. The second produces the behaviour you expected.

4.a) For this rad.conf:

	interface igc1 {
		prefix fdaa:aaaa:aaaa:1:: {
			preferred lifetime 2000
			valid lifetime 3000
		}
		prefix 2001:db80:eeee:eeee:: {
			preferred lifetime 4000
			valid lifetime 5000
		}
	}

we get RA messages from rad(8) containing these two prefix info options
(the ones I originally reported):

	- fdaa:aaaa:aaaa:1::/64 with valid time 3000, pref. time 2000
	- 2001:db80:eeee:eeee::/64 with valid time 103481,
	    pref. time 103481

4.b) For this rad.conf:

	interface igc1 {
		no auto prefix   # Difference from 4.a is this line 
		prefix fdaa:aaaa:aaaa:1:: {
			preferred lifetime 2000
			valid lifetime 3000
		}
		prefix 2001:db80:eeee:eeee:: {
			preferred lifetime 4000
			valid lifetime 5000
		}
	}

we see the behaviour you expected:

	- fdaa:aaaa:aaaa:1::/64 with valid time 3000, pref. time 2000
	- 2001:db80:eeee:eeee::/64 with valid time 5000,
	    pref. time 4000

Some investigation --

In your previous message, you highlighted a few lines of code in
merge_ra_interfaces():

	if (ra_iface_conf->autoprefix)
		get_interface_prefixes(ra_iface,
		    ra_iface_conf->autoprefix, ifap);

Just before the highlighted code, merge_ra_interfaces() deals with
adding the static prefixes (there's an "add static prefixes for %s"
debug message). For both versions of rad.conf (4.a and 4.b): both
fdaa:aaaa:aaaa:1::/64 and 2001:db80:eeee:eeee::/64 get added as static
prefixes here -- each with a call to add_new_prefix_to_ra_iface().

After the static prefixes are dealt with in merge_ra_interfaces(), we
reach the highlighted lines of code. Here's where the behaviour
becomes different without the "no auto prefix" line in rad.conf.
Without the "no auto prefix" line, we call
add_new_prefix_to_ra_iface() again for both prefixes.

The first prefix, fdaa:aaaa:aaaa:1::/64, gets ignored during its
second call to add_new_prefix_to_ra_iface(), producing an
"ignoring duplicate %s/%d prefix" debug message.

The second prefix, 2001:db80:eeee:eeee::/64, however, takes a
different path through add_new_prefix_to_ra_iface() during its second
call:

	- The call to find_ra_prefix_conf(), right at the top of
	    add_new_prefix_to_ra_iface(), returns non-NULL
	- decaying_vltime != ND6_INFINITE_LIFETIME and
	    decaying_pltime != ND6_INFINITE_LIFETIME, so we go through
	    that branch of the following ifs and else-ifs.

I believe what's happening is that add_new_prefix_to_ra_iface() isn't
recognizing 2001:db80:eeee:eeee::/64 as a duplicate prefix because of
the check for decaying preferred/valid lifetimes.