Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bgpd: implement zlog_ferr facility for enhance error messages in bgp #2

Merged
merged 1 commit into from
Jun 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
128 changes: 77 additions & 51 deletions bgpd/bgp_attr.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
#include "bgpd/bgp_aspath.h"
#include "bgpd/bgp_community.h"
#include "bgpd/bgp_debug.h"
#include "bgpd/bgp_errors.h"
#include "bgpd/bgp_label.h"
#include "bgpd/bgp_packet.h"
#include "bgpd/bgp_ecommunity.h"
Expand Down Expand Up @@ -996,12 +997,14 @@ bgp_attr_flags_diagnose(struct bgp_attr_parser_args *args,
for (i = 0; i <= 2; i++) /* O,T,P, but not E */
if (CHECK_FLAG(desired_flags, attr_flag_str[i].key)
!= CHECK_FLAG(real_flags, attr_flag_str[i].key)) {
zlog_err("%s attribute must%s be flagged as \"%s\"",
lookup_msg(attr_str, attr_code, NULL),
CHECK_FLAG(desired_flags, attr_flag_str[i].key)
? ""
: " not",
attr_flag_str[i].str);
zlog_ferr(
BGP_ERR_ATTR_FLAG,
"%s attribute must%s be flagged as \"%s\"",
lookup_msg(attr_str, attr_code, NULL),
CHECK_FLAG(desired_flags, attr_flag_str[i].key)
? ""
: " not",
attr_flag_str[i].str);
seen = 1;
}
if (!seen) {
Expand Down Expand Up @@ -1059,7 +1062,8 @@ static int bgp_attr_flag_invalid(struct bgp_attr_parser_args *args)
*/
if (!CHECK_FLAG(BGP_ATTR_FLAG_OPTIONAL, flags)
&& !CHECK_FLAG(BGP_ATTR_FLAG_TRANS, flags)) {
zlog_err(
zlog_ferr(
BGP_ERR_ATTR_FLAG,
"%s well-known attributes must have transitive flag set (%x)",
lookup_msg(attr_str, attr_code, NULL), flags);
return 1;
Expand All @@ -1071,18 +1075,18 @@ static int bgp_attr_flag_invalid(struct bgp_attr_parser_args *args)
*/
if (CHECK_FLAG(flags, BGP_ATTR_FLAG_PARTIAL)) {
if (!CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL)) {
zlog_err(
"%s well-known attribute "
"must NOT have the partial flag set (%x)",
lookup_msg(attr_str, attr_code, NULL), flags);
zlog_ferr(BGP_ERR_ATTR_FLAG,
"%s well-known attribute "
"must NOT have the partial flag set (%x)",
lookup_msg(attr_str, attr_code, NULL), flags);
return 1;
}
if (CHECK_FLAG(flags, BGP_ATTR_FLAG_OPTIONAL)
&& !CHECK_FLAG(flags, BGP_ATTR_FLAG_TRANS)) {
zlog_err(
"%s optional + transitive attribute "
"must NOT have the partial flag set (%x)",
lookup_msg(attr_str, attr_code, NULL), flags);
zlog_ferr(BGP_ERR_ATTR_FLAG,
"%s optional + transitive attribute "
"must NOT have the partial flag set (%x)",
lookup_msg(attr_str, attr_code, NULL), flags);
return 1;
}
}
Expand Down Expand Up @@ -1114,7 +1118,8 @@ static bgp_attr_parse_ret_t bgp_attr_origin(struct bgp_attr_parser_args *args)
field contains the erroneous attribute (type, length and
value). */
if (length != 1) {
zlog_err("Origin attribute length is not one %d", length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"Origin attribute length is not one %d", length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
Expand All @@ -1127,7 +1132,8 @@ static bgp_attr_parse_ret_t bgp_attr_origin(struct bgp_attr_parser_args *args)
contains the unrecognized attribute (type, length and value). */
if ((attr->origin != BGP_ORIGIN_IGP) && (attr->origin != BGP_ORIGIN_EGP)
&& (attr->origin != BGP_ORIGIN_INCOMPLETE)) {
zlog_err("Origin attribute value is invalid %d", attr->origin);
zlog_ferr(BGP_ERR_ATTR_ORIGIN,
"Origin attribute value is invalid %d", attr->origin);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_INVAL_ORIGIN,
args->total);
}
Expand Down Expand Up @@ -1155,8 +1161,9 @@ static int bgp_attr_aspath(struct bgp_attr_parser_args *args)

/* In case of IBGP, length will be zero. */
if (!attr->aspath) {
zlog_err("Malformed AS path from %s, length is %d", peer->host,
length);
zlog_ferr(BGP_ERR_ATTR_MAL_AS_PATH,
"Malformed AS path from %s, length is %d", peer->host,
length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_MAL_AS_PATH,
0);
}
Expand Down Expand Up @@ -1184,7 +1191,8 @@ static bgp_attr_parse_ret_t bgp_attr_aspath_check(struct peer *const peer,
&& !aspath_left_confed_check(attr->aspath))
|| (peer->sort == BGP_PEER_EBGP
&& aspath_confed_check(attr->aspath))) {
zlog_err("Malformed AS path from %s", peer->host);
zlog_ferr(BGP_ERR_ATTR_MAL_AS_PATH, "Malformed AS path from %s",
peer->host);
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_AS_PATH);
return BGP_ATTR_PARSE_ERROR;
Expand All @@ -1194,8 +1202,9 @@ static bgp_attr_parse_ret_t bgp_attr_aspath_check(struct peer *const peer,
if (CHECK_FLAG(peer->flags, PEER_FLAG_ENFORCE_FIRST_AS)) {
if (peer->sort == BGP_PEER_EBGP
&& !aspath_firstas_check(attr->aspath, peer->as)) {
zlog_err("%s incorrect first AS (must be %u)",
peer->host, peer->as);
zlog_ferr(BGP_ERR_ATTR_FIRST_AS,
"%s incorrect first AS (must be %u)",
peer->host, peer->as);
bgp_notify_send(peer, BGP_NOTIFY_UPDATE_ERR,
BGP_NOTIFY_UPDATE_MAL_AS_PATH);
return BGP_ATTR_PARSE_ERROR;
Expand Down Expand Up @@ -1227,8 +1236,9 @@ static int bgp_attr_as4_path(struct bgp_attr_parser_args *args,

/* In case of IBGP, length will be zero. */
if (!*as4_path) {
zlog_err("Malformed AS4 path from %s, length is %d", peer->host,
length);
zlog_ferr(BGP_ERR_ATTR_MAL_AS_PATH,
"Malformed AS4 path from %s, length is %d",
peer->host, length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_MAL_AS_PATH,
0);
}
Expand All @@ -1250,7 +1260,8 @@ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args)

/* Check nexthop attribute length. */
if (length != 4) {
zlog_err("Nexthop attribute length isn't four [%d]", length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"Nexthop attribute length isn't four [%d]", length);

return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand All @@ -1274,7 +1285,7 @@ static bgp_attr_parse_ret_t bgp_attr_nexthop(struct bgp_attr_parser_args *args)
{
char buf[INET_ADDRSTRLEN];
inet_ntop(AF_INET, &nexthop_n, buf, INET_ADDRSTRLEN);
zlog_err("Martian nexthop %s", buf);
zlog_ferr(BGP_ERR_ATTR_MARTIAN_NH, "Martian nexthop %s", buf);
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_INVAL_NEXT_HOP, args->total);
}
Expand All @@ -1294,7 +1305,8 @@ static bgp_attr_parse_ret_t bgp_attr_med(struct bgp_attr_parser_args *args)

/* Length check. */
if (length != 4) {
zlog_err("MED attribute length isn't four [%d]", length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"MED attribute length isn't four [%d]", length);

return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand All @@ -1317,7 +1329,8 @@ bgp_attr_local_pref(struct bgp_attr_parser_args *args)

/* Length check. */
if (length != 4) {
zlog_err("LOCAL_PREF attribute length isn't 4 [%u]", length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"LOCAL_PREF attribute length isn't 4 [%u]", length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
Expand Down Expand Up @@ -1346,8 +1359,9 @@ static int bgp_attr_atomic(struct bgp_attr_parser_args *args)

/* Length check. */
if (length != 0) {
zlog_err("ATOMIC_AGGREGATE attribute length isn't 0 [%u]",
length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"ATOMIC_AGGREGATE attribute length isn't 0 [%u]",
length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
Expand All @@ -1372,8 +1386,9 @@ static int bgp_attr_aggregator(struct bgp_attr_parser_args *args)
wantedlen = 8;

if (length != wantedlen) {
zlog_err("AGGREGATOR attribute length isn't %u [%u]", wantedlen,
length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"AGGREGATOR attribute length isn't %u [%u]",
wantedlen, length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
Expand Down Expand Up @@ -1401,7 +1416,8 @@ bgp_attr_as4_aggregator(struct bgp_attr_parser_args *args,
const bgp_size_t length = args->length;

if (length != 8) {
zlog_err("New Aggregator length is not 8 [%d]", length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"New Aggregator length is not 8 [%d]", length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
0);
}
Expand Down Expand Up @@ -1557,7 +1573,8 @@ bgp_attr_originator_id(struct bgp_attr_parser_args *args)

/* Length check. */
if (length != 4) {
zlog_err("Bad originator ID length %d", length);
zlog_ferr(BGP_ERR_ATTR_LEN, "Bad originator ID length %d",
length);

return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand All @@ -1580,7 +1597,8 @@ bgp_attr_cluster_list(struct bgp_attr_parser_args *args)

/* Check length. */
if (length % 4) {
zlog_err("Bad cluster list length %d", length);
zlog_ferr(BGP_ERR_ATTR_LEN, "Bad cluster list length %d",
length);

return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand Down Expand Up @@ -2039,10 +2057,10 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,

if (type == BGP_PREFIX_SID_LABEL_INDEX) {
if (length != BGP_PREFIX_SID_LABEL_INDEX_LENGTH) {
zlog_err(
"Prefix SID label index length is %d instead of %d",
length,
BGP_PREFIX_SID_LABEL_INDEX_LENGTH);
zlog_ferr(
BGP_ERR_ATTR_LEN,
"Prefix SID label index length is %d instead of %d",
length, BGP_PREFIX_SID_LABEL_INDEX_LENGTH);
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand Down Expand Up @@ -2075,8 +2093,9 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
/* Placeholder code for the IPv6 SID type */
else if (type == BGP_PREFIX_SID_IPV6) {
if (length != BGP_PREFIX_SID_IPV6_LENGTH) {
zlog_err("Prefix SID IPv6 length is %d instead of %d",
length, BGP_PREFIX_SID_IPV6_LENGTH);
zlog_ferr(BGP_ERR_ATTR_LEN,
"Prefix SID IPv6 length is %d instead of %d",
length, BGP_PREFIX_SID_IPV6_LENGTH);
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand All @@ -2097,7 +2116,8 @@ static bgp_attr_parse_ret_t bgp_attr_psid_sub(int32_t type,
length -= 2;

if (length % BGP_PREFIX_SID_ORIGINATOR_SRGB_LENGTH) {
zlog_err(
zlog_ferr(
BGP_ERR_ATTR_LEN,
"Prefix SID Originator SRGB length is %d, it must be a multiple of %d ",
length, BGP_PREFIX_SID_ORIGINATOR_SRGB_LENGTH);
return bgp_attr_malformed(
Expand Down Expand Up @@ -2146,8 +2166,10 @@ bgp_attr_prefix_sid(int32_t tlength, struct bgp_attr_parser_args *args,
tlength -= length + 3;

if (tlength < 0) {
zlog_err("Prefix SID internal length %d causes us to read beyond the total Prefix SID length",
length);
zlog_ferr(
BGP_ERR_ATTR_LEN,
"Prefix SID internal length %d causes us to read beyond the total Prefix SID length",
length);
return bgp_attr_malformed(args,
BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand All @@ -2172,21 +2194,24 @@ bgp_attr_pmsi_tunnel(struct bgp_attr_parser_args *args)
* can only support that.
*/
if (length < 2) {
zlog_err("Bad PMSI tunnel attribute length %d", length);
zlog_ferr(BGP_ERR_ATTR_LEN,
"Bad PMSI tunnel attribute length %d", length);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
}
stream_getc(peer->curr); /* Flags */
tnl_type = stream_getc(peer->curr);
if (tnl_type > PMSI_TNLTYPE_MAX) {
zlog_err("Invalid PMSI tunnel attribute type %d", tnl_type);
zlog_ferr(BGP_ERR_ATTR_PMSI_TYPE,
"Invalid PMSI tunnel attribute type %d", tnl_type);
return bgp_attr_malformed(args, BGP_NOTIFY_UPDATE_OPT_ATTR_ERR,
args->total);
}
if (tnl_type == PMSI_TNLTYPE_INGR_REPL) {
if (length != 9) {
zlog_err("Bad PMSI tunnel attribute length %d for IR",
length);
zlog_ferr(BGP_ERR_ATTR_PMSI_LEN,
"Bad PMSI tunnel attribute length %d for IR",
length);
return bgp_attr_malformed(
args, BGP_NOTIFY_UPDATE_ATTR_LENG_ERR,
args->total);
Expand Down Expand Up @@ -2786,9 +2811,10 @@ size_t bgp_packet_mpattr_start(struct stream *s, struct peer *peer, afi_t afi,
break;
default:
if (safi != SAFI_FLOWSPEC)
zlog_err(
"Bad nexthop when sending to %s, AFI %u SAFI %u nhlen %d",
peer->host, afi, safi, attr->mp_nexthop_len);
zlog_ferr(
BGP_ERR_ATTR_NH_SEND_LEN,
"Bad nexthop when sending to %s, AFI %u SAFI %u nhlen %d",
peer->host, afi, safi, attr->mp_nexthop_len);
break;
}

Expand Down
Loading