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 3f5cf0d259ab8f21068d045208d9da69d01d9d10..2533044158c524738452c6e138af582aaa5ec5e5 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 71b839168914ac0446112a1da4ca462852a1c77d..d6f69a31e7a714d4482d5aa3294036dfe3803020 100644 (file)
--- a/nest/protocol.h
+++ b/nest/protocol.h
@@ -659,7 +659,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 a5ec4e6acab483bfb7a1cd7331cc8d1669e40920..aaf8c96c3a8b5d2922469e3a5e7ab247a979ed75 100644 (file)
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -756,7 +756,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;