*: merge branch stable/0.99.23
bgp extcommunity fixes from stable branch
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index a1fd165..fcf8255 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -794,7 +794,7 @@
return BGP_ATTR_PARSE_WITHDRAW;
/* default to reset */
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
/* Find out what is wrong with the path attribute flag bits and log the error.
@@ -1110,7 +1110,7 @@
if (IPV4_NET0 (nexthop_h) || IPV4_NET127 (nexthop_h) || IPV4_CLASS_DE (nexthop_h))
{
char buf[INET_ADDRSTRLEN];
- inet_ntop (AF_INET, &nexthop_h, buf, INET_ADDRSTRLEN);
+ inet_ntop (AF_INET, &nexthop_n, buf, INET_ADDRSTRLEN);
zlog (peer->log, LOG_ERR, "Martian nexthop %s", buf);
return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP,
@@ -1483,7 +1483,7 @@
{
zlog_info ("%s: %s sent invalid length, %lu",
__func__, peer->host, (unsigned long)length);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
/* Load AFI, SAFI. */
@@ -1497,7 +1497,7 @@
{
zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute",
__func__, peer->host, attre->mp_nexthop_len);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
/* Nexthop length check. */
@@ -1540,14 +1540,14 @@
default:
zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d",
__func__, peer->host, attre->mp_nexthop_len);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
if (!LEN_LEFT)
{
zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)",
__func__, peer->host);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
{
@@ -1563,7 +1563,7 @@
{
zlog_info ("%s: (%s) Failed to read NLRI",
__func__, peer->host);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
if (safi != SAFI_MPLS_LABELED_VPN)
@@ -1573,7 +1573,7 @@
{
zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
__func__, peer->host);
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
}
@@ -1605,7 +1605,7 @@
#define BGP_MP_UNREACH_MIN_SIZE 3
if ((length > STREAM_READABLE(s)) || (length < BGP_MP_UNREACH_MIN_SIZE))
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
afi = stream_getw (s);
safi = stream_getc (s);
@@ -1616,7 +1616,7 @@
{
ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
if (ret < 0)
- return BGP_ATTR_PARSE_ERROR;
+ return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
}
mp_withdraw->afi = afi;
@@ -1913,6 +1913,14 @@
break;
}
+ if (ret == BGP_ATTR_PARSE_ERROR_NOTIFYPLS)
+ {
+ bgp_notify_send (peer,
+ BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_MAL_ATTR);
+ ret = BGP_ATTR_PARSE_ERROR;
+ }
+
/* If hard error occured immediately return to the caller. */
if (ret == BGP_ATTR_PARSE_ERROR)
{
@@ -1920,9 +1928,6 @@
"%s: Attribute %s, parse error",
peer->host,
LOOKUP (attr_str, type));
- bgp_notify_send (peer,
- BGP_NOTIFY_UPDATE_ERR,
- BGP_NOTIFY_UPDATE_MAL_ATTR);
if (as4_path)
aspath_unintern (&as4_path);
return ret;
@@ -1979,9 +1984,14 @@
* all attributes first, including these 32bit ones, and now,
* afterwards, we look what and if something is to be done for as4.
*/
+ /* actually... this doesn't ever return failure currently, but
+ * better safe than sorry */
if (bgp_attr_munge_as4_attrs (peer, attr, as4_path,
as4_aggregator, &as4_aggregator_addr))
{
+ bgp_notify_send (peer,
+ BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_MAL_ATTR);
if (as4_path)
aspath_unintern (&as4_path);
return BGP_ATTR_PARSE_ERROR;
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index cdd5467..cb401e7 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -136,6 +136,9 @@
BGP_ATTR_PARSE_PROCEED = 0,
BGP_ATTR_PARSE_ERROR = -1,
BGP_ATTR_PARSE_WITHDRAW = -2,
+
+ /* only used internally, send notify + convert to BGP_ATTR_PARSE_ERROR */
+ BGP_ATTR_PARSE_ERROR_NOTIFYPLS = -3,
} bgp_attr_parse_ret_t;
/* Prototypes. */
diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c
index 80651f1..65c6cac 100644
--- a/bgpd/bgp_packet.c
+++ b/bgpd/bgp_packet.c
@@ -1720,7 +1720,10 @@
attr_parse_ret = bgp_attr_parse (peer, &attr, attribute_len,
&mp_update, &mp_withdraw);
if (attr_parse_ret == BGP_ATTR_PARSE_ERROR)
- return -1;
+ {
+ bgp_attr_unintern_sub (&attr);
+ return -1;
+ }
}
/* Logging the attribute. */
diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c
index 232a6a1..04cbb8a 100644
--- a/bgpd/bgp_route.c
+++ b/bgpd/bgp_route.c
@@ -713,11 +713,8 @@
peer->rmap_type = 0;
if (ret == RMAP_DENYMATCH)
- {
- /* Free newly generated AS path and community by route-map. */
- bgp_attr_flush (attr);
- return RMAP_DENY;
- }
+ /* caller has multiple error paths with bgp_attr_flush() */
+ return RMAP_DENY;
}
return RMAP_PERMIT;
}
@@ -2144,10 +2141,14 @@
new_attr.extra = &new_extra;
bgp_attr_dup (&new_attr, attr);
- /* Apply incoming route-map. */
+ /* Apply incoming route-map.
+ * NB: new_attr may now contain newly allocated values from route-map "set"
+ * commands, so we need bgp_attr_flush in the error paths, until we intern
+ * the attr (which takes over the memory references) */
if (bgp_input_modifier (peer, p, &new_attr, afi, safi) == RMAP_DENY)
{
reason = "route-map;";
+ bgp_attr_flush (&new_attr);
goto filtered;
}
@@ -2161,6 +2162,7 @@
&& ! CHECK_FLAG (peer->flags, PEER_FLAG_DISABLE_CONNECTED_CHECK))
{
reason = "non-connected next-hop;";
+ bgp_attr_flush (&new_attr);
goto filtered;
}
@@ -2171,6 +2173,7 @@
|| bgp_nexthop_self (&new_attr))
{
reason = "martian next-hop;";
+ bgp_attr_flush (&new_attr);
goto filtered;
}
}
diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c
index a0b1fb4..c498f58 100644
--- a/bgpd/bgp_routemap.c
+++ b/bgpd/bgp_routemap.c
@@ -1506,10 +1506,10 @@
/* `set extcommunity rt COMMUNITY' */
-/* For community set mechanism. */
+/* For community set mechanism. Used by _rt and _soo. */
static route_map_result_t
-route_set_ecommunity_rt (void *rule, struct prefix *prefix,
- route_map_object_t type, void *object)
+route_set_ecommunity (void *rule, struct prefix *prefix,
+ route_map_object_t type, void *object)
{
struct ecommunity *ecom;
struct ecommunity *new_ecom;
@@ -1528,14 +1528,19 @@
old_ecom = (bgp_attr_extra_get (bgp_info->attr))->ecommunity;
if (old_ecom)
- new_ecom = ecommunity_merge (ecommunity_dup (old_ecom), ecom);
+ {
+ new_ecom = ecommunity_merge (ecommunity_dup (old_ecom), ecom);
+
+ /* old_ecom->refcnt = 1 => owned elsewhere, e.g. bgp_update_receive()
+ * ->refcnt = 0 => set by a previous route-map statement */
+ if (!old_ecom->refcnt)
+ ecommunity_free (&old_ecom);
+ }
else
new_ecom = ecommunity_dup (ecom);
- bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
-
- if (old_ecom)
- ecommunity_unintern (&old_ecom);
+ /* will be intern()'d or attr_flush()'d by bgp_update_main() */
+ bgp_info->attr->extra->ecommunity = new_ecom;
bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
}
@@ -1554,9 +1559,9 @@
return ecommunity_intern (ecom);
}
-/* Free function for set community. */
+/* Free function for set community. Used by _rt and _soo */
static void
-route_set_ecommunity_rt_free (void *rule)
+route_set_ecommunity_free (void *rule)
{
struct ecommunity *ecom = rule;
ecommunity_unintern (&ecom);
@@ -1566,46 +1571,13 @@
struct route_map_rule_cmd route_set_ecommunity_rt_cmd =
{
"extcommunity rt",
- route_set_ecommunity_rt,
+ route_set_ecommunity,
route_set_ecommunity_rt_compile,
- route_set_ecommunity_rt_free,
+ route_set_ecommunity_free,
};
/* `set extcommunity soo COMMUNITY' */
-/* For community set mechanism. */
-static route_map_result_t
-route_set_ecommunity_soo (void *rule, struct prefix *prefix,
- route_map_object_t type, void *object)
-{
- struct ecommunity *ecom, *old_ecom, *new_ecom;
- struct bgp_info *bgp_info;
-
- if (type == RMAP_BGP)
- {
- ecom = rule;
- bgp_info = object;
-
- if (! ecom)
- return RMAP_OKAY;
-
- old_ecom = (bgp_attr_extra_get (bgp_info->attr))->ecommunity;
-
- if (old_ecom)
- new_ecom = ecommunity_merge (ecommunity_dup (old_ecom), ecom);
- else
- new_ecom = ecommunity_dup (ecom);
-
- bgp_info->attr->extra->ecommunity = ecommunity_intern (new_ecom);
-
- if (old_ecom)
- ecommunity_unintern (&old_ecom);
-
- bgp_info->attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
- }
- return RMAP_OKAY;
-}
-
/* Compile function for set community. */
static void *
route_set_ecommunity_soo_compile (const char *arg)
@@ -1619,21 +1591,13 @@
return ecommunity_intern (ecom);
}
-/* Free function for set community. */
-static void
-route_set_ecommunity_soo_free (void *rule)
-{
- struct ecommunity *ecom = rule;
- ecommunity_unintern (&ecom);
-}
-
/* Set community rule structure. */
struct route_map_rule_cmd route_set_ecommunity_soo_cmd =
{
"extcommunity soo",
- route_set_ecommunity_soo,
+ route_set_ecommunity,
route_set_ecommunity_soo_compile,
- route_set_ecommunity_soo_free,
+ route_set_ecommunity_free,
};
/* `set origin ORIGIN' */