On Sun, Feb 19, 2017 at 12:57:29AM +0100, Ondrej Zajicek wrote:
> > 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).
>
> Hello
Hi!
> I think it is a useful feature. But rx/tx values seems like a bit
> overengineering. If for some obscure reason someone wants it just
> one way, it can easily be done in filters.
Good point, I agree. Here's v2, also with the corresponding change to
bird.sgml, against current 1.6 git HEAD. Are you happy with the name of
the config option?
diff --git a/doc/bird.sgml b/doc/bird.sgml
index 6af0e0f..513382d 100644
--- a/doc/bird.sgml
+++ b/doc/bird.sgml
@@ -2043,6 +2043,14 @@ using the following configuration parameters:
TX direction. When active, all available routes accepted by the export
filter are advertised to the neighbor. Default: off.
+ <tag><label id="bgp-ebgp-localpref">ebgp localpref <m/switch/</tag>
+ A standard BGP implementation will not send the Local Preference
+ attribute to eBGP neighbors and will ignore this attribute if received
+ from eBGP neighbors, as per <rfc id="4271">. When this option is
+ enabled on an eBGP session, this attribute will be sent to and
+ accepted from the peer, which is useful for example if you have a
+ setup like in <rfc id="7938">.
+
<tag><label id="bgp-allow-local-as">allow local as [<m/number/]</tag>
BGP prevents routing loops by rejecting received routes with the local
AS number in the AS path. This option allows to loose or disable the
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index 9d23374..257c0ff 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)
+ 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)
+ continue;
+ }
if (desc->validate)
{
errcode = desc->validate(bgp, z, l);
diff --git a/proto/bgp/bgp.c b/proto/bgp/bgp.c
index 0f1c944..dee4d91 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",
p->is_internal ? "internal" : "external",
p->cf->multihop ? " multihop" : "",
p->rr_client ? " route-reflector" : "",
@@ -1564,7 +1564,8 @@ 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-localpref" : "");
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..69c06d6 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 */
diff --git a/proto/bgp/config.Y b/proto/bgp/config.Y
index 32ae88a..4cc1291 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,7 @@ 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 bool ';' { BGP_CFG->ebgp_local_pref = $4; }
| 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; }
