mirrors/bird
mirrors/bird
0
0
Fork 0
mirror of https://gitlab.nic.cz/labs/bird.git synced 2024-11-17 16:48:43 +00:00
Code Releases Activity

BGP: Use proper class in attribute error messages

Browse Source 
Most error messages in attribute processing are in rx/decode step and
these use L_REMOTE log class. But there are few that are in tx/export
step and these should use L_ERR log class.

Use tx-specific macro (REJECT()) in tx/export code and rename field
err_withdraw to err_reject in struct bgp_export_state to ensure that
appropriate error reporting macros are called in proper contexts.
This commit is contained in:
Ondrej Zajicek (work) 2022-01-28 05:35:22 +01:00
parent 75d01ecc2d
commit 963b2c7ce2
3 changed files with 21 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
proto/bgp/attrs.c
View File

@ -45,9 +45,9 @@
* *
* export - Hook that validates and normalizes attribute during export phase. * export - Hook that validates and normalizes attribute during export phase.
* Receives eattr, may modify it (e.g., sort community lists for canonical * Receives eattr, may modify it (e.g., sort community lists for canonical
* representation), UNSET() it (e.g., skip empty lists), or WITHDRAW() it if * representation), UNSET() it (e.g., skip empty lists), or REJECT() the route
* necessary. May assume that eattr has value valid w.r.t. its type, but may be * if necessary. May assume that eattr has value valid w.r.t. its type, but may
* invalid w.r.t. BGP constraints. Optional. * be invalid w.r.t. BGP constraints. Optional.
* *
* encode - Hook that converts internal representation to external one during * encode - Hook that converts internal representation to external one during
* packet writing. Receives eattr and puts it in the buffer (including attribute * packet writing. Receives eattr and puts it in the buffer (including attribute
@ -108,6 +108,9 @@ bgp_set_attr(ea_list **attrs, struct linpool *pool, uint code, uint flags, uintp
#define UNSET(a) \ #define UNSET(a) \
({ a->type = EAF_TYPE_UNDEF; return; }) ({ a->type = EAF_TYPE_UNDEF; return; })
#define REJECT(msg, args...) \
({ log(L_ERR "%s: " msg, s->proto->p.name, ## args); s->err_reject = 1; return; })
#define NEW_BGP "Discarding %s attribute received from AS4-aware neighbor" #define NEW_BGP "Discarding %s attribute received from AS4-aware neighbor"
#define BAD_EBGP "Discarding %s attribute received from EBGP neighbor" #define BAD_EBGP "Discarding %s attribute received from EBGP neighbor"
#define BAD_LENGTH "Malformed %s attribute - invalid length (%u)" #define BAD_LENGTH "Malformed %s attribute - invalid length (%u)"
@ -380,7 +383,7 @@ static void
bgp_export_origin(struct bgp_export_state *s, eattr *a) bgp_export_origin(struct bgp_export_state *s, eattr *a)
{ {
if (a->u.data > 2) if (a->u.data > 2)
WITHDRAW(BAD_VALUE, "ORIGIN", a->u.data); REJECT(BAD_VALUE, "ORIGIN", a->u.data);
} }
static void static void
@ -902,20 +905,20 @@ bgp_export_mpls_label_stack(struct bgp_export_state *s, eattr *a)
/* Perhaps we should just ignore it? */ /* Perhaps we should just ignore it? */
if (!s->mpls) if (!s->mpls)
WITHDRAW("Unexpected MPLS stack"); REJECT("Unexpected MPLS stack");
/* Empty MPLS stack is not allowed */ /* Empty MPLS stack is not allowed */
if (!lnum) if (!lnum)
WITHDRAW("Malformed MPLS stack - empty"); REJECT("Malformed MPLS stack - empty");
/* This is ugly, but we must ensure that labels fit into NLRI field */ /* This is ugly, but we must ensure that labels fit into NLRI field */
if ((24*lnum + (net_is_vpn(n) ? 64 : 0) + net_pxlen(n)) > 255) if ((24*lnum + (net_is_vpn(n) ? 64 : 0) + net_pxlen(n)) > 255)
WITHDRAW("Malformed MPLS stack - too many labels (%u)", lnum); REJECT("Malformed MPLS stack - too many labels (%u)", lnum);
for (uint i = 0; i < lnum; i++) for (uint i = 0; i < lnum; i++)
{ {
if (labels[i] > 0xfffff) if (labels[i] > 0xfffff)
WITHDRAW("Malformed MPLS stack - invalid label (%u)", labels[i]); REJECT("Malformed MPLS stack - invalid label (%u)", labels[i]);
/* TODO: Check for special-purpose label values? */ /* TODO: Check for special-purpose label values? */
} }
@ -1196,7 +1199,7 @@ bgp_export_attrs(struct bgp_export_state *s, ea_list *attrs)
for (i = 0; i < count; i++) for (i = 0; i < count; i++)
bgp_export_attr(s, &new->attrs[i], new); bgp_export_attr(s, &new->attrs[i], new);
if (s->err_withdraw) if (s->err_reject)
return NULL; return NULL;
return new; return new;

2
proto/bgp/bgp.h
View File

@ -397,7 +397,7 @@ struct bgp_export_state {
int mpls; int mpls;
u32 attrs_seen[1]; u32 attrs_seen[1];
uint err_withdraw; uint err_reject;
uint local_next_hop; uint local_next_hop;
}; };

13
proto/bgp/packets.c
View File

@ -932,6 +932,9 @@ bgp_rx_open(struct bgp_conn *conn, byte *pkt, uint len)
#define WITHDRAW(msg, args...) \ #define WITHDRAW(msg, args...) \
({ REPORT(msg, ## args); s->err_withdraw = 1; return; }) ({ REPORT(msg, ## args); s->err_withdraw = 1; return; })
#define REJECT(msg, args...) \
({ log(L_ERR "%s: " msg, s->proto->p.name, ## args); s->err_reject = 1; return; })
#define BAD_AFI "Unexpected AF <%u/%u> in UPDATE" #define BAD_AFI "Unexpected AF <%u/%u> in UPDATE"
#define BAD_NEXT_HOP "Invalid NEXT_HOP attribute" #define BAD_NEXT_HOP "Invalid NEXT_HOP attribute"
#define NO_NEXT_HOP "Missing NEXT_HOP attribute" #define NO_NEXT_HOP "Missing NEXT_HOP attribute"
@ -1126,7 +1129,7 @@ bgp_update_next_hop_ip(struct bgp_export_state *s, eattr *a, ea_list **to)
/* Check if next hop is valid */ /* Check if next hop is valid */
a = bgp_find_attr(*to, BA_NEXT_HOP); a = bgp_find_attr(*to, BA_NEXT_HOP);
if (!a) if (!a)
WITHDRAW(NO_NEXT_HOP); REJECT(NO_NEXT_HOP);
ip_addr *nh = (void *) a->u.ptr->data; ip_addr *nh = (void *) a->u.ptr->data;
ip_addr peer = s->proto->remote_ip; ip_addr peer = s->proto->remote_ip;
@ -1134,20 +1137,20 @@ bgp_update_next_hop_ip(struct bgp_export_state *s, eattr *a, ea_list **to)
/* Forbid zero next hop */ /* Forbid zero next hop */
if (ipa_zero2(nh[0]) && ((len != 32) || ipa_zero(nh[1]))) if (ipa_zero2(nh[0]) && ((len != 32) || ipa_zero(nh[1])))
WITHDRAW(BAD_NEXT_HOP " - zero address"); REJECT(BAD_NEXT_HOP " - zero address");
/* Forbid next hop equal to neighbor IP */ /* Forbid next hop equal to neighbor IP */
if (ipa_equal(peer, nh[0]) || ((len == 32) && ipa_equal(peer, nh[1]))) if (ipa_equal(peer, nh[0]) || ((len == 32) && ipa_equal(peer, nh[1])))
WITHDRAW(BAD_NEXT_HOP " - neighbor address %I", peer); REJECT(BAD_NEXT_HOP " - neighbor address %I", peer);
/* Forbid next hop with non-matching AF */ /* Forbid next hop with non-matching AF */
if ((ipa_is_ip4(nh[0]) != bgp_channel_is_ipv4(s->channel)) && if ((ipa_is_ip4(nh[0]) != bgp_channel_is_ipv4(s->channel)) &&
!s->channel->ext_next_hop) !s->channel->ext_next_hop)
WITHDRAW(BAD_NEXT_HOP MISMATCHED_AF, nh[0], s->channel->desc->name); REJECT(BAD_NEXT_HOP MISMATCHED_AF, nh[0], s->channel->desc->name);
/* Just check if MPLS stack */ /* Just check if MPLS stack */
if (s->mpls && !bgp_find_attr(*to, BA_MPLS_LABEL_STACK)) if (s->mpls && !bgp_find_attr(*to, BA_MPLS_LABEL_STACK))
WITHDRAW(NO_LABEL_STACK); REJECT(NO_LABEL_STACK);
} }
static uint static uint