[PATCH v2] ndp: Don't send unsolicited router advertisement if we can't, yet

[PATCH v2] ndp: Don't send unsolicited router advertisement if we can't, yet

[Stefano Brivio]

ndp_timer() is called right away on the first epoll_wait() cycle,
when the communication channel to the guest isn't ready yet:

  1.0038: NDP: sending unsolicited RA, next in 264s
  1.0038: tap: failed to send 1 frames of 1

check that it's up before sending it. This effectively delays the
first gratuitous router advertisement, which is probably a good idea
given that we expect the guest to send a router solicitation right
away.

Fixes: 6e1e44293ef9 ("ndp: Send unsolicited Router Advertisements")
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
Oops, I sent a previous/wrong version of the patch as v1.

 ndp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ndp.c b/ndp.c
index 7ee44b2..1752d64 100644
--- a/ndp.c
+++ b/ndp.c
@@ -391,7 +391,7 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
 	time_t max_rtr_adv_interval = DEFAULT_MAX_RTR_ADV_INTERVAL;
 	time_t min_rtr_adv_interval, interval;
 
-	if (c->no_ra || now->tv_sec < next_ra)
+	if (c->fd_tap < 0 || c->no_ra || now->tv_sec < next_ra)
 		return;
 
 	/* We must advertise before the route's lifetime expires */
-- 
@@ -391,7 +391,7 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
 	time_t max_rtr_adv_interval = DEFAULT_MAX_RTR_ADV_INTERVAL;
 	time_t min_rtr_adv_interval, interval;
 
-	if (c->no_ra || now->tv_sec < next_ra)
+	if (c->fd_tap < 0 || c->no_ra || now->tv_sec < next_ra)
 		return;
 
 	/* We must advertise before the route's lifetime expires */
-- 
2.43.0

Re: [PATCH v2] ndp: Don't send unsolicited router advertisement if we can't, yet

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1702 bytes --]

On Fri, Nov 15, 2024 at 03:22:06PM +0100, Stefano Brivio wrote:
> ndp_timer() is called right away on the first epoll_wait() cycle,
> when the communication channel to the guest isn't ready yet:
> 
>   1.0038: NDP: sending unsolicited RA, next in 264s
>   1.0038: tap: failed to send 1 frames of 1
> 
> check that it's up before sending it. This effectively delays the
> first gratuitous router advertisement, which is probably a good idea
> given that we expect the guest to send a router solicitation right
> away.
> 
> Fixes: 6e1e44293ef9 ("ndp: Send unsolicited Router Advertisements")
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

I spotted another buglet in the RA stuff shortly after I sent it: we
should suppress calling ndp_timer() at all if c->no_ndp, but I forgot
to test for it.

> ---
> Oops, I sent a previous/wrong version of the patch as v1.
> 
>  ndp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/ndp.c b/ndp.c
> index 7ee44b2..1752d64 100644
> --- a/ndp.c
> +++ b/ndp.c
> @@ -391,7 +391,7 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
>  	time_t max_rtr_adv_interval = DEFAULT_MAX_RTR_ADV_INTERVAL;
>  	time_t min_rtr_adv_interval, interval;
>  
> -	if (c->no_ra || now->tv_sec < next_ra)
> +	if (c->fd_tap < 0 || c->no_ra || now->tv_sec < next_ra)
>  		return;
>  
>  	/* We must advertise before the route's lifetime expires */

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]