Re: bgpd: only run one roa softreconfig process at a time

2022-08-30 Thread Sebastian Benoit
ok 

Claudio Jeker(cje...@diehard.n-r-g.com) on 2022.08.30 22:54:43 +0200:
> Currently if bgpd takes a long time to re-evaluate all prefixes because of
> a ROA change a second update can come in before the first is processed.
> This is not good. So add a barrier to only run one rde_roa_softreconfig
> dump at a time.
> 
> If a dump is pending while a new roa set is received ignore that roa-set
> and wait for the next one. Later on the roa update should be done with
> deltas and only the networks affected by the delta should be touched but
> that is way more complicated.
> 
> The following diff should do that.
> -- 
> :wq Claudio
> 
> Index: rde.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
> retrieving revision 1.570
> diff -u -p -r1.570 rde.c
> --- rde.c 30 Aug 2022 18:50:21 -  1.570
> +++ rde.c 30 Aug 2022 19:54:45 -
> @@ -1083,6 +1083,7 @@ rde_dispatch_imsg_rtr(struct imsgbuf *ib
>   switch (imsg.hdr.type) {
>   case IMSG_RECONF_ROA_SET:
>   /* start of update */
> + trie_free(_new.th); /* clear new roa */
>   break;
>   case IMSG_RECONF_ROA_ITEM:
>   if (imsg.hdr.len - IMSG_HEADER_SIZE !=
> @@ -3923,11 +3924,14 @@ rde_roa_softreload(struct rib_entry *re,
>   }
>  }
>  
> +static int roa_update_pending;
> +
>  static void
>  rde_roa_softreload_done(void *arg, uint8_t aid)
>  {
>   /* the roa update is done */
>   log_info("ROA softreload done");
> + roa_update_pending = 0;
>  }
>  
>  static void
> @@ -3935,6 +3939,11 @@ rde_roa_reload(void)
>  {
>   struct rde_prefixset roa_old;
>  
> + if (roa_update_pending) {
> + log_info("ROA softreload skipped, old still running");
> + return;
> + }
> +
>   roa_old = rde_roa;
>   rde_roa = roa_new;
>   memset(_new, 0, sizeof(roa_new));
> @@ -3950,6 +3959,7 @@ rde_roa_reload(void)
>   trie_free(_old.th); /* old roa no longer needed */
>  
>   log_debug("ROA change: reloading Adj-RIB-In");
> + roa_update_pending = 1;
>   if (rib_dump_new(RIB_ADJ_IN, AID_UNSPEC, RDE_RUNNER_ROUNDS,
>   rib_byid(RIB_ADJ_IN), rde_roa_softreload,
>   rde_roa_softreload_done, NULL) == -1)
> Index: rde_trie.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rde_trie.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 rde_trie.c
> --- rde_trie.c23 May 2022 13:40:12 -  1.15
> +++ rde_trie.c30 Aug 2022 19:53:11 -
> @@ -468,6 +468,7 @@ trie_free(struct trie_head *th)
>  {
>   trie_free_v4(th->root_v4);
>   trie_free_v6(th->root_v6);
> + memset(th, 0, sizeof(*th));
>  }
>  
>  static int
> Index: rtr.c
> ===
> RCS file: /cvs/src/usr.sbin/bgpd/rtr.c,v
> retrieving revision 1.6
> diff -u -p -r1.6 rtr.c
> --- rtr.c 17 Aug 2022 15:15:26 -  1.6
> +++ rtr.c 30 Aug 2022 19:59:20 -
> @@ -76,7 +76,7 @@ rtr_expire_roas(time_t now)
>   }
>   }
>   if (recalc != 0)
> - log_warnx("%u roa-set entries expired", recalc);
> + log_info("%u roa-set entries expired", recalc);
>   return recalc;
>  }
>  
> 



bgpd: only run one roa softreconfig process at a time

2022-08-30 Thread Claudio Jeker
Currently if bgpd takes a long time to re-evaluate all prefixes because of
a ROA change a second update can come in before the first is processed.
This is not good. So add a barrier to only run one rde_roa_softreconfig
dump at a time.

If a dump is pending while a new roa set is received ignore that roa-set
and wait for the next one. Later on the roa update should be done with
deltas and only the networks affected by the delta should be touched but
that is way more complicated.

The following diff should do that.
-- 
:wq Claudio

Index: rde.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde.c,v
retrieving revision 1.570
diff -u -p -r1.570 rde.c
--- rde.c   30 Aug 2022 18:50:21 -  1.570
+++ rde.c   30 Aug 2022 19:54:45 -
@@ -1083,6 +1083,7 @@ rde_dispatch_imsg_rtr(struct imsgbuf *ib
switch (imsg.hdr.type) {
case IMSG_RECONF_ROA_SET:
/* start of update */
+   trie_free(_new.th); /* clear new roa */
break;
case IMSG_RECONF_ROA_ITEM:
if (imsg.hdr.len - IMSG_HEADER_SIZE !=
@@ -3923,11 +3924,14 @@ rde_roa_softreload(struct rib_entry *re,
}
 }
 
+static int roa_update_pending;
+
 static void
 rde_roa_softreload_done(void *arg, uint8_t aid)
 {
/* the roa update is done */
log_info("ROA softreload done");
+   roa_update_pending = 0;
 }
 
 static void
@@ -3935,6 +3939,11 @@ rde_roa_reload(void)
 {
struct rde_prefixset roa_old;
 
+   if (roa_update_pending) {
+   log_info("ROA softreload skipped, old still running");
+   return;
+   }
+
roa_old = rde_roa;
rde_roa = roa_new;
memset(_new, 0, sizeof(roa_new));
@@ -3950,6 +3959,7 @@ rde_roa_reload(void)
trie_free(_old.th); /* old roa no longer needed */
 
log_debug("ROA change: reloading Adj-RIB-In");
+   roa_update_pending = 1;
if (rib_dump_new(RIB_ADJ_IN, AID_UNSPEC, RDE_RUNNER_ROUNDS,
rib_byid(RIB_ADJ_IN), rde_roa_softreload,
rde_roa_softreload_done, NULL) == -1)
Index: rde_trie.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_trie.c,v
retrieving revision 1.15
diff -u -p -r1.15 rde_trie.c
--- rde_trie.c  23 May 2022 13:40:12 -  1.15
+++ rde_trie.c  30 Aug 2022 19:53:11 -
@@ -468,6 +468,7 @@ trie_free(struct trie_head *th)
 {
trie_free_v4(th->root_v4);
trie_free_v6(th->root_v6);
+   memset(th, 0, sizeof(*th));
 }
 
 static int
Index: rtr.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rtr.c,v
retrieving revision 1.6
diff -u -p -r1.6 rtr.c
--- rtr.c   17 Aug 2022 15:15:26 -  1.6
+++ rtr.c   30 Aug 2022 19:59:20 -
@@ -76,7 +76,7 @@ rtr_expire_roas(time_t now)
}
}
if (recalc != 0)
-   log_warnx("%u roa-set entries expired", recalc);
+   log_info("%u roa-set entries expired", recalc);
return recalc;
 }