BGP: Extend 'next hop keep' and 'next hop self' options

Extend 'next hop keep' and 'next hop self' options to have boolean values
(enabled / disabled) and also values 'ibgp'/ 'ebgp' to restrict it to
routes received from IBGP / EBGP. This allows to have it enabled by
default in some cases, matches features of other implementations, and
allows to handle some strange cases like EBGP border router with 'next
hop self' also doing IBGP route reflecting.

Change default of 'next hop keep' to enabled for route servers, and
'ibgp' for route reflectors.

Update documentation for these options.

4 files changed, 45 insertions, 13 deletions

diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index ae4a1b0a..fb6c9881 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -1814,6 +1814,10 @@ bgp_postconfig(struct proto_config *CF)
     if ((cc->c.in_limit.action == PLA_RESTART) && cf->disable_after_error)
       cc->c.in_limit.action = PLA_DISABLE;
 
+    /* Different default based on rr_client, rs_client */
+    if (cc->next_hop_keep == 0xff)
+      cc->next_hop_keep = cf->rr_client ? NH_IBGP : (cf->rs_client ? NH_ALL : NH_NO);
+
     /* Different default based on rs_client */
     if (!cc->missing_lladdr)
       cc->missing_lladdr = cf->rs_client ? MLL_IGNORE : MLL_SELF;
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index 76b835fa..2b60f90f 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -134,8 +134,8 @@ struct bgp_channel_config {
   const struct bgp_af_desc *desc;
 
   ip_addr next_hop_addr;		/* Local address for NEXT_HOP attribute */
-  u8 next_hop_self;			/* Always set next hop to local IP address */
-  u8 next_hop_keep;			/* Do not touch next hop attribute */
+  u8 next_hop_self;			/* Always set next hop to local IP address (NH_*) */
+  u8 next_hop_keep;			/* Do not modify next hop attribute (NH_*) */
   u8 missing_lladdr;			/* What we will do when we don' know link-local addr, see MLL_* */
   u8 gw_mode;				/* How we compute route gateway from next_hop attr, see GW_* */
   u8 secondary;				/* Accept also non-best routes (i.e. RA_ACCEPTED) */
@@ -151,12 +151,17 @@ struct bgp_channel_config {
   struct rtable_config *igp_table_ip6;	/* Table for recursive IPv6 next hop lookups */
 };
 
-#define MLL_SELF 1
-#define MLL_DROP 2
-#define MLL_IGNORE 3
+#define NH_NO			0
+#define NH_ALL			1
+#define NH_IBGP			2
+#define NH_EBGP			3
 
-#define GW_DIRECT 1
-#define GW_RECURSIVE 2
+#define MLL_SELF		1
+#define MLL_DROP		2
+#define MLL_IGNORE		3
+
+#define GW_DIRECT		1
+#define GW_RECURSIVE		2
 
 #define BGP_ADD_PATH_RX		1
 #define BGP_ADD_PATH_TX		2
diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y
index f155eee2..ac8d024a 100644
--- a/proto/bgp/config.Y
+++ b/proto/bgp/config.Y
@@ -29,8 +29,9 @@ CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, CONNECT, RETRY, KEEPALIVE,
 	SECURITY, DETERMINISTIC, SECONDARY, ALLOW, BFD, ADD, PATHS, RX, TX,
 	GRACEFUL, RESTART, AWARE, CHECK, LINK, PORT, EXTENDED, MESSAGES, SETKEY,
 	STRICT, BIND, CONFEDERATION, MEMBER, MULTICAST, FLOW4, FLOW6, LONG,
-	LIVED, STALE, IMPORT)
+	LIVED, STALE, IMPORT, IBGP, EBGP)
 
+%type <i> bgp_nh
 %type <i32> bgp_afi
 
 CF_KEYWORDS(CEASE, PREFIX, LIMIT, HIT, ADMINISTRATIVE, SHUTDOWN, RESET, PEER,
@@ -205,17 +206,23 @@ bgp_channel_start: bgp_afi
     BGP_CC->c.ra_mode = RA_UNDEF;
     BGP_CC->afi = $1;
     BGP_CC->desc = desc;
+    BGP_CC->next_hop_keep = 0xff; /* undefined */
     BGP_CC->gr_able = 0xff;	/* undefined */
     BGP_CC->llgr_able = 0xff;	/* undefined */
     BGP_CC->llgr_time = ~0U;	/* undefined */
   }
 };
 
+bgp_nh:
+   bool { $$ = $1; }
+ | IBGP { $$ = NH_IBGP; }
+ | EBGP { $$ = NH_EBGP; }
+
 bgp_channel_item:
    channel_item
  | NEXT HOP ADDRESS ipa { BGP_CC->next_hop_addr = $4; }
- | NEXT HOP SELF { BGP_CC->next_hop_self = 1; BGP_CC->next_hop_keep = 0; }
- | NEXT HOP KEEP { BGP_CC->next_hop_keep = 1; BGP_CC->next_hop_self = 0; }
+ | NEXT HOP SELF bgp_nh { BGP_CC->next_hop_self = $4; }
+ | NEXT HOP KEEP bgp_nh { BGP_CC->next_hop_keep = $4; }
  | MISSING LLADDR SELF { BGP_CC->missing_lladdr = MLL_SELF; }
  | MISSING LLADDR DROP { BGP_CC->missing_lladdr = MLL_DROP; }
  | MISSING LLADDR IGNORE { BGP_CC->missing_lladdr = MLL_IGNORE; }
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index e7089eba..26716573 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -851,6 +851,19 @@ bgp_apply_mpls_labels(struct bgp_parse_state *s, rta *a, u32 *labels, uint lnum)
 }
 
 
+static int
+bgp_match_src(struct bgp_export_state *s, int mode)
+{
+  switch (mode)
+  {
+  case NH_NO:		return 0;
+  case NH_ALL:		return 1;
+  case NH_IBGP:		return s->src && s->src->is_internal;
+  case NH_EBGP:		return s->src && !s->src->is_internal;
+  default:		return 0;
+  }
+}
+
 static inline int
 bgp_use_next_hop(struct bgp_export_state *s, eattr *a)
 {
@@ -858,10 +871,12 @@ bgp_use_next_hop(struct bgp_export_state *s, eattr *a)
   struct bgp_channel *c = s->channel;
   ip_addr *nh = (void *) a->u.ptr->data;
 
-  if (s->channel->cf->next_hop_self)
+  /* Handle next hop self option */
+  if (c->cf->next_hop_self && bgp_match_src(s, c->cf->next_hop_self))
     return 0;
 
-  if (s->channel->cf->next_hop_keep)
+  /* Handle next hop keep option */
+  if (c->cf->next_hop_keep && bgp_match_src(s, c->cf->next_hop_keep))
     return 1;
 
   /* Keep it when explicitly set in export filter */
@@ -888,7 +903,8 @@ bgp_use_gateway(struct bgp_export_state *s)
   struct bgp_channel *c = s->channel;
   rta *ra = s->route->attrs;
 
-  if (s->channel->cf->next_hop_self)
+  /* Handle next hop self option - also applies to gateway */
+  if (c->cf->next_hop_self && bgp_match_src(s, c->cf->next_hop_self))
     return 0;
 
   /* We need one valid global gateway */