From 8029b21687a4de7364607fc2aa9925d770a73507 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Wed, 10 Oct 2018 14:28:32 -0700 Subject: [PATCH 1/8] bgpd: hidden commands to add/del a local mac local mac add/del comes from zebra. the hidden commands help verify various race conditions between bgp and zebra. Ticket: CM-22687 Reviewed By: CCR-7939 Signed-off-by: Anuradha Karuppiah --- bgpd/bgp_vty.c | 85 ++++++++++++++++++++++++++++++++++++++++++++++++++ bgpd/bgp_vty.h | 2 ++ 2 files changed, 87 insertions(+) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index ecbe33ff8c..70e49a90da 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -822,6 +822,87 @@ DEFUN_HIDDEN (no_bgp_multiple_instance, return CMD_SUCCESS; } +DEFUN_HIDDEN (bgp_local_mac, + bgp_local_mac_cmd, + "bgp local-mac vni " BGP_CMD_VNI_RANGE " mac WORD seq (0-4294967295)", + BGP_STR + "Local MAC config\n" + "VxLAN Network Identifier\n" + "VNI number\n" + "local mac\n" + "mac address\n" + "mac-mobility sequence\n" + "seq number\n") +{ + int rv; + vni_t vni; + struct ethaddr mac; + struct ipaddr ip; + uint32_t seq; + struct bgp *bgp; + + vni = strtoul(argv[3]->arg, NULL, 10); + if (!prefix_str2mac(argv[5]->arg, &mac)) { + vty_out(vty, "%% Malformed MAC address\n"); + return CMD_WARNING; + } + memset(&ip, 0, sizeof(ip)); + seq = strtoul(argv[7]->arg, NULL, 10); + + bgp = bgp_get_default(); + if (!bgp) { + vty_out(vty, "Default BGP instance is not there\n"); + return CMD_WARNING; + } + + rv = bgp_evpn_local_macip_add(bgp, vni, &mac, &ip, 0 /* flags */, seq); + if (rv < 0) { + vty_out(vty, "Internal error\n"); + return CMD_WARNING; + } + + return CMD_SUCCESS; +} + +DEFUN_HIDDEN (no_bgp_local_mac, + no_bgp_local_mac_cmd, + "no bgp local-mac vni " BGP_CMD_VNI_RANGE " mac WORD", + NO_STR + BGP_STR + "Local MAC config\n" + "VxLAN Network Identifier\n" + "VNI number\n" + "local mac\n" + "mac address\n") +{ + int rv; + vni_t vni; + struct ethaddr mac; + struct ipaddr ip; + struct bgp *bgp; + + vni = strtoul(argv[4]->arg, NULL, 10); + if (!prefix_str2mac(argv[6]->arg, &mac)) { + vty_out(vty, "%% Malformed MAC address\n"); + return CMD_WARNING; + } + memset(&ip, 0, sizeof(ip)); + + bgp = bgp_get_default(); + if (!bgp) { + vty_out(vty, "Default BGP instance is not there\n"); + return CMD_WARNING; + } + + rv = bgp_evpn_local_macip_del(bgp, vni, &mac, &ip); + if (rv < 0) { + vty_out(vty, "Internal error\n"); + return CMD_WARNING; + } + + return CMD_SUCCESS; +} + #if (CONFDATE > 20190601) CPP_NOTICE("bgpd: time to remove deprecated cli bgp config-type cisco") CPP_NOTICE("This includes BGP_OPT_CISCO_CONFIG") @@ -12579,6 +12660,10 @@ void bgp_vty_init(void) install_element(CONFIG_NODE, &bgp_config_type_cmd); install_element(CONFIG_NODE, &no_bgp_config_type_cmd); + /* "bgp local-mac" hidden commands. */ + install_element(CONFIG_NODE, &bgp_local_mac_cmd); + install_element(CONFIG_NODE, &no_bgp_local_mac_cmd); + /* bgp route-map delay-timer commands. */ install_element(CONFIG_NODE, &bgp_set_route_map_delay_timer_cmd); install_element(CONFIG_NODE, &no_bgp_set_route_map_delay_timer_cmd); diff --git a/bgpd/bgp_vty.h b/bgpd/bgp_vty.h index d9df2b4cfe..efb8902d97 100644 --- a/bgpd/bgp_vty.h +++ b/bgpd/bgp_vty.h @@ -44,6 +44,8 @@ struct bgp; "Address Family modifier\n" \ "Address Family modifier\n" +#define BGP_CMD_VNI_RANGE "(1-16777215)" + extern void bgp_vty_init(void); extern const char *afi_safi_print(afi_t afi, safi_t safi); extern const char *afi_safi_json(afi_t afi, safi_t safi); From e98e4b8899a696e13bcfcc8fa25b3546c1f4c244 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Thu, 11 Oct 2018 08:48:42 -0700 Subject: [PATCH 2/8] zebra: send a local-mac del to bgpd on mac mod to remote When events cross paths between bgp and zebra bgpd could end up with a dangling local MAC entry. Consider the following sequence of events on rack-1 - 1. MAC1 has MM sequence number 1 and points to rack-3 2. Now a packet is rxed locally on rack-1 and rack-2 (simultaneously) with source-mac=MAC1. 3. This would cause rack-1 and rack-2 to set the MM seq to 2 and simultaneously report the MAC as local. 4. Now let's say on rack-1 zebra's MACIP_ADD is in bgpd's queue. bgpd accepts rack-3's update and sends a remote MACIP add to zebra with MM=2. 5. zebra updates the MAC entry from local=>remote. 6. bgpd now processes zebra's "stale local" making it the best path. However zebra no longer has a local MAC entry. At this point bgpd and zebra are effectively out of sync i.e. bgpd has a local-MAC which is not present in the kernel or in zebra. To handle this window zebra should send a local MAC delete to bgpd on modifying its cache to remote. Ticket: CM-22687 Reviewed By: CCR-7935 Signed-off-by: Anuradha Karuppiah --- zebra/zebra_vxlan.c | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index 58cf6eb30f..9ecf834333 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -148,8 +148,7 @@ static void zvni_mac_del_all(zebra_vni_t *zvni, int uninstall, int upd_client, static zebra_mac_t *zvni_mac_lookup(zebra_vni_t *zvni, struct ethaddr *macaddr); static int zvni_mac_send_add_to_client(vni_t vni, struct ethaddr *macaddr, uint8_t flags, uint32_t seq); -static int zvni_mac_send_del_to_client(vni_t vni, struct ethaddr *macaddr, - uint8_t flags); +static int zvni_mac_send_del_to_client(vni_t vni, struct ethaddr *macaddr); static zebra_vni_t *zvni_map_vlan(struct interface *ifp, struct interface *br_if, vlanid_t vid); static int zvni_mac_install(zebra_vni_t *zvni, zebra_mac_t *mac); @@ -2321,7 +2320,7 @@ static void zvni_mac_del_hash_entry(struct hash_backet *backet, void *arg) &wctx->r_vtep_ip))) { if (wctx->upd_client && (mac->flags & ZEBRA_MAC_LOCAL)) { zvni_mac_send_del_to_client(wctx->zvni->vni, - &mac->macaddr, mac->flags); + &mac->macaddr); } if (wctx->uninstall) @@ -2408,18 +2407,10 @@ static int zvni_mac_send_add_to_client(vni_t vni, struct ethaddr *macaddr, /* * Inform BGP about local MAC deletion. */ -static int zvni_mac_send_del_to_client(vni_t vni, struct ethaddr *macaddr, - uint8_t mac_flags) +static int zvni_mac_send_del_to_client(vni_t vni, struct ethaddr *macaddr) { - uint8_t flags = 0; - - if (CHECK_FLAG(mac_flags, ZEBRA_MAC_STICKY)) - SET_FLAG(flags, ZEBRA_MACIP_TYPE_STICKY); - if (CHECK_FLAG(mac_flags, ZEBRA_MAC_DEF_GW)) - SET_FLAG(flags, ZEBRA_MACIP_TYPE_GW); - - return zvni_macip_send_msg_to_client(vni, macaddr, NULL, flags, - 0, ZEBRA_MACIP_DEL); + return zvni_macip_send_msg_to_client(vni, macaddr, NULL, 0 /* flags */, + 0 /* seq */, ZEBRA_MACIP_DEL); } /* @@ -4218,6 +4209,10 @@ static void process_remote_macip_add(vni_t vni, } } + /* Remove local MAC from BGP. */ + if (CHECK_FLAG(mac->flags, ZEBRA_MAC_LOCAL)) + zvni_mac_send_del_to_client(zvni->vni, macaddr); + /* Set "auto" and "remote" forwarding info. */ UNSET_FLAG(mac->flags, ZEBRA_MAC_LOCAL); memset(&mac->fwd_info, 0, sizeof(mac->fwd_info)); @@ -4238,6 +4233,7 @@ static void process_remote_macip_add(vni_t vni, /* Install the entry. */ zvni_mac_install(zvni, mac); + } /* Update seq number. */ @@ -5631,7 +5627,7 @@ int zebra_vxlan_check_del_local_mac(struct interface *ifp, ifp->ifindex, vni); /* Remove MAC from BGP. */ - zvni_mac_send_del_to_client(zvni->vni, macaddr, mac->flags); + zvni_mac_send_del_to_client(zvni->vni, macaddr); /* * If there are no neigh associated with the mac delete the mac @@ -5742,7 +5738,7 @@ int zebra_vxlan_local_mac_del(struct interface *ifp, struct interface *br_if, zvni_process_neigh_on_local_mac_del(zvni, mac); /* Remove MAC from BGP. */ - zvni_mac_send_del_to_client(zvni->vni, macaddr, mac->flags); + zvni_mac_send_del_to_client(zvni->vni, macaddr); /* * If there are no neigh associated with the mac delete the mac From 3e3aa88e5fbd6a19e37b7c211013b8dc73d61ef7 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Fri, 12 Oct 2018 08:43:19 -0700 Subject: [PATCH 3/8] bgpd: perform route selection again when the local path is deleted This is needed to install the remote dst when a more preferred local path is removed. Ticket: CM-22685 Reviewed By: CCR-7936 Signed-off-by: Anuradha Karuppiah --- bgpd/bgp_evpn.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 574ece8cc5..945094a671 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -1921,8 +1921,10 @@ static int delete_evpn_route(struct bgp *bgp, struct bgpevpn *vpn, /* Delete route entry in the VNI route table. This can just be removed. */ delete_evpn_route_entry(bgp, afi, safi, rn, &pi); - if (pi) + if (pi) { bgp_path_info_reap(rn, pi); + evpn_route_select_install(bgp, vpn, rn); + } bgp_unlock_node(rn); return 0; From 6d8c603a930f229aac931aac2f556f79f4b7b437 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Mon, 15 Oct 2018 08:16:51 -0700 Subject: [PATCH 4/8] bgpd: use IP address as tie breaker if the MM seq number is the same Same sequence number handling is specified by RFC 7432 - [ If two (or more) PEs advertise the same MAC address with the same sequence number but different Ethernet segment identifiers, a PE that receives these routes selects the route advertised by the PE with the lowest IP address as the best route. If the PE is the originator of the MAC route and it receives the same MAC address with the same sequence number that it generated, it will compare its own IP address with the IP address of the remote PE and will select the lowest IP. If its own route is not the best one, it will withdraw the route. ] To implement that specification this commit uses nexthop IP as a tie breaker between two paths of equal seq number with lower IP winning. Now if a local path already exists with the same sequence number but higher (local-VTEP) IP it is evicted (deleted and withdrawn from the peers) and the winning new remote path is installed in zebra. This is existing code and handled implicitly via evpn_route_select_install. If a local path is rxed from zebra with the same sequence as the current remote winner it is rejected (not installed in the bgp routing tables) and zebra is asked to re-install the older/remote winner. This is a race condition that can only happen if bgp's add and zebra's add cross paths. Additional handling has been added in this commit via evpn_cleanup_local_non_best_route to take care of the race condition. Ticket: CM-22674 Reviewed By: CCR-7937 Signed-off-by: Anuradha Karuppiah --- bgpd/bgp_evpn.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++ bgpd/bgp_route.c | 23 ++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 945094a671..961e3b84de 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -1685,6 +1685,65 @@ static int update_evpn_route_entry(struct bgp *bgp, struct bgpevpn *vpn, return route_change; } +/* + * If the local route was not selected evict it and tell zebra to re-add + * the best remote dest. + * + * Typically a local path added by zebra is expected to be selected as + * best. In which case when a remote path wins as best (later) + * evpn_route_select_install itself evicts the older-local-best path. + * + * However if bgp's add and zebra's add cross paths (race condition) it + * is possible that the local path is no longer the "older" best path. + * It is a path that was never designated as best and hence requires + * additional handling to prevent bgp from injecting and holding on to a + * non-best local path. + */ +static void evpn_cleanup_local_non_best_route(struct bgp *bgp, + struct bgpevpn *vpn, + struct bgp_node *rn, + struct bgp_path_info *local_pi, + int *route_change) +{ + struct bgp_path_info *tmp_pi; + struct bgp_path_info *curr_select = NULL; + uint8_t flags = 0; + char buf[PREFIX_STRLEN]; + + if (CHECK_FLAG(local_pi->flags, BGP_PATH_SELECTED)) { + /* local path is the winner; no additional cleanup needed */ + return; + } + + /* local path was not picked as the winner; kick it out */ + if (bgp_debug_zebra(NULL)) { + zlog_debug("evicting local evpn prefix %s as remote won", + prefix2str(&rn->p, buf, sizeof(buf))); + } + *route_change = 0; + evpn_delete_old_local_route(bgp, vpn, rn, local_pi); + bgp_path_info_reap(rn, local_pi); + + /* tell zebra to re-add the best remote path */ + for (tmp_pi = rn->info; tmp_pi; tmp_pi = tmp_pi->next) { + if (CHECK_FLAG(tmp_pi->flags, BGP_PATH_SELECTED)) { + curr_select = tmp_pi; + break; + } + } + if (curr_select && + curr_select->type == ZEBRA_ROUTE_BGP + && curr_select->sub_type == BGP_ROUTE_IMPORTED) { + if (curr_select->attr->sticky) + SET_FLAG(flags, ZEBRA_MACIP_TYPE_STICKY); + if (curr_select->attr->default_gw) + SET_FLAG(flags, ZEBRA_MACIP_TYPE_GW); + evpn_zebra_install(bgp, vpn, (struct prefix_evpn *)&rn->p, + curr_select->attr->nexthop, flags, + mac_mobility_seqnum(curr_select->attr)); + } +} + /* * Create or update EVPN route (of type based on prefix) for specified VNI * and schedule for processing. @@ -1747,10 +1806,19 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn, assert(pi); attr_new = pi->attr; + /* lock ri to prevent freeing in evpn_route_select_install */ + bgp_path_info_lock(pi); /* Perform route selection; this is just to set the flags correctly * as local route in the VNI always wins. */ evpn_route_select_install(bgp, vpn, rn); + /* + * if the new local route was not selected evict it and tell zebra + * to add the best remote dest + */ + evpn_cleanup_local_non_best_route(bgp, vpn, rn, pi, &route_change); + bgp_path_info_unlock(pi); + bgp_unlock_node(rn); /* If this is a new route or some attribute has changed, export the diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index c20b404f19..4c408ff9dd 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -453,6 +453,7 @@ static int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, char exist_buf[PATH_ADDPATH_STR_BUFFER]; uint32_t new_mm_seq; uint32_t exist_mm_seq; + int nh_cmp; *paths_eq = 0; @@ -545,6 +546,28 @@ static int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, exist_mm_seq); return 0; } + + /* + * if sequence numbers are the same path with the lowest IP + * wins + */ + nh_cmp = bgp_path_info_nexthop_cmp(new, exist); + if (nh_cmp < 0) { + if (debug) + zlog_debug( + "%s: %s wins over %s due to same MM seq %u and lower IP %s", + pfx_buf, new_buf, exist_buf, new_mm_seq, + inet_ntoa(new->attr->nexthop)); + return 1; + } + if (nh_cmp > 0) { + if (debug) + zlog_debug( + "%s: %s loses to %s due to same MM seq %u and higher IP %s", + pfx_buf, new_buf, exist_buf, new_mm_seq, + inet_ntoa(new->attr->nexthop)); + return 0; + } } /* 1. Weight check. */ From 4fd5ea4b3edf602e35373b6548639aa0a6dda6c2 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Tue, 16 Oct 2018 08:23:22 -0700 Subject: [PATCH 5/8] zebra: make neigh active when it is modified from local to remote This is a fixup to commit - f32ea5c07 - zebra: act on kernel notifications for remote neighbors The original commit handled a race condition between kernel and zebra that would result in an inconsistent state i.e. kernel has an offload/remote neigh zebra has a local neigh The original commit missed setting the neigh to active when zebra tried to resolve the inconsistency by modifying the local neigh to remote neigh on hearing back its own kernel update. Fixed here. Signed-off-by: Anuradha Karuppiah Signed-off-by: Vivek Venkatraman Ticket: CM-22700 --- zebra/zebra_vxlan.c | 1 + 1 file changed, 1 insertion(+) diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index 9ecf834333..b98b7381f4 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -2218,6 +2218,7 @@ static int zvni_remote_neigh_update(zebra_vni_t *zvni, UNSET_FLAG(n->flags, ZEBRA_NEIGH_LOCAL); SET_FLAG(n->flags, ZEBRA_NEIGH_REMOTE); + ZEBRA_NEIGH_SET_ACTIVE(n); n->r_vtep_ip = zmac->fwd_info.r_vtep_ip; } From f3a930da157cd9febea3d8f0f723fb8351e75d9c Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Tue, 16 Oct 2018 12:59:24 -0700 Subject: [PATCH 6/8] zebra: set remoteseq to 0 when remote mac is deleted by bgpd When the remote mac is deleted by bgpd we can end up with an auto mac entry in zebra if there are neighs referring to the mac. The remote sequence number in the auto mac entry needs to be reset to 0 as the mac entry may have been removed on all VTEPs (including the originating one). Now if the MAC comes back on a remote VTEP it may be added with MM=0 which will NOT be accepted if the remote seq was not reset in the previous step. Ticket: CM-22707 Signed-off-by: Anuradha Karuppiah Signed-off-by: Vivek Venkatraman --- zebra/zebra_vxlan.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index b98b7381f4..ab7ccc381c 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -4433,6 +4433,13 @@ static void process_remote_macip_del(vni_t vni, } else { if (CHECK_FLAG(mac->flags, ZEBRA_MAC_REMOTE)) { zvni_process_neigh_on_remote_mac_del(zvni, mac); + /* + * the remote sequence number in the auto mac entry + * needs to be reset to 0 as the mac entry may have + * been removed on all VTEPs (including + * the originating one) + */ + mac->rem_seq = 0; /* If all remote neighbors referencing a remote MAC * go away, we need to uninstall the MAC. From 093e3f23f63ae90ce2a7407ecdec9a86b99dacd9 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 18 Oct 2018 20:44:52 -0400 Subject: [PATCH 7/8] bgpd, lib, vtysh, zebra: Convert to using CMD_VNI_RANGE For the vni range use a macro to keep track of it. Signed-off-by: Donald Sharp --- bgpd/bgp_evpn_vty.c | 20 ++++++++++---------- bgpd/bgp_routemap.c | 4 ++-- bgpd/bgp_vty.c | 4 ++-- bgpd/bgp_vty.h | 2 -- lib/command.h | 1 + vtysh/vtysh.c | 2 +- zebra/zebra_vty.c | 3 --- 7 files changed, 16 insertions(+), 20 deletions(-) diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index 29f9f64cca..aa5eabeade 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -3195,7 +3195,7 @@ DEFUN (no_bgp_evpn_advertise_type5, */ DEFUN(show_bgp_l2vpn_evpn_vni, show_bgp_l2vpn_evpn_vni_cmd, - "show bgp l2vpn evpn vni [(1-16777215)] [json]", + "show bgp l2vpn evpn vni [" CMD_VNI_RANGE "] [json]", SHOW_STR BGP_STR L2VPN_HELP_STR @@ -3623,7 +3623,7 @@ DEFUN(show_bgp_l2vpn_evpn_route_esi, * Display per-VNI EVPN routing table. */ DEFUN(show_bgp_l2vpn_evpn_route_vni, show_bgp_l2vpn_evpn_route_vni_cmd, - "show bgp l2vpn evpn route vni (1-16777215) [ | vtep A.B.C.D>] [json]", + "show bgp l2vpn evpn route vni " CMD_VNI_RANGE " [ | vtep A.B.C.D>] [json]", SHOW_STR BGP_STR L2VPN_HELP_STR @@ -3696,7 +3696,7 @@ DEFUN(show_bgp_l2vpn_evpn_route_vni, show_bgp_l2vpn_evpn_route_vni_cmd, */ DEFUN(show_bgp_l2vpn_evpn_route_vni_macip, show_bgp_l2vpn_evpn_route_vni_macip_cmd, - "show bgp l2vpn evpn route vni (1-16777215) mac WORD [ip WORD] [json]", + "show bgp l2vpn evpn route vni " CMD_VNI_RANGE " mac WORD [ip WORD] [json]", SHOW_STR BGP_STR L2VPN_HELP_STR @@ -3766,7 +3766,7 @@ DEFUN(show_bgp_l2vpn_evpn_route_vni_macip, */ DEFUN(show_bgp_l2vpn_evpn_route_vni_multicast, show_bgp_l2vpn_evpn_route_vni_multicast_cmd, - "show bgp l2vpn evpn route vni (1-16777215) multicast A.B.C.D [json]", + "show bgp l2vpn evpn route vni " CMD_VNI_RANGE " multicast A.B.C.D [json]", SHOW_STR BGP_STR L2VPN_HELP_STR @@ -4019,7 +4019,7 @@ DEFUN(test_withdraw_evpn_type4_route, } ALIAS_HIDDEN(show_bgp_l2vpn_evpn_vni, show_bgp_evpn_vni_cmd, - "show bgp evpn vni [(1-16777215)]", SHOW_STR BGP_STR EVPN_HELP_STR + "show bgp evpn vni [" CMD_VNI_RANGE "]", SHOW_STR BGP_STR EVPN_HELP_STR "Show VNI\n" "VNI number\n") @@ -4060,7 +4060,7 @@ ALIAS_HIDDEN( ALIAS_HIDDEN( show_bgp_l2vpn_evpn_route_vni, show_bgp_evpn_route_vni_cmd, - "show bgp evpn route vni (1-16777215) [ | vtep A.B.C.D>]", + "show bgp evpn route vni " CMD_VNI_RANGE " [ | vtep A.B.C.D>]", SHOW_STR BGP_STR EVPN_HELP_STR "EVPN route information\n" "VXLAN Network Identifier\n" @@ -4073,7 +4073,7 @@ ALIAS_HIDDEN( ALIAS_HIDDEN(show_bgp_l2vpn_evpn_route_vni_macip, show_bgp_evpn_route_vni_macip_cmd, - "show bgp evpn route vni (1-16777215) mac WORD [ip WORD]", + "show bgp evpn route vni " CMD_VNI_RANGE " mac WORD [ip WORD]", SHOW_STR BGP_STR EVPN_HELP_STR "EVPN route information\n" "VXLAN Network Identifier\n" @@ -4085,7 +4085,7 @@ ALIAS_HIDDEN(show_bgp_l2vpn_evpn_route_vni_macip, ALIAS_HIDDEN(show_bgp_l2vpn_evpn_route_vni_multicast, show_bgp_evpn_route_vni_multicast_cmd, - "show bgp evpn route vni (1-16777215) multicast A.B.C.D", + "show bgp evpn route vni " CMD_VNI_RANGE " multicast A.B.C.D", SHOW_STR BGP_STR EVPN_HELP_STR "EVPN route information\n" "VXLAN Network Identifier\n" @@ -4108,7 +4108,7 @@ ALIAS_HIDDEN(show_bgp_l2vpn_evpn_import_rt, show_bgp_evpn_import_rt_cmd, DEFUN_NOSH (bgp_evpn_vni, bgp_evpn_vni_cmd, - "vni (1-16777215)", + "vni " CMD_VNI_RANGE, "VXLAN Network Identifier\n" "VNI number\n") { @@ -4134,7 +4134,7 @@ DEFUN_NOSH (bgp_evpn_vni, DEFUN (no_bgp_evpn_vni, no_bgp_evpn_vni_cmd, - "no vni (1-16777215)", + "no vni " CMD_VNI_RANGE, NO_STR "VXLAN Network Identifier\n" "VNI number\n") diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 60a4e994c9..f7c4175383 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -3438,7 +3438,7 @@ DEFUN (no_match_evpn_route_type, DEFUN (match_evpn_vni, match_evpn_vni_cmd, - "match evpn vni (1-16777215)", + "match evpn vni " CMD_VNI_RANGE, MATCH_STR EVPN_HELP_STR "Match VNI\n" @@ -3450,7 +3450,7 @@ DEFUN (match_evpn_vni, DEFUN (no_match_evpn_vni, no_match_evpn_vni_cmd, - "no match evpn vni (1-16777215)", + "no match evpn vni " CMD_VNI_RANGE, NO_STR MATCH_STR EVPN_HELP_STR diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index 70e49a90da..c8b325e72b 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -824,7 +824,7 @@ DEFUN_HIDDEN (no_bgp_multiple_instance, DEFUN_HIDDEN (bgp_local_mac, bgp_local_mac_cmd, - "bgp local-mac vni " BGP_CMD_VNI_RANGE " mac WORD seq (0-4294967295)", + "bgp local-mac vni " CMD_VNI_RANGE " mac WORD seq (0-4294967295)", BGP_STR "Local MAC config\n" "VxLAN Network Identifier\n" @@ -866,7 +866,7 @@ DEFUN_HIDDEN (bgp_local_mac, DEFUN_HIDDEN (no_bgp_local_mac, no_bgp_local_mac_cmd, - "no bgp local-mac vni " BGP_CMD_VNI_RANGE " mac WORD", + "no bgp local-mac vni " CMD_VNI_RANGE " mac WORD", NO_STR BGP_STR "Local MAC config\n" diff --git a/bgpd/bgp_vty.h b/bgpd/bgp_vty.h index efb8902d97..d9df2b4cfe 100644 --- a/bgpd/bgp_vty.h +++ b/bgpd/bgp_vty.h @@ -44,8 +44,6 @@ struct bgp; "Address Family modifier\n" \ "Address Family modifier\n" -#define BGP_CMD_VNI_RANGE "(1-16777215)" - extern void bgp_vty_init(void); extern const char *afi_safi_print(afi_t afi, safi_t safi); extern const char *afi_safi_json(afi_t afi, safi_t safi); diff --git a/lib/command.h b/lib/command.h index de65c8bd98..cfe0114297 100644 --- a/lib/command.h +++ b/lib/command.h @@ -377,6 +377,7 @@ struct cmd_node { #define WATCHFRR_STR "watchfrr information\n" #define ZEBRA_STR "Zebra information\n" +#define CMD_VNI_RANGE "(1-16777215)" #define CONF_BACKUP_EXT ".sav" /* Command warnings. */ diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index 35f719fa54..cd78551cb4 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -1458,7 +1458,7 @@ DEFUNSH_HIDDEN(VTYSH_BGPD, address_family_evpn2, address_family_evpn2_cmd, } #endif -DEFUNSH(VTYSH_BGPD, bgp_evpn_vni, bgp_evpn_vni_cmd, "vni (1-16777215)", +DEFUNSH(VTYSH_BGPD, bgp_evpn_vni, bgp_evpn_vni_cmd, "vni " CMD_VNI_RANGE, "VXLAN Network Identifier\n" "VNI number\n") { diff --git a/zebra/zebra_vty.c b/zebra/zebra_vty.c index 17609a03fe..8361dc61f6 100644 --- a/zebra/zebra_vty.c +++ b/zebra/zebra_vty.c @@ -68,9 +68,6 @@ static void vty_show_ip_route_summary(struct vty *vty, static void vty_show_ip_route_summary_prefix(struct vty *vty, struct route_table *table); -/* VNI range as per RFC 7432 */ -#define CMD_VNI_RANGE "(1-16777215)" - DEFUN (ip_multicast_mode, ip_multicast_mode_cmd, "ip multicast rpf-lookup-mode ", From 9a8897aa9a51c2151a1629c9c491ddc6d0dfb589 Mon Sep 17 00:00:00 2001 From: Anuradha Karuppiah Date: Fri, 19 Oct 2018 08:46:46 -0700 Subject: [PATCH 8/8] bgpd: move non-best local path checks outside the function This change is a fixup to - 7b5e18 - bgpd: use IP address as tie breaker if the MM seq number is the same And is being done in response to review comments. This commit brings no functional change; simply moves around code for easier maintanence. Signed-off-by: Anuradha Karuppiah --- bgpd/bgp_evpn.c | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index 961e3b84de..ae00534c39 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -1702,25 +1702,18 @@ static int update_evpn_route_entry(struct bgp *bgp, struct bgpevpn *vpn, static void evpn_cleanup_local_non_best_route(struct bgp *bgp, struct bgpevpn *vpn, struct bgp_node *rn, - struct bgp_path_info *local_pi, - int *route_change) + struct bgp_path_info *local_pi) { struct bgp_path_info *tmp_pi; struct bgp_path_info *curr_select = NULL; uint8_t flags = 0; char buf[PREFIX_STRLEN]; - if (CHECK_FLAG(local_pi->flags, BGP_PATH_SELECTED)) { - /* local path is the winner; no additional cleanup needed */ - return; - } - /* local path was not picked as the winner; kick it out */ if (bgp_debug_zebra(NULL)) { zlog_debug("evicting local evpn prefix %s as remote won", prefix2str(&rn->p, buf, sizeof(buf))); } - *route_change = 0; evpn_delete_old_local_route(bgp, vpn, rn, local_pi); bgp_path_info_reap(rn, local_pi); @@ -1813,10 +1806,14 @@ static int update_evpn_route(struct bgp *bgp, struct bgpevpn *vpn, */ evpn_route_select_install(bgp, vpn, rn); /* - * if the new local route was not selected evict it and tell zebra - * to add the best remote dest + * If the new local route was not selected evict it and tell zebra + * to re-add the best remote dest. BGP doesn't retain non-best local + * routes. */ - evpn_cleanup_local_non_best_route(bgp, vpn, rn, pi, &route_change); + if (!CHECK_FLAG(pi->flags, BGP_PATH_SELECTED)) { + route_change = 0; + evpn_cleanup_local_non_best_route(bgp, vpn, rn, pi); + } bgp_path_info_unlock(pi); bgp_unlock_node(rn);