[PATCH,RFC] Allow exchanging LOCAL_PREF with eBGP peers.

18 Feb 2017

Hello!

I've attached a patch that allows (selectively) exchanging LOCAL_PREF
with eBGP peers.

The BGP RFC (RFC4271) says that you shouldn't send LOCAL_PREF to eBGP
peers, but most modern BGP implementations have an override to allow
doing this anyway, and it is very useful in some scenarios, for example
if you have a network topology a la RFC7938.

This patch enables this functionality by the 'ebgp localpref' clause
in a bgp protocol statement, for example:

	protocol bgp mypeer {
		[...];
		ebgp localpref rx;
	}

This option works like 'add paths', in that the possible arguments are
"rx" or "tx" or a bool (where "yes" means both rx and tx).

RFC4271 also specifies that if LOCAL_PREF is received from an eBGP
speaker, it should be ignored (and you should not reply with a
NOTIFICATION), so it should be safe to send LOCAL_PREF to eBGP peers
that are not expecting it, and it's safe to send it to another bird
-- but there are various reasons why you might not want to do that,
of course.

I'm not submitting this patch for inclusion yet at this point (which is
why there is no documentation), but this kind of functionality is useful
for us, and I'd like to hear from people whether they think this could
be useful for them, too.  And of course, there will have to be a lot of
bikeshedding about the name of the config option! :-)


Cheers,
Lennert


Signed-off-by: Lennert Buytenhek <lbuytenhek@fastly.com>

diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 9d23374..3730021 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -307,7 +307,7 @@ static struct attr_desc bgp_attr_table[] = {
     bgp_check_next_hop, bgp_format_next_hop },
   { "med", 4, BAF_OPTIONAL, EAF_TYPE_INT, 1,					/* BA_MULTI_EXIT_DISC */
     NULL, NULL },
