bgpd: general MP/SAFI improvements
This fixes some minor mixups particularly in MPLS-related SAFIs, as well
as doing some stylistic changes & adding comments.
Signed-off-by: Lou Berger <lberger@labn.net>
Reviewed-by: David Lamparter <equinox@opensourcerouting.org>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index ecf1c31..a25ea76 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -1613,7 +1613,7 @@
if (safi != SAFI_MPLS_LABELED_VPN)
{
- ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), nlri_len);
+ ret = bgp_nlri_sanity_check (peer, afi, safi, stream_pnt (s), nlri_len);
if (ret < 0)
{
zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
@@ -1662,7 +1662,7 @@
if (safi != SAFI_MPLS_LABELED_VPN)
{
- ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
+ ret = bgp_nlri_sanity_check (peer, afi, safi, stream_pnt (s), withdraw_len);
if (ret < 0)
return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
@@ -2160,8 +2160,9 @@
stream_putc (s, BGP_ATTR_MP_REACH_NLRI);
sizep = stream_get_endp (s);
stream_putw (s, 0); /* Marker: Attribute length. */
- stream_putw (s, afi); /* AFI */
- stream_putc (s, safi); /* SAFI */
+
+ stream_putw (s, afi);
+ stream_putc (s, (safi == SAFI_MPLS_VPN) ? SAFI_MPLS_LABELED_VPN : safi);
/* Nexthop */
switch (afi)
@@ -2169,17 +2170,17 @@
case AFI_IP:
switch (safi)
{
- case SAFI_UNICAST:
case SAFI_MULTICAST:
stream_putc (s, 4);
stream_put_ipv4 (s, attr->nexthop.s_addr);
break;
case SAFI_MPLS_VPN:
stream_putc (s, 12);
- stream_putl (s, 0);
+ stream_putl (s, 0); /* RD = 0, per RFC */
stream_putl (s, 0);
stream_put (s, &attr->extra->mp_nexthop_global_in, 4);
break;
+ case SAFI_UNICAST: /* invalid for IPv4 */
default:
break;
}
@@ -2199,6 +2200,28 @@
if (attre->mp_nexthop_len == 32)
stream_put (s, &attre->mp_nexthop_local, 16);
}
+ break;
+ case SAFI_MPLS_VPN:
+ {
+ struct attr_extra *attre = attr->extra;
+
+ assert (attr->extra);
+ if (attre->mp_nexthop_len == 16) {
+ stream_putc (s, 24);
+ stream_putl (s, 0); /* RD = 0, per RFC */
+ stream_putl (s, 0);
+ stream_put (s, &attre->mp_nexthop_global, 16);
+ } else if (attre->mp_nexthop_len == 32) {
+ stream_putc (s, 48);
+ stream_putl (s, 0); /* RD = 0, per RFC */
+ stream_putl (s, 0);
+ stream_put (s, &attre->mp_nexthop_global, 16);
+ stream_putl (s, 0); /* RD = 0, per RFC */
+ stream_putl (s, 0);
+ stream_put (s, &attre->mp_nexthop_local, 16);
+ }
+ }
+ break;
default:
break;
}
@@ -2218,20 +2241,25 @@
struct prefix *p, struct prefix_rd *prd,
u_char *tag)
{
- switch (safi)
+ if (safi == SAFI_MPLS_VPN)
{
- case SAFI_MPLS_VPN:
/* Tag, RD, Prefix write. */
stream_putc (s, p->prefixlen + 88);
stream_put (s, tag, 3);
stream_put (s, prd->val, 8);
stream_put (s, &p->u.prefix, PSIZE (p->prefixlen));
- break;
- default:
- /* Prefix write. */
- stream_put_prefix (s, p);
- break;
}
+ else
+ stream_put_prefix (s, p);
+}
+
+size_t
+bgp_packet_mpattr_prefix_size (afi_t afi, safi_t safi, struct prefix *p)
+{
+ int size = PSIZE (p->prefixlen);
+ if (safi == SAFI_MPLS_VPN)
+ size += 88;
+ return size;
}
void
@@ -2255,7 +2283,6 @@
int send_as4_path = 0;
int send_as4_aggregator = 0;
int use32bit = (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV)) ? 1 : 0;
- size_t mpattrlen_pos = 0;
if (! bgp)
bgp = bgp_get_default ();
@@ -2265,6 +2292,7 @@
if (p && !(afi == AFI_IP && safi == SAFI_UNICAST))
{
+ size_t mpattrlen_pos = 0;
mpattrlen_pos = bgp_packet_mpattr_start(s, afi, safi, attr);
bgp_packet_mpattr_prefix(s, afi, safi, p, prd, tag);
bgp_packet_mpattr_end(s, mpattrlen_pos);
@@ -2338,7 +2366,8 @@
send_as4_path = 1; /* we'll do this later, at the correct place */
/* Nexthop attribute. */
- if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP) && afi == AFI_IP)
+ if (attr->flag & ATTR_FLAG_BIT (BGP_ATTR_NEXT_HOP) && afi == AFI_IP &&
+ safi == SAFI_UNICAST) /* only write NH attr for unicast safi */
{
stream_putc (s, BGP_ATTR_FLAG_TRANS);
stream_putc (s, BGP_ATTR_NEXT_HOP);
@@ -2612,8 +2641,7 @@
stream_putw (s, 0); /* Length of this attribute. */
stream_putw (s, afi);
- safi = (safi == SAFI_MPLS_VPN) ? SAFI_MPLS_LABELED_VPN : safi;
- stream_putc (s, safi);
+ stream_putc (s, (safi == SAFI_MPLS_VPN) ? SAFI_MPLS_LABELED_VPN : safi);
return attrlen_pnt;
}
@@ -2622,26 +2650,13 @@
afi_t afi, safi_t safi, struct prefix_rd *prd,
u_char *tag)
{
- if (safi == SAFI_MPLS_VPN)
- {
- stream_putc (s, p->prefixlen + 88);
- stream_put (s, tag, 3);
- stream_put (s, prd->val, 8);
- stream_put (s, &p->u.prefix, PSIZE (p->prefixlen));
- }
- else
- stream_put_prefix (s, p);
+ bgp_packet_mpattr_prefix (s, afi, safi, p, prd, tag);
}
void
bgp_packet_mpunreach_end (struct stream *s, size_t attrlen_pnt)
{
- bgp_size_t size;
-
- /* Set MP attribute length. Don't count the (2) bytes used to encode
- the attr length */
- size = stream_get_endp (s) - attrlen_pnt - 2;
- stream_putw_at (s, attrlen_pnt, size);
+ bgp_packet_mpattr_end (s, attrlen_pnt);
}
/* Initialization of attribute. */
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index b59fa8e..6fdf1c1 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -205,6 +205,8 @@
extern void bgp_packet_mpattr_prefix(struct stream *s, afi_t afi, safi_t safi,
struct prefix *p, struct prefix_rd *prd,
u_char *tag);
+extern size_t bgp_packet_mpattr_prefix_size(afi_t afi, safi_t safi,
+ struct prefix *p);
extern void bgp_packet_mpattr_end(struct stream *s, size_t sizep);
extern size_t bgp_packet_mpunreach_start (struct stream *s, afi_t afi,
diff --git a/bgpd/bgp_mplsvpn.c b/bgpd/bgp_mplsvpn.c
index 78a71e7..e922665 100644
--- a/bgpd/bgp_mplsvpn.c
+++ b/bgpd/bgp_mplsvpn.c
@@ -178,13 +178,10 @@
decode_rd_ip (pnt + 5, &rd_ip);
break;
- case RD_TYPE_EOI:
- break;
-
- default:
- zlog_err ("Invalid RD type %d", type);
- return -1;
- }
+ default:
+ zlog_err ("Unknown RD type %d", type);
+ break; /* just report */
+ }
p.prefixlen = prefixlen - VPN_PREFIXLEN_MIN_BYTES*8;
memcpy (&p.u.prefix, pnt + VPN_PREFIXLEN_MIN_BYTES,
@@ -323,14 +320,6 @@
snprintf (buf, size, "%s:%d", inet_ntoa (rd_ip.ip), rd_ip.val);
return buf;
}
- else if (type == RD_TYPE_EOI)
- {
- snprintf(buf, size, "LHI:%d, %02x:%02x:%02x:%02x:%02x:%02x",
- pnt[1], /* LHI */
- pnt[2], pnt[3], pnt[4], pnt[5], pnt[6], pnt[7]); /* MAC */
- return buf;
- }
-
return NULL;
}
diff --git a/bgpd/bgp_mplsvpn.h b/bgpd/bgp_mplsvpn.h
index 4e9b317..3299b9c 100644
--- a/bgpd/bgp_mplsvpn.h
+++ b/bgpd/bgp_mplsvpn.h
@@ -24,7 +24,6 @@
#define RD_TYPE_AS 0
#define RD_TYPE_IP 1
#define RD_TYPE_AS4 2
-#define RD_TYPE_EOI 0xff00
#define RD_ADDRSTRLEN 28
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 16e1435..841eaab 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -172,16 +172,24 @@
/* When remaining space can't include NLRI and it's length. */
if (STREAM_CONCAT_REMAIN (s, snlri, STREAM_SIZE(s)) <=
- (BGP_NLRI_LENGTH + PSIZE (rn->p.prefixlen)))
+ (BGP_NLRI_LENGTH + bgp_packet_mpattr_prefix_size(afi,safi,&rn->p)))
break;
/* If packet is empty, set attribute. */
if (stream_empty (s))
{
+ struct prefix_rd *prd = NULL;
+ u_char *tag = NULL;
struct peer *from = NULL;
+ if (rn->prn)
+ prd = (struct prefix_rd *) &rn->prn->p;
if (binfo)
- from = binfo->peer;
+ {
+ from = binfo->peer;
+ if (binfo->extra)
+ tag = binfo->extra->tag;
+ }
/* 1: Write the BGP message header - 16 bytes marker, 2 bytes length,
* one byte message type.
@@ -204,8 +212,8 @@
/* 5: Encode all the attributes, except MP_REACH_NLRI attr. */
total_attr_len = bgp_packet_attribute (NULL, peer, s,
adv->baa->attr,
- NULL, afi, safi,
- from, NULL, NULL);
+ &rn->p, afi, safi,
+ from, prd, tag);
}
if (afi == AFI_IP && safi == SAFI_UNICAST)
@@ -1653,7 +1661,7 @@
/* Unfeasible Route packet format check. */
if (withdraw_len > 0)
{
- ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_pnt (s), withdraw_len);
+ ret = bgp_nlri_sanity_check (peer, AFI_IP, SAFI_UNICAST, stream_pnt (s), withdraw_len);
if (ret < 0)
return -1;
@@ -1713,6 +1721,7 @@
if (attr_parse_ret == BGP_ATTR_PARSE_ERROR)
{
bgp_attr_unintern_sub (&attr);
+ bgp_attr_flush (&attr);
return -1;
}
}
@@ -1744,10 +1753,11 @@
if (update_len)
{
/* Check NLRI packet format and prefix length. */
- ret = bgp_nlri_sanity_check (peer, AFI_IP, stream_pnt (s), update_len);
+ ret = bgp_nlri_sanity_check (peer, AFI_IP, SAFI_UNICAST, stream_pnt (s), update_len);
if (ret < 0)
{
bgp_attr_unintern_sub (&attr);
+ bgp_attr_flush (&attr);
return -1;
}
@@ -1933,6 +1943,7 @@
/* Everything is done. We unintern temporary structures which
interned in bgp_attr_parse(). */
bgp_attr_unintern_sub (&attr);
+ bgp_attr_flush (&attr);
/* If peering is stopped due to some reason, do not generate BGP
event. */
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 868fbe5..b024d80 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -1480,6 +1480,9 @@
struct attr attr;
struct attr_extra extra;
+ memset (&attr, 0, sizeof(struct attr));
+ memset (&extra, 0, sizeof(struct attr_extra));
+
p = &rn->p;
/* Announce route to Established peer. */
@@ -1519,6 +1522,7 @@
break;
}
+ bgp_attr_flush (&attr);
return 0;
}
@@ -1661,7 +1665,7 @@
}
}
- /* Reap old select bgp_info, it it has been removed */
+ /* Reap old select bgp_info, if it has been removed */
if (old_select && CHECK_FLAG (old_select->flags, BGP_INFO_REMOVED))
bgp_info_reap (rn, old_select);
@@ -1869,7 +1873,7 @@
static void
bgp_rib_withdraw (struct bgp_node *rn, struct bgp_info *ri, struct peer *peer,
- afi_t afi, safi_t safi)
+ afi_t afi, safi_t safi, struct prefix_rd *prd)
{
int status = BGP_DAMP_NONE;
@@ -1991,7 +1995,7 @@
bgp_unlock_node (rn);
bgp_attr_unintern (&attr_new);
-
+ bgp_attr_flush(&new_attr);
return;
}
@@ -2099,7 +2103,7 @@
/* Withdraw specified route from routing table. */
if (ri && ! CHECK_FLAG (ri->flags, BGP_INFO_HISTORY))
- bgp_rib_withdraw (rn, ri, peer, afi, safi);
+ bgp_rib_withdraw (rn, ri, peer, afi, safi, prd);
else if (BGP_DEBUG (update, UPDATE_IN))
zlog (peer->log, LOG_DEBUG,
"%s Can't find the route %s/%d", peer->host,
@@ -2274,6 +2278,7 @@
bgp_unlock_node (rn);
bgp_attr_unintern (&attr_new);
+ bgp_attr_flush (&new_attr);
return 0;
}
@@ -2326,6 +2331,8 @@
if (safi == SAFI_MPLS_VPN)
memcpy ((bgp_info_extra_get (ri))->tag, tag, 3);
+ bgp_attr_flush (&new_attr);
+
/* Update bgp route dampening information. */
if (CHECK_FLAG (bgp->af_flags[afi][safi], BGP_CONFIG_DAMPENING)
&& peer->sort == BGP_PEER_EBGP)
@@ -2355,6 +2362,8 @@
else
bgp_info_set_flag (rn, ri, BGP_INFO_VALID);
+ bgp_attr_flush (&new_attr);
+
/* Process change. */
bgp_aggregate_increment (bgp, p, ri, afi, safi);
@@ -2410,6 +2419,8 @@
/* route_node_get lock */
bgp_unlock_node (rn);
+ bgp_attr_flush (&new_attr);
+
/* If maximum prefix count is configured and current prefix
count exeed it. */
if (bgp_maximum_prefix_overflow (peer, afi, safi, 0))
@@ -2434,6 +2445,7 @@
bgp_rib_remove (rn, ri, peer, afi, safi);
bgp_unlock_node (rn);
+ bgp_attr_flush (&new_attr);
return 0;
}
@@ -2520,7 +2532,7 @@
/* Withdraw specified route from routing table. */
if (ri && ! CHECK_FLAG (ri->flags, BGP_INFO_HISTORY))
- bgp_rib_withdraw (rn, ri, peer, afi, safi);
+ bgp_rib_withdraw (rn, ri, peer, afi, safi, prd);
else if (BGP_DEBUG (update, UPDATE_IN))
zlog (peer->log, LOG_DEBUG,
"%s Can't find the route %s/%d", peer->host,
@@ -3006,8 +3018,7 @@
* unlock happens at the end of this function.
*/
if (!peer->clear_node_queue->thread)
- peer_lock (peer);
-
+ peer_lock (peer); /* bgp_clear_node_complete */
switch (purpose)
{
case BGP_CLEAR_ROUTE_NORMAL:
@@ -3026,6 +3037,11 @@
break;
case BGP_CLEAR_ROUTE_MY_RSCLIENT:
+ /*
+ * gpz 091009: TBD why don't we have special handling for
+ * SAFI_MPLS_VPN here in the original quagga code?
+ * (and, by extension, for SAFI_ENCAP)
+ */
bgp_clear_route_table (peer, afi, safi, NULL, peer, purpose);
break;
@@ -3304,8 +3320,8 @@
/* NLRI encode syntax check routine. */
int
-bgp_nlri_sanity_check (struct peer *peer, int afi, u_char *pnt,
- bgp_size_t length)
+bgp_nlri_sanity_check (struct peer *peer, int afi, safi_t safi,
+ u_char *pnt, bgp_size_t length)
{
u_char *end;
u_char prefixlen;
@@ -6739,6 +6755,7 @@
struct bgp_info *ri;
struct bgp_table *table;
+ memset (&match, 0, sizeof (struct prefix)); /* keep valgrind happy */
/* Check IP address argument. */
ret = str2prefix (ip_str, &match);
if (! ret)
diff --git a/bgpd/bgp_route.h b/bgpd/bgp_route.h
index 5a93b44..8e9bcbd 100644
--- a/bgpd/bgp_route.h
+++ b/bgpd/bgp_route.h
@@ -196,7 +196,7 @@
extern void bgp_info_set_flag (struct bgp_node *, struct bgp_info *, u_int32_t);
extern void bgp_info_unset_flag (struct bgp_node *, struct bgp_info *, u_int32_t);
-extern int bgp_nlri_sanity_check (struct peer *, int, u_char *, bgp_size_t);
+extern int bgp_nlri_sanity_check (struct peer *, int, safi_t, u_char *, bgp_size_t);
extern int bgp_nlri_parse (struct peer *, struct attr *, struct bgp_nlri *);
extern int bgp_maximum_prefix_overflow (struct peer *, afi_t, safi_t, int);
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index b449cae..a6d5a0e 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -2090,6 +2090,7 @@
/* Set next hop value. */
(bgp_attr_extra_get (bgp_info->attr))->mp_nexthop_global_in = *address;
+ (bgp_attr_extra_get (bgp_info->attr))->mp_nexthop_len = 4;
}
return RMAP_OKAY;