BGP: fix warnings for roa_check() with no import table and route refresh on

When roa_check() appears in import filters, the autoreload is switched
on but prohibited for BGP if no import table is available. By fixing the
route refresh feature in 9edc421148fe9c5e7d6038b667ba8fafb587a1eb, we
inadvertently exposed another bug where Nest wasn't distinguishing
between locally and remotely available reload.

Whereas on manual reconfiguration, the route refresh is expected to be
invoked, the autoreload should not trigger any remote actions. Therefore
now the channels actualy indicate whether their reload hook triggers
remote actions or not. This information can be then used to decide
whether to allow autoreload or not.

diff --git a/nest/proto.c b/nest/proto.c
index 3565f229d49885bb084eb8c640907a1827da73fa..739c212c73a541f237e51435d85d85ad7fb26b69 100644 (file)
--- a/nest/proto.c
+++ b/nest/proto.c
@@ -55,6 +55,22 @@ static inline void channel_reimport(struct channel *c, struct rt_feeding_request
   ev_send(proto_event_list(c->proto), &c->reimport_event);
 }
 
+static inline bool channel_reload(struct channel *c, struct rt_feeding_request *rfr)
+{
+  if ((c->in_keep & RIK_PREFILTER) == RIK_PREFILTER)
+  {
+    channel_reimport(c, rfr);
+    return true;
+  }
+  else
+    return c->proto->reload_routes(c, rfr);
+}
+
+static inline void channel_reload_roa(struct channel *c, struct rt_feeding_request *rfr)
+{
+  ASSERT_DIE(channel_reload(c, rfr));
+}
+
 static inline void channel_refeed(struct channel *c, struct rt_feeding_request *rfr)
 {
   CALL(c->proto->refeed_begin, c, rfr);
@@ -67,10 +83,18 @@ static inline int proto_is_done(struct proto *p)
 static inline int channel_is_active(struct channel *c)
 { return (c->channel_state != CS_DOWN); }
 
-static inline int channel_reloadable(struct channel *c)
+static inline enum channel_reloadable channel_reloadable(struct channel *c)
 {
-  return c->reloadable && c->proto->reload_routes
-      || ((c->in_keep & RIK_PREFILTER) == RIK_PREFILTER);
+  /* Import table always reloads locally */
+  if ((c->in_keep & RIK_PREFILTER) == RIK_PREFILTER)
+    return CHANNEL_RELOADABLE_LOCALLY;
+
+  /* The protocol should indicate how the reload actually works */
+  if (c->proto->reload_routes)
+    return c->reloadable;
+
+  /* No reload possible */
+  return CHANNEL_RELOADABLE_NEVER;
 }
 
 static inline void
@@ -232,7 +256,7 @@ proto_add_channel(struct proto *p, struct channel_config *cf)
 
   c->channel_state = CS_DOWN;
   c->last_state_change = current_time();
-  c->reloadable = 1;
+  c->reloadable = CHANNEL_RELOADABLE_LOCALLY;
 
   init_list(&c->roa_subscriptions);
 
@@ -514,7 +538,7 @@ channel_roa_changed(void *_s)
 
 static inline void (*channel_roa_reload_hook(int dir))(struct channel *, struct rt_feeding_request *)
 {
-  return dir ? channel_reimport : channel_refeed;
+  return dir ? channel_reload_roa : channel_refeed;
 }
 
 static int
@@ -587,7 +611,7 @@ channel_roa_subscribe_filter(struct channel *c, int dir)
     return;
 
   /* No automatic reload for non-reloadable channels */
-  if (dir && !channel_reloadable(c))
+  if (dir && (channel_reloadable(c) != CHANNEL_RELOADABLE_LOCALLY))
     valid = 0;
 
   struct filter_iterator fit;
@@ -1040,10 +1064,11 @@ channel_request_reload(struct channel *c, struct rt_feeding_request *cir)
   else
     CD(c, "Full import reload requested");
 
-  if ((c->in_keep & RIK_PREFILTER) == RIK_PREFILTER)
-    channel_reimport(c, cir);
-  else if (! c->proto->reload_routes(c, cir))
-    cli_msg(-15, "%s.%s: partial reload refused, please run full reload instead", c->proto->name, c->name);
+  if (!channel_reload(c, cir))
+    if (cir && cir->prefilter.mode)
+      cli_msg(-15, "%s.%s: partial reload refused, please run full reload instead", c->proto->name, c->name);
+    else
+      bug("%s.%s: full reload refused");
 }
 
 const struct channel_class channel_basic = {

diff --git a/nest/protocol.h b/nest/protocol.h
index ae02ebb8e7c98ec1562442cdc98a619be812f220..f5d8a757181bf10fd4217212e983a82f7710d5a8 100644 (file)
--- a/nest/protocol.h
+++ b/nest/protocol.h
@@ -668,7 +668,12 @@ struct channel {
   u8 stale;                            /* Used in reconfiguration */
 
   u8 channel_state;
-  u8 reloadable;                       /* Hook reload_routes() is allowed on the channel */
+  PACKED enum channel_reloadable {
+    CHANNEL_RELOADABLE_NEVER = 0,      /* Never reload */
+    CHANNEL_RELOADABLE_REMOTELY = 1,   /* Hook reload_routes() causes remote actions */
+    CHANNEL_RELOADABLE_LOCALLY = 2,    /* Hook reload_routes() reloads routes locally */
+  } reloadable;
+
   OBSREF(struct graceful_recovery_context) gr_lock;    /* Graceful restart mechanism should wait for this channel */
   u8 gr_wait;                          /* Route export to channel is postponed until graceful restart */
 

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 5d5f85a2a0f8f10a97c60b24183fb97ff425d882..7c75978c5d9357e7f925ee52ca874870c5b92794 100644 (file)
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -1207,7 +1207,11 @@ bgp_conn_enter_established_state(struct bgp_conn *conn)
 
     int active = loc->ready && rem->ready;
     c->c.disabled = !active;
-    c->c.reloadable = p->route_refresh;
+
+    if (p->route_refresh)
+      c->c.reloadable = CHANNEL_RELOADABLE_REMOTELY;
+    else
+      c->c.reloadable = CHANNEL_RELOADABLE_NEVER;
 
     c->index = active ? num++ : 0;