From a08ca0a7e101cbc44fc9300520bd0ab6a2c9b784 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Mon, 31 Jul 2017 21:09:01 -0300 Subject: [PATCH 1/5] lib: remove SAFI_RESERVED_4 and SAFI_RESERVED_5 SAFI values have been a major source of confusion over the last few years. That's because each SAFI needs to be represented in two different ways: * IANA's value used to send/receive packets over the network; * Internal value used for array indexing. In the second case, defining reserved values makes no sense because we don't want to index SAFIs that simply don't exist. The sole purpose of the internal SAFI values is to remove the gaps we have among the IANA values, which would represent wasted memory in C arrays. With that said, remove these reserved SAFIs to avoid further confusion in the future. Signed-off-by: Renato Westphal --- bgpd/bgp_fsm.c | 8 ++++---- bgpd/bgp_vty.c | 6 ------ bgpd/bgpd.c | 2 +- lib/zebra.h | 18 +++++------------- 4 files changed, 10 insertions(+), 24 deletions(-) diff --git a/bgpd/bgp_fsm.c b/bgpd/bgp_fsm.c index cf1cb18689..b609abac69 100644 --- a/bgpd/bgp_fsm.c +++ b/bgpd/bgp_fsm.c @@ -488,7 +488,7 @@ static int bgp_graceful_restart_timer_expire(struct thread *thread) /* NSF delete stale route */ for (afi = AFI_IP; afi < AFI_MAX; afi++) - for (safi = SAFI_UNICAST; safi < SAFI_RESERVED_4; safi++) + for (safi = SAFI_UNICAST; safi <= SAFI_MPLS_VPN; safi++) if (peer->nsf[afi][safi]) bgp_clear_stale_route(peer, afi, safi); @@ -521,7 +521,7 @@ static int bgp_graceful_stale_timer_expire(struct thread *thread) /* NSF delete stale route */ for (afi = AFI_IP; afi < AFI_MAX; afi++) - for (safi = SAFI_UNICAST; safi < SAFI_RESERVED_4; safi++) + for (safi = SAFI_UNICAST; safi <= SAFI_MPLS_VPN; safi++) if (peer->nsf[afi][safi]) bgp_clear_stale_route(peer, afi, safi); @@ -1022,7 +1022,7 @@ int bgp_stop(struct peer *peer) for (afi = AFI_IP; afi < AFI_MAX; afi++) for (safi = SAFI_UNICAST; - safi < SAFI_RESERVED_4; safi++) + safi <= SAFI_MPLS_VPN; safi++) peer->nsf[afi][safi] = 0; } @@ -1425,7 +1425,7 @@ static int bgp_establish(struct peer *peer) /* graceful restart */ UNSET_FLAG(peer->sflags, PEER_STATUS_NSF_WAIT); for (afi = AFI_IP; afi < AFI_MAX; afi++) - for (safi = SAFI_UNICAST; safi < SAFI_RESERVED_4; safi++) { + for (safi = SAFI_UNICAST; safi <= SAFI_MPLS_VPN; safi++) { if (peer->afc_nego[afi][safi] && CHECK_FLAG(peer->cap, PEER_CAP_RESTART_ADV) && CHECK_FLAG(peer->af_cap[afi][safi], diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 65a1473f75..db503516d5 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -7079,12 +7079,6 @@ static void bgp_show_summary_afi_safi(struct vty *vty, struct bgp *bgp, int afi, json); } safi++; - if (safi == SAFI_RESERVED_4 - || safi - == SAFI_RESERVED_5) /* handle special - cases to match - zebra.h */ - safi++; if (!safi_wildcard) safi = SAFI_MAX; } diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a0e2d6749a..a078bdfd7e 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -1842,7 +1842,7 @@ static void peer_nsf_stop(struct peer *peer) UNSET_FLAG(peer->sflags, PEER_STATUS_NSF_MODE); for (afi = AFI_IP; afi < AFI_MAX; afi++) - for (safi = SAFI_UNICAST; safi < SAFI_RESERVED_4; safi++) + for (safi = SAFI_UNICAST; safi <= SAFI_MPLS_VPN; safi++) peer->nsf[afi][safi] = 0; if (peer->t_gr_restart) { diff --git a/lib/zebra.h b/lib/zebra.h index 8e1c4db804..7b705bb83f 100644 --- a/lib/zebra.h +++ b/lib/zebra.h @@ -419,19 +419,10 @@ typedef enum { AFI_IP = 1, AFI_IP6 = 2, AFI_L2VPN = 3, AFI_MAX = 4 } afi_t; #define SAFI_UNICAST 1 #define SAFI_MULTICAST 2 #define SAFI_MPLS_VPN 3 -#define SAFI_RESERVED_4 4 -#define SAFI_ENCAP 5 -#define SAFI_RESERVED_5 5 -#define SAFI_EVPN 6 -#define SAFI_LABELED_UNICAST 7 -#define SAFI_MAX 8 - -#define IANA_SAFI_RESERVED 0 -#define IANA_SAFI_UNICAST 1 -#define IANA_SAFI_MULTICAST 2 -#define IANA_SAFI_LABELED_UNICAST 4 -#define IANA_SAFI_ENCAP 7 -#define IANA_SAFI_MPLS_VPN 128 +#define SAFI_ENCAP 4 +#define SAFI_EVPN 5 +#define SAFI_LABELED_UNICAST 6 +#define SAFI_MAX 7 /* * The above AFI and SAFI definitions are for internal use. The protocol @@ -454,6 +445,7 @@ typedef enum { #define IANA_SAFI_RESERVED 0 #define IANA_SAFI_UNICAST 1 #define IANA_SAFI_MULTICAST 2 +#define IANA_SAFI_LABELED_UNICAST 4 #define IANA_SAFI_ENCAP 7 #define IANA_SAFI_EVPN 70 #define IANA_SAFI_MPLS_VPN 128 From 5c5255381e5b457e263081455a9677afa6b9e470 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Mon, 31 Jul 2017 21:06:40 -0300 Subject: [PATCH 2/5] lib/bgpd: introduce the iana_safi_t enum We had afi_t/iana_afi_t for AFIs but only safi_t for SAFIs. Fix this inconsistency. Signed-off-by: Renato Westphal --- bgpd/bgp_attr.c | 10 ++++++---- bgpd/bgp_open.c | 15 +++++++++------ bgpd/bgp_open.h | 2 +- bgpd/bgp_packet.c | 15 +++++++++------ bgpd/bgp_route.c | 2 +- bgpd/bgp_updgrp_packet.c | 4 ++-- bgpd/bgp_vty.c | 8 ++++++++ bgpd/bgpd.c | 6 +++--- bgpd/bgpd.h | 4 ++-- bgpd/rfapi/rfapi_import.c | 4 ++++ lib/prefix.c | 3 ++- lib/zebra.h | 38 ++++++++++++++++++++------------------ tests/bgpd/test_mp_attr.c | 2 +- 13 files changed, 68 insertions(+), 45 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index ef32b9cf92..ca9a50db55 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1677,7 +1677,8 @@ int bgp_mp_reach_parse(struct bgp_attr_parser_args *args, { iana_afi_t pkt_afi; afi_t afi; - safi_t pkt_safi, safi; + iana_safi_t pkt_safi; + safi_t safi; bgp_size_t nlri_len; size_t start; struct stream *s; @@ -1826,7 +1827,8 @@ int bgp_mp_unreach_parse(struct bgp_attr_parser_args *args, struct stream *s; iana_afi_t pkt_afi; afi_t afi; - safi_t pkt_safi, safi; + iana_safi_t pkt_safi; + safi_t safi; u_int16_t withdraw_len; struct peer *const peer = args->peer; struct attr *const attr = args->attr; @@ -2593,7 +2595,7 @@ size_t bgp_packet_mpattr_start(struct stream *s, struct peer *peer, afi_t afi, { size_t sizep; iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; afi_t nh_afi; /* Set extended bit always to encode the attribute length as 2 bytes */ @@ -3280,7 +3282,7 @@ size_t bgp_packet_mpunreach_start(struct stream *s, afi_t afi, safi_t safi) { unsigned long attrlen_pnt; iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; /* Set extended bit always to encode the attribute length as 2 bytes */ stream_putc(s, BGP_ATTR_FLAG_OPTIONAL | BGP_ATTR_FLAG_EXTLEN); diff --git a/bgpd/bgp_open.c b/bgpd/bgp_open.c index b18a4b7c46..2c988f86cc 100644 --- a/bgpd/bgp_open.c +++ b/bgpd/bgp_open.c @@ -298,7 +298,8 @@ static int bgp_capability_orf_entry(struct peer *peer, u_char num; iana_afi_t pkt_afi; afi_t afi; - safi_t pkt_safi, safi; + iana_safi_t pkt_safi; + safi_t safi; u_char type; u_char mode; u_int16_t sm_cap = 0; /* capability send-mode receive */ @@ -466,7 +467,7 @@ static int bgp_capability_restart(struct peer *peer, afi_t afi; safi_t safi; iana_afi_t pkt_afi = stream_getw(s); - safi_t pkt_safi = stream_getc(s); + iana_safi_t pkt_safi = stream_getc(s); u_char flag = stream_getc(s); /* Convert AFI, SAFI to internal values, check. */ @@ -543,7 +544,7 @@ static int bgp_capability_addpath(struct peer *peer, afi_t afi; safi_t safi; iana_afi_t pkt_afi = stream_getw(s); - safi_t pkt_safi = stream_getc(s); + iana_safi_t pkt_safi = stream_getc(s); u_char send_receive = stream_getc(s); if (bgp_debug_neighbor_events(peer)) @@ -600,7 +601,8 @@ static int bgp_capability_enhe(struct peer *peer, struct capability_header *hdr) while (stream_get_getp(s) + 6 <= end) { iana_afi_t pkt_afi = stream_getw(s); afi_t afi; - safi_t safi, pkt_safi = stream_getw(s); + iana_safi_t pkt_safi = stream_getw(s); + safi_t safi; iana_afi_t pkt_nh_afi = stream_getw(s); afi_t nh_afi; @@ -1199,7 +1201,7 @@ static void bgp_open_capability_orf(struct stream *s, struct peer *peer, unsigned long numberp; int number_of_orfs = 0; iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; /* Convert AFI, SAFI to values for packet. */ bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi, &pkt_safi); @@ -1264,7 +1266,8 @@ void bgp_open_capability(struct stream *s, struct peer *peer) unsigned long cp, capp, rcapp; iana_afi_t pkt_afi; afi_t afi; - safi_t safi, pkt_safi; + safi_t safi; + iana_safi_t pkt_safi; as_t local_as; u_int32_t restart_time; u_char afi_safi_count = 0; diff --git a/bgpd/bgp_open.h b/bgpd/bgp_open.h index cbd32116b4..6b92e6e38c 100644 --- a/bgpd/bgp_open.h +++ b/bgpd/bgp_open.h @@ -31,7 +31,7 @@ struct capability_header { struct capability_mp_data { iana_afi_t afi; u_char reserved; - safi_t safi; + iana_safi_t safi; }; struct capability_as4 { diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index e92f2d6977..003fbaff63 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -142,7 +142,7 @@ static struct stream *bgp_update_packet_eor(struct peer *peer, afi_t afi, { struct stream *s; iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; if (DISABLE_BGP_ANNOUNCE) return NULL; @@ -671,7 +671,7 @@ void bgp_route_refresh_send(struct peer *peer, afi_t afi, safi_t safi, struct bgp_filter *filter; int orf_refresh = 0; iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; if (DISABLE_BGP_ANNOUNCE) return; @@ -761,7 +761,7 @@ void bgp_capability_send(struct peer *peer, afi_t afi, safi_t safi, { struct stream *s; iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; /* Convert AFI, SAFI to values for packet. */ bgp_map_afi_safi_int2iana(afi, safi, &pkt_afi, &pkt_safi); @@ -1338,8 +1338,9 @@ int bgp_nlri_parse(struct peer *peer, struct attr *attr, packet); case SAFI_EVPN: return bgp_nlri_parse_evpn(peer, attr, packet, mp_withdraw); + default: + return -1; } - return -1; } /* Parse BGP Update packet and make attribute object. */ @@ -1697,7 +1698,8 @@ static void bgp_route_refresh_receive(struct peer *peer, bgp_size_t size) { iana_afi_t pkt_afi; afi_t afi; - safi_t pkt_safi, safi; + iana_safi_t pkt_safi; + safi_t safi; struct stream *s; struct peer_af *paf; struct update_group *updgrp; @@ -1965,7 +1967,8 @@ static int bgp_capability_msg_parse(struct peer *peer, u_char *pnt, u_char action; iana_afi_t pkt_afi; afi_t afi; - safi_t pkt_safi, safi; + iana_safi_t pkt_safi; + safi_t safi; end = pnt + length; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index b554aeb32b..8c2278339f 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2288,7 +2288,7 @@ int bgp_maximum_prefix_overflow(struct peer *peer, afi_t afi, safi_t safi, int always) { iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; if (!CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_MAX_PREFIX)) return 0; diff --git a/bgpd/bgp_updgrp_packet.c b/bgpd/bgp_updgrp_packet.c index 1a23a36e91..692db32fa4 100644 --- a/bgpd/bgp_updgrp_packet.c +++ b/bgpd/bgp_updgrp_packet.c @@ -845,7 +845,7 @@ struct bpacket *subgroup_update_packet(struct update_subgroup *subgrp) send_attr_str); if (!stream_empty(snlri)) { iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; pkt_afi = afi_int2iana(afi); pkt_safi = safi_int2iana(safi); @@ -989,7 +989,7 @@ struct bpacket *subgroup_withdraw_packet(struct update_subgroup *subgrp) /* If first time, format the MP_UNREACH header */ if (first_time) { iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; pkt_afi = afi_int2iana(afi); pkt_safi = safi_int2iana(safi); diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index db503516d5..0220a7e55d 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -78,6 +78,10 @@ static enum node_type bgp_node_type(afi_t afi, safi_t safi) case SAFI_MPLS_VPN: return BGP_VPNV4_NODE; break; + default: + /* not expected */ + return BGP_IPV4_NODE; + break; } break; case AFI_IP6: @@ -94,6 +98,10 @@ static enum node_type bgp_node_type(afi_t afi, safi_t safi) case SAFI_MPLS_VPN: return BGP_VPNV6_NODE; break; + default: + /* not expected */ + return BGP_IPV4_NODE; + break; } break; case AFI_L2VPN: diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index a078bdfd7e..0729d0a1b3 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -611,8 +611,8 @@ int bgp_listen_limit_unset(struct bgp *bgp) return 0; } -int bgp_map_afi_safi_iana2int(iana_afi_t pkt_afi, safi_t pkt_safi, afi_t *afi, - safi_t *safi) +int bgp_map_afi_safi_iana2int(iana_afi_t pkt_afi, iana_safi_t pkt_safi, + afi_t *afi, safi_t *safi) { /* Map from IANA values to internal values, return error if * values are unrecognized. @@ -626,7 +626,7 @@ int bgp_map_afi_safi_iana2int(iana_afi_t pkt_afi, safi_t pkt_safi, afi_t *afi, } int bgp_map_afi_safi_int2iana(afi_t afi, safi_t safi, iana_afi_t *pkt_afi, - safi_t *pkt_safi) + iana_safi_t *pkt_safi) { /* Map from internal values to IANA values, return error if * internal values are bad (unexpected). diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 67b8289c70..ff15115e03 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -1381,10 +1381,10 @@ extern void bgp_route_map_terminate(void); extern int peer_cmp(struct peer *p1, struct peer *p2); -extern int bgp_map_afi_safi_iana2int(iana_afi_t pkt_afi, safi_t pkt_safi, +extern int bgp_map_afi_safi_iana2int(iana_afi_t pkt_afi, iana_safi_t pkt_safi, afi_t *afi, safi_t *safi); extern int bgp_map_afi_safi_int2iana(afi_t afi, safi_t safi, - iana_afi_t *pkt_afi, safi_t *pkt_safi); + iana_afi_t *pkt_afi, iana_safi_t *pkt_safi); extern struct peer_af *peer_af_create(struct peer *, afi_t, safi_t); extern struct peer_af *peer_af_find(struct peer *, afi_t, safi_t); diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c index 0bbbe12cce..61739d38ab 100644 --- a/bgpd/rfapi/rfapi_import.c +++ b/bgpd/rfapi/rfapi_import.c @@ -3872,6 +3872,10 @@ rfapiBgpInfoFilteredImportFunction(safi_t safi) case SAFI_ENCAP: return rfapiBgpInfoFilteredImportEncap; + + default: + /* not expected */ + return NULL; } zlog_err("%s: bad safi %d", __func__, safi); return NULL; diff --git a/lib/prefix.c b/lib/prefix.c index 88b13cd99f..0ba0025c68 100644 --- a/lib/prefix.c +++ b/lib/prefix.c @@ -507,8 +507,9 @@ const char *safi2str(safi_t safi) return "evpn"; case SAFI_LABELED_UNICAST: return "labeled-unicast"; + default: + return "unknown"; } - return NULL; } /* If n includes p prefix then return 1 else return 0. */ diff --git a/lib/zebra.h b/lib/zebra.h index 7b705bb83f..1924675a14 100644 --- a/lib/zebra.h +++ b/lib/zebra.h @@ -416,13 +416,15 @@ extern const char *zserv_command_string(unsigned int command); typedef enum { AFI_IP = 1, AFI_IP6 = 2, AFI_L2VPN = 3, AFI_MAX = 4 } afi_t; /* Subsequent Address Family Identifier. */ -#define SAFI_UNICAST 1 -#define SAFI_MULTICAST 2 -#define SAFI_MPLS_VPN 3 -#define SAFI_ENCAP 4 -#define SAFI_EVPN 5 -#define SAFI_LABELED_UNICAST 6 -#define SAFI_MAX 7 +typedef enum { + SAFI_UNICAST = 1, + SAFI_MULTICAST = 2, + SAFI_MPLS_VPN = 3, + SAFI_ENCAP = 4, + SAFI_EVPN = 5, + SAFI_LABELED_UNICAST = 6, + SAFI_MAX = 7 +} safi_t; /* * The above AFI and SAFI definitions are for internal use. The protocol @@ -442,13 +444,15 @@ typedef enum { IANA_AFI_IP6MR = 129 } iana_afi_t; -#define IANA_SAFI_RESERVED 0 -#define IANA_SAFI_UNICAST 1 -#define IANA_SAFI_MULTICAST 2 -#define IANA_SAFI_LABELED_UNICAST 4 -#define IANA_SAFI_ENCAP 7 -#define IANA_SAFI_EVPN 70 -#define IANA_SAFI_MPLS_VPN 128 +typedef enum { + IANA_SAFI_RESERVED = 0, + IANA_SAFI_UNICAST = 1, + IANA_SAFI_MULTICAST = 2, + IANA_SAFI_LABELED_UNICAST = 4, + IANA_SAFI_ENCAP = 7, + IANA_SAFI_EVPN = 70, + IANA_SAFI_MPLS_VPN = 128 +} iana_safi_t; /* Default Administrative Distance of each protocol. */ #define ZEBRA_KERNEL_DISTANCE_DEFAULT 0 @@ -469,8 +473,6 @@ typedef enum { #define UNSET_FLAG(V,F) (V) &= ~(F) #define RESET_FLAG(V) (V) = 0 -typedef u_int8_t safi_t; - /* Zebra types. Used in Zserv message header. */ typedef u_int16_t zebra_size_t; typedef u_int16_t zebra_command_t; @@ -504,7 +506,7 @@ static inline iana_afi_t afi_int2iana(afi_t afi) return IANA_AFI_RESERVED; } -static inline safi_t safi_iana2int(safi_t safi) +static inline safi_t safi_iana2int(iana_safi_t safi) { if (safi == IANA_SAFI_UNICAST) return SAFI_UNICAST; @@ -521,7 +523,7 @@ static inline safi_t safi_iana2int(safi_t safi) return SAFI_MAX; } -static inline safi_t safi_int2iana(safi_t safi) +static inline iana_safi_t safi_int2iana(safi_t safi) { if (safi == SAFI_UNICAST) return IANA_SAFI_UNICAST; diff --git a/tests/bgpd/test_mp_attr.c b/tests/bgpd/test_mp_attr.c index 7c0afa1b92..30d5fdd6cd 100644 --- a/tests/bgpd/test_mp_attr.c +++ b/tests/bgpd/test_mp_attr.c @@ -1059,7 +1059,7 @@ static void parse_test(struct peer *peer, struct test_segment *t, int type) parse_ret = bgp_mp_unreach_parse(&attr_args, &nlri); if (!parse_ret) { iana_afi_t pkt_afi; - safi_t pkt_safi; + iana_safi_t pkt_safi; /* Convert AFI, SAFI to internal values, check. */ if (bgp_map_afi_safi_int2iana(nlri.afi, nlri.safi, &pkt_afi, From 085347cfadbca04a7dfee175971c23e266becc6b Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Mon, 31 Jul 2017 21:22:20 -0300 Subject: [PATCH 3/5] lib: use switch statements in the AFI/SAFI conversion functions Switch statements are more elegant (and potentially faster... but that's not the main motivation). Signed-off-by: Renato Westphal --- lib/zebra.h | 56 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/zebra.h b/lib/zebra.h index 1924675a14..c31fe0809c 100644 --- a/lib/zebra.h +++ b/lib/zebra.h @@ -486,58 +486,70 @@ typedef uint32_t route_tag_t; static inline afi_t afi_iana2int(iana_afi_t afi) { - if (afi == IANA_AFI_IPV4) + switch (afi) { + case IANA_AFI_IPV4: return AFI_IP; - if (afi == IANA_AFI_IPV6) + case IANA_AFI_IPV6: return AFI_IP6; - if (afi == IANA_AFI_L2VPN) + case IANA_AFI_L2VPN: return AFI_L2VPN; - return AFI_MAX; + default: + return AFI_MAX; + } } static inline iana_afi_t afi_int2iana(afi_t afi) { - if (afi == AFI_IP) + switch (afi) { + case AFI_IP: return IANA_AFI_IPV4; - if (afi == AFI_IP6) + case AFI_IP6: return IANA_AFI_IPV6; - if (afi == AFI_L2VPN) + case AFI_L2VPN: return IANA_AFI_L2VPN; - return IANA_AFI_RESERVED; + default: + return IANA_AFI_RESERVED; + } } static inline safi_t safi_iana2int(iana_safi_t safi) { - if (safi == IANA_SAFI_UNICAST) + switch (safi) { + case IANA_SAFI_UNICAST: return SAFI_UNICAST; - if (safi == IANA_SAFI_MULTICAST) + case IANA_SAFI_MULTICAST: return SAFI_MULTICAST; - if (safi == IANA_SAFI_MPLS_VPN) + case IANA_SAFI_MPLS_VPN: return SAFI_MPLS_VPN; - if (safi == IANA_SAFI_ENCAP) + case IANA_SAFI_ENCAP: return SAFI_ENCAP; - if (safi == IANA_SAFI_EVPN) + case IANA_SAFI_EVPN: return SAFI_EVPN; - if (safi == IANA_SAFI_LABELED_UNICAST) + case IANA_SAFI_LABELED_UNICAST: return SAFI_LABELED_UNICAST; - return SAFI_MAX; + default: + return SAFI_MAX; + } } static inline iana_safi_t safi_int2iana(safi_t safi) { - if (safi == SAFI_UNICAST) + switch (safi) { + case SAFI_UNICAST: return IANA_SAFI_UNICAST; - if (safi == SAFI_MULTICAST) + case SAFI_MULTICAST: return IANA_SAFI_MULTICAST; - if (safi == SAFI_MPLS_VPN) + case SAFI_MPLS_VPN: return IANA_SAFI_MPLS_VPN; - if (safi == SAFI_ENCAP) + case SAFI_ENCAP: return IANA_SAFI_ENCAP; - if (safi == SAFI_EVPN) + case SAFI_EVPN: return IANA_SAFI_EVPN; - if (safi == SAFI_LABELED_UNICAST) + case SAFI_LABELED_UNICAST: return IANA_SAFI_LABELED_UNICAST; - return IANA_SAFI_RESERVED; + default: + return IANA_SAFI_RESERVED; + } } #endif /* _ZEBRA_H */ From a46a2e9b4e8de782ac07e01429a80ed7ec167dcb Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Mon, 31 Jul 2017 21:37:46 -0300 Subject: [PATCH 4/5] bgpd: don't make any assumptions about the size of an enum The size of an enum is compiler dependent and thus we shouldn't use enums inside structures that represent fields of a packet. Problem detected by the 'test_capability' unit test. The problem was not apparent before because the 'iana_safi_t' enum didn't exist and 'safi_t' was a typedef to uint8_t. Now we have two different enums, 'iana_afi_t' and 'iana_safi_t', and both need to be encoded in different ways on the wire (2 bytes vs 1 byte). Signed-off-by: Renato Westphal --- bgpd/bgp_open.h | 4 ++-- bgpd/bgpd.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_open.h b/bgpd/bgp_open.h index 6b92e6e38c..83b79a589a 100644 --- a/bgpd/bgp_open.h +++ b/bgpd/bgp_open.h @@ -29,9 +29,9 @@ struct capability_header { /* Generic MP capability data */ struct capability_mp_data { - iana_afi_t afi; + uint16_t afi; /* iana_afi_t */ u_char reserved; - iana_safi_t safi; + uint8_t safi; /* iana_safi_t */ }; struct capability_as4 { diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index ff15115e03..250ee5f4e8 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -922,10 +922,10 @@ DECLARE_QOBJ_TYPE(peer) stream. */ struct bgp_nlri { /* AFI. */ - afi_t afi; + uint16_t afi; /* iana_afi_t */ /* SAFI. */ - safi_t safi; + uint8_t safi; /* iana_safi_t */ /* Pointer to NLRI byte stream. */ u_char *nlri; From 399aedd6375f48ca5e99cc9391973566540b20a2 Mon Sep 17 00:00:00 2001 From: Renato Westphal Date: Mon, 31 Jul 2017 21:01:34 -0300 Subject: [PATCH 5/5] tests: fix small typo Signed-off-by: Renato Westphal --- tests/bgpd/test_capability.c | 2 +- tests/bgpd/test_capability.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/bgpd/test_capability.c b/tests/bgpd/test_capability.c index 9ec2b5df19..e8700a8b4a 100644 --- a/tests/bgpd/test_capability.c +++ b/tests/bgpd/test_capability.c @@ -170,7 +170,7 @@ static struct test_segment mp_segments[] = { /* 8 */ { "MP6", - "MP IP4/MPLS-laveled VPN", + "MP IP4/MPLS-labeled VPN", {CAPABILITY_CODE_MP, 0x4, 0x0, 0x1, 0x0, 0x80}, 6, SHOULD_PARSE, diff --git a/tests/bgpd/test_capability.py b/tests/bgpd/test_capability.py index 4cb650092b..872fcb6d12 100644 --- a/tests/bgpd/test_capability.py +++ b/tests/bgpd/test_capability.py @@ -8,7 +8,7 @@ TestCapability.okfail("MPv6: MP IPv6/Uni") TestCapability.okfail("MP2: MP IP/Multicast") TestCapability.okfail("MP3: MP IP6/MPLS-labeled VPN") TestCapability.okfail("MP5: MP IP6/MPLS-VPN") -TestCapability.okfail("MP6: MP IP4/MPLS-laveled VPN") +TestCapability.okfail("MP6: MP IP4/MPLS-labeled VPN") TestCapability.okfail("MP8: MP unknown AFI/SAFI") TestCapability.okfail("MP-short: MP IP4/Unicast, length too short (< minimum)") TestCapability.okfail("MP-overflow: MP IP4/Unicast, length too long")