I've created attached patches to prevent BGP route loops from turning
into ghosts (issue reported earlier to the mailing list:
http://marc.info/?l=bird-users&m=129433651229774&w=2).
Instead of completely ignoring received BGP routes containing the
router's own AS number, it should now handle them as if the prefixes are
unreachable.
We have tested the patch against git and it fixed the issue there. I've
also made an (untested) patch against bird 1.2.5.
--
Ivo
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index ff231b1..fe83be8 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -1278,7 +1278,7 @@ bgp_remove_as4_attrs(struct bgp_proto *p, rta *a)
* by a &rta.
*/
struct rta *
-bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct
linpool *pool, int mandatory)
+bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct
linpool *pool, int mandatory, int *status)
{
struct bgp_proto *bgp = conn->bgp;
rta *a = lp_alloc(pool, sizeof(struct rta));
@@ -1297,6 +1297,8 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr,
unsigned int len, struct lin
/* a->dest = RTD_ROUTER; -- set in bgp_set_next_hop() */
a->from = bgp->cf->remote_ip;
+ *status = 0;
+
/* Parse the attributes */
bzero(seen, sizeof(seen));
DBG("BGP: Parsing attributes\n");
@@ -1438,12 +1440,12 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr,
unsigned int len, struct lin
/* If the AS path attribute contains our AS, reject the routes */
if (bgp_as_path_loopy(bgp, a))
- goto loop;
+ *status = 1;
/* Two checks for IBGP loops caused by route reflection, RFC 4456 */
if (bgp_originator_id_loopy(bgp, a) ||
bgp_cluster_list_loopy(bgp, a))
- goto loop;
+ *status = 1;
/* If there's no local preference, define one */
if (!(seen[0] & (1 << BA_LOCAL_PREF)))
@@ -1451,10 +1453,6 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr,
unsigned int len, struct lin
return a;
-loop:
- DBG("BGP: Path loop!\n");
- return NULL;
-
malformed:
bgp_error(conn, 3, 1, NULL, 0);
return NULL;
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index b06f20a..ce0a178 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -180,7 +180,7 @@ static inline void set_next_hop(byte *b, ip_addr addr) {
((ip_addr *) b)[0] = ad
void bgp_attach_attr(struct ea_list **to, struct linpool *pool, unsigned attr,
uintptr_t val);
byte *bgp_attach_attr_wa(struct ea_list **to, struct linpool *pool, unsigned
attr, unsigned len);
-struct rta *bgp_decode_attrs(struct bgp_conn *conn, byte *a, unsigned int len,
struct linpool *pool, int mandatory);
+struct rta *bgp_decode_attrs(struct bgp_conn *conn, byte *a, unsigned int len,
struct linpool *pool, int mandatory, int *status);
int bgp_get_attr(struct eattr *e, byte *buf, int buflen);
int bgp_rte_better(struct rte *, struct rte *);
void bgp_rt_notify(struct proto *P, rtable *tbl UNUSED, net *n, rte *new, rte
*old UNUSED, ea_list *attrs);
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index bf98640..fa96216 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -845,6 +845,7 @@ bgp_do_rx_update(struct bgp_conn *conn,
ip_addr prefix;
net *n;
int err = 0, pxlen;
+ int attrstatus;
/* Withdraw routes */
while (withdrawn_len)
@@ -858,8 +859,21 @@ bgp_do_rx_update(struct bgp_conn *conn,
if (!attr_len && !nlri_len) /* shortcut */
return;
- a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, nlri_len);
- if (a0 && nlri_len && bgp_set_next_hop(p, a0))
+ a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, nlri_len,
&attrstatus);
+ if (!a0 || !nlri_len)
+ {
+ }
+ else if (attrstatus)
+ {
+ while (nlri_len)
+ {
+ DECODE_PREFIX(nlri, nlri_len);
+ DBG("Withdraw loop %I/%d\n", prefix, pxlen);
+ if (n = net_find(p->p.table, prefix, pxlen))
+ rte_update(p->p.table, n, &p->p, &p->p, NULL);
+ }
+ }
+ else if (bgp_set_next_hop(p, a0))
{
a = rta_lookup(a0);
while (nlri_len)
@@ -921,10 +935,11 @@ bgp_do_rx_update(struct bgp_conn *conn,
ip_addr prefix;
net *n;
int err = 0, pxlen;
+ int attrstatus;
p->mp_reach_len = 0;
p->mp_unreach_len = 0;
- a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, 0);
+ a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, 0, &attrstatus);
if (!a0)
return;
@@ -941,6 +956,17 @@ bgp_do_rx_update(struct bgp_conn *conn,
DO_NLRI(mp_reach)
{
+ if (attrstatus) {
+ len -= *x + 2;
+ x += *x + 2;
+ while (len)
+ {
+ DECODE_PREFIX(x, len);
+ DBG("Withdraw loop %I/%d\n", prefix, pxlen);
+ if (n = net_find(p->p.table, prefix, pxlen))
+ rte_update(p->p.table, n, &p->p, &p->p, NULL);
+ }
+ } else {
/* Create fake NEXT_HOP attribute */
if (len < 1 || (*x != 16 && *x != 32) || len < *x + 2)
goto bad;
@@ -980,6 +1006,7 @@ bgp_do_rx_update(struct bgp_conn *conn,
}
rta_free(a);
}
+ } //if attrstatus
}
return;
diff --git a/proto/bgp/attrs.c b/proto/bgp/attrs.c
index ff231b1..fe83be8 100644
--- a/proto/bgp/attrs.c
+++ b/proto/bgp/attrs.c
@@ -1278,7 +1278,7 @@ bgp_remove_as4_attrs(struct bgp_proto *p, rta *a)
* by a &rta.
*/
struct rta *
-bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct
linpool *pool, int mandatory)
+bgp_decode_attrs(struct bgp_conn *conn, byte *attr, unsigned int len, struct
linpool *pool, int mandatory, int *status)
{
struct bgp_proto *bgp = conn->bgp;
rta *a = lp_alloc(pool, sizeof(struct rta));
@@ -1297,6 +1297,8 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr,
unsigned int len, struct lin
/* a->dest = RTD_ROUTER; -- set in bgp_set_next_hop() */
a->from = bgp->cf->remote_ip;
+ *status = 0;
+
/* Parse the attributes */
bzero(seen, sizeof(seen));
DBG("BGP: Parsing attributes\n");
@@ -1438,12 +1440,12 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr,
unsigned int len, struct lin
/* If the AS path attribute contains our AS, reject the routes */
if (bgp_as_path_loopy(bgp, a))
- goto loop;
+ *status = 1;
/* Two checks for IBGP loops caused by route reflection, RFC 4456 */
if (bgp_originator_id_loopy(bgp, a) ||
bgp_cluster_list_loopy(bgp, a))
- goto loop;
+ *status = 1;
/* If there's no local preference, define one */
if (!(seen[0] & (1 << BA_LOCAL_PREF)))
@@ -1451,10 +1453,6 @@ bgp_decode_attrs(struct bgp_conn *conn, byte *attr,
unsigned int len, struct lin
return a;
-loop:
- DBG("BGP: Path loop!\n");
- return NULL;
-
malformed:
bgp_error(conn, 3, 1, NULL, 0);
return NULL;
diff --git a/proto/bgp/bgp.h b/proto/bgp/bgp.h
index b06f20a..ce0a178 100644
--- a/proto/bgp/bgp.h
+++ b/proto/bgp/bgp.h
@@ -180,7 +180,7 @@ static inline void set_next_hop(byte *b, ip_addr addr) {
((ip_addr *) b)[0] = ad
void bgp_attach_attr(struct ea_list **to, struct linpool *pool, unsigned attr,
uintptr_t val);
byte *bgp_attach_attr_wa(struct ea_list **to, struct linpool *pool, unsigned
attr, unsigned len);
-struct rta *bgp_decode_attrs(struct bgp_conn *conn, byte *a, unsigned int len,
struct linpool *pool, int mandatory);
+struct rta *bgp_decode_attrs(struct bgp_conn *conn, byte *a, unsigned int len,
struct linpool *pool, int mandatory, int *status);
int bgp_get_attr(struct eattr *e, byte *buf, int buflen);
int bgp_rte_better(struct rte *, struct rte *);
void bgp_rt_notify(struct proto *P, rtable *tbl UNUSED, net *n, rte *new, rte
*old UNUSED, ea_list *attrs);
diff --git a/proto/bgp/packets.c b/proto/bgp/packets.c
index bf98640..fa96216 100644
--- a/proto/bgp/packets.c
+++ b/proto/bgp/packets.c
@@ -845,6 +845,7 @@ bgp_do_rx_update(struct bgp_conn *conn,
ip_addr prefix;
net *n;
int err = 0, pxlen;
+ int attrstatus;
/* Withdraw routes */
while (withdrawn_len)
@@ -858,8 +859,21 @@ bgp_do_rx_update(struct bgp_conn *conn,
if (!attr_len && !nlri_len) /* shortcut */
return;
- a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, nlri_len);
- if (a0 && nlri_len && bgp_get_nexthop(p, a0))
+ a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, nlri_len,
&attrstatus);
+ if (!a0 || !nlri_len)
+ {
+ }
+ else if (attrstatus)
+ {
+ while (nlri_len)
+ {
+ DECODE_PREFIX(nlri, nlri_len);
+ DBG("Withdraw loop %I/%d\n", prefix, pxlen);
+ if (n = net_find(p->p.table, prefix, pxlen))
+ rte_update(p->p.table, n, &p->p, &p->p, NULL);
+ }
+ }
+ else if (bgp_get_nexthop(p, a0))
{
a = rta_lookup(a0);
while (nlri_len)
@@ -921,10 +935,11 @@ bgp_do_rx_update(struct bgp_conn *conn,
ip_addr prefix;
net *n;
int err = 0, pxlen;
+ int attrstatus;
p->mp_reach_len = 0;
p->mp_unreach_len = 0;
- a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, 0);
+ a0 = bgp_decode_attrs(conn, attrs, attr_len, bgp_linpool, 0, &attrstatus);
if (!a0)
return;
@@ -941,6 +956,17 @@ bgp_do_rx_update(struct bgp_conn *conn,
DO_NLRI(mp_reach)
{
+ if (attrstatus) {
+ len -= *x + 2;
+ x += *x + 2;
+ while (len)
+ {
+ DECODE_PREFIX(x, len);
+ DBG("Withdraw loop %I/%d\n", prefix, pxlen);
+ if (n = net_find(p->p.table, prefix, pxlen))
+ rte_update(p->p.table, n, &p->p, &p->p, NULL);
+ }
+ } else {
/* Create fake NEXT_HOP attribute */
if (len < 1 || (*x != 16 && *x != 32) || len < *x + 2)
goto bad;
@@ -980,6 +1006,7 @@ bgp_do_rx_update(struct bgp_conn *conn,
}
rta_free(a);
}
+ } //if attrstatus
}
return;
