diff mbox series

[odhcpd,v2,1/2] router: improve RA logging

Message ID 20230216203041.2203509-1-stijn@linux-ipv6.be
State Accepted, archived
Delegated to: Stijn Tintel
Headers show
Series [odhcpd,v2,1/2] router: improve RA logging | expand

Commit Message

Stijn Tintel Feb. 16, 2023, 8:30 p.m. UTC
We only set the RA lifetime to what is configured in UCI when there is a
default route and valid prefix. In any other case, we set it to 0. This
leads to confusion where people believe ra_lifetime is completely
ignored. In case there is a default route, but no valid prefix, a debug
message explains this, but if there is no default route, we silently
override ra_lifetime.

Add a debug message for the latter case, and explicitly mention
overriding ra_lifetime in both cases.

Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
---
 src/router.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Hans Dedecker Feb. 17, 2023, 12:22 p.m. UTC | #1
On Thu, Feb 16, 2023 at 9:30 PM <stijn@linux-ipv6.be> wrote:
>
> We only set the RA lifetime to what is configured in UCI when there is a
> default route and valid prefix. In any other case, we set it to 0. This
> leads to confusion where people believe ra_lifetime is completely
> ignored. In case there is a default route, but no valid prefix, a debug
> message explains this, but if there is no default route, we silently
> override ra_lifetime.
>
> Add a debug message for the latter case, and explicitly mention
> overriding ra_lifetime in both cases.
>
> Signed-off-by: Stijn Tintel <stijn@linux-ipv6.be>
Acked-by: Hans Dedecker <dedeckeh@gmail.com>
> ---
>  src/router.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/src/router.c b/src/router.c
> index 949cbe7..1c11849 100644
> --- a/src/router.c
> +++ b/src/router.c
> @@ -618,17 +618,19 @@ static int send_router_advert(struct interface *iface, const struct in6_addr *fr
>         msecs = calc_adv_interval(iface, minvalid, &maxival);
>         lifetime = calc_ra_lifetime(iface, maxival);
>
> -       if (default_route) {
> -               if (!valid_prefix) {
> -                       syslog(LOG_WARNING, "A default route is present but there is no public prefix "
> -                                       "on %s thus we don't announce a default route!", iface->name);
> -                       adv.h.nd_ra_router_lifetime = 0;
> -               } else
> -                       adv.h.nd_ra_router_lifetime = htons(lifetime < UINT16_MAX ? lifetime : UINT16_MAX);
> -
> -       } else
> +       if (default_route && valid_prefix) {
> +               adv.h.nd_ra_router_lifetime = htons(lifetime < UINT16_MAX ? lifetime : UINT16_MAX);
> +       } else {
>                 adv.h.nd_ra_router_lifetime = 0;
>
> +               if (default_route) {
> +                       syslog(LOG_WARNING, "A default route is present but there is no public prefix "
> +                                           "on %s thus we don't announce a default route by overriding ra_lifetime!", iface->name);
> +               } else {
> +                       syslog(LOG_WARNING, "No default route present, overriding ra_lifetime!");
> +               }
> +       }
> +
>         syslog(LOG_DEBUG, "Using a RA lifetime of %d seconds on %s", ntohs(adv.h.nd_ra_router_lifetime), iface->name);
>
>         /* DNS options */
> --
> 2.39.1
>
diff mbox series

Patch

diff --git a/src/router.c b/src/router.c
index 949cbe7..1c11849 100644
--- a/src/router.c
+++ b/src/router.c
@@ -618,17 +618,19 @@  static int send_router_advert(struct interface *iface, const struct in6_addr *fr
 	msecs = calc_adv_interval(iface, minvalid, &maxival);
 	lifetime = calc_ra_lifetime(iface, maxival);
 
-	if (default_route) {
-		if (!valid_prefix) {
-			syslog(LOG_WARNING, "A default route is present but there is no public prefix "
-					"on %s thus we don't announce a default route!", iface->name);
-			adv.h.nd_ra_router_lifetime = 0;
-		} else
-			adv.h.nd_ra_router_lifetime = htons(lifetime < UINT16_MAX ? lifetime : UINT16_MAX);
-
-	} else
+	if (default_route && valid_prefix) {
+		adv.h.nd_ra_router_lifetime = htons(lifetime < UINT16_MAX ? lifetime : UINT16_MAX);
+	} else {
 		adv.h.nd_ra_router_lifetime = 0;
 
+		if (default_route) {
+			syslog(LOG_WARNING, "A default route is present but there is no public prefix "
+					    "on %s thus we don't announce a default route by overriding ra_lifetime!", iface->name);
+		} else {
+			syslog(LOG_WARNING, "No default route present, overriding ra_lifetime!");
+		}
+	}
+
 	syslog(LOG_DEBUG, "Using a RA lifetime of %d seconds on %s", ntohs(adv.h.nd_ra_router_lifetime), iface->name);
 
 	/* DNS options */