On Fri, May 10, 2024 at 12:30:35AM +0200, Paul Donald wrote:
> From: Paul Donald <newt...@gmail.com>
>
> Signed-off-by: Paul Donald <newt...@gmail.com>
> ---
> src/config.c | 48 +++++++++++++++++++++++++++++++++++++++++-------
> src/router.c | 10 ----------
> src/router.h | 2 +-
> 3 files changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/src/config.c b/src/config.c
> index 346d74a..2ccf742 100644
> --- a/src/config.c
> +++ b/src/config.c
> @@ -18,6 +18,7 @@
> #include <libubox/vlist.h>
>
> #include "odhcpd.h"
> +#include "router.h"
>
> static struct blob_buf b;
> static int reload_pipe[2] = { -1, -1 };
> @@ -227,8 +228,12 @@ static void set_interface_defaults(struct interface
> *iface)
> iface->ra_flags = ND_RA_FLAG_OTHER;
> iface->ra_slaac = true;
> iface->ra_maxinterval = 600;
> + /*
> + * RFC4861: MinRtrAdvInterval: Default: 0.33 * MaxRtrAdvInterval If
> + * MaxRtrAdvInterval >= 9 seconds; otherwise, the Default is
> MaxRtrAdvInterval.
> + */
Any idea how to handle the if condition? If possible?
Maybe we set here min to -1 and handle later in calc?
> iface->ra_mininterval = iface->ra_maxinterval/3;
> - iface->ra_lifetime = -1;
> + iface->ra_lifetime = 3 * iface->ra_maxinterval; /* RFC4861:
> AdvDefaultLifetime: Default: 3 * MaxRtrAdvInterval */
> iface->ra_dns = true;
> }
>
> @@ -966,14 +971,43 @@ int config_parse_interface(void *data, size_t len,
> const char *name, bool overwr
> if ((c = tb[IFACE_ATTR_RA_ADVROUTER]))
> iface->ra_advrouter = blobmsg_get_bool(c);
>
> - if ((c = tb[IFACE_ATTR_RA_MININTERVAL]))
> - iface->ra_mininterval = blobmsg_get_u32(c);
> + /*
> + * RFC4861: MaxRtrAdvInterval: MUST be no less than 4 seconds and no
> greater than 1800 seconds.
> + * RFC8319: MaxRtrAdvInterval: MUST be no less than 4 seconds and no
> greater than 65535 seconds.
> + * Default: 600 seconds
> + */
> + if ((c = tb[IFACE_ATTR_RA_MAXINTERVAL])){
> + uint32_t ra_maxinterval = blobmsg_get_u32(c);
> + iface->ra_maxinterval = (ra_maxinterval < 4) ? 4 :
> + (ra_maxinterval > MaxRtrAdvInterval_CEILING) ?
> + MaxRtrAdvInterval_CEILING : ra_maxinterval;
> + }
Mhhh ? : are ok for simple condition but here we have nested ones...
Better to use the full if condition form.
>
> - if ((c = tb[IFACE_ATTR_RA_MAXINTERVAL]))
> - iface->ra_maxinterval = blobmsg_get_u32(c);
> + /*
> + * RFC4861: MinRtrAdvInterval: MUST be no less than 3 seconds and no
> greater than .75 * MaxRtrAdvInterval.
> + * Default: 0.33 * MaxRtrAdvInterval If MaxRtrAdvInterval >= 9 seconds;
> otherwise, the
> + * Default is MaxRtrAdvInterval.
> + */
> + if ((c = tb[IFACE_ATTR_RA_MININTERVAL])){
> + uint32_t ra_mininterval = blobmsg_get_u32(c);
> + iface->ra_mininterval = (ra_mininterval <
> MinRtrAdvInterval_FLOOR) ? // clamp min
> + MinRtrAdvInterval_FLOOR : (ra_mininterval > 0.75 *
> (uint32_t)iface->ra_maxinterval) ? // clamp max
> + 0.75 * (uint32_t)iface->ra_maxinterval :
> ra_mininterval;
> + }
Ditto.
>
> - if ((c = tb[IFACE_ATTR_RA_LIFETIME]))
> - iface->ra_lifetime = blobmsg_get_u32(c);
> + /*
> + * RFC4861: AdvDefaultLifetime: MUST be either zero or between
> MaxRtrAdvInterval and 9000 seconds.
> + * RFC8319: AdvDefaultLifetime: MUST be either zero or between
> MaxRtrAdvInterval and 65535 seconds.
> + * Default: 3 * MaxRtrAdvInterval
> + * i.e. 3 * 65535 => 65535 seconds.
> + */
> + if ((c = tb[IFACE_ATTR_RA_LIFETIME])){
> + uint32_t ra_lifetime = blobmsg_get_u32(c);
> + iface->ra_lifetime = (ra_lifetime == 0) ? 0 : // leave at 0
> + (ra_lifetime < (uint32_t)iface->ra_maxinterval) ?
> (uint32_t)iface->ra_maxinterval : // clamp min
> + (ra_lifetime > AdvDefaultLifetime_CEILING) ?
> + AdvDefaultLifetime_CEILING :
> ra_lifetime; // clamp max
> + }
Ditto.
>
> if ((c = tb[IFACE_ATTR_RA_USELEASETIME]))
> iface->ra_useleasetime = blobmsg_get_bool(c);
> diff --git a/src/router.c b/src/router.c
> index ae0c545..96237d6 100644
> --- a/src/router.c
> +++ b/src/router.c
> @@ -350,16 +350,6 @@ static int calc_adv_interval(struct interface *iface,
> uint32_t lowest_found_life
> if (*maxival > lowest_found_lifetime/3)
> *maxival = lowest_found_lifetime/3;
>
> - if (*maxival > MaxRtrAdvInterval)
> - *maxival = MaxRtrAdvInterval;
> - else if (*maxival < 4)
> - *maxival = 4;
> -
> - if (minival < MinRtrAdvInterval)
> - minival = MinRtrAdvInterval;
> - else if (minival > (*maxival * 3)/4)
> - minival = (*maxival >= 9 ? *maxival/3 : *maxival);
> -
> odhcpd_urandom(&msecs, sizeof(msecs));
> msecs = (labs(msecs) % ((*maxival != minival) ? (*maxival -
> minival)*1000 : 500)) +
> minival*1000;
> diff --git a/src/router.h b/src/router.h
> index 1f8d156..8e9499e 100644
> --- a/src/router.h
> +++ b/src/router.h
> @@ -43,7 +43,7 @@ struct icmpv6_opt {
> AdvDefaultLifetime, change 9000 to 65535 seconds.
> */
> #define MaxRtrAdvInterval_CEILING 65535
> -#define MinRtrAdvInterval 3
> +#define MinRtrAdvInterval_FLOOR 3
> #define AdvDefaultLifetime_CEILING 65535
> /* RFC8319 §4
> This document updates §4.2 and 6.2.1 of [RFC4861] to change
Same here and for rest of the series, do we really need these additional
define? (and bisectability problem)
--
Ansuel
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
