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>
This commit is contained in:
David Lamparter 2014-06-04 01:01:10 +02:00
parent bb02b82354
commit f57000c0db
2 changed files with 25 additions and 12 deletions

View File

@ -794,7 +794,7 @@ bgp_attr_malformed (struct bgp_attr_parser_args *args, u_char subcode,
return BGP_ATTR_PARSE_WITHDRAW; return BGP_ATTR_PARSE_WITHDRAW;
/* default to reset */ /* 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. /* Find out what is wrong with the path attribute flag bits and log the error.
@ -1483,7 +1483,7 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
{ {
zlog_info ("%s: %s sent invalid length, %lu", zlog_info ("%s: %s sent invalid length, %lu",
__func__, peer->host, (unsigned long)length); __func__, peer->host, (unsigned long)length);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
/* Load AFI, SAFI. */ /* Load AFI, SAFI. */
@ -1497,7 +1497,7 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
{ {
zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute", zlog_info ("%s: %s, MP nexthop length, %u, goes past end of attribute",
__func__, peer->host, attre->mp_nexthop_len); __func__, peer->host, attre->mp_nexthop_len);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
/* Nexthop length check. */ /* Nexthop length check. */
@ -1540,14 +1540,14 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
default: default:
zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d", zlog_info ("%s: (%s) Wrong multiprotocol next hop length: %d",
__func__, peer->host, attre->mp_nexthop_len); __func__, peer->host, attre->mp_nexthop_len);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
if (!LEN_LEFT) if (!LEN_LEFT)
{ {
zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)", zlog_info ("%s: (%s) Failed to read SNPA and NLRI(s)",
__func__, peer->host); __func__, peer->host);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
{ {
@ -1563,7 +1563,7 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
{ {
zlog_info ("%s: (%s) Failed to read NLRI", zlog_info ("%s: (%s) Failed to read NLRI",
__func__, peer->host); __func__, peer->host);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
if (safi != SAFI_MPLS_LABELED_VPN) if (safi != SAFI_MPLS_LABELED_VPN)
@ -1573,7 +1573,7 @@ bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
{ {
zlog_info ("%s: (%s) NLRI doesn't pass sanity check", zlog_info ("%s: (%s) NLRI doesn't pass sanity check",
__func__, peer->host); __func__, peer->host);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
} }
@ -1605,7 +1605,7 @@ bgp_mp_unreach_parse (struct bgp_attr_parser_args *args,
#define BGP_MP_UNREACH_MIN_SIZE 3 #define BGP_MP_UNREACH_MIN_SIZE 3
if ((length > STREAM_READABLE(s)) || (length < BGP_MP_UNREACH_MIN_SIZE)) 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); afi = stream_getw (s);
safi = stream_getc (s); safi = stream_getc (s);
@ -1616,7 +1616,7 @@ bgp_mp_unreach_parse (struct bgp_attr_parser_args *args,
{ {
ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len); ret = bgp_nlri_sanity_check (peer, afi, stream_pnt (s), withdraw_len);
if (ret < 0) if (ret < 0)
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR_NOTIFYPLS;
} }
mp_withdraw->afi = afi; mp_withdraw->afi = afi;
@ -1913,6 +1913,14 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
break; 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 hard error occured immediately return to the caller. */
if (ret == BGP_ATTR_PARSE_ERROR) if (ret == BGP_ATTR_PARSE_ERROR)
{ {
@ -1920,9 +1928,6 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
"%s: Attribute %s, parse error", "%s: Attribute %s, parse error",
peer->host, peer->host,
LOOKUP (attr_str, type)); LOOKUP (attr_str, type));
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR);
if (as4_path) if (as4_path)
aspath_unintern (&as4_path); aspath_unintern (&as4_path);
return ret; return ret;
@ -1979,9 +1984,14 @@ bgp_attr_parse (struct peer *peer, struct attr *attr, bgp_size_t size,
* all attributes first, including these 32bit ones, and now, * all attributes first, including these 32bit ones, and now,
* afterwards, we look what and if something is to be done for as4. * 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, if (bgp_attr_munge_as4_attrs (peer, attr, as4_path,
as4_aggregator, &as4_aggregator_addr)) as4_aggregator, &as4_aggregator_addr))
{ {
bgp_notify_send (peer,
BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_ATTR);
if (as4_path) if (as4_path)
aspath_unintern (&as4_path); aspath_unintern (&as4_path);
return BGP_ATTR_PARSE_ERROR; return BGP_ATTR_PARSE_ERROR;

View File

@ -136,6 +136,9 @@ typedef enum {
BGP_ATTR_PARSE_PROCEED = 0, BGP_ATTR_PARSE_PROCEED = 0,
BGP_ATTR_PARSE_ERROR = -1, BGP_ATTR_PARSE_ERROR = -1,
BGP_ATTR_PARSE_WITHDRAW = -2, 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; } bgp_attr_parse_ret_t;
/* Prototypes. */ /* Prototypes. */