Index | Thread | Search

From:
Ryan Vogt <rvogt.ca@gmail.com>
Subject:
Re: Allow rad(8) to advertise shorter lifetimes
To:
tech@openbsd.org
Cc:
florian@narrans.de
Date:
Sun, 14 Sep 2025 17:32:27 -0700

Download raw body.

Thread
On Sun, Sep 14, 2025 at 01:05:33PM +0200, Florian Obser wrote:
> Apologies for my silence and very late response, life happened...
> 
> I fully agree that lifetimes should be limited and a excessively long
> lifetimes handed out by a DHCPv6 server are a problem.
> 
> I don't think we need a config option for this. The minimum of what we
> find on an interface and what we have in the config file (including
> defaults) should win.

I like this approach far better. I was overly concerned with
maintaining the existing behaviour of rad as a configuration-file
option. If a user actually wants rad to hand out unbounded lifetimes,
they can use the maximum lifetime (4294967295) in the configuration
file.

> The following diff, inspired by your work, implements that.
> Does this work for you?
> Florian

It absolutely does. I have only one tiny comment about the code,
inline below.

I ran this patch through 10 different tests, and all came out with the
behaviour I'd expect. For posterity, here's what I tested.

For each test, I've used a shorthand for the configuration file. So,
[auto:4000/2000, static:9999/8888] refers to the configuration file:

    interface em1 {
        auto prefix {
            valid lifetime 4000
            preferred lifetime 2000
        }
        prefix wwww:xxxx:yyyy:zzzz {
            valid lifetime 9999
            preferred lifetime 8888
        }
    }

I'll always the use notation vvvv/pppp to refer to a valid lifetime of
vvvv and a preferred lifetime of pppp.

The static prefix in the configuration file (wwww:xxxx:yyyy:zzzz)
always matches the decaying configured address on interface em1, to
test the behaviour when an auto prefix configuration and static prefix
give different lifetime bounds.

And I'll use "->" to indicate what rad sends out as advertisements.

For the first 5 tests, a configured address of wwww:xxxx:yyyy:zzzz::1
exists on em1, lifetime 6000/3000, when rad starts. Tests 6-9
introduce a configured address while rad is running. Test 10 resets
the lifetime on a configured address while rad is running.

1) Ensure that rad starts using the decaying lifetime when it crosses
the bound in the configuration file.

[auto:5980/2980]
...decaying configured lifetime on em1 6000/3000...
...rad starts...
-> 5980/2980
...time passes...
-> 5800/2800-ish

2) We should get the same behaviour as test 1, even if the prefix is
defined as a static prefix in the configuration file.

[static:5980/2980]
...decaying configured lifetime on em1 6000/3000...
...rad starts...
-> 5980/2980
...time passes...
-> 5800/2800-ish

3) Users should be able to use a maximum value to unbound lifetimes.

[auto:4294967295/4294967295]
...decaying configured lifetime on em1 6000/3000...
...rad starts...
-> 6000/3000

4) A static configuration should override the auto configuration if it
provides a lower lifetime bound.

[auto:9999/8888, static:4000/2000]
...decaying configured lifetime on em1 6000/3000...
...rad starts...
-> 4000/2000

5) A static configuration should override the auto configuration, even
when the static configuration provides a higher lifetime bound.

[auto:4000/2000, static:9999/8888]
...decaying configured lifetime on em1 6000/3000...
...rad starts...
-> 6000/3000

6) Ensure the behaviour of test 1 is unchanged if the configured
lifetime appears while rad is running.

[auto:5980/2980]
...no configured lifetime on em1...
...rad starts...
-> No prefix information
...place decaying configured lifetime on em1 with 6000/3000...
...almost instantly...
-> 5980/2980
...time passes...
-> 5800/2800-ish

7) As with test 6, but without the bound coming into play.

[auto:9999/8888]
...no configured lifetime on em1...
...rad starts...
-> No prefix information
...place decaying configured address on em1 with 6000/3000...
...almost instantly...
-> 5999/2999-ish

8) Ensure the behaviour of test 2 is unchanged if the configured
lifetime appears while rad is running.

[static:5980/2980]
...no configured lifetime on em1...
...start rad...
-> 5980/2980
...place decaying configured address on em1 with 6000/3000...
...time passes...
-> 5800/2800-ish

9) As with test 8, but without the bound coming into play.

