bgpd: Move up flag-check calls, parcel up attr-parser args, and other cleanups
* bgp_attr.h: (struct bgp_attr_parser_args) Attribute parsing context,
containing common arguments.
* bgp_attr.c: (general) Move the bgp_attr_flag_invalid flag-check calls up,
out of each individual attr parser function, to be done once in attr_parse.
Similarly move the calculation of the 'total' attribute length field up
to attr_parse.
Bundle together common arguments to attr-parsing functions and helpers
into (struct bgp_attr_parser_args), so it can be passed by reference down
the stack & also de-clutter the argument lists & make it easier to
add/modify the context for attr-parsing - add local const aliases to avoid
modifying body of code too much. This also should help avoid cut & paste
errors, where calls to helpers with hard-coded attribute types are pasted
to other functions but the code isn't changed.
(bgp_attr_flags_diagnose) as above.
(bgp_attr_flag_invalid) as above.
(bgp_attr_{origin,aspath,as4_path,nexthop,med,local_pref,atomic}) as above.
(bgp_attr_{aggregator,as4_aggregator,community,originator_id}) as above
(bgp_attr_{cluster_list,ext_communities},bgp_mp_{un,}reach_parse) as above
(bgp_attr_unknown) as above.
(bgp_attr_malformed) as above. Also, startp and length have to be
special-cased, because whether or not to send attribute data depends
on the particular error - a separate length argument, distinct from
args->length, indicates whether or not the attribute data should be sent
in the NOTIFY.
(bgp_attr_aspath_check) Call to bgp_attr_malformed is wrong here, there is
no attribute parsing context - e.g. the 'flag' argument is unlikely to be
right, remove it. Explicitly handle the error instead.
(bgp_attr_munge_as4_attrs) Flag argument is pointless.
As the comment notes, the check here is pointless as AS_PATH presence
already checked elsewhere.
(bgp_attr_parse) Do bgp_attr_flag_invalid call here.
Use (struct bgp_attr_parser_args) for args to attr parser functions.
Remove out-of-context 'flag' argument to as4 checking functions.
diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c
index d8ca831..b02cfee 100644
--- a/bgpd/bgp_attr.c
+++ b/bgpd/bgp_attr.c
@@ -710,9 +710,17 @@
* introduced by the sending neighbour.
*/
static bgp_attr_parse_ret_t
-bgp_attr_malformed (struct peer *peer, u_char type, u_char flag,
- u_char subcode, u_char *startp, bgp_size_t length)
+bgp_attr_malformed (struct bgp_attr_parser_args *args, u_char subcode,
+ bgp_size_t length)
{
+ struct peer *const peer = args->peer;
+ const u_int8_t flags = args->flags;
+ /* startp and length must be special-cased, as whether or not to
+ * send the attribute data with the NOTIFY depends on the error,
+ * the caller therefore signals this with the seperate length argument
+ */
+ u_char *startp = (length > 0 ? args->startp : NULL);
+
/* Only relax error handling for eBGP peers */
if (peer_sort (peer) != BGP_PEER_EBGP)
{
@@ -722,7 +730,7 @@
}
- switch (type) {
+ switch (args->type) {
/* where an optional attribute is inconsequential, e.g. it does not affect
* route selection, and can be safely ignored then any such attributes
* which are malformed should just be ignored and the route processed as
@@ -756,9 +764,9 @@
* the whole session to be reset. Instead treat it as a withdrawal
* of the routes, if possible.
*/
- if (CHECK_FLAG (flag, BGP_ATTR_FLAG_TRANS)
- && CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL)
- && CHECK_FLAG (flag, BGP_ATTR_FLAG_PARTIAL))
+ if (CHECK_FLAG (flags, BGP_ATTR_FLAG_TRANS)
+ && CHECK_FLAG (flags, BGP_ATTR_FLAG_OPTIONAL)
+ && CHECK_FLAG (flags, BGP_ATTR_FLAG_PARTIAL))
return BGP_ATTR_PARSE_WITHDRAW;
/* default to reset */
@@ -771,16 +779,14 @@
being diagnosed is defined by RFC as either a "well-known" or an "optional,
non-transitive" attribute. */
static void
-bgp_attr_flags_diagnose
-(
- struct peer * peer,
- const u_int8_t attr_code,
- u_int8_t desired_flags, /* how RFC says it must be */
- u_int8_t real_flags /* on wire */
+bgp_attr_flags_diagnose (struct bgp_attr_parser_args *args,
+ u_int8_t desired_flags /* how RFC says it must be */
)
{
u_char seen = 0, i;
-
+ u_char real_flags = args->flags;
+ const u_int8_t attr_code = args->type;
+
desired_flags &= ~BGP_ATTR_FLAG_EXTLEN;
real_flags &= ~BGP_ATTR_FLAG_EXTLEN;
for (i = 0; i <= 2; i++) /* O,T,P, but not E */
@@ -790,7 +796,7 @@
CHECK_FLAG (real_flags, attr_flag_str[i].key)
)
{
- zlog (peer->log, LOG_ERR, "%s attribute must%s be flagged as \"%s\"",
+ zlog (args->peer->log, LOG_ERR, "%s attribute must%s be flagged as \"%s\"",
LOOKUP (attr_str, attr_code),
CHECK_FLAG (desired_flags, attr_flag_str[i].key) ? "" : " not",
attr_flag_str[i].str);
@@ -823,9 +829,12 @@
sizeof (attr_flags_values) / sizeof (attr_flags_values[0]);
static int
-bgp_attr_flag_invalid (struct peer *peer, u_int8_t attr_code, u_int8_t flags)
+bgp_attr_flag_invalid (struct bgp_attr_parser_args *args)
{
u_int8_t mask = BGP_ATTR_FLAG_EXTLEN;
+ const u_int8_t flags = args->flags;
+ const u_int8_t attr_code = args->type;
+ struct peer *const peer = args->peer;
/* there may be attributes we don't know about */
if (attr_code > attr_flags_values_max)
@@ -880,31 +889,18 @@
== attr_flags_values[attr_code])
return 0;
- bgp_attr_flags_diagnose (peer, attr_code, attr_flags_values[attr_code],
- flags);
+ bgp_attr_flags_diagnose (args, attr_flags_values[attr_code]);
return 1;
}
/* Get origin attribute of the update message. */
static bgp_attr_parse_ret_t
-bgp_attr_origin (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_origin (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- /* total is entire attribute length include Attribute Flags (1),
- Attribute Type code (1) and Attribute length (1 or 2). */
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
- /* If any recognized attribute has Attribute Flags that conflict
- with the Attribute Type Code, then the Error Subcode is set to
- Attribute Flags Error. The Data field contains the erroneous
- attribute (type, length and value). */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_ORIGIN, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
-
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
/* If any recognized attribute has Attribute Length that conflicts
with the expected length (based on the attribute type code), then
the Error Subcode is set to Attribute Length Error. The Data
@@ -914,9 +910,9 @@
{
zlog (peer->log, LOG_ERR, "Origin attribute length is not one %d",
length);
- return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
/* Fetch origin attribute. */
@@ -931,9 +927,9 @@
{
zlog (peer->log, LOG_ERR, "Origin attribute value is invalid %d",
attr->origin);
- return bgp_attr_malformed (peer, BGP_ATTR_ORIGIN, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
- startp, total);
+ args->total);
}
/* Set oring attribute flag. */
@@ -945,17 +941,12 @@
/* Parse AS path information. This function is wrapper of
aspath_parse. */
static int
-bgp_attr_aspath (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_aspath (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+ struct attr *const attr = args->attr;
+ struct peer *const peer = args->peer;
+ const bgp_size_t length = args->length;
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_AS_PATH, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
/*
* peer with AS4 => will get 4Byte ASnums
* otherwise, will get 16 Bit
@@ -969,9 +960,7 @@
zlog (peer->log, LOG_ERR,
"Malformed AS path from %s, length is %d",
peer->host, length);
- return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH,
- NULL, 0);
+ return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_MAL_AS_PATH, 0);
}
/* Set aspath attribute flag. */
@@ -981,7 +970,7 @@
}
static bgp_attr_parse_ret_t
-bgp_attr_aspath_check (struct peer *peer, struct attr *attr, u_char flag)
+bgp_attr_aspath_check (struct peer *const peer, struct attr *const attr)
{
/* These checks were part of bgp_attr_aspath, but with
* as4 we should to check aspath things when
@@ -1000,9 +989,9 @@
(peer_sort (peer) == BGP_PEER_EBGP && aspath_confed_check (attr->aspath)))
{
zlog (peer->log, LOG_ERR, "Malformed AS path from %s", peer->host);
- return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH,
- NULL, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_MAL_AS_PATH);
+ return BGP_ATTR_PARSE_ERROR;
}
/* First AS check for EBGP. */
@@ -1013,9 +1002,9 @@
{
zlog (peer->log, LOG_ERR,
"%s incorrect first AS (must be %u)", peer->host, peer->as);
- return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
- BGP_NOTIFY_UPDATE_MAL_AS_PATH,
- NULL, 0);
+ bgp_notify_send (peer, BGP_NOTIFY_UPDATE_ERR,
+ BGP_NOTIFY_UPDATE_MAL_AS_PATH);
+ return BGP_ATTR_PARSE_ERROR;
}
}
@@ -1035,19 +1024,11 @@
/* Parse AS4 path information. This function is another wrapper of
aspath_parse. */
static int
-bgp_attr_as4_path (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp,
- struct aspath **as4_path)
+bgp_attr_as4_path (struct bgp_attr_parser_args *args, struct aspath **as4_path)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
- /* Flag check. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_AS4_PATH, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_AS4_PATH, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
*as4_path = aspath_parse (peer->ibuf, length, 1);
@@ -1057,9 +1038,9 @@
zlog (peer->log, LOG_ERR,
"Malformed AS4 path from %s, length is %d",
peer->host, length);
- return bgp_attr_malformed (peer, BGP_ATTR_AS4_PATH, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_MAL_AS_PATH,
- NULL, 0);
+ 0);
}
/* Set aspath attribute flag. */
@@ -1071,29 +1052,23 @@
/* Nexthop attribute. */
static bgp_attr_parse_ret_t
-bgp_attr_nexthop (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_nexthop (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
in_addr_t nexthop_h, nexthop_n;
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
- /* Flags check. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_NEXT_HOP, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
-
/* Check nexthop attribute length. */
if (length != 4)
{
zlog (peer->log, LOG_ERR, "Nexthop attribute length isn't four [%d]",
length);
- return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
/* According to section 6.3 of RFC4271, syntactically incorrect NEXT_HOP
@@ -1108,9 +1083,9 @@
char buf[INET_ADDRSTRLEN];
inet_ntop (AF_INET, &nexthop_h, buf, INET_ADDRSTRLEN);
zlog (peer->log, LOG_ERR, "Martian nexthop %s", buf);
- return bgp_attr_malformed (peer, BGP_ATTR_NEXT_HOP, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP,
- startp, total);
+ args->total);
}
attr->nexthop.s_addr = nexthop_n;
@@ -1121,28 +1096,21 @@
/* MED atrribute. */
static bgp_attr_parse_ret_t
-bgp_attr_med (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_med (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_MULTI_EXIT_DISC, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
-
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
/* Length check. */
if (length != 4)
{
zlog (peer->log, LOG_ERR,
"MED attribute length isn't four [%d]", length);
- return bgp_attr_malformed (peer, BGP_ATTR_MULTI_EXIT_DISC, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
attr->med = stream_getl (peer->ibuf);
@@ -1154,25 +1122,20 @@
/* Local preference attribute. */
static bgp_attr_parse_ret_t
-bgp_attr_local_pref (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_local_pref (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_LOCAL_PREF, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_LOCAL_PREF, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
/* Length check. */
if (length != 4)
{
- zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute length isn't 4 [%u]", length);
- return bgp_attr_malformed (peer, BGP_ATTR_LOCAL_PREF, flag,
+ zlog (peer->log, LOG_ERR, "LOCAL_PREF attribute length isn't 4 [%u]",
+ length);
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
/* If it is contained in an UPDATE message that is received from an
@@ -1194,25 +1157,20 @@
/* Atomic aggregate. */
static int
-bgp_attr_atomic (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_atomic (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
/* Length check. */
if (length != 0)
{
- zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute length isn't 0 [%u]", length);
- return bgp_attr_malformed (peer, BGP_ATTR_ATOMIC_AGGREGATE, flag,
+ zlog (peer->log, LOG_ERR, "ATOMIC_AGGREGATE attribute length isn't 0 [%u]",
+ length);
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
/* Set atomic aggregate flag. */
@@ -1223,19 +1181,14 @@
/* Aggregator attribute */
static int
-bgp_attr_aggregator (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_aggregator (struct bgp_attr_parser_args *args)
{
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
int wantedlen = 6;
struct attr_extra *attre = bgp_attr_extra_get (attr);
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flags check. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_AGGREGATOR, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
/* peer with AS4 will send 4 Byte AS, peer without will send 2 Byte */
if (CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV))
@@ -1243,10 +1196,11 @@
if (length != wantedlen)
{
- zlog (peer->log, LOG_ERR, "AGGREGATOR attribute length isn't %u [%u]", wantedlen, length);
- return bgp_attr_malformed (peer, BGP_ATTR_AGGREGATOR, flag,
+ zlog (peer->log, LOG_ERR, "AGGREGATOR attribute length isn't %u [%u]",
+ wantedlen, length);
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
if ( CHECK_FLAG (peer->cap, PEER_CAP_AS4_RCV ) )
@@ -1263,27 +1217,22 @@
/* New Aggregator attribute */
static bgp_attr_parse_ret_t
-bgp_attr_as4_aggregator (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag,
- as_t *as4_aggregator_as,
- struct in_addr *as4_aggregator_addr,
- u_char *startp)
+bgp_attr_as4_aggregator (struct bgp_attr_parser_args *args,
+ as_t *as4_aggregator_as,
+ struct in_addr *as4_aggregator_addr)
{
- bgp_size_t total;
-
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
if (length != 8)
{
- zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]", length);
- return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag,
+ zlog (peer->log, LOG_ERR, "New Aggregator length is not 8 [%d]",
+ length);
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- NULL, 0);
+ 0);
}
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flags check. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_AS4_AGGREGATOR, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_AS4_AGGREGATOR, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
*as4_aggregator_as = stream_getl (peer->ibuf);
as4_aggregator_addr->s_addr = stream_get_ipv4 (peer->ibuf);
@@ -1296,7 +1245,8 @@
/* Munge Aggregator and New-Aggregator, AS_PATH and NEW_AS_PATH.
*/
static bgp_attr_parse_ret_t
-bgp_attr_munge_as4_attrs (struct peer *peer, struct attr *attr, u_char flag,
+bgp_attr_munge_as4_attrs (struct peer *const peer,
+ struct attr *const attr,
struct aspath *as4_path, as_t as4_aggregator,
struct in_addr *as4_aggregator_addr)
{
@@ -1325,24 +1275,6 @@
return BGP_ATTR_PARSE_PROCEED;
}
- if (attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS4_PATH))
- && !(attr->flag & (ATTR_FLAG_BIT (BGP_ATTR_AS_PATH))))
- {
- /* Hu? This is not supposed to happen at all!
- * got as4_path and no aspath,
- * This should already
- * have been handled by 'well known attributes missing'
- * But... yeah, paranoia
- * Take this as a "malformed attribute"
- */
- zlog (peer->log, LOG_ERR,
- "%s BGP not AS4 capable peer sent AS4_PATH but"
- " no AS_PATH, cant do anything here", peer->host);
- return bgp_attr_malformed (peer, BGP_ATTR_AS_PATH, flag,
- BGP_NOTIFY_UPDATE_MAL_ATTR,
- NULL, 0);
- }
-
/* We have a asn16 peer. First, look for AS4_AGGREGATOR
* because that may override AS4_PATH
*/
@@ -1410,11 +1342,11 @@
/* Community attribute. */
static bgp_attr_parse_ret_t
-bgp_attr_community (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_community (struct bgp_attr_parser_args *args)
{
- bgp_size_t total
- = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
if (length == 0)
{
@@ -1422,12 +1354,6 @@
return BGP_ATTR_PARSE_PROCEED;
}
- /* Flags check. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_COMMUNITIES, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
-
attr->community =
community_parse ((u_int32_t *)stream_pnt (peer->ibuf), length);
@@ -1435,9 +1361,9 @@
stream_forward_getp (peer->ibuf, length);
if (!attr->community)
- return bgp_attr_malformed (peer, BGP_ATTR_COMMUNITIES, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
- startp, total);
+ args->total);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_COMMUNITIES);
@@ -1446,25 +1372,20 @@
/* Originator ID attribute. */
static bgp_attr_parse_ret_t
-bgp_attr_originator_id (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_originator_id (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_ORIGINATOR_ID, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
/* Length check. */
if (length != 4)
{
zlog (peer->log, LOG_ERR, "Bad originator ID length %d", length);
- return bgp_attr_malformed (peer, BGP_ATTR_ORIGINATOR_ID, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ args->total);
}
(bgp_attr_extra_get (attr))->originator_id.s_addr
@@ -1477,25 +1398,19 @@
/* Cluster list attribute. */
static bgp_attr_parse_ret_t
-bgp_attr_cluster_list (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_cluster_list (struct bgp_attr_parser_args *args)
{
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_CLUSTER_LIST, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
+
/* Check length. */
if (length % 4)
{
zlog (peer->log, LOG_ERR, "Bad cluster list length %d", length);
- return bgp_attr_malformed (peer, BGP_ATTR_CLUSTER_LIST, flag,
- BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
- startp, total);
+ return bgp_attr_malformed (args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
+ args->total);
}
(bgp_attr_extra_get (attr))->cluster
@@ -1511,8 +1426,8 @@
/* Multiprotocol reachability information parse. */
int
-bgp_mp_reach_parse (struct peer *peer, const bgp_size_t length,
- struct attr *attr, const u_char flag, u_char *startp, struct bgp_nlri *mp_update)
+bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
+ struct bgp_nlri *mp_update)
{
afi_t afi;
safi_t safi;
@@ -1520,15 +1435,11 @@
size_t start;
int ret;
struct stream *s;
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
struct attr_extra *attre = bgp_attr_extra_get(attr);
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_MP_REACH_NLRI, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_MP_REACH_NLRI, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+
/* Set end of packet. */
s = BGP_INPUT(peer);
start = stream_get_getp(s);
@@ -1647,8 +1558,7 @@
/* Multiprotocol unreachable parse */
int
-bgp_mp_unreach_parse (struct peer *peer, const bgp_size_t length,
- const u_char flag, u_char *startp,
+bgp_mp_unreach_parse (struct bgp_attr_parser_args *args,
struct bgp_nlri *mp_withdraw)
{
struct stream *s;
@@ -1656,14 +1566,8 @@
safi_t safi;
u_int16_t withdraw_len;
int ret;
- bgp_size_t total;
-
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
- /* Flag checks. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_MP_UNREACH_NLRI, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_MP_UNREACH_NLRI, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
+ struct peer *const peer = args->peer;
+ const bgp_size_t length = args->length;
s = peer->ibuf;
@@ -1695,11 +1599,11 @@
/* Extended Community attribute. */
static bgp_attr_parse_ret_t
-bgp_attr_ext_communities (struct peer *peer, bgp_size_t length,
- struct attr *attr, u_char flag, u_char *startp)
+bgp_attr_ext_communities (struct bgp_attr_parser_args *args)
{
- bgp_size_t total
- = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ const bgp_size_t length = args->length;
if (length == 0)
{
@@ -1709,21 +1613,15 @@
return BGP_ATTR_PARSE_PROCEED;
}
- /* Flags check. */
- if (bgp_attr_flag_invalid (peer, BGP_ATTR_EXT_COMMUNITIES, flag))
- return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES, flag,
- BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
- startp, total);
-
(bgp_attr_extra_get (attr))->ecommunity =
ecommunity_parse ((u_int8_t *)stream_pnt (peer->ibuf), length);
/* XXX: fix ecommunity_parse to use stream API */
stream_forward_getp (peer->ibuf, length);
if (!attr->extra->ecommunity)
- return bgp_attr_malformed (peer, BGP_ATTR_EXT_COMMUNITIES,
- flag, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
- startp, total);
+ return bgp_attr_malformed (args,
+ BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
+ args->total);
attr->flag |= ATTR_FLAG_BIT (BGP_ATTR_EXT_COMMUNITIES);
@@ -1732,12 +1630,18 @@
/* BGP unknown attribute treatment. */
static bgp_attr_parse_ret_t
-bgp_attr_unknown (struct peer *peer, struct attr *attr, u_char flag,
- u_char type, bgp_size_t length, u_char *startp)
+bgp_attr_unknown (struct bgp_attr_parser_args *args)
{
bgp_size_t total;
struct transit *transit;
struct attr_extra *attre;
+ struct peer *const peer = args->peer;
+ struct attr *const attr = args->attr;
+ u_char *const startp = args->startp;
+ const u_char type = args->type;
+ const u_char flag = args->flags;
+ const bgp_size_t length = args->length;
+
if (BGP_DEBUG (normal, NORMAL))
zlog_debug ("%s Unknown attribute is received (type %d, length %d)",
@@ -1750,18 +1654,15 @@
/* Forward read pointer of input stream. */
stream_forward_getp (peer->ibuf, length);
- /* Adjest total length to include type and length. */
- total = length + (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN) ? 4 : 3);
-
/* If any of the mandatory well-known attributes are not recognized,
then the Error Subcode is set to Unrecognized Well-known
Attribute. The Data field contains the unrecognized attribute
(type, length and value). */
if (!CHECK_FLAG (flag, BGP_ATTR_FLAG_OPTIONAL))
{
- return bgp_attr_malformed (peer, type, flag,
+ return bgp_attr_malformed (args,
BGP_NOTIFY_UPDATE_UNREC_ATTR,
- startp, total);
+ args->total);
}
/* Unrecognized non-transitive optional attributes must be quietly
@@ -1858,7 +1759,7 @@
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return BGP_ATTR_PARSE_ERROR;
}
-
+
/* Check extended attribue length bit. */
if (CHECK_FLAG (flag, BGP_ATTR_FLAG_EXTLEN))
length = stream_getw (BGP_INPUT (peer));
@@ -1898,59 +1799,79 @@
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR);
return BGP_ATTR_PARSE_ERROR;
}
+
+ struct bgp_attr_parser_args attr_args = {
+ .peer = peer,
+ .length = length,
+ .attr = attr,
+ .type = type,
+ .flags = flag,
+ .startp = startp,
+ .total = attr_endp - startp,
+ };
+
+
+ /* If any recognized attribute has Attribute Flags that conflict
+ with the Attribute Type Code, then the Error Subcode is set to
+ Attribute Flags Error. The Data field contains the erroneous
+ attribute (type, length and value). */
+ if (bgp_attr_flag_invalid (&attr_args))
+ return bgp_attr_malformed (&attr_args,
+ BGP_NOTIFY_UPDATE_ATTR_FLAG_ERR,
+ attr_args.total);
/* OK check attribute and store it's value. */
switch (type)
{
case BGP_ATTR_ORIGIN:
- ret = bgp_attr_origin (peer, length, attr, flag, startp);
+ ret = bgp_attr_origin (&attr_args);
break;
case BGP_ATTR_AS_PATH:
- ret = bgp_attr_aspath (peer, length, attr, flag, startp);
+ ret = bgp_attr_aspath (&attr_args);
break;
case BGP_ATTR_AS4_PATH:
- ret = bgp_attr_as4_path (peer, length, attr, flag, startp, &as4_path);
+ ret = bgp_attr_as4_path (&attr_args, &as4_path);
break;
case BGP_ATTR_NEXT_HOP:
- ret = bgp_attr_nexthop (peer, length, attr, flag, startp);
+ ret = bgp_attr_nexthop (&attr_args);
break;
case BGP_ATTR_MULTI_EXIT_DISC:
- ret = bgp_attr_med (peer, length, attr, flag, startp);
+ ret = bgp_attr_med (&attr_args);
break;
case BGP_ATTR_LOCAL_PREF:
- ret = bgp_attr_local_pref (peer, length, attr, flag, startp);
+ ret = bgp_attr_local_pref (&attr_args);
break;
case BGP_ATTR_ATOMIC_AGGREGATE:
- ret = bgp_attr_atomic (peer, length, attr, flag, startp);
+ ret = bgp_attr_atomic (&attr_args);
break;
case BGP_ATTR_AGGREGATOR:
- ret = bgp_attr_aggregator (peer, length, attr, flag, startp);
+ ret = bgp_attr_aggregator (&attr_args);
break;
case BGP_ATTR_AS4_AGGREGATOR:
- ret = bgp_attr_as4_aggregator (peer, length, attr, flag,
- &as4_aggregator,
- &as4_aggregator_addr, startp);
+ ret = bgp_attr_as4_aggregator (&attr_args,
+ &as4_aggregator,
+ &as4_aggregator_addr);
break;
case BGP_ATTR_COMMUNITIES:
- ret = bgp_attr_community (peer, length, attr, flag, startp);
+ ret = bgp_attr_community (&attr_args);
break;
case BGP_ATTR_ORIGINATOR_ID:
- ret = bgp_attr_originator_id (peer, length, attr, flag, startp);
+ ret = bgp_attr_originator_id (&attr_args);
break;
case BGP_ATTR_CLUSTER_LIST:
- ret = bgp_attr_cluster_list (peer, length, attr, flag, startp);
+ ret = bgp_attr_cluster_list (&attr_args);
break;
case BGP_ATTR_MP_REACH_NLRI:
- ret = bgp_mp_reach_parse (peer, length, attr, flag, startp, mp_update);
+ ret = bgp_mp_reach_parse (&attr_args, mp_update);
break;
case BGP_ATTR_MP_UNREACH_NLRI:
- ret = bgp_mp_unreach_parse (peer, length, flag, startp, mp_withdraw);
+ ret = bgp_mp_unreach_parse (&attr_args, mp_withdraw);
break;
case BGP_ATTR_EXT_COMMUNITIES:
- ret = bgp_attr_ext_communities (peer, length, attr, flag, startp);
+ ret = bgp_attr_ext_communities (&attr_args);
break;
default:
- ret = bgp_attr_unknown (peer, attr, flag, type, length, startp);
+ ret = bgp_attr_unknown (&attr_args);
break;
}
@@ -2020,7 +1941,7 @@
* all attributes first, including these 32bit ones, and now,
* afterwards, we look what and if something is to be done for as4.
*/
- if (bgp_attr_munge_as4_attrs (peer, attr, flag, as4_path,
+ if (bgp_attr_munge_as4_attrs (peer, attr, as4_path,
as4_aggregator, &as4_aggregator_addr))
{
if (as4_path)
@@ -2052,7 +1973,7 @@
*/
if (attr->flag & (ATTR_FLAG_BIT(BGP_ATTR_AS_PATH)))
{
- ret = bgp_attr_aspath_check (peer, attr, flag);
+ ret = bgp_attr_aspath_check (peer, attr);
if (ret != BGP_ATTR_PARSE_PROCEED)
return ret;
}
diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h
index e630074..df87c86 100644
--- a/bgpd/bgp_attr.h
+++ b/bgpd/bgp_attr.h
@@ -179,10 +179,19 @@
/* Transit attribute prototypes. */
void transit_unintern (struct transit *);
-/* Exported for unit-test purposes only */
-extern int bgp_mp_reach_parse (struct peer *, const bgp_size_t, struct attr *,
- const u_char, u_char *, struct bgp_nlri *);
-extern int bgp_mp_unreach_parse (struct peer *, const bgp_size_t, const u_char,
- u_char *, struct bgp_nlri *);
+/* Below exported for unit-test purposes only */
+struct bgp_attr_parser_args {
+ struct peer *peer;
+ bgp_size_t length; /* attribute data length; */
+ bgp_size_t total; /* total length, inc header */
+ struct attr *attr;
+ u_int8_t type;
+ u_int8_t flags;
+ u_char *startp;
+};
+extern int bgp_mp_reach_parse (struct bgp_attr_parser_args *args,
+ struct bgp_nlri *);
+extern int bgp_mp_unreach_parse (struct bgp_attr_parser_args *args,
+ struct bgp_nlri *);
#endif /* _QUAGGA_BGP_ATTR_H */