bgpd: don't send NOTIFY twice for malformed attrs
Most of the attribute parsing functions were already sending a notify,
let's clean up the code to make it happen only once.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index f9fde9f..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.
@@ -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. */