[static:9999/8888]
...no configured lifetime on em1...
...start rad...
-> 9999/8888
...place decaying configured address on em1 with 6000/3000...
...almost instantly...
-> 5999/2999-ish

10) A configured lifetime changes while rad is running.

[auto:5890/2890]
...decaying configured lifetime on em1 6000/3000...
...rad starts...
-> 5980/2980
...time passes...
-> 5800/2800-ish
...reset decaying configured lifetime on em1 to 6000/3000...
...almost instantly...
-> 5980/2980

Every one of these tests seems correct to me. Inline comment below.

Cheers,
Ryan

> diff --git frontend.c frontend.c
> index 2a519dac0b1..97f4d0ba708 100644
> --- frontend.c
> +++ frontend.c
> @@ -110,7 +110,7 @@ struct ra_iface {
>  	int				 removed;
>  	int				 link_state;
>  	int				 prefix_count;
> -	int				 ltime_decaying;
> +	int				 has_autoconf_prefix;
>  	size_t				 datalen;
>  	uint8_t				 data[RA_MAX_SIZE];
>  };
> @@ -151,11 +151,12 @@ void			 unref_icmp6ev(struct ra_iface *);
>  void			 set_icmp6sock(int, int);
>  void			 add_new_prefix_to_ra_iface(struct ra_iface *r,
>  			    struct in6_addr *, int, struct ra_prefix_conf *,
> -			    uint32_t, uint32_t);
> +			    int, uint32_t, uint32_t);
>  void			 free_ra_iface(struct ra_iface *);
>  int			 in6_mask2prefixlen(struct in6_addr *);
>  void			 get_interface_prefixes(struct ra_iface *,
>  			     struct ra_prefix_conf *, struct ifaddrs *);
> +uint32_t		 calc_autoconf_ltime(time_t, uint32_t, uint32_t);
>  int			 build_packet(struct ra_iface *);
>  void			 build_leaving_packet(struct ra_iface *);
>  void			 ra_output(struct ra_iface *, struct sockaddr_in6 *);
> @@ -979,7 +980,7 @@ merge_ra_interfaces(void)
>  		    entry) {
>  			add_new_prefix_to_ra_iface(ra_iface,
>  			    &ra_prefix_conf->prefix,
> -			    ra_prefix_conf->prefixlen, ra_prefix_conf,
> +			    ra_prefix_conf->prefixlen, ra_prefix_conf, 0,
>  			    ND6_INFINITE_LIFETIME, ND6_INFINITE_LIFETIME);
>  		}
>  
> @@ -1046,7 +1047,7 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf
>  	struct in6_ifreq	 ifr6;
>  	struct ifaddrs		*ifa;
>  	struct sockaddr_in6	*sin6;
> -	uint32_t		 decaying_vltime, decaying_pltime;
> +	uint32_t		 if_vltime, if_pltime;
>  	int			 prefixlen;
>  
>  	for (ifa = ifap; ifa != NULL; ifa = ifa->ifa_next) {
> @@ -1066,8 +1067,8 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf
>  		strlcpy(ifr6.ifr_name, ra_iface->name, sizeof(ifr6.ifr_name));
>  		memcpy(&ifr6.ifr_addr, sin6, sizeof(ifr6.ifr_addr));
>  
> -		decaying_vltime = ND6_INFINITE_LIFETIME;
> -		decaying_pltime = ND6_INFINITE_LIFETIME;
> +		if_vltime = ND6_INFINITE_LIFETIME;
> +		if_pltime = ND6_INFINITE_LIFETIME;
>  
>  		if (ioctl(ioctlsock, SIOCGIFALIFETIME_IN6,
>  		    (caddr_t)&ifr6) != -1) {
> @@ -1075,9 +1076,9 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf
>  
>  			lifetime = &ifr6.ifr_ifru.ifru_lifetime;
>  			if (lifetime->ia6t_preferred)
> -				decaying_pltime = lifetime->ia6t_preferred;
> +				if_pltime = lifetime->ia6t_preferred;
>  			if (lifetime->ia6t_expire)
> -				decaying_vltime = lifetime->ia6t_expire;
> +				if_vltime = lifetime->ia6t_expire;
>  		}
>  
>  		memset(&ifr6, 0, sizeof(ifr6));
> @@ -1096,8 +1097,7 @@ get_interface_prefixes(struct ra_iface *ra_iface, struct ra_prefix_conf
>  		mask_prefix(&sin6->sin6_addr, prefixlen);
>  
>  		add_new_prefix_to_ra_iface(ra_iface, &sin6->sin6_addr,
> -		    prefixlen, autoprefix_conf, decaying_vltime,
> -		    decaying_pltime);
> +		    prefixlen, autoprefix_conf, 1, if_vltime, if_pltime);
>  	}
>  }
>  
> @@ -1118,41 +1118,30 @@ find_ra_prefix_conf(struct ra_prefix_conf_head* head, struct in6_addr *prefix,
>  
>  void
>  add_new_prefix_to_ra_iface(struct ra_iface *ra_iface, struct in6_addr *addr,
> -    int prefixlen, struct ra_prefix_conf *ra_prefix_conf,
> -    uint32_t decaying_vltime, uint32_t decaying_pltime)
> +    int prefixlen, struct ra_prefix_conf *ra_prefix_conf, int autoconf,
> +    uint32_t if_vltime, uint32_t if_pltime)
>  {
>  	struct ra_prefix_conf	*new_ra_prefix_conf;
>  
>  	if ((new_ra_prefix_conf = find_ra_prefix_conf(&ra_iface->prefixes, addr,
>  	    prefixlen)) != NULL) {
> -		if (decaying_vltime != ND6_INFINITE_LIFETIME ||
> -		    decaying_pltime != ND6_INFINITE_LIFETIME) {
> -			ra_iface->ltime_decaying = 1;
> -			new_ra_prefix_conf->ltime_decaying = 0;
> -			if (decaying_vltime != ND6_INFINITE_LIFETIME) {
> -				new_ra_prefix_conf->vltime = decaying_vltime;
> -				new_ra_prefix_conf->ltime_decaying |=
> -				    VLTIME_DECAYING;
> -			}
> -			if (decaying_pltime != ND6_INFINITE_LIFETIME) {
> -				new_ra_prefix_conf->pltime = decaying_pltime;
> -				new_ra_prefix_conf->ltime_decaying |=
> -				    PLTIME_DECAYING;
> -			}
> -		} else if (new_ra_prefix_conf->ltime_decaying) {
> +		int changed = new_ra_prefix_conf->autoconf != autoconf;
> +
> +		new_ra_prefix_conf->autoconf = autoconf;
> +		new_ra_prefix_conf->if_vltime = if_vltime;
> +		new_ra_prefix_conf->if_pltime = if_pltime;
> +
> +		if (changed) {
>  			struct ra_prefix_conf *pc;
>  
> -			new_ra_prefix_conf->ltime_decaying = 0;
> -			ra_iface->ltime_decaying = 0;
> +			ra_iface->has_autoconf_prefix = 0;
>  			SIMPLEQ_FOREACH(pc, &ra_iface->prefixes, entry) {
> -				if (pc->ltime_decaying) {
> -					ra_iface->ltime_decaying = 1;
> +				if (pc->autoconf) {
> +					ra_iface->has_autoconf_prefix = 1;
>  					break;
>  				}
>  			}
> -		} else
> -			log_debug("ignoring duplicate %s/%d prefix",
> -			    in6_to_str(addr), prefixlen);
> +		}
>  		return;
>  	}
>  
> @@ -1164,24 +1153,36 @@ add_new_prefix_to_ra_iface(struct ra_iface *ra_iface, struct in6_addr *addr,
>  	new_ra_prefix_conf->prefixlen = prefixlen;
>  	new_ra_prefix_conf->vltime = ra_prefix_conf->vltime;
>  	new_ra_prefix_conf->pltime = ra_prefix_conf->pltime;
> -	if (decaying_vltime != ND6_INFINITE_LIFETIME ||
> -	    decaying_pltime != ND6_INFINITE_LIFETIME) {
> -		ra_iface->ltime_decaying = 1;
> -		if (decaying_vltime != ND6_INFINITE_LIFETIME) {
> -			new_ra_prefix_conf->vltime = decaying_vltime;
> -			new_ra_prefix_conf->ltime_decaying |= VLTIME_DECAYING;
> -		}
> -		if (decaying_pltime != ND6_INFINITE_LIFETIME) {
> -			new_ra_prefix_conf->pltime = decaying_pltime;
> -			new_ra_prefix_conf->ltime_decaying |= PLTIME_DECAYING;
> -		}
> -	}
> +	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"?

> +{
> +	uint32_t ltime;
> +
> +	if (if_ltime == ND6_INFINITE_LIFETIME)
> +		return conf_ltime;
> +
> +	if (if_ltime > t)
> +		ltime = if_ltime - t;
> +	else
> +		ltime = 0;
> +
> +	if (ltime < conf_ltime)
> +		return ltime;
> +	else
> +		return conf_ltime;
> +}
> +
>  int
>  build_packet(struct ra_iface *ra_iface)
>  {
> @@ -1294,16 +1295,15 @@ build_packet(struct ra_iface *ra_iface)
>  			ndopt_pi->nd_opt_pi_flags_reserved |=
>  			    ND_OPT_PI_FLAG_AUTO;
>  
> -		if (ra_prefix_conf->ltime_decaying & VLTIME_DECAYING)
> -			vltime = ra_prefix_conf->vltime < t ? 0 :
> -			    ra_prefix_conf->vltime - t;
> -		else
> -			vltime = ra_prefix_conf->vltime;
> -		if (ra_prefix_conf->ltime_decaying & PLTIME_DECAYING)
> -			pltime = ra_prefix_conf->pltime < t ? 0 :
> -			    ra_prefix_conf->pltime - t;
> -		else
> +		if (ra_prefix_conf->autoconf) {
> +			pltime = calc_autoconf_ltime(t,
> +			    ra_prefix_conf->pltime, ra_prefix_conf->if_pltime);
> +			vltime = calc_autoconf_ltime(t,
> +			    ra_prefix_conf->vltime, ra_prefix_conf->if_vltime);
> +		} else {
>  			pltime = ra_prefix_conf->pltime;
> +			vltime = ra_prefix_conf->vltime;
> +		}
>  
>  		ndopt_pi->nd_opt_pi_valid_time = htonl(vltime);
>  		ndopt_pi->nd_opt_pi_preferred_time = htonl(pltime);
> @@ -1430,7 +1430,7 @@ ra_output(struct ra_iface *ra_iface, struct sockaddr_in6 *to)
>  	if (!LINK_STATE_IS_UP(ra_iface->link_state))
>  		return;
>  
> -	if (ra_iface->ltime_decaying)
> +	if (ra_iface->has_autoconf_prefix)
>  		/* update vltime & pltime */
>  		build_packet(ra_iface);
>  
> diff --git rad.conf.5 rad.conf.5
> index d03bcc139aa..7211c48d0a1 100644
> --- rad.conf.5
> +++ rad.conf.5
> @@ -207,13 +207,13 @@ The preferred lifetime (pltime) in seconds for addresses generated from this
>  prefix.
>  The default is 2700.
>  This option is ignored if the prefix is discovered from a network interface
> -and it has a preferred lifetime configured.
> +and it has a lower preferred lifetime.
>  .It Cm valid lifetime Ar seconds
>  The valid lifetime (vltime) in seconds for addresses generated from this
>  prefix.
>  The default is 5400.
>  This option is ignored if the prefix is discovered from a network interface
> -and it has a valid lifetime configured.
> +and it has a lower valid lifetime.
>  .El
>  .El
>  .El
> diff --git rad.h rad.h
> index 7775ac4a0aa..00b4f1bfeb0 100644
> --- rad.h
> +++ rad.h
> @@ -35,8 +35,6 @@
>  #define	MIN_DELAY_BETWEEN_RAS	3	/* 3 seconds */
>  #define	MAX_SEARCH		1025	/* MAXDNAME in arpa/nameser.h */
>  #define	DEFAULT_RDNS_LIFETIME	3 * MAX_RTR_ADV_INTERVAL
> -#define	PLTIME_DECAYING		1
> -#define	VLTIME_DECAYING		2
>  
>  #define	IMSG_DATA_SIZE(imsg)	((imsg).hdr.len - IMSG_HEADER_SIZE)
>  
> @@ -116,7 +114,9 @@ struct ra_prefix_conf {
>  	int				 prefixlen;	/* prefix length */
>  	uint32_t			 vltime;	/* valid lifetime */
>  	uint32_t			 pltime;	/* preferred lifetime */
> -	int				 ltime_decaying;
> +	uint32_t			 if_vltime;	/* valid lifetime */
> +	uint32_t			 if_pltime;	/* preferred lifetime */
> +	int				 autoconf;
>  	int				 lflag;		/* on-link flag*/
>  	int				 aflag;		/* autonom. addr flag */
>  };
>