-  { "local_pref", 4, BAF_TRANSITIVE, EAF_TYPE_INT, 0,				/* BA_LOCAL_PREF */
+  { "local_pref", 4, BAF_TRANSITIVE, EAF_TYPE_INT, 1,				/* BA_LOCAL_PREF */
     NULL, NULL },
   { "atomic_aggr", 0, BAF_TRANSITIVE, EAF_TYPE_OPAQUE, 1,			/* BA_ATOMIC_AGGR */
     NULL, NULL },
@@ -822,8 +822,13 @@ bgp_get_bucket(struct bgp_proto *p, net *n, ea_list *attrs, int originate)
       code = EA_ID(a->id);
       if (ATTR_KNOWN(code))
 	{
-	  if (!bgp_attr_table[code].allow_in_ebgp && !p->is_internal)
-	    continue;
+	  if (!p->is_internal)
+	    {
+	      if (!bgp_attr_table[code].allow_in_ebgp)
+		continue;
+	      if (code == BA_LOCAL_PREF && !(p->cf->ebgp_local_pref & EBGP_LOCAL_PREF_TX))
+		continue;
+	    }
 	  /* The flags might have been zero if the attr was added by filters */
 	  a->flags = (a->flags & BAF_PARTIAL) | bgp_attr_table[code].expected_flags;
 	  if (code < 32)
@@ -1777,8 +1782,13 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr, uint len, struct linpool *po
 	    { errcode = 5; goto err; }
 	  if ((desc->expected_flags ^ flags) & (BAF_OPTIONAL | BAF_TRANSITIVE))
 	    { errcode = 4; goto err; }
-	  if (!desc->allow_in_ebgp && !bgp->is_internal)
-	    continue;
+	  if (!bgp->is_internal)
+	    {
+	      if (!desc->allow_in_ebgp)
+		continue;
+	      if (code == BA_LOCAL_PREF && !(bgp->cf->ebgp_local_pref & EBGP_LOCAL_PREF_RX))
+		continue;
+	    }
 	  if (desc->validate)
 	    {
 	      errcode = desc->validate(bgp, z, l);
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 0f1c944..fae4be5 100644
--- a/proto/bgp/bgp.c
+++ b/proto/bgp/bgp.c
@@ -1556,7 +1556,7 @@ bgp_show_proto_info(struct proto *P)
 	      (c->peer_add_path & ADD_PATH_RX) ? " add-path-rx" : "",
 	      (c->peer_add_path & ADD_PATH_TX) ? " add-path-tx" : "",
 	      c->peer_ext_messages_support ? " ext-messages" : "");
-      cli_msg(-1006, "    Session:          %s%s%s%s%s%s%s%s",
+      cli_msg(-1006, "    Session:          %s%s%s%s%s%s%s%s%s%s",
 	      p->is_internal ? "internal" : "external",
 	      p->cf->multihop ? " multihop" : "",
 	      p->rr_client ? " route-reflector" : "",
@@ -1564,7 +1564,9 @@ bgp_show_proto_info(struct proto *P)
 	      p->as4_session ? " AS4" : "",
 	      p->add_path_rx ? " add-path-rx" : "",
 	      p->add_path_tx ? " add-path-tx" : "",
-	      p->ext_messages ? " ext-messages" : "");
+	      p->ext_messages ? " ext-messages" : "",
+	      (p->cf->ebgp_local_pref & EBGP_LOCAL_PREF_RX) ? " ebgp-localpref-rx" : "",
+	      (p->cf->ebgp_local_pref & EBGP_LOCAL_PREF_TX) ? " ebgp-localpref-tx" : "");
       cli_msg(-1006, "    Source address:   %I", p->source_addr);
       if (P->cf->in_limit)
 	cli_msg(-1006, "    Route limit:      %d/%d",
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index d028bef..6e68d35 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -49,6 +49,7 @@ struct bgp_config {
   int interpret_communities;		/* Hardwired handling of well-known communities */
   int secondary;			/* Accept also non-best routes (i.e. RA_ACCEPTED) */
   int add_path;				/* Use ADD-PATH extension [draft] */
+  int ebgp_local_pref;			/* Exchange LOCAL_PREF with eBGP peers */
   int allow_local_as;			/* Allow that number of local ASNs in incoming AS_PATHs */
   int gr_mode;				/* Graceful restart mode (BGP_GR_*) */
   int setkey;				/* Set MD5 password to system SA/SP database */
@@ -79,6 +80,9 @@ struct bgp_config {
 #define ADD_PATH_TX 2
 #define ADD_PATH_FULL 3
 
+#define EBGP_LOCAL_PREF_RX 1
+#define EBGP_LOCAL_PREF_TX 2
+
 #define BGP_GR_ABLE 1
 #define BGP_GR_AWARE 2
 
diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y
index 32ae88a..5db122d 100644
--- a/proto/bgp/config.Y
+++ b/proto/bgp/config.Y
@@ -27,7 +27,8 @@ CF_KEYWORDS(BGP, LOCAL, NEIGHBOR, AS, HOLD, TIME, CONNECT, RETRY,
 	INTERPRET, COMMUNITIES, BGP_ORIGINATOR_ID, BGP_CLUSTER_LIST, IGP,
 	TABLE, GATEWAY, DIRECT, RECURSIVE, MED, TTL, SECURITY, DETERMINISTIC,
 	SECONDARY, ALLOW, BFD, ADD, PATHS, RX, TX, GRACEFUL, RESTART, AWARE,
-	CHECK, LINK, PORT, EXTENDED, MESSAGES, SETKEY, BGP_LARGE_COMMUNITY)
+	CHECK, LINK, PORT, EXTENDED, MESSAGES, SETKEY, BGP_LARGE_COMMUNITY,
+	EBGP, LOCALPREF)
 
 CF_GRAMMAR
 
@@ -126,6 +127,9 @@ bgp_proto:
  | bgp_proto ADD PATHS RX ';' { BGP_CFG->add_path = ADD_PATH_RX; }
  | bgp_proto ADD PATHS TX ';' { BGP_CFG->add_path = ADD_PATH_TX; }
  | bgp_proto ADD PATHS bool ';' { BGP_CFG->add_path = $4 ? ADD_PATH_FULL : 0; }
+ | bgp_proto EBGP LOCALPREF RX ';' { BGP_CFG->ebgp_local_pref = EBGP_LOCAL_PREF_RX; }
+ | bgp_proto EBGP LOCALPREF TX ';' { BGP_CFG->ebgp_local_pref = EBGP_LOCAL_PREF_TX; }
+ | bgp_proto EBGP LOCALPREF bool ';' { BGP_CFG->ebgp_local_pref = $4 ? (EBGP_LOCAL_PREF_RX | EBGP_LOCAL_PREF_TX) : 0; }
  | bgp_proto ALLOW LOCAL AS ';' { BGP_CFG->allow_local_as = -1; }
  | bgp_proto ALLOW LOCAL AS expr ';' { BGP_CFG->allow_local_as = $5; }
  | bgp_proto GRACEFUL RESTART bool ';' { BGP_CFG->gr_mode = $4; }

    