Index | Thread | Search

From:
Florian Obser <florian@narrans.de>
Subject:
Re: Allow rad(8) to advertise shorter lifetimes
To:
Ryan Vogt <rvogt.ca@gmail.com>
Cc:
tech@openbsd.org
Date:
Mon, 15 Sep 2025 11:08:09 +0200

Download raw body.

Thread
On 2025-09-14 17:32 -07, Ryan Vogt <rvogt.ca@gmail.com> wrote:
> On Sun, Sep 14, 2025 at 01:05:33PM +0200, Florian Obser wrote:
[...]
>> +	new_ra_prefix_conf->autoconf = autoconf;
>> +	if (autoconf)
>> +		ra_iface->has_autoconf_prefix = 1;
>> +	new_ra_prefix_conf->if_vltime = if_vltime;
>> +	new_ra_prefix_conf->if_pltime = if_pltime;
>>  	new_ra_prefix_conf->aflag = ra_prefix_conf->aflag;
>>  	new_ra_prefix_conf->lflag = ra_prefix_conf->lflag;
>>  	SIMPLEQ_INSERT_TAIL(&ra_iface->prefixes, new_ra_prefix_conf, entry);
>>  	ra_iface->prefix_count++;
>>  }
>>  
>> +uint32_t
>> +calc_autoconf_ltime(time_t t, uint32_t conf_ltime, uint32_t if_ltime)
>
> A tiny comment about two of the parameters. Here,
>
> - "conf_ltime" refers to the lifetime specified in the prefix
>   configuration; and,
> - "if_ltime" refers to the configured lifetime on the interface.
>
> I misinterpreted "conf_ltime" as "configured lifetime" and had to
> reread things a few times. Please feel free to disregard, but could I
> suggest calling the lifetime from the prefix configuration
> "prefix_conf_ltime" instead of "conf_ltime"?

I hear what you are saying. It took me so long to tackle this problem
because rad(8) was trying to be too clever in places and poorly named
variables. Being explicit about which information came from the config
file and what information came from the interface helped a lot.

That being said, I committed it as is, prefix_conf_ltime would confuse
me, I'd read that as the "configured lifetime" of the prefix. "conf" is
the variable that holds the contents of the configuration file.

This probably suggests that neither conf_ltime nor prefix_conf_ltime are
good variable names.

Thank you for your persistence and extensive testing!

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