Thank you for the detailed write-up.
I now understand what is going on.

We should always announce the minimum of static configured and interface
discovered lifetimes.

For that rad(8) needs to track more information, I already have a plan
on how to implement this, but currently not enough time.

Since I think the two outstanding diffs are strictly better than what we
have I chose to commit those and work on a diff on top.

For the time being I went with your suggestion of documenting this
behaviour in the man page with a slightly tweaked text.

On 2024-05-25 11:40 -07, Ryan Vogt <rvogt.ca@gmail.com> wrote:
> 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.
>

-- 
In my defence, I have been left unsupervised.