Re: [PATCH] ndp: Don't send first periodic router advertisement right after guest connects

On Tue, 26 Nov 2024 15:22:38 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:

...
  On Mon, Nov 25, 2024 at 09:09:05AM +0100, Stefano
Brivio wrote:
 
 [...]

 @@ -420,9 +420,13 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
  	interval = min_rtr_adv_interval +
  		random() % (max_rtr_adv_interval - min_rtr_adv_interval);
  
 +	if (!next_ra)
 +		goto first;   
 
 I don't think avoiding re-indenting two lines is sufficient reason to
 introduce yet another goto, though..
 
 > +
 >  	info("NDP: sending unsolicited RA, next in %llds", (long
long)interval);
 >  
 >  	ndp_ra(c, &in6addr_ll_all_nodes);
 >  
 > +first:
 >  	next_ra = now->tv_sec + interval;
 >  }   
At the cost of one additional line (and zero non-blank lines):

--
$ git diff --patch --stat
 ndp.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/ndp.c b/ndp.c
index 37bf7a3..d1ba867 100644
--- a/ndp.c
+++ b/ndp.c
@@ -420,13 +420,12 @@ void ndp_timer(const struct ctx *c, const struct timespec *now)
        interval = min_rtr_adv_interval +
                random() % (max_rtr_adv_interval - min_rtr_adv_interval);
 
-       if (!next_ra)
-               goto first;
+       if (next_ra) {
+               info("NDP: sending unsolicited RA, next in %llds",
+                    (long long)interval);
 
-       info("NDP: sending unsolicited RA, next in %llds", (long
long)interval);
-
-       ndp_ra(c, &in6addr_ll_all_nodes);
+               ndp_ra(c, &in6addr_ll_all_nodes);
+       }
 
-first:
        next_ra = now->tv_sec + interval;
 }
--

we get:

- clarity about the fact that 'next_ra' happens to be 0 on the *first*
  run (the label says it)

- clarity about the fact that it's a special case (it's a goto)

- no wrapped lines

-- 
Stefano