From 1632988acf944b7f5e7d24c4aacbcfd4c5f626b0 Mon Sep 17 00:00:00 2001 From: Rajasekar Raja Date: Tue, 6 Aug 2024 11:46:45 -0700 Subject: [PATCH 01/46] zebra: vlan to dplane, Relocating some functions Relocating functions used by vlan in if_netlink into zebra vxlan Note: Static variable to the functions will be added back in the next commit. Ticket :#3878175 Signed-off-by: Rajasekar Raja --- zebra/if_netlink.c | 65 +-------------------------------------------- zebra/zebra_vxlan.c | 64 ++++++++++++++++++++++++++++++++++++++++++++ zebra/zebra_vxlan.h | 6 ++++- 3 files changed, 70 insertions(+), 65 deletions(-) diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 5fb908eb0d70..3de6661f5d5e 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -63,6 +63,7 @@ #include "zebra/zebra_l2.h" #include "zebra/netconf_netlink.h" #include "zebra/zebra_trace.h" +#include "zebra/zebra_vxlan.h" extern struct zebra_privs_t zserv_privs; @@ -1816,70 +1817,6 @@ int netlink_tunneldump_read(struct zebra_ns *zns) return 0; } -static const char *port_state2str(uint8_t state) -{ - switch (state) { - case BR_STATE_DISABLED: - return "DISABLED"; - case BR_STATE_LISTENING: - return "LISTENING"; - case BR_STATE_LEARNING: - return "LEARNING"; - case BR_STATE_FORWARDING: - return "FORWARDING"; - case BR_STATE_BLOCKING: - return "BLOCKING"; - } - - return "UNKNOWN"; -} - -static void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, - uint8_t state) -{ - struct zebra_vxlan_vni *vnip; - - vnip = zebra_vxlan_if_vlanid_vni_find(zif, id); - - if (!vnip) { - if (IS_ZEBRA_DEBUG_VXLAN) - zlog_debug( - "Cannot find VNI for VID (%u) IF %s for vlan state update", - id, zif->ifp->name); - - return; - } - - switch (state) { - case BR_STATE_FORWARDING: - zebra_vxlan_if_vni_up(zif->ifp, vnip); - break; - case BR_STATE_BLOCKING: - zebra_vxlan_if_vni_down(zif->ifp, vnip); - break; - case BR_STATE_DISABLED: - case BR_STATE_LISTENING: - case BR_STATE_LEARNING: - default: - /* Not used for anything at the moment */ - break; - } -} - -static void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, - uint16_t id_end, uint8_t state) -{ - struct zebra_if *zif; - - zif = (struct zebra_if *)ifp->info; - - if (!zif) - return; - - for (uint16_t i = id_start; i <= id_end; i++) - vxlan_vni_state_change(zif, i, state); -} - /** * netlink_vlan_change() - Read in change about vlans from the kernel * diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index f1ae42e32043..0f0464e486fa 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -41,6 +41,8 @@ #include "zebra/zebra_evpn_mh.h" #include "zebra/zebra_evpn_vxlan.h" #include "zebra/zebra_router.h" +#include "zebra/zebra_trace.h" +#include DEFINE_MTYPE_STATIC(ZEBRA, HOST_PREFIX, "host prefix"); DEFINE_MTYPE_STATIC(ZEBRA, ZL3VNI, "L3 VNI hash"); @@ -6246,3 +6248,65 @@ extern void zebra_evpn_init(void) { hook_register(zserv_client_close, zebra_evpn_cfg_clean_up); } + +const char *port_state2str(uint8_t state) +{ + switch (state) { + case BR_STATE_DISABLED: + return "DISABLED"; + case BR_STATE_LISTENING: + return "LISTENING"; + case BR_STATE_LEARNING: + return "LEARNING"; + case BR_STATE_FORWARDING: + return "FORWARDING"; + case BR_STATE_BLOCKING: + return "BLOCKING"; + } + + return "UNKNOWN"; +} + +void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, uint8_t state) +{ + struct zebra_vxlan_vni *vnip; + + vnip = zebra_vxlan_if_vlanid_vni_find(zif, id); + + if (!vnip) { + if (IS_ZEBRA_DEBUG_VXLAN) + zlog_debug("Cannot find VNI for VID (%u) IF %s for vlan state update", + id, zif->ifp->name); + + return; + } + + switch (state) { + case BR_STATE_FORWARDING: + zebra_vxlan_if_vni_up(zif->ifp, vnip); + break; + case BR_STATE_BLOCKING: + zebra_vxlan_if_vni_down(zif->ifp, vnip); + break; + case BR_STATE_DISABLED: + case BR_STATE_LISTENING: + case BR_STATE_LEARNING: + default: + /* Not used for anything at the moment */ + break; + } +} + +void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, + uint16_t id_end, uint8_t state) +{ + struct zebra_if *zif; + + zif = (struct zebra_if *)ifp->info; + + if (!zif) + return; + + for (uint16_t i = id_start; i <= id_end; i++) + vxlan_vni_state_change(zif, i, state); +} diff --git a/zebra/zebra_vxlan.h b/zebra/zebra_vxlan.h index eb02de6f7b40..0b37e58cfdfd 100644 --- a/zebra/zebra_vxlan.h +++ b/zebra/zebra_vxlan.h @@ -221,7 +221,11 @@ extern int zebra_vxlan_dp_network_mac_del(struct interface *ifp, extern void zebra_vxlan_set_accept_bgp_seq(bool set); extern bool zebra_vxlan_get_accept_bgp_seq(void); - +extern void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, + uint16_t id_end, uint8_t state); +extern void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, + uint8_t state); +extern const char *port_state2str(uint8_t state); #ifdef __cplusplus } #endif From aa4786642c9a65c282d0fd5247a35b0f14fa1c3c Mon Sep 17 00:00:00 2001 From: Rajasekar Raja Date: Mon, 12 Aug 2024 11:51:06 -0700 Subject: [PATCH 02/46] zebra: vlan to dplane Offload from main Trigger: Zebra core seen when we convert l2vni to l3vni and back BackTrace: /usr/lib/x86_64-linux-gnu/frr/libfrr.so.0(_zlog_assert_failed+0xe9) [0x7f4af96989d9] /usr/lib/frr/zebra(zebra_vxlan_if_vni_up+0x250) [0x5561022ae030] /usr/lib/frr/zebra(netlink_vlan_change+0x2f4) [0x5561021fd354] /usr/lib/frr/zebra(netlink_parse_info+0xff) [0x55610220d37f] /usr/lib/frr/zebra(+0xc264a) [0x55610220d64a] /usr/lib/x86_64-linux-gnu/frr/libfrr.so.0(thread_call+0x7d) [0x7f4af967e96d] /usr/lib/x86_64-linux-gnu/frr/libfrr.so.0(frr_run+0xe8) [0x7f4af9637588] /usr/lib/frr/zebra(main+0x402) [0x5561021f4d32] /lib/x86_64-linux-gnu/libc.so.6(+0x2724a) [0x7f4af932624a] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85) [0x7f4af9326305] /usr/lib/frr/zebra(_start+0x21) [0x5561021f72f1] Root Cause: In working case, - We get a RTM_NEWLINK whose ctx is enqueued by zebra dplane and dequeued by zebra main and processed i.e. (102000 is deleted from vxlan99) before we handle RTM_NEWVLAN. - So in handling of NEWVLAN (vxlan99) we bail out since find with vlan id 703 does not exist. root@leaf2:mgmt:/var/log/frr# cat ~/raja_logs/working/nocras.log | grep "RTM_NEWLINK\|QUEUED\|vxlan99\|in thread" 2024/07/18 23:09:33.741105 ZEBRA: [KMXEB-K771Y] netlink_parse_info: netlink-dp-in (NS 0) type RTM_NEWLINK(16), len=616, seq=0, pid=0 2024/07/18 23:09:33.744061 ZEBRA: [K8FXY-V65ZJ] Intf dplane ctx 0x7f2244000cf0, op INTF_INSTALL, ifindex (65), result QUEUED 2024/07/18 23:09:33.767240 ZEBRA: [KMXEB-K771Y] netlink_parse_info: netlink-dp-in (NS 0) type RTM_NEWLINK(16), len=508, seq=0, pid=0 2024/07/18 23:09:33.767380 ZEBRA: [K8FXY-V65ZJ] Intf dplane ctx 0x7f2244000cf0, op INTF_INSTALL, ifindex (73), result QUEUED 2024/07/18 23:09:33.767389 ZEBRA: [NVFT0-HS1EX] INTF_INSTALL for vxlan99(73) 2024/07/18 23:09:33.767404 ZEBRA: [TQR2A-H2RFY] Vlan-Vni(1186:1186-6000002:6000002) update for VxLAN IF vxlan99(73) 2024/07/18 23:09:33.767422 ZEBRA: [TP4VP-XZ627] Del L2-VNI 102000 intf vxlan99(73) 2024/07/18 23:09:33.767858 ZEBRA: [QYXB9-6RNNK] RTM_NEWVLAN bridge IF vxlan99 NS 0 2024/07/18 23:09:33.767866 ZEBRA: [KKZGZ-8PCDW] Cannot find VNI for VID (703) IF vxlan99 for vlan state update >>>>BAIL OUT In failure case, - The NEWVLAN is received first even before processing RTM_NEWLINK. - Since the vxlan id 102000 is not removed from the vxlan99, the find with vlan id 703 returns the 102000 one and we invoke zebra_vxlan_if_vni_up where the interfaces don't match and assert. root@leaf2:mgmt:/var/log/frr# cat ~/raja_logs/noworking/crash.log | grep "RTM_NEWLINK\|QUEUED\|vxlan99\|in thread" 2024/07/18 22:26:43.829370 ZEBRA: [KMXEB-K771Y] netlink_parse_info: netlink-dp-in (NS 0) type RTM_NEWLINK(16), len=616, seq=0, pid=0 2024/07/18 22:26:43.829646 ZEBRA: [K8FXY-V65ZJ] Intf dplane ctx 0x7fe07c026d30, op INTF_INSTALL, ifindex (65), result QUEUED 2024/07/18 22:26:43.853930 ZEBRA: [QYXB9-6RNNK] RTM_NEWVLAN bridge IF vxlan99 NS 0 2024/07/18 22:26:43.853949 ZEBRA: [K61WJ-XQQ3X] Intf vxlan99(73) L2-VNI 102000 is UP >>> VLAN PROCESSED BEFORE INTF EVENT 2024/07/18 22:26:43.853951 ZEBRA: [SPV50-BX2RP] RAJA zevpn_vxlanif vxlan48 and ifp vxlan99 2024/07/18 22:26:43.854005 ZEBRA: [KMXEB-K771Y] netlink_parse_info: netlink-dp-in (NS 0) type RTM_NEWLINK(16), len=508, seq=0, pid=0 2024/07/18 22:26:43.854241 ZEBRA: [KMXEB-K771Y] netlink_parse_info: netlink-dp-in (NS 0) type RTM_NEWLINK(16), len=516, seq=0, pid=0 2024/07/18 22:26:43.854251 ZEBRA: [KMXEB-K771Y] netlink_parse_info: netlink-dp-in (NS 0) type RTM_NEWLINK(16), len=544, seq=0, pid=0 ZEBRA: in thread kernel_read scheduled from zebra/kernel_netlink.c:505 kernel_read() Fix: Similar to #13396, where link change handling was offloaded to dplane, same is being done for vlan events. Note: Prior to this change, zebra main thread was interested in the RTNLGRP_BRVLAN. So all the kernel events pertaining to vlan was handled by zebra main. With this change change as well the handling of vlan events is still with Zebra main. However we offload it via Dplane thread. Ticket :#3878175 Signed-off-by: Rajasekar Raja --- zebra/dpdk/zebra_dplane_dpdk.c | 2 + zebra/dplane_fpm_nl.c | 1 + zebra/if_netlink.c | 79 ++++++++++++++++++++-------------- zebra/kernel_netlink.c | 15 ++++--- zebra/kernel_socket.c | 1 + zebra/zebra_dplane.c | 56 ++++++++++++++++++++++++ zebra/zebra_dplane.h | 32 ++++++++++++++ zebra/zebra_l2.h | 11 +++++ zebra/zebra_rib.c | 3 ++ zebra/zebra_script.c | 1 + zebra/zebra_vxlan.c | 79 +++++++++++++++++++++++++++------- zebra/zebra_vxlan.h | 6 +-- 12 files changed, 226 insertions(+), 60 deletions(-) diff --git a/zebra/dpdk/zebra_dplane_dpdk.c b/zebra/dpdk/zebra_dplane_dpdk.c index 411155167f64..ae1a3743ce58 100644 --- a/zebra/dpdk/zebra_dplane_dpdk.c +++ b/zebra/dpdk/zebra_dplane_dpdk.c @@ -400,6 +400,7 @@ static void zd_dpdk_rule_update(struct zebra_dplane_ctx *ctx) case DPLANE_OP_INTF_INSTALL: case DPLANE_OP_INTF_UPDATE: case DPLANE_OP_INTF_DELETE: + case DPLANE_OP_VLAN_INSTALL, break; } } @@ -459,6 +460,7 @@ static void zd_dpdk_process_update(struct zebra_dplane_ctx *ctx) case DPLANE_OP_INTF_INSTALL: case DPLANE_OP_INTF_UPDATE: case DPLANE_OP_INTF_DELETE: + case DPLANE_OP_VLAN_INSTALL, atomic_fetch_add_explicit(&dpdk_stat->ignored_updates, 1, memory_order_relaxed); diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c index 1d2f9e695f30..234908703e68 100644 --- a/zebra/dplane_fpm_nl.c +++ b/zebra/dplane_fpm_nl.c @@ -1058,6 +1058,7 @@ static int fpm_nl_enqueue(struct fpm_nl_ctx *fnc, struct zebra_dplane_ctx *ctx) case DPLANE_OP_SRV6_ENCAP_SRCADDR_SET: case DPLANE_OP_NONE: case DPLANE_OP_STARTUP_STAGE: + case DPLANE_OP_VLAN_INSTALL: break; } diff --git a/zebra/if_netlink.c b/zebra/if_netlink.c index 3de6661f5d5e..c9861b0b5e11 100644 --- a/zebra/if_netlink.c +++ b/zebra/if_netlink.c @@ -63,7 +63,6 @@ #include "zebra/zebra_l2.h" #include "zebra/netconf_netlink.h" #include "zebra/zebra_trace.h" -#include "zebra/zebra_vxlan.h" extern struct zebra_privs_t zserv_privs; @@ -1817,6 +1816,20 @@ int netlink_tunneldump_read(struct zebra_ns *zns) return 0; } +static uint8_t netlink_get_dplane_vlan_state(uint8_t state) +{ + if (state == BR_STATE_LISTENING) + return ZEBRA_DPLANE_BR_STATE_LISTENING; + else if (state == BR_STATE_LEARNING) + return ZEBRA_DPLANE_BR_STATE_LEARNING; + else if (state == BR_STATE_FORWARDING) + return ZEBRA_DPLANE_BR_STATE_FORWARDING; + else if (state == BR_STATE_BLOCKING) + return ZEBRA_DPLANE_BR_STATE_BLOCKING; + + return ZEBRA_DPLANE_BR_STATE_DISABLED; +} + /** * netlink_vlan_change() - Read in change about vlans from the kernel * @@ -1829,7 +1842,6 @@ int netlink_tunneldump_read(struct zebra_ns *zns) int netlink_vlan_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) { int len, rem; - struct interface *ifp; struct br_vlan_msg *bvm; struct bridge_vlan_info *vinfo; struct rtattr *vtb[BRIDGE_VLANDB_ENTRY_MAX + 1] = {}; @@ -1837,6 +1849,9 @@ int netlink_vlan_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) uint8_t state; uint32_t vrange; int type; + uint32_t count = 0; + struct zebra_dplane_ctx *ctx = NULL; + struct zebra_vxlan_vlan_array *vlan_array = NULL; /* We only care about state changes for now */ if (!(h->nlmsg_type == RTM_NEWVLAN)) @@ -1856,25 +1871,10 @@ int netlink_vlan_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (bvm->family != AF_BRIDGE) return 0; - ifp = if_lookup_by_index_per_ns(zebra_ns_lookup(ns_id), bvm->ifindex); - if (!ifp) { - zlog_debug("Cannot find bridge-vlan IF (%u) for vlan update", - bvm->ifindex); - return 0; - } - - if (!IS_ZEBRA_IF_VXLAN(ifp)) { - if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug("Ignoring non-vxlan IF (%s) for vlan update", - ifp->name); - - return 0; - } - - if (IS_ZEBRA_DEBUG_KERNEL || IS_ZEBRA_DEBUG_VXLAN) - zlog_debug("%s %s IF %s NS %u", - nl_msg_type_to_str(h->nlmsg_type), - nl_family_to_str(bvm->family), ifp->name, ns_id); + ctx = dplane_ctx_alloc(); + dplane_ctx_set_ns_id(ctx, ns_id); + dplane_ctx_set_op(ctx, DPLANE_OP_VLAN_INSTALL); + dplane_ctx_set_vlan_ifindex(ctx, bvm->ifindex); /* Loop over "ALL" BRIDGE_VLANDB_ENTRY */ rem = len; @@ -1905,26 +1905,39 @@ int netlink_vlan_change(struct nlmsghdr *h, ns_id_t ns_id, int startup) if (!vtb[BRIDGE_VLANDB_ENTRY_STATE]) continue; + count++; + vlan_array = + XREALLOC(MTYPE_VLAN_CHANGE_ARR, vlan_array, + sizeof(struct zebra_vxlan_vlan_array) + + count * sizeof(struct zebra_vxlan_vlan)); + + memset(&vlan_array->vlans[count - 1], 0, + sizeof(struct zebra_vxlan_vlan)); + state = *(uint8_t *)RTA_DATA(vtb[BRIDGE_VLANDB_ENTRY_STATE]); if (vtb[BRIDGE_VLANDB_ENTRY_RANGE]) vrange = *(uint32_t *)RTA_DATA( vtb[BRIDGE_VLANDB_ENTRY_RANGE]); - if (IS_ZEBRA_DEBUG_KERNEL || IS_ZEBRA_DEBUG_VXLAN) { - if (vrange) - zlog_debug("VLANDB_ENTRY: VID (%u-%u) state=%s", - vinfo->vid, vrange, - port_state2str(state)); - else - zlog_debug("VLANDB_ENTRY: VID (%u) state=%s", - vinfo->vid, port_state2str(state)); - } - - vlan_id_range_state_change( - ifp, vinfo->vid, (vrange ? vrange : vinfo->vid), state); + vlan_array->vlans[count - 1].state = + netlink_get_dplane_vlan_state(state); + vlan_array->vlans[count - 1].vid = vinfo->vid; + vlan_array->vlans[count - 1].vrange = vrange; } + if (count) { + vlan_array->count = count; + dplane_ctx_set_vxlan_vlan_array(ctx, vlan_array); + if (IS_ZEBRA_DEBUG_KERNEL || IS_ZEBRA_DEBUG_VXLAN) + zlog_debug("RTM_NEWVLAN for ifindex %u NS %u, enqueuing for zebra main", + bvm->ifindex, ns_id); + + dplane_provider_enqueue_to_zebra(ctx); + } else + dplane_ctx_fini(&ctx); + + return 0; } diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c index 84aabc425456..3547314f8452 100644 --- a/zebra/kernel_netlink.c +++ b/zebra/kernel_netlink.c @@ -430,10 +430,6 @@ static int netlink_information_fetch(struct nlmsghdr *h, ns_id_t ns_id, case RTM_NEWTFILTER: case RTM_DELTFILTER: return netlink_tfilter_change(h, ns_id, startup); - case RTM_NEWVLAN: - return netlink_vlan_change(h, ns_id, startup); - case RTM_DELVLAN: - return netlink_vlan_change(h, ns_id, startup); /* Messages we may receive, but ignore */ case RTM_NEWCHAIN: @@ -449,6 +445,8 @@ static int netlink_information_fetch(struct nlmsghdr *h, ns_id_t ns_id, case RTM_NEWTUNNEL: case RTM_DELTUNNEL: case RTM_GETTUNNEL: + case RTM_NEWVLAN: + case RTM_DELVLAN: return 0; default: /* @@ -492,6 +490,10 @@ static int dplane_netlink_information_fetch(struct nlmsghdr *h, ns_id_t ns_id, case RTM_DELLINK: return netlink_link_change(h, ns_id, startup); + case RTM_NEWVLAN: + case RTM_DELVLAN: + return netlink_vlan_change(h, ns_id, startup); + default: break; } @@ -1621,6 +1623,7 @@ static enum netlink_msg_status nl_put_msg(struct nl_batch *bth, case DPLANE_OP_IPSET_ENTRY_ADD: case DPLANE_OP_IPSET_ENTRY_DELETE: case DPLANE_OP_STARTUP_STAGE: + case DPLANE_OP_VLAN_INSTALL: return FRR_NETLINK_ERROR; case DPLANE_OP_GRE_SET: @@ -1862,8 +1865,8 @@ void kernel_init(struct zebra_ns *zns) * setsockopt multicast group subscriptions that don't fit in nl_groups */ grp = RTNLGRP_BRVLAN; - ret = setsockopt(zns->netlink.sock, SOL_NETLINK, NETLINK_ADD_MEMBERSHIP, - &grp, sizeof(grp)); + ret = setsockopt(zns->netlink_dplane_in.sock, SOL_NETLINK, + NETLINK_ADD_MEMBERSHIP, &grp, sizeof(grp)); if (ret < 0) zlog_notice( diff --git a/zebra/kernel_socket.c b/zebra/kernel_socket.c index 5cfbe7a896dd..4789cb62f257 100644 --- a/zebra/kernel_socket.c +++ b/zebra/kernel_socket.c @@ -1627,6 +1627,7 @@ void kernel_update_multi(struct dplane_ctx_list_head *ctx_list) case DPLANE_OP_INTF_ADDR_DEL: case DPLANE_OP_STARTUP_STAGE: case DPLANE_OP_SRV6_ENCAP_SRCADDR_SET: + case DPLANE_OP_VLAN_INSTALL: zlog_err("Unhandled dplane data for %s", dplane_op2str(dplane_ctx_get_op(ctx))); res = ZEBRA_DPLANE_REQUEST_FAILURE; diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 75147e713649..f88464d7450a 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -35,6 +35,8 @@ DEFINE_MTYPE_STATIC(ZEBRA, DP_PROV, "Zebra DPlane Provider"); DEFINE_MTYPE_STATIC(ZEBRA, DP_NETFILTER, "Zebra Netfilter Internal Object"); DEFINE_MTYPE_STATIC(ZEBRA, DP_NS, "DPlane NSes"); +DEFINE_MTYPE(ZEBRA, VLAN_CHANGE_ARR, "Vlan Change Array"); + #ifndef AOK # define AOK 0 #endif @@ -370,6 +372,14 @@ struct dplane_srv6_encap_ctx { struct in6_addr srcaddr; }; +/* + * VLAN info for the dataplane + */ +struct dplane_vlan_info { + ifindex_t ifindex; + struct zebra_vxlan_vlan_array *vlan_array; +}; + /* * The context block used to exchange info about route updates across * the boundary between the zebra main context (and pthread) and the @@ -416,6 +426,7 @@ struct zebra_dplane_ctx { struct dplane_pw_info pw; struct dplane_br_port_info br_port; struct dplane_intf_info intf; + struct dplane_vlan_info vlan_info; struct dplane_mac_info macinfo; struct dplane_neigh_info neigh; struct dplane_rule_info rule; @@ -885,6 +896,11 @@ static void dplane_ctx_free_internal(struct zebra_dplane_ctx *ctx) case DPLANE_OP_STARTUP_STAGE: case DPLANE_OP_SRV6_ENCAP_SRCADDR_SET: break; + case DPLANE_OP_VLAN_INSTALL: + if (ctx->u.vlan_info.vlan_array) + XFREE(MTYPE_VLAN_CHANGE_ARR, + ctx->u.vlan_info.vlan_array); + break; } } @@ -1219,6 +1235,10 @@ const char *dplane_op2str(enum dplane_op_e op) case DPLANE_OP_SRV6_ENCAP_SRCADDR_SET: ret = "SRV6_ENCAP_SRCADDR_SET"; break; + + case DPLANE_OP_VLAN_INSTALL: + ret = "NEW_VLAN"; + break; } return ret; @@ -3321,6 +3341,35 @@ uint32_t dplane_get_in_queue_len(void) memory_order_seq_cst); } +void dplane_ctx_set_vlan_ifindex(struct zebra_dplane_ctx *ctx, ifindex_t ifindex) +{ + DPLANE_CTX_VALID(ctx); + ctx->u.vlan_info.ifindex = ifindex; +} + +ifindex_t dplane_ctx_get_vlan_ifindex(struct zebra_dplane_ctx *ctx) +{ + DPLANE_CTX_VALID(ctx); + + return ctx->u.vlan_info.ifindex; +} + +void dplane_ctx_set_vxlan_vlan_array(struct zebra_dplane_ctx *ctx, + struct zebra_vxlan_vlan_array *vlan_array) +{ + DPLANE_CTX_VALID(ctx); + + ctx->u.vlan_info.vlan_array = vlan_array; +} + +const struct zebra_vxlan_vlan_array * +dplane_ctx_get_vxlan_vlan_array(struct zebra_dplane_ctx *ctx) +{ + DPLANE_CTX_VALID(ctx); + + return ctx->u.vlan_info.vlan_array; +} + /* * Internal helper that copies information from a zebra ns object; this is * called in the zebra main pthread context as part of dplane ctx init. @@ -6720,6 +6769,12 @@ static void kernel_dplane_log_detail(struct zebra_dplane_ctx *ctx) dplane_op2str(dplane_ctx_get_op(ctx)), &ctx->u.srv6_encap.srcaddr); break; + + case DPLANE_OP_VLAN_INSTALL: + zlog_debug("Dplane %s on idx %u", + dplane_op2str(dplane_ctx_get_op(ctx)), + dplane_ctx_get_vlan_ifindex(ctx)); + break; } } @@ -6888,6 +6943,7 @@ static void kernel_dplane_handle_result(struct zebra_dplane_ctx *ctx) case DPLANE_OP_INTF_ADDR_ADD: case DPLANE_OP_INTF_ADDR_DEL: case DPLANE_OP_INTF_NETCONFIG: + case DPLANE_OP_VLAN_INSTALL: break; case DPLANE_OP_SRV6_ENCAP_SRCADDR_SET: diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h index 0e9a8bfb99f5..3e143e8dbac9 100644 --- a/zebra/zebra_dplane.h +++ b/zebra/zebra_dplane.h @@ -24,6 +24,8 @@ extern "C" { #endif +DECLARE_MTYPE(VLAN_CHANGE_ARR); + /* Retrieve the dataplane API version number; see libfrr.h to decode major, * minor, sub version values. * Plugins should pay attention to the major version number, at least, to @@ -204,6 +206,9 @@ enum dplane_op_e { DPLANE_OP_TC_FILTER_DELETE, DPLANE_OP_TC_FILTER_UPDATE, + /* VLAN update */ + DPLANE_OP_VLAN_INSTALL, + /* Startup Control */ DPLANE_OP_STARTUP_STAGE, @@ -211,6 +216,13 @@ enum dplane_op_e { DPLANE_OP_SRV6_ENCAP_SRCADDR_SET, }; +/* Operational status of Bridge Ports */ +#define ZEBRA_DPLANE_BR_STATE_DISABLED 0x01 +#define ZEBRA_DPLANE_BR_STATE_LISTENING 0x02 +#define ZEBRA_DPLANE_BR_STATE_LEARNING 0x04 +#define ZEBRA_DPLANE_BR_STATE_FORWARDING 0x08 +#define ZEBRA_DPLANE_BR_STATE_BLOCKING 0x10 + /* * The vxlan/evpn neighbor management code needs some values to use * when programming neighbor changes. Offer some platform-neutral values @@ -1078,6 +1090,26 @@ void dplane_set_in_queue_limit(uint32_t limit, bool set); /* Retrieve the current queue depth of incoming, unprocessed updates */ uint32_t dplane_get_in_queue_len(void); +void dplane_ctx_set_vlan_ifindex(struct zebra_dplane_ctx *ctx, + ifindex_t ifindex); +ifindex_t dplane_ctx_get_vlan_ifindex(struct zebra_dplane_ctx *ctx); +struct zebra_vxlan_vlan_array; + +/* + * In netlink_vlan_change(), the memory allocated for vlan_array is freed + * in two cases + * 1) Inline free in netlink_vlan_change() when there are no new + * vlans to process i.e. nothing is enqueued to main thread. + * 2) Dplane-ctx takes over the vlan memory which gets freed in + * rib_process_dplane_results() after handling the vlan install + * + * Note: MTYPE of interest for this purpose is MTYPE_VLAN_CHANGE_ARR + */ +void dplane_ctx_set_vxlan_vlan_array(struct zebra_dplane_ctx *ctx, + struct zebra_vxlan_vlan_array *vlan_array); +const struct zebra_vxlan_vlan_array * +dplane_ctx_get_vxlan_vlan_array(struct zebra_dplane_ctx *ctx); + /* * Vty/cli apis */ diff --git a/zebra/zebra_l2.h b/zebra/zebra_l2.h index 588917f4c036..ad5f5eeee008 100644 --- a/zebra/zebra_l2.h +++ b/zebra/zebra_l2.h @@ -146,6 +146,17 @@ union zebra_l2if_info { struct zebra_l2info_gre gre; }; +struct zebra_vxlan_vlan { + uint8_t state; + uint32_t vrange; + vlanid_t vid; +}; + +struct zebra_vxlan_vlan_array { + uint16_t count; + struct zebra_vxlan_vlan vlans[0]; +}; + /* NOTE: These macros are to be invoked only in the "correct" context. * IOW, the macro VNI_FROM_ZEBRA_IF() will assume the interface is * of type ZEBRA_IF_VXLAN. diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index 402a3104b941..5fe79b159d11 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -5083,6 +5083,9 @@ static void rib_process_dplane_results(struct event *thread) zebra_ns_startup_continue(ctx); break; + case DPLANE_OP_VLAN_INSTALL: + zebra_vlan_dplane_result(ctx); + break; } /* Dispatch by op code */ dplane_ctx_fini(&ctx); diff --git a/zebra/zebra_script.c b/zebra/zebra_script.c index 6c34d12c6409..a0d5e2054cca 100644 --- a/zebra/zebra_script.c +++ b/zebra/zebra_script.c @@ -418,6 +418,7 @@ void lua_pushzebra_dplane_ctx(lua_State *L, const struct zebra_dplane_ctx *ctx) case DPLANE_OP_SRV6_ENCAP_SRCADDR_SET: case DPLANE_OP_NONE: case DPLANE_OP_STARTUP_STAGE: + case DPLANE_OP_VLAN_INSTALL: break; } /* Dispatch by op code */ } diff --git a/zebra/zebra_vxlan.c b/zebra/zebra_vxlan.c index 0f0464e486fa..0f72259951b0 100644 --- a/zebra/zebra_vxlan.c +++ b/zebra/zebra_vxlan.c @@ -41,8 +41,6 @@ #include "zebra/zebra_evpn_mh.h" #include "zebra/zebra_evpn_vxlan.h" #include "zebra/zebra_router.h" -#include "zebra/zebra_trace.h" -#include DEFINE_MTYPE_STATIC(ZEBRA, HOST_PREFIX, "host prefix"); DEFINE_MTYPE_STATIC(ZEBRA, ZL3VNI, "L3 VNI hash"); @@ -6249,25 +6247,26 @@ extern void zebra_evpn_init(void) hook_register(zserv_client_close, zebra_evpn_cfg_clean_up); } -const char *port_state2str(uint8_t state) +static const char *port_state2str(uint8_t state) { switch (state) { - case BR_STATE_DISABLED: + case ZEBRA_DPLANE_BR_STATE_DISABLED: return "DISABLED"; - case BR_STATE_LISTENING: + case ZEBRA_DPLANE_BR_STATE_LISTENING: return "LISTENING"; - case BR_STATE_LEARNING: + case ZEBRA_DPLANE_BR_STATE_LEARNING: return "LEARNING"; - case BR_STATE_FORWARDING: + case ZEBRA_DPLANE_BR_STATE_FORWARDING: return "FORWARDING"; - case BR_STATE_BLOCKING: + case ZEBRA_DPLANE_BR_STATE_BLOCKING: return "BLOCKING"; } return "UNKNOWN"; } -void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, uint8_t state) +static void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, + uint8_t state) { struct zebra_vxlan_vni *vnip; @@ -6282,23 +6281,23 @@ void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, uint8_t state) } switch (state) { - case BR_STATE_FORWARDING: + case ZEBRA_DPLANE_BR_STATE_FORWARDING: zebra_vxlan_if_vni_up(zif->ifp, vnip); break; - case BR_STATE_BLOCKING: + case ZEBRA_DPLANE_BR_STATE_BLOCKING: zebra_vxlan_if_vni_down(zif->ifp, vnip); break; - case BR_STATE_DISABLED: - case BR_STATE_LISTENING: - case BR_STATE_LEARNING: + case ZEBRA_DPLANE_BR_STATE_DISABLED: + case ZEBRA_DPLANE_BR_STATE_LISTENING: + case ZEBRA_DPLANE_BR_STATE_LEARNING: default: /* Not used for anything at the moment */ break; } } -void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, - uint16_t id_end, uint8_t state) +static void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, + uint16_t id_end, uint8_t state) { struct zebra_if *zif; @@ -6310,3 +6309,51 @@ void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, for (uint16_t i = id_start; i <= id_end; i++) vxlan_vni_state_change(zif, i, state); } + +void zebra_vlan_dplane_result(struct zebra_dplane_ctx *ctx) +{ + int i; + struct interface *ifp = NULL; + ns_id_t ns_id = dplane_ctx_get_ns_id(ctx); + enum dplane_op_e op = dplane_ctx_get_op(ctx); + const struct zebra_vxlan_vlan_array *vlan_array = + dplane_ctx_get_vxlan_vlan_array(ctx); + ifindex_t ifindex = dplane_ctx_get_vlan_ifindex(ctx); + + ifp = if_lookup_by_index_per_ns(zebra_ns_lookup(ns_id), ifindex); + if (!ifp) { + zlog_debug("Cannot find bridge-vlan IF (%u) for vlan update", + ifindex); + return; + } + + if (!IS_ZEBRA_IF_VXLAN(ifp)) { + if (IS_ZEBRA_DEBUG_KERNEL) + zlog_debug("Ignoring non-vxlan IF (%s) for vlan update", + ifp->name); + + return; + } + + if (IS_ZEBRA_DEBUG_KERNEL || IS_ZEBRA_DEBUG_VXLAN) + zlog_debug("Dequeuing in zebra main..%s IF %s ifindex %u NS %u", + dplane_op2str(op), ifp->name, ifindex, ns_id); + + for (i = 0; i < vlan_array->count; i++) { + vlanid_t vid = vlan_array->vlans[i].vid; + uint8_t state = vlan_array->vlans[i].state; + uint32_t vrange = vlan_array->vlans[i].vrange; + + if (IS_ZEBRA_DEBUG_KERNEL || IS_ZEBRA_DEBUG_VXLAN) { + if (vrange) + zlog_debug("VLANDB_ENTRY: VID (%u-%u) state=%s", + vid, vrange, port_state2str(state)); + else + zlog_debug("VLANDB_ENTRY: VID (%u) state=%s", + vid, port_state2str(state)); + } + + vlan_id_range_state_change(ifp, vid, (vrange ? vrange : vid), + state); + } +} diff --git a/zebra/zebra_vxlan.h b/zebra/zebra_vxlan.h index 0b37e58cfdfd..ef4c39c060fe 100644 --- a/zebra/zebra_vxlan.h +++ b/zebra/zebra_vxlan.h @@ -221,11 +221,7 @@ extern int zebra_vxlan_dp_network_mac_del(struct interface *ifp, extern void zebra_vxlan_set_accept_bgp_seq(bool set); extern bool zebra_vxlan_get_accept_bgp_seq(void); -extern void vlan_id_range_state_change(struct interface *ifp, uint16_t id_start, - uint16_t id_end, uint8_t state); -extern void vxlan_vni_state_change(struct zebra_if *zif, uint16_t id, - uint8_t state); -extern const char *port_state2str(uint8_t state); +extern void zebra_vlan_dplane_result(struct zebra_dplane_ctx *ctx); #ifdef __cplusplus } #endif From 5913d49d5bd340a1b85c5f6eb540bead7c747543 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 10 Oct 2024 12:53:51 +0300 Subject: [PATCH 03/46] bgpd: Drop deprecated `bgp network import-check exact` command Signed-off-by: Donatas Abraitis --- bgpd/bgp_vty.c | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/bgpd/bgp_vty.c b/bgpd/bgp_vty.c index e7be2a33d2b6..69e5a6c0c60f 100644 --- a/bgpd/bgp_vty.c +++ b/bgpd/bgp_vty.c @@ -4494,16 +4494,6 @@ DEFUN (bgp_network_import_check, return CMD_SUCCESS; } -#if CONFDATE > 20241013 -CPP_NOTICE("Drop `bgp network import-check exact` command") -#endif -ALIAS_HIDDEN(bgp_network_import_check, bgp_network_import_check_exact_cmd, - "bgp network import-check exact", - BGP_STR - "BGP network command\n" - "Check BGP network route exists in IGP\n" - "Match route precisely\n") - DEFUN (no_bgp_network_import_check, no_bgp_network_import_check_cmd, "no bgp network import-check", @@ -20678,7 +20668,6 @@ void bgp_vty_init(void) /* "bgp network import-check" commands. */ install_element(BGP_NODE, &bgp_network_import_check_cmd); - install_element(BGP_NODE, &bgp_network_import_check_exact_cmd); install_element(BGP_NODE, &no_bgp_network_import_check_cmd); /* "bgp default local-preference" commands. */ From 310ca4f33de15ab6aeb98401fe28d6a0991b835d Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Thu, 10 Oct 2024 16:24:24 +0300 Subject: [PATCH 04/46] lib: Apply and generate route-map commands earlier before any other protocol If e.g. BGP neighbor is using a route-map at the boot, that is not yet created, then the log is spammed with `The route-map 'X' does not exist`. Processing earlier, should do the trick. Signed-off-by: Donatas Abraitis --- lib/command.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/command.h b/lib/command.h index f369a35243f6..c60751789f66 100644 --- a/lib/command.h +++ b/lib/command.h @@ -84,6 +84,7 @@ enum node_type { CONFIG_NODE, /* Config node. Default mode of config file. */ PREFIX_NODE, /* ip prefix-list node. */ PREFIX_IPV6_NODE, /* ipv6 prefix-list node. */ + RMAP_NODE, /* Route map node. */ LIB_DEBUG_NODE, /* frrlib debug node. */ DEBUG_NODE, /* Debug node. */ VRF_DEBUG_NODE, /* Vrf Debug node. */ @@ -136,7 +137,6 @@ enum node_type { AS_LIST_NODE, /* AS list node. */ COMMUNITY_LIST_NODE, /* Community list node. */ COMMUNITY_ALIAS_NODE, /* Community alias node. */ - RMAP_NODE, /* Route map node. */ PBRMAP_NODE, /* PBR map node. */ SMUX_NODE, /* SNMP configuration node. */ DUMP_NODE, /* Packet dump node. */ From dd135843ad3454f4a12c70b729ff8ba04f2aa4d6 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 10 Oct 2024 13:29:43 +0200 Subject: [PATCH 05/46] tests: rework bgp_route_server_client Rework bgp_route_server_client in a more standard form in order to facilitate the next commut changes. Cosmetic change. Signed-off-by: Louis Scalbert --- .../r1/show_bgp_ipv6.json | 58 ++++++++++ .../r2/show_bgp_ipv6.json | 78 ++++++++++++++ .../r2/show_bgp_ipv6_summary.json | 6 ++ .../r3/show_bgp_ipv6.json | 32 ++++++ .../test_bgp_route_server_client.py | 100 ++++++++---------- 5 files changed, 219 insertions(+), 55 deletions(-) create mode 100644 tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json create mode 100644 tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json create mode 100644 tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json create mode 100644 tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json diff --git a/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json new file mode 100644 index 000000000000..1776b1924802 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json @@ -0,0 +1,58 @@ +{ + "routerId": "10.10.10.1", + "localAS": 65001, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r1", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r1", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json new file mode 100644 index 000000000000..78b4aaefda3f --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json @@ -0,0 +1,78 @@ +{ + "routerId": "10.10.10.2", + "localAS": 65000, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json new file mode 100644 index 000000000000..b40192b452e0 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json @@ -0,0 +1,6 @@ +{ + "ipv6Unicast": { + "failedPeers": 0, + "totalPeers": 2 + } +} diff --git a/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json new file mode 100644 index 000000000000..aebffb2c552f --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json @@ -0,0 +1,32 @@ +{ + "routerId": "10.10.10.3", + "localAS": 65003, + "routes": { + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r3", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r3", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py b/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py index 29d9842d59dd..8582b09c8601 100644 --- a/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py +++ b/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py @@ -13,6 +13,7 @@ import sys import json import pytest +from functools import partial import functools pytestmark = [pytest.mark.bgpd] @@ -60,67 +61,56 @@ def teardown_module(mod): tgen.stop_topology() -def test_bgp_route_server_client(): - tgen = get_topogen() +def test_converge_protocols(): + "Wait for protocol convergence" + tgen = get_topogen() + # Don't run this test if we have any failure. if tgen.routers_have_failure(): pytest.skip(tgen.errors) - r1 = tgen.gears["r1"] r2 = tgen.gears["r2"] + ref_file = "{}/{}/show_bgp_ipv6_summary.json".format(CWD, r2.name) + expected = json.loads(open(ref_file).read()) + + test_func = partial( + topotest.router_json_cmp, + r2, + "show bgp view RS ipv6 summary json", + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = "{}: BGP convergence failed".format(r2.name) + assert res is None, assertmsg + + +def test_bgp_route_server_client_step1(): + tgen = get_topogen() - def _bgp_converge(router): - output = json.loads(router.vtysh_cmd("show bgp ipv6 unicast summary json")) - expected = {"peers": {"2001:db8:1::1": {"state": "Established", "pfxRcd": 2}}} - return topotest.json_cmp(output, expected) - - test_func = functools.partial(_bgp_converge, r1) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) - assert result is None, "Cannot see BGP sessions to be up" - - def _bgp_prefix_received(router): - output = json.loads(router.vtysh_cmd("show bgp 2001:db8:f::3/128 json")) - expected = { - "prefix": "2001:db8:f::3/128", - "paths": [{"nexthops": [{"ip": "2001:db8:3::2"}]}], - } - return topotest.json_cmp(output, expected) - - test_func = functools.partial(_bgp_prefix_received, r1) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) - assert result is None, "Cannot see BGP GUA next hop from r3 in r1" - - def _bgp_single_next_hop(router): - output = json.loads(router.vtysh_cmd("show bgp 2001:db8:f::3/128 json")) - return len(output["paths"][0]["nexthops"]) - - assert ( - _bgp_single_next_hop(r1) == 1 - ), "Not ONLY one Next Hop received for 2001:db8:f::3/128" - - def _bgp_gua_lla_next_hop(router): - output = json.loads(router.vtysh_cmd("show bgp view RS 2001:db8:f::3/128 json")) - expected = { - "prefix": "2001:db8:f::3/128", - "paths": [ - { - "nexthops": [ - { - "ip": "2001:db8:3::2", - "hostname": "r3", - "afi": "ipv6", - "scope": "global", - }, - {"hostname": "r3", "afi": "ipv6", "scope": "link-local"}, - ] - } - ], - } - return topotest.json_cmp(output, expected) - - test_func = functools.partial(_bgp_gua_lla_next_hop, r2) - _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) - assert result is None, "Cannot see BGP LLA next hop from r3 in r2" + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + router_list = tgen.routers().values() + for router in router_list: + if router.name == "r2": + # route-server + cmd = "show bgp view RS ipv6 unicast json" + else: + cmd = "show bgp ipv6 unicast json" + + # router.cmd("vtysh -c 'sh bgp ipv6 json' >/tmp/show_bgp_ipv6_%s.json" % router.name) + ref_file = "{}/{}/show_bgp_ipv6.json".format(CWD, router.name) + expected = json.loads(open(ref_file).read()) + + test_func = partial( + topotest.router_json_cmp, + router, + cmd, + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = "{}: BGP IPv6 table failure".format(router.name) + assert res is None, assertmsg if __name__ == "__main__": From da7b2d9831fb87c6b20b41a803214b06354f5cd8 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 10 Oct 2024 14:59:50 +0200 Subject: [PATCH 06/46] tests: unset r3 enforce-first-as bgp_route_server_client Unset enforce-first-as on r3 of bgp_route_server_client to enable the reception of routes on this router. Signed-off-by: Louis Scalbert --- .../bgp_route_server_client/r3/bgpd.conf | 1 + .../r3/show_bgp_ipv6.json | 26 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/tests/topotests/bgp_route_server_client/r3/bgpd.conf b/tests/topotests/bgp_route_server_client/r3/bgpd.conf index 60a5ffc559a0..f7daba87face 100644 --- a/tests/topotests/bgp_route_server_client/r3/bgpd.conf +++ b/tests/topotests/bgp_route_server_client/r3/bgpd.conf @@ -2,6 +2,7 @@ router bgp 65003 bgp router-id 10.10.10.3 no bgp ebgp-requires-policy + no bgp enforce-first-as neighbor 2001:db8:3::1 remote-as external neighbor 2001:db8:3::1 timers 3 10 neighbor 2001:db8:3::1 timers connect 5 diff --git a/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json index aebffb2c552f..5ebcbe4c6856 100644 --- a/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json +++ b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json @@ -2,6 +2,19 @@ "routerId": "10.10.10.3", "localAS": 65003, "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], "2001:db8:3::/64": [ { "nexthops": [ @@ -15,6 +28,19 @@ ] } ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], "2001:db8:f::3/128": [ { "nexthops": [ From 91512c30487ceb1d306529886f4e5a629cd0e2f8 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 11 Oct 2024 07:12:23 +0200 Subject: [PATCH 07/46] bgpd: split nexthop-local unchanged peer subgroup 5bb99ccad2 ("bgpd: reset ipv6 invalid link-local nexthop") now resets the link-local when originating and destination peers are not on the same network segment. However, it does not work all the time. The fix compares the 'from' and 'peer' global IPv6 address. However, 'peer' refers to one of the peers of subgroup. The subgroup may contain peers located on different network segment. Split nexthop-local unchanged peer subgroup by network segment. Fixes: 5bb99ccad2 ("bgpd: reset ipv6 invalid link-local nexthop") Signed-off-by: Louis Scalbert --- bgpd/bgp_updgrp.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/bgpd/bgp_updgrp.c b/bgpd/bgp_updgrp.c index 90c43b938ffd..ef036067078f 100644 --- a/bgpd/bgp_updgrp.c +++ b/bgpd/bgp_updgrp.c @@ -455,6 +455,10 @@ static unsigned int updgrp_hash_key_make(const void *p) key = jhash_1word(jhash(soo_str, strlen(soo_str), SEED1), key); } + if (afi == AFI_IP6 && + (CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED))) + key = jhash(&peer->nexthop.v6_global, IPV6_MAX_BYTELEN, key); + /* * ANY NEW ITEMS THAT ARE ADDED TO THE key, ENSURE DEBUG * STATEMENT STAYS UP TO DATE @@ -521,6 +525,12 @@ static unsigned int updgrp_hash_key_make(const void *p) peer->soo[afi][safi] ? ecommunity_str(peer->soo[afi][safi]) : "(NONE)"); + zlog_debug("%pBP Update Group Hash: IPv6 nexthop-local unchanged: %d IPv6 global %pI6", + peer, + afi == AFI_IP6 && (CHECK_FLAG(peer->af_flags[afi][safi], + PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED)), + &peer->nexthop.v6_global); + zlog_debug("%pBP Update Group Hash key: %u", peer, key); } return key; @@ -655,6 +665,12 @@ static bool updgrp_hash_cmp(const void *p1, const void *p2) !sockunion_same(&pe1->connection->su, &pe2->connection->su)) return false; + if (afi == AFI_IP6 && + (CHECK_FLAG(flags1, PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED) || + CHECK_FLAG(flags2, PEER_FLAG_NEXTHOP_LOCAL_UNCHANGED)) && + !IPV6_ADDR_SAME(&pe1->nexthop.v6_global, &pe2->nexthop.v6_global)) + return false; + return true; } From 5f035edf25119c416951aba1ddac941d59edae75 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Thu, 10 Oct 2024 14:51:11 +0200 Subject: [PATCH 08/46] tests: test nexthop-local unchanged with route-server Test nexthop-local unchanged with route-server. Signed-off-by: Louis Scalbert --- .../bgp_route_server_client/exabgp.env | 53 +++++ ...bgp_ipv6.json => show_bgp_ipv6_step1.json} | 37 +++ .../r1/show_bgp_ipv6_step2.json | 113 ++++++++++ .../bgp_route_server_client/r2/bgpd.conf | 10 + .../r2/show_bgp_ipv6_step1.json | 208 +++++++++++++++++ .../r2/show_bgp_ipv6_step2.json | 208 +++++++++++++++++ .../r2/show_bgp_ipv6_summary.json | 2 +- ...bgp_ipv6.json => show_bgp_ipv6_step1.json} | 28 ++- .../show_bgp_ipv6_step2.json} | 60 +++-- .../bgp_route_server_client/r4/bgpd.conf | 13 ++ .../r4/show_bgp_ipv6_step1.json | 95 ++++++++ .../r4/show_bgp_ipv6_step2.json | 113 ++++++++++ .../bgp_route_server_client/r4/zebra.conf | 7 + .../bgp_route_server_client/r5/exabgp.cfg | 16 ++ .../test_bgp_route_server_client.py | 212 +++++++++++++++++- 15 files changed, 1144 insertions(+), 31 deletions(-) create mode 100644 tests/topotests/bgp_route_server_client/exabgp.env rename tests/topotests/bgp_route_server_client/r1/{show_bgp_ipv6.json => show_bgp_ipv6_step1.json} (59%) create mode 100644 tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step2.json create mode 100644 tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step1.json create mode 100644 tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step2.json rename tests/topotests/bgp_route_server_client/r3/{show_bgp_ipv6.json => show_bgp_ipv6_step1.json} (65%) rename tests/topotests/bgp_route_server_client/{r2/show_bgp_ipv6.json => r3/show_bgp_ipv6_step2.json} (56%) create mode 100644 tests/topotests/bgp_route_server_client/r4/bgpd.conf create mode 100644 tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step1.json create mode 100644 tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step2.json create mode 100644 tests/topotests/bgp_route_server_client/r4/zebra.conf create mode 100644 tests/topotests/bgp_route_server_client/r5/exabgp.cfg diff --git a/tests/topotests/bgp_route_server_client/exabgp.env b/tests/topotests/bgp_route_server_client/exabgp.env new file mode 100644 index 000000000000..28e642360a39 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/exabgp.env @@ -0,0 +1,53 @@ +[exabgp.api] +encoder = text +highres = false +respawn = false +socket = '' + +[exabgp.bgp] +openwait = 60 + +[exabgp.cache] +attributes = true +nexthops = true + +[exabgp.daemon] +daemonize = true +pid = '/var/run/exabgp/exabgp.pid' +user = 'exabgp' +##daemonize = false + +[exabgp.log] +all = false +configuration = true +daemon = true +destination = '/var/log/exabgp.log' +enable = true +level = INFO +message = false +network = true +packets = false +parser = false +processes = true +reactor = true +rib = false +routes = false +short = false +timers = false + +[exabgp.pdb] +enable = false + +[exabgp.profile] +enable = false +file = '' + +[exabgp.reactor] +speed = 1.0 + +[exabgp.tcp] +acl = false +bind = '' +delay = 0 +once = false +port = 179 diff --git a/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step1.json similarity index 59% rename from tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json rename to tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step1.json index 1776b1924802..387d7b3374b7 100644 --- a/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6.json +++ b/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step1.json @@ -13,6 +13,17 @@ "used": true } ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] } ], "2001:db8:3::/64": [ @@ -53,6 +64,32 @@ } ] } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } ] } } diff --git a/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step2.json b/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step2.json new file mode 100644 index 000000000000..f9e68b897723 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r1/show_bgp_ipv6_step2.json @@ -0,0 +1,113 @@ +{ + "routerId": "10.10.10.1", + "localAS": 65001, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r1", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r2:r2-eth0", + "hostname": "r2", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r1", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r2:r2-eth0", + "hostname": "r2", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r2", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r2", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r2/bgpd.conf b/tests/topotests/bgp_route_server_client/r2/bgpd.conf index 3b0a24b8ba5b..19607660f98f 100644 --- a/tests/topotests/bgp_route_server_client/r2/bgpd.conf +++ b/tests/topotests/bgp_route_server_client/r2/bgpd.conf @@ -4,14 +4,24 @@ router bgp 65000 view RS neighbor 2001:db8:1::2 remote-as external neighbor 2001:db8:1::2 timers 3 10 neighbor 2001:db8:1::2 timers connect 5 + neighbor 2001:db8:1::3 remote-as external + neighbor 2001:db8:1::3 timers 3 10 + neighbor 2001:db8:1::3 timers connect 5 + neighbor 2001:db8:1::4 remote-as external + neighbor 2001:db8:1::4 timers 3 10 + neighbor 2001:db8:1::4 timers connect 5 neighbor 2001:db8:3::2 remote-as external neighbor 2001:db8:3::2 timers 3 10 neighbor 2001:db8:3::2 timers connect 5 address-family ipv6 unicast redistribute connected neighbor 2001:db8:1::2 activate + neighbor 2001:db8:1::3 activate + neighbor 2001:db8:1::4 activate neighbor 2001:db8:3::2 activate neighbor 2001:db8:1::2 route-server-client + neighbor 2001:db8:1::3 route-server-client + neighbor 2001:db8:1::4 route-server-client neighbor 2001:db8:3::2 route-server-client exit-address-family ! diff --git a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step1.json b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step1.json new file mode 100644 index 000000000000..c2f31f8c32ed --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step1.json @@ -0,0 +1,208 @@ +{ + "routerId": "10.10.10.2", + "localAS": 65000, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r3:r3-eth0", + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r3:r3-eth0", + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step2.json b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step2.json new file mode 100644 index 000000000000..c2f31f8c32ed --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_step2.json @@ -0,0 +1,208 @@ +{ + "routerId": "10.10.10.2", + "localAS": 65000, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r3:r3-eth0", + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r3", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r3:r3-eth0", + "hostname": "r3", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r1", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r1", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r4", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r4:r4-eth0", + "hostname": "r4", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json index b40192b452e0..8a42a11c47a9 100644 --- a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json +++ b/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6_summary.json @@ -1,6 +1,6 @@ { "ipv6Unicast": { "failedPeers": 0, - "totalPeers": 2 + "totalPeers": 4 } } diff --git a/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6_step1.json similarity index 65% rename from tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json rename to tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6_step1.json index 5ebcbe4c6856..bf8d74801dc3 100644 --- a/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6.json +++ b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6_step1.json @@ -6,7 +6,7 @@ { "nexthops": [ { - "ip": "2001:db8:1::2", + "ip": "2001:db8:1::4", "hostname": "r2", "afi": "ipv6", "scope": "global", @@ -53,6 +53,32 @@ } ] } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } ] } } diff --git a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6_step2.json similarity index 56% rename from tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json rename to tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6_step2.json index 78b4aaefda3f..31c1eb781611 100644 --- a/tests/topotests/bgp_route_server_client/r2/show_bgp_ipv6.json +++ b/tests/topotests/bgp_route_server_client/r3/show_bgp_ipv6_step2.json @@ -1,20 +1,15 @@ { - "routerId": "10.10.10.2", - "localAS": 65000, + "routerId": "10.10.10.3", + "localAS": 65003, "routes": { "2001:db8:1::/64": [ { "nexthops": [ { - "ip": "2001:db8:1::2", - "hostname": "r1", - "afi": "ipv6", - "scope": "global" - }, - { - "hostname": "r1", + "ip": "2001:db8:1::4", + "hostname": "r2", "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -24,15 +19,10 @@ { "nexthops": [ { - "ip": "2001:db8:3::2", - "hostname": "r3", - "afi": "ipv6", - "scope": "global" - }, - { + "ip": "::", "hostname": "r3", "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -43,12 +33,13 @@ "nexthops": [ { "ip": "2001:db8:1::2", - "hostname": "r1", + "hostname": "r2", "afi": "ipv6", "scope": "global" }, { - "hostname": "r1", + "ip": "link-local:r2:r2-eth1", + "hostname": "r2", "afi": "ipv6", "scope": "link-local", "used": true @@ -60,19 +51,46 @@ { "nexthops": [ { - "ip": "2001:db8:3::2", + "ip": "::", "hostname": "r3", "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::3", + "hostname": "r2", + "afi": "ipv6", "scope": "global" }, { - "hostname": "r3", + "ip": "link-local:r2:r2-eth1", + "hostname": "r2", "afi": "ipv6", "scope": "link-local", "used": true } ] } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } ] } } diff --git a/tests/topotests/bgp_route_server_client/r4/bgpd.conf b/tests/topotests/bgp_route_server_client/r4/bgpd.conf new file mode 100644 index 000000000000..c907d7284e22 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r4/bgpd.conf @@ -0,0 +1,13 @@ +! +router bgp 65004 + bgp router-id 10.10.10.4 + no bgp ebgp-requires-policy + no bgp enforce-first-as + neighbor 2001:db8:1::1 remote-as external + neighbor 2001:db8:1::1 timers 3 10 + neighbor 2001:db8:1::1 timers connect 5 + address-family ipv6 unicast + redistribute connected + neighbor 2001:db8:1::1 activate + exit-address-family +! diff --git a/tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step1.json b/tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step1.json new file mode 100644 index 000000000000..5c090d9cf947 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step1.json @@ -0,0 +1,95 @@ +{ + "routerId": "10.10.10.4", + "localAS": 65004, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step2.json b/tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step2.json new file mode 100644 index 000000000000..01db18e926d4 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r4/show_bgp_ipv6_step2.json @@ -0,0 +1,113 @@ +{ + "routerId": "10.10.10.4", + "localAS": 65004, + "routes": { + "2001:db8:1::/64": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + }, + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:3::/64": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r2:r2-eth0", + "hostname": "r2", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::1/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r1:r1-eth0", + "hostname": "r2", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::3/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:3::2", + "hostname": "r2", + "afi": "ipv6", + "scope": "global" + }, + { + "ip": "link-local:r2:r2-eth0", + "hostname": "r2", + "afi": "ipv6", + "scope": "link-local", + "used": true + } + ] + } + ], + "2001:db8:f::4/128": [ + { + "nexthops": [ + { + "ip": "::", + "hostname": "r4", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ], + "2001:db8:f::5/128": [ + { + "nexthops": [ + { + "ip": "2001:db8:1::4", + "hostname": "r2", + "afi": "ipv6", + "scope": "global", + "used": true + } + ] + } + ] + } +} diff --git a/tests/topotests/bgp_route_server_client/r4/zebra.conf b/tests/topotests/bgp_route_server_client/r4/zebra.conf new file mode 100644 index 000000000000..849884045d98 --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r4/zebra.conf @@ -0,0 +1,7 @@ +! +int lo + ipv6 address 2001:db8:f::4/128 +! +int r4-eth0 + ipv6 address 2001:db8:1::3/64 +! diff --git a/tests/topotests/bgp_route_server_client/r5/exabgp.cfg b/tests/topotests/bgp_route_server_client/r5/exabgp.cfg new file mode 100644 index 000000000000..b151f16caaeb --- /dev/null +++ b/tests/topotests/bgp_route_server_client/r5/exabgp.cfg @@ -0,0 +1,16 @@ +neighbor 2001:db8:1::1{ + router-id 10.10.10.5; + local-address 2001:db8:1::4; + local-as 65005; + peer-as 65000; + + family { + ipv6 unicast; + } + + static { + route 2001:db8:1::0/64 next-hop 2001:db8:1::4; + route 2001:db8:f::5/128 next-hop 2001:db8:1::4; + } + hold-time 10; +} diff --git a/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py b/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py index 8582b09c8601..a6334918dfcb 100644 --- a/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py +++ b/tests/topotests/bgp_route_server_client/test_bgp_route_server_client.py @@ -27,16 +27,49 @@ def build_topo(tgen): - for routern in range(1, 4): + """ + All peers are FRR BGP peers except r5 that is a exabgp peer. + Exabgp does not send any IPv6 Link-Local nexthop + + r2 is a route-server view RS AS 65000 + Other routers rX has AS 6500X + + +---+ + | r3| + +---+ + | + 2001:db8:3::0/64 + | + eth1 + +---+ + |r2 | + +---+ + eth0 + | + 2001:db8:1::0/64 + / | \ + +---+ +---+ +---+ + | r1| | r4| |r5 | + +---+ +---+ +---+ + """ + + for routern in range(1, 5): tgen.add_router("r{}".format(routern)) - switch = tgen.add_switch("s1") - switch.add_link(tgen.gears["r1"]) - switch.add_link(tgen.gears["r2"]) + sw1 = tgen.add_switch("s1") + sw1.add_link(tgen.gears["r1"]) + sw1.add_link(tgen.gears["r2"]) + sw1.add_link(tgen.gears["r4"]) - switch = tgen.add_switch("s2") - switch.add_link(tgen.gears["r2"]) - switch.add_link(tgen.gears["r3"]) + sw2 = tgen.add_switch("s2") + sw2.add_link(tgen.gears["r2"]) + sw2.add_link(tgen.gears["r3"]) + + ## Add iBGP ExaBGP neighbor + peer_ip = "2001:db8:1::4" ## peer + peer_route = "via 2001:db8:1::1" ## router + r5 = tgen.add_exabgp_peer("r5", ip=peer_ip, defaultRoute=peer_route) + sw1.add_link(r5) def setup_module(mod): @@ -55,12 +88,59 @@ def setup_module(mod): tgen.start_router() + # Start r5 exabgp peer + r5 = tgen.gears["r5"] + r5.start(os.path.join(CWD, "r5"), os.path.join(CWD, "exabgp.env")) + def teardown_module(mod): tgen = get_topogen() tgen.stop_topology() +def get_link_local(rname, ifname, cache): + ip = cache.get(rname, {}).get(ifname) + if ip: + return ip + + tgen = get_topogen() + out = tgen.gears[rname].vtysh_cmd("show interface %s json" % ifname, isjson=True) + for address in out[ifname]["ipAddresses"]: + if not address["address"].startswith("fe80::"): + continue + ip = address["address"].split("/")[0] + cache.setdefault(rname, {})[ifname] = ip + return ip + + +def replace_link_local(expected, cache): + for prefix, prefix_infos in expected.get("routes", {}).items(): + for prefix_info in prefix_infos: + for nexthop in prefix_info.get("nexthops", []): + ip = nexthop.get("ip", "") + if not ip.startswith("link-local:"): + continue + rname = ip.split(":")[1] + ifname = ip.split(":")[2] + ip = get_link_local(rname, ifname, cache) + nexthop["ip"] = ip + + +def check_r2_sub_group(expected): + tgen = get_topogen() + + r2 = tgen.gears["r2"] + + output = json.loads(r2.vtysh_cmd("show bgp view RS update-groups json")) + actual = [ + subgroup["peers"] + for entry in output.get("RS", {}).values() + for subgroup in entry["subGroup"] + ] + + return topotest.json_cmp(actual, expected) + + def test_converge_protocols(): "Wait for protocol convergence" @@ -90,6 +170,58 @@ def test_bgp_route_server_client_step1(): if tgen.routers_have_failure(): pytest.skip(tgen.errors) + global link_local_cache + link_local_cache = {} + router_list = tgen.routers().values() + for router in router_list: + if router.name == "r2": + # route-server + cmd = "show bgp view RS ipv6 unicast json" + else: + cmd = "show bgp ipv6 unicast json" + + # router.cmd("vtysh -c 'sh bgp ipv6 json' >/tmp/show_bgp_ipv6_%s.json" % router.name) + ref_file = "{}/{}/show_bgp_ipv6_step1.json".format(CWD, router.name) + expected = json.loads(open(ref_file).read()) + replace_link_local(expected, link_local_cache) + + test_func = partial( + topotest.router_json_cmp, + router, + cmd, + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = "{}: BGP IPv6 table failure".format(router.name) + assert res is None, assertmsg + + # check r2 sub-groups + expected = [["2001:db8:1::4"], ["2001:db8:1::3", "2001:db8:1::2", "2001:db8:3::2"]] + + test_func = functools.partial(check_r2_sub_group, expected) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Peer group split failed" + + +def test_bgp_route_server_client_step2(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r2 = tgen.gears["r2"] + r2.vtysh_cmd( + """ +configure terminal +router bgp 65000 view RS + address-family ipv6 unicast + neighbor 2001:db8:1::2 nexthop-local unchanged + neighbor 2001:db8:1::3 nexthop-local unchanged + neighbor 2001:db8:1::4 nexthop-local unchanged + neighbor 2001:db8:3::2 nexthop-local unchanged +""" + ) + router_list = tgen.routers().values() for router in router_list: if router.name == "r2": @@ -99,8 +231,9 @@ def test_bgp_route_server_client_step1(): cmd = "show bgp ipv6 unicast json" # router.cmd("vtysh -c 'sh bgp ipv6 json' >/tmp/show_bgp_ipv6_%s.json" % router.name) - ref_file = "{}/{}/show_bgp_ipv6.json".format(CWD, router.name) + ref_file = "{}/{}/show_bgp_ipv6_step2.json".format(CWD, router.name) expected = json.loads(open(ref_file).read()) + replace_link_local(expected, link_local_cache) test_func = partial( topotest.router_json_cmp, @@ -112,6 +245,69 @@ def test_bgp_route_server_client_step1(): assertmsg = "{}: BGP IPv6 table failure".format(router.name) assert res is None, assertmsg + # check r2 sub-groups + expected = [ + ["2001:db8:1::4"], + ["2001:db8:1::3", "2001:db8:1::2"], + ["2001:db8:3::2"], + ] + + test_func = functools.partial(check_r2_sub_group, expected) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Peer group split failed" + + +def test_bgp_route_server_client_step3(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r2 = tgen.gears["r2"] + r2.vtysh_cmd( + """ +configure terminal +router bgp 65000 view RS + address-family ipv6 unicast + no neighbor 2001:db8:1::2 nexthop-local unchanged + no neighbor 2001:db8:1::3 nexthop-local unchanged + no neighbor 2001:db8:1::4 nexthop-local unchanged + no neighbor 2001:db8:3::2 nexthop-local unchanged +""" + ) + + global link_local_cache + link_local_cache = {} + router_list = tgen.routers().values() + for router in router_list: + if router.name == "r2": + # route-server + cmd = "show bgp view RS ipv6 unicast json" + else: + cmd = "show bgp ipv6 unicast json" + + # router.cmd("vtysh -c 'sh bgp ipv6 json' >/tmp/show_bgp_ipv6_%s.json" % router.name) + ref_file = "{}/{}/show_bgp_ipv6_step1.json".format(CWD, router.name) + expected = json.loads(open(ref_file).read()) + replace_link_local(expected, link_local_cache) + + test_func = partial( + topotest.router_json_cmp, + router, + cmd, + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = "{}: BGP IPv6 table failure".format(router.name) + assert res is None, assertmsg + + # check r2 sub-groups + expected = [["2001:db8:1::4"], ["2001:db8:1::3", "2001:db8:1::2", "2001:db8:3::2"]] + + test_func = functools.partial(check_r2_sub_group, expected) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Peer group split failed" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] From 1005c147684024a71412e44146e6f62673496cf2 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 11 Oct 2024 13:14:25 +0200 Subject: [PATCH 09/46] tests: test nexthop-local unchanged with reflector Test nexthop-local unchanged with route-reflector. Signed-off-by: Louis Scalbert --- .../r1/show_bgp_ipv6_step1.json | 64 ++++++++++-- .../r2/show_bgp_ipv6_step1.json | 64 ++++++++++-- .../r5/show_bgp_ipv6_step1.json | 16 +-- .../r6/show_bgp_ipv6_step1.json | 64 ++++++++++-- tests/topotests/bgp_nexthop_ipv6/rr/bgpd.conf | 5 + .../test_bgp_nexthop_ipv6_topo1.py | 98 +++++++++++++++++++ 6 files changed, 273 insertions(+), 38 deletions(-) diff --git a/tests/topotests/bgp_nexthop_ipv6/r1/show_bgp_ipv6_step1.json b/tests/topotests/bgp_nexthop_ipv6/r1/show_bgp_ipv6_step1.json index 9923edb348da..f468ae1b3e37 100644 --- a/tests/topotests/bgp_nexthop_ipv6/r1/show_bgp_ipv6_step1.json +++ b/tests/topotests/bgp_nexthop_ipv6/r1/show_bgp_ipv6_step1.json @@ -22,7 +22,13 @@ "ip": "fd00:0:2::2", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r2:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -48,7 +54,13 @@ "ip": "fd00:0:2::4", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r4:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -61,7 +73,13 @@ "ip": "fd00:0:3::5", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -74,7 +92,13 @@ "ip": "fd00:0:4::6", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -100,7 +124,13 @@ "ip": "fd00:0:2::2", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r2:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -126,7 +156,13 @@ "ip": "fd00:0:2::4", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r4:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -139,7 +175,13 @@ "ip": "fd00:0:3::5", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -152,7 +194,13 @@ "ip": "fd00:0:4::6", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] diff --git a/tests/topotests/bgp_nexthop_ipv6/r2/show_bgp_ipv6_step1.json b/tests/topotests/bgp_nexthop_ipv6/r2/show_bgp_ipv6_step1.json index bb2efa16d9e8..824db383a9ef 100644 --- a/tests/topotests/bgp_nexthop_ipv6/r2/show_bgp_ipv6_step1.json +++ b/tests/topotests/bgp_nexthop_ipv6/r2/show_bgp_ipv6_step1.json @@ -9,7 +9,13 @@ "ip": "fd00:0:2::1", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r1:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -48,7 +54,13 @@ "ip": "fd00:0:2::4", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r4:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -61,7 +73,13 @@ "ip": "fd00:0:3::5", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -74,7 +92,13 @@ "ip": "fd00:0:4::6", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -87,7 +111,13 @@ "ip": "fd00:0:2::1", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r1:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -126,7 +156,13 @@ "ip": "fd00:0:2::4", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:r4:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -139,7 +175,13 @@ "ip": "fd00:0:3::5", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -152,7 +194,13 @@ "ip": "fd00:0:4::6", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-sw", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] diff --git a/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step1.json b/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step1.json index d0875474ae94..88e3efb61757 100644 --- a/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step1.json +++ b/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step1.json @@ -47,13 +47,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -155,13 +149,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] diff --git a/tests/topotests/bgp_nexthop_ipv6/r6/show_bgp_ipv6_step1.json b/tests/topotests/bgp_nexthop_ipv6/r6/show_bgp_ipv6_step1.json index cd48dd4697ce..1407eca359f1 100644 --- a/tests/topotests/bgp_nexthop_ipv6/r6/show_bgp_ipv6_step1.json +++ b/tests/topotests/bgp_nexthop_ipv6/r6/show_bgp_ipv6_step1.json @@ -9,7 +9,13 @@ "ip": "fd00:0:2::1", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -22,7 +28,13 @@ "ip": "fd00:0:2::2", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -48,7 +60,13 @@ "ip": "fd00:0:2::4", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -61,7 +79,13 @@ "ip": "fd00:0:3::5", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -87,7 +111,13 @@ "ip": "fd00:0:2::1", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -100,7 +130,13 @@ "ip": "fd00:0:2::2", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -126,7 +162,13 @@ "ip": "fd00:0:2::4", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] @@ -139,7 +181,13 @@ "ip": "fd00:0:3::5", "hostname": "rr", "afi": "ipv6", - "scope": "global", + "scope": "global" + }, + { + "ip": "link-local:rr:eth-r6", + "hostname": "rr", + "afi": "ipv6", + "scope": "link-local", "used": true } ] diff --git a/tests/topotests/bgp_nexthop_ipv6/rr/bgpd.conf b/tests/topotests/bgp_nexthop_ipv6/rr/bgpd.conf index 6dcded15258c..705ae78b8e1e 100644 --- a/tests/topotests/bgp_nexthop_ipv6/rr/bgpd.conf +++ b/tests/topotests/bgp_nexthop_ipv6/rr/bgpd.conf @@ -19,12 +19,17 @@ router bgp 65000 neighbor fd00:0:4::6 route-reflector-client address-family ipv6 unicast neighbor fd00:0:2::1 route-reflector-client + neighbor fd00:0:2::1 nexthop-local unchanged neighbor fd00:0:2::1 activate neighbor fd00:0:2::2 route-reflector-client + neighbor fd00:0:2::2 nexthop-local unchanged neighbor fd00:0:2::2 activate neighbor fd00:0:2::3 route-reflector-client + neighbor fd00:0:2::3 nexthop-local unchanged neighbor fd00:0:2::3 activate neighbor fd00:0:2::4 nexthop-local unchanged neighbor fd00:0:2::4 activate + neighbor fd00:0:3::5 nexthop-local unchanged neighbor fd00:0:3::5 activate + neighbor fd00:0:4::6 nexthop-local unchanged neighbor fd00:0:4::6 activate diff --git a/tests/topotests/bgp_nexthop_ipv6/test_bgp_nexthop_ipv6_topo1.py b/tests/topotests/bgp_nexthop_ipv6/test_bgp_nexthop_ipv6_topo1.py index 24d71f5622ae..e478139eb1e6 100644 --- a/tests/topotests/bgp_nexthop_ipv6/test_bgp_nexthop_ipv6_topo1.py +++ b/tests/topotests/bgp_nexthop_ipv6/test_bgp_nexthop_ipv6_topo1.py @@ -165,6 +165,21 @@ def replace_link_local(expected, cache): nexthop["ip"] = ip +def check_rr_sub_group(expected): + tgen = get_topogen() + + rr = tgen.gears["rr"] + + output = json.loads(rr.vtysh_cmd("show bgp update-groups json")) + actual = [ + subgroup["peers"] + for entry in output.get("default", {}).values() + for subgroup in entry["subGroup"] + ] + + return topotest.json_cmp(actual, expected) + + def teardown_module(_mod): "Teardown the pytest environment" tgen = get_topogen() @@ -222,6 +237,19 @@ def test_bgp_ipv6_table_step1(): assertmsg = "{}: BGP IPv6 Nexthop failure".format(router.name) assert res is None, assertmsg + # check rr sub-groups + expected = [ + ["fd00:0:2::1", "fd00:0:2::2"], + ["fd00:0:2::3"], + ["fd00:0:2::4"], + ["fd00:0:3::5"], + ["fd00:0:4::6"], + ] + + test_func = partial(check_rr_sub_group, expected) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Peer group split failed" + def test_bgp_ipv6_table_step2(): tgen = get_topogen() @@ -236,7 +264,12 @@ def test_bgp_ipv6_table_step2(): configure terminal router bgp 65000 address-family ipv6 unicast + no neighbor fd00:0:2::1 nexthop-local unchanged + no neighbor fd00:0:2::2 nexthop-local unchanged + no neighbor fd00:0:2::3 nexthop-local unchanged no neighbor fd00:0:2::4 nexthop-local unchanged + no neighbor fd00:0:3::5 nexthop-local unchanged + no neighbor fd00:0:4::6 nexthop-local unchanged """ ) @@ -257,6 +290,71 @@ def test_bgp_ipv6_table_step2(): assertmsg = "{}: BGP IPv6 Nexthop failure".format(router.name) assert res is None, assertmsg + # check rr sub-groups + expected = [ + ["fd00:0:2::1", "fd00:0:2::2"], + ["fd00:0:2::3"], + ["fd00:0:3::5", "fd00:0:2::4"], + ["fd00:0:4::6"], + ] + + test_func = partial(check_rr_sub_group, expected) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Peer group split failed" + + +def test_bgp_ipv6_table_step3(): + tgen = get_topogen() + + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + rr = tgen.gears["rr"] + rr.vtysh_cmd( + """ +configure terminal +router bgp 65000 + address-family ipv6 unicast + neighbor fd00:0:2::1 nexthop-local unchanged + neighbor fd00:0:2::2 nexthop-local unchanged + neighbor fd00:0:2::3 nexthop-local unchanged + neighbor fd00:0:2::4 nexthop-local unchanged + neighbor fd00:0:3::5 nexthop-local unchanged + neighbor fd00:0:4::6 nexthop-local unchanged +""" + ) + + router_list = tgen.routers().values() + for router in router_list: + # router.cmd("vtysh -c 'sh bgp ipv6 json' >/tmp/show_bgp_ipv6_%s.json" % router.name) + ref_file = "{}/{}/show_bgp_ipv6_step1.json".format(CWD, router.name) + expected = json.loads(open(ref_file).read()) + replace_link_local(expected, link_local_cache) + + test_func = partial( + topotest.router_json_cmp, + router, + "show bgp ipv6 unicast json", + expected, + ) + _, res = topotest.run_and_expect(test_func, None, count=30, wait=1) + assertmsg = "{}: BGP IPv6 Nexthop failure".format(router.name) + assert res is None, assertmsg + + # check rr sub-groups + expected = [ + ["fd00:0:2::1", "fd00:0:2::2"], + ["fd00:0:2::3"], + ["fd00:0:2::4"], + ["fd00:0:3::5"], + ["fd00:0:4::6"], + ] + + test_func = partial(check_rr_sub_group, expected) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=0.5) + assert result is None, "Peer group split failed" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] From 33189510e8d008e894b686291236be7c59b8c603 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 27 Feb 2024 19:32:21 +0100 Subject: [PATCH 10/46] topotests: check export labels to pre-policy bmp Check export labels to pre-policy bmp Signed-off-by: Louis Scalbert --- tests/topotests/bgp_bmp/test_bgp_bmp.py | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp.py b/tests/topotests/bgp_bmp/test_bgp_bmp.py index 80e291b2bdb6..de5db9eb3e28 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp.py @@ -234,15 +234,11 @@ def vpn_prefixes(policy): prefixes = ["172.31.10.1/32", "2001::2222/128"] - if policy == PRE_POLICY: - # labels are not yet supported in adj-RIB-in. Do not test for the moment - labels = None - else: - # "label vpn export" value in r2/bgpd.conf - labels = { - "172.31.10.1/32": 102, - "2001::2222/128": 105, - } + # "label vpn export" value in r2/bgpd.conf + labels = { + "172.31.10.1/32": 102, + "2001::2222/128": 105, + } # add prefixes configure_prefixes(tgen, "r2", 65502, "unicast", prefixes, vrf="vrf1") From 035304c25a3890a040acbe23ca385750b062cdce Mon Sep 17 00:00:00 2001 From: Maxence Younsi Date: Thu, 13 Apr 2023 15:28:32 +0200 Subject: [PATCH 11/46] bgpd: bmp loc-rib peer up/down for vrfs added bmp bgp peer for vrfs added peer up vrf in bmp peer up state added vrf state in bmpbgp added safe bmp_peer_sendall : bmp_peer_sendall_safe changed bgp_open_send to call new bgp_open_make bgp_open_make creates a bgp open packet, now used in bmp for peer up vrf added hook and call to bgp instance state vrf peer state is recomputed when interfaces (including vrf itf) go up / down and when it gets created or removed Link: https://github.com/mxyns/frr/commit/e48ba380700d53124131f4e4419f646c05b40c86 Signed-off-by: Philippe Guibert Signed-off-by: Louis Scalbert Signed-off-by: Maxence Younsi --- bgpd/bgp_bmp.c | 267 ++++++++++++++++++++++++++++++++++++++++------ bgpd/bgp_bmp.h | 24 ++++- bgpd/bgp_packet.c | 51 +++++---- bgpd/bgp_packet.h | 1 + bgpd/bgpd.c | 7 ++ bgpd/bgpd.h | 1 + 6 files changed, 289 insertions(+), 62 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 9d99c2c7fda8..08f8b8b734ff 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -39,6 +39,8 @@ #include "bgpd/bgp_trace.h" #include "bgpd/bgp_network.h" #include "bgpd/bgp_label.h" +#include "bgpd/bgp_open.h" +#include "bgpd/bgp_aspath.h" static void bmp_close(struct bmp *bmp); static struct bmp_bgp *bmp_bgp_find(struct bgp *bgp); @@ -246,6 +248,33 @@ static void bmp_free(struct bmp *bmp) #define BMP_PEER_TYPE_LOCAL_INSTANCE 2 #define BMP_PEER_TYPE_LOC_RIB_INSTANCE 3 +/* determine the peer type for per-peer headers from a vrf_id + * for non loc-rib peer type only + */ +static inline int bmp_get_peer_type_vrf(vrf_id_t vrf_id) +{ + switch (vrf_id) { + case VRF_DEFAULT: + return BMP_PEER_TYPE_GLOBAL_INSTANCE; + case VRF_UNKNOWN: + /* when the VRF is unknown consider it a local instance */ + return BMP_PEER_TYPE_LOCAL_INSTANCE; + default: + return BMP_PEER_TYPE_RD_INSTANCE; + } +} + +/* determine the peer type for per-peer headers from a struct peer + * provide a bgp->peer_self for loc-rib + */ +static inline int bmp_get_peer_type(struct peer *peer) +{ + if (peer->bgp->peer_self == peer) + return BMP_PEER_TYPE_LOC_RIB_INSTANCE; + + return bmp_get_peer_type_vrf(peer->bgp->vrf_id); +} + static inline int bmp_get_peer_distinguisher(struct bmp *bmp, afi_t afi, uint8_t peer_type, uint64_t *result_ref) @@ -370,17 +399,18 @@ static void bmp_put_info_tlv(struct stream *s, uint16_t type, stream_put(s, string, len); } -static void __attribute__((unused)) -bmp_put_vrftablename_info_tlv(struct stream *s, struct bmp *bmp) +/* put the vrf table name of the bgp instance bmp is bound to in a tlv on the + * stream + */ +static void bmp_put_vrftablename_info_tlv(struct stream *s, struct bgp *bgp) { + const char *vrftablename = "global"; #define BMP_INFO_TYPE_VRFTABLENAME 3 - const char *vrftablename = "global"; - if (bmp->targets->bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) { - struct vrf *vrf = vrf_lookup_by_id(bmp->targets->bgp->vrf_id); - vrftablename = vrf ? vrf->name : NULL; - } + if (bgp->inst_type != BGP_INSTANCE_TYPE_DEFAULT) + vrftablename = bgp->name; + if (vrftablename != NULL) bmp_put_info_tlv(s, BMP_INFO_TYPE_VRFTABLENAME, vrftablename); } @@ -428,6 +458,10 @@ static void bmp_notify_put(struct stream *s, struct bgp_notify *nfy) + sizeof(marker)); } +/* send peer up/down for peer based on down boolean value + * returns the message to send or NULL if the peer_distinguisher is not + * available + */ static struct stream *bmp_peerstate(struct peer *peer, bool down) { struct stream *s; @@ -438,11 +472,14 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) uptime.tv_usec = 0; monotime_to_realtime(&uptime, &uptime_real); + uint8_t peer_type = bmp_get_peer_type(peer); + bool is_locrib = peer_type == BMP_PEER_TYPE_LOC_RIB_INSTANCE; + #define BGP_BMP_MAX_PACKET_SIZE 1024 #define BMP_PEERUP_INFO_TYPE_STRING 0 s = stream_new(BGP_MAX_PACKET_SIZE); - if (peer_established(peer->connection) && !down) { + if ((peer_established(peer->connection) || is_locrib) && !down) { struct bmp_bgp_peer *bbpeer; bmp_common_hdr(s, BMP_VERSION_3, @@ -452,7 +489,9 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) &uptime_real); /* Local Address (16 bytes) */ - if (peer->su_local->sa.sa_family == AF_INET6) + if (!peer->su_local || is_locrib) + stream_put(s, 0, 16); + else if (peer->su_local->sa.sa_family == AF_INET6) stream_put(s, &peer->su_local->sin6.sin6_addr, 16); else if (peer->su_local->sa.sa_family == AF_INET) { stream_putl(s, 0); @@ -462,15 +501,21 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) } /* Local Port, Remote Port */ - if (peer->su_local->sa.sa_family == AF_INET6) + if (is_locrib) + stream_putw(s, 0); + else if (peer->su_local->sa.sa_family == AF_INET6) stream_putw(s, htons(peer->su_local->sin6.sin6_port)); else if (peer->su_local->sa.sa_family == AF_INET) stream_putw(s, htons(peer->su_local->sin.sin_port)); - if (peer->su_remote->sa.sa_family == AF_INET6) + + if (is_locrib) + stream_putw(s, 0); + else if (peer->su_remote->sa.sa_family == AF_INET6) stream_putw(s, htons(peer->su_remote->sin6.sin6_port)); else if (peer->su_remote->sa.sa_family == AF_INET) stream_putw(s, htons(peer->su_remote->sin.sin_port)); + /* TODO craft message with fields & capabilities for loc-rib */ static const uint8_t dummy_open[] = { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, @@ -510,32 +555,40 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) type_pos = stream_get_endp(s); stream_putc(s, 0); /* placeholder for down reason */ - switch (peer->last_reset) { - case PEER_DOWN_NOTIFY_RECEIVED: - type = BMP_PEERDOWN_REMOTE_NOTIFY; - bmp_notify_put(s, &peer->notify); - break; - case PEER_DOWN_CLOSE_SESSION: - type = BMP_PEERDOWN_REMOTE_CLOSE; - break; - case PEER_DOWN_WAITING_NHT: - type = BMP_PEERDOWN_LOCAL_FSM; - stream_putw(s, BGP_FSM_TcpConnectionFails); - break; - /* - * TODO: Map remaining PEER_DOWN_* reasons to RFC event codes. - * TODO: Implement BMP_PEERDOWN_LOCAL_NOTIFY. - * - * See RFC7854 ss. 4.9 - */ - default: - type = BMP_PEERDOWN_LOCAL_FSM; - stream_putw(s, BMP_PEER_DOWN_NO_RELEVANT_EVENT_CODE); - break; + if (is_locrib) { + type = BMP_PEERDOWN_LOCAL_TLV; + } else { + switch (peer->last_reset) { + case PEER_DOWN_NOTIFY_RECEIVED: + type = BMP_PEERDOWN_REMOTE_NOTIFY; + bmp_notify_put(s, &peer->notify); + break; + case PEER_DOWN_CLOSE_SESSION: + type = BMP_PEERDOWN_REMOTE_CLOSE; + break; + case PEER_DOWN_WAITING_NHT: + type = BMP_PEERDOWN_LOCAL_FSM; + stream_putw(s, BGP_FSM_TcpConnectionFails); + break; + /* + * TODO: Map remaining PEER_DOWN_* reasons to RFC event + * codes. + * TODO: Implement BMP_PEERDOWN_LOCAL_NOTIFY. + * + * See RFC7854 ss. 4.9 + */ + default: + type = BMP_PEERDOWN_LOCAL_FSM; + stream_putw(s, BMP_PEER_DOWN_NO_RELEVANT_EVENT_CODE); + break; + } } stream_putc_at(s, type_pos, type); } + if (is_locrib) + bmp_put_vrftablename_info_tlv(s, peer->bgp); + len = stream_get_endp(s); stream_putl_at(s, BMP_LENGTH_POS, len); /* message length is set. */ return s; @@ -558,6 +611,25 @@ static int bmp_send_peerup(struct bmp *bmp) return 0; } +static int bmp_send_peerup_vrf(struct bmp *bmp) +{ + struct bmp_bgp *bmpbgp = bmp->targets->bmpbgp; + struct stream *s; + + /* send unconditionally because state may has been set before the + * session was up. and in this case the peer up has not been sent. + */ + bmp_bgp_update_vrf_status(bmpbgp, vrf_state_unknown); + + s = bmp_peerstate(bmpbgp->bgp->peer_self, bmpbgp->vrf_state == vrf_state_down); + + pullwr_write_stream(bmp->pullwr, s); + stream_free(s); + + return 0; +} + +/* send a stream to all bmp sessions configured in a bgp instance */ /* XXX: kludge - filling the pullwr's buffer */ static void bmp_send_all(struct bmp_bgp *bmpbgp, struct stream *s) { @@ -570,6 +642,16 @@ static void bmp_send_all(struct bmp_bgp *bmpbgp, struct stream *s) stream_free(s); } +static void bmp_send_all_safe(struct bmp_bgp *bmpbgp, struct stream *s) +{ + if (!bmpbgp) { + stream_free(s); + return; + } + + bmp_send_all(bmpbgp, s); +} + /* * Route Mirroring */ @@ -816,7 +898,7 @@ static int bmp_peer_status_changed(struct peer *peer) } } - bmp_send_all(bmpbgp, bmp_peerstate(peer, false)); + bmp_send_all_safe(bmpbgp, bmp_peerstate(peer, false)); return 0; } @@ -838,7 +920,7 @@ static int bmp_peer_backward(struct peer *peer) bbpeer->open_rx_len = 0; } - bmp_send_all(bmpbgp, bmp_peerstate(peer, true)); + bmp_send_all_safe(bmpbgp, bmp_peerstate(peer, true)); return 0; } @@ -1476,6 +1558,7 @@ static void bmp_wrfill(struct bmp *bmp, struct pullwr *pullwr) { switch(bmp->state) { case BMP_PeerUp: + bmp_send_peerup_vrf(bmp); bmp_send_peerup(bmp); bmp->state = BMP_Run; break; @@ -1839,6 +1922,7 @@ static struct bmp_bgp *bmp_bgp_get(struct bgp *bgp) bmpbgp = XCALLOC(MTYPE_BMP, sizeof(*bmpbgp)); bmpbgp->bgp = bgp; + bmpbgp->vrf_state = vrf_state_unknown; bmpbgp->mirror_qsizelimit = ~0UL; bmp_mirrorq_init(&bmpbgp->mirrorq); bmp_bgph_add(&bmp_bgph, bmpbgp); @@ -1873,6 +1957,79 @@ static int bmp_bgp_del(struct bgp *bgp) return 0; } +static void bmp_bgp_peer_vrf(struct bmp_bgp_peer *bbpeer, struct bgp *bgp) +{ + struct peer *peer = bgp->peer_self; + uint16_t send_holdtime; + as_t local_as; + + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) + send_holdtime = peer->holdtime; + else + send_holdtime = peer->bgp->default_holdtime; + + /* local-as Change */ + if (peer->change_local_as) + local_as = peer->change_local_as; + else + local_as = peer->local_as; + + struct stream *s = bgp_open_make(peer, send_holdtime, local_as); + size_t open_len = stream_get_endp(s); + + bbpeer->open_rx_len = open_len; + bbpeer->open_rx = XMALLOC(MTYPE_BMP_OPEN, open_len); + memcpy(bbpeer->open_rx, s->data, open_len); + + bbpeer->open_tx_len = open_len; + bbpeer->open_tx = bbpeer->open_rx; +} + +/* update the vrf status of the bmpbgp struct for vrf peer up/down + * + * if force is unknown, use zebra vrf state + * + * returns true if state has changed + */ +bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force) +{ + enum bmp_vrf_state old_state; + struct bmp_bgp_peer *bbpeer; + struct peer *peer; + struct vrf *vrf; + struct bgp *bgp; + bool changed; + + if (!bmpbgp || !bmpbgp->bgp) + return false; + + bgp = bmpbgp->bgp; + old_state = bmpbgp->vrf_state; + + vrf = bgp_vrf_lookup_by_instance_type(bgp); + bmpbgp->vrf_state = force != vrf_state_unknown ? force + : vrf_is_enabled(vrf) ? vrf_state_up + : vrf_state_down; + + changed = old_state != bmpbgp->vrf_state; + if (changed) { + peer = bmpbgp->bgp->peer_self; + if (bmpbgp->vrf_state == vrf_state_up) { + bbpeer = bmp_bgp_peer_get(peer); + bmp_bgp_peer_vrf(bbpeer, bgp); + } else { + bbpeer = bmp_bgp_peer_find(peer->qobj_node.nid); + if (bbpeer) { + XFREE(MTYPE_BMP_OPEN, bbpeer->open_rx); + bmp_peerh_del(&bmp_peerh, bbpeer); + XFREE(MTYPE_BMP_PEER, bbpeer); + } + } + } + + return changed; +} + static struct bmp_bgp_peer *bmp_bgp_peer_find(uint64_t peerid) { struct bmp_bgp_peer dummy = { .peerid = peerid }; @@ -2953,6 +3110,44 @@ static int bgp_bmp_early_fini(void) return 0; } +/* called when a bgp instance goes up/down, implying that the underlying VRF + * has been created or deleted in zebra + */ +static int bmp_vrf_state_changed(struct bgp *bgp) +{ + struct bmp_bgp *bmpbgp = bmp_bgp_find(bgp); + + if (!bmp_bgp_update_vrf_status(bmpbgp, vrf_state_unknown)) + return 1; + + bmp_send_all_safe(bmpbgp, + bmp_peerstate(bgp->peer_self, bmpbgp->vrf_state == vrf_state_down)); + + return 0; +} + +/* called when an interface goes up/down in a vrf, this may signal that the + * VRF changed state and is how bgp_snmp detects vrf state changes + */ +static int bmp_vrf_itf_state_changed(struct bgp *bgp, struct interface *itf) +{ + struct bmp_bgp *bmpbgp; + enum bmp_vrf_state new_state; + + /* if the update is not about the vrf device double-check + * the zebra status of the vrf + */ + if (!itf || !if_is_vrf(itf)) + return bmp_vrf_state_changed(bgp); + + bmpbgp = bmp_bgp_find(bgp); + new_state = if_is_up(itf) ? vrf_state_up : vrf_state_down; + if (bmp_bgp_update_vrf_status(bmpbgp, new_state)) + bmp_send_all(bmpbgp, bmp_peerstate(bgp->peer_self, new_state == vrf_state_down)); + + return 0; +} + static int bgp_bmp_module_init(void) { hook_register(bgp_packet_dump, bmp_mirror_packet); @@ -2965,6 +3160,8 @@ static int bgp_bmp_module_init(void) hook_register(frr_late_init, bgp_bmp_init); hook_register(bgp_route_update, bmp_route_update); hook_register(frr_early_fini, bgp_bmp_early_fini); + hook_register(bgp_instance_state, bmp_vrf_state_changed); + hook_register(bgp_vrf_status_changed, bmp_vrf_itf_state_changed); return 0; } diff --git a/bgpd/bgp_bmp.h b/bgpd/bgp_bmp.h index 33247c402504..d45a4278f69e 100644 --- a/bgpd/bgp_bmp.h +++ b/bgpd/bgp_bmp.h @@ -268,10 +268,19 @@ PREDECL_HASH(bmp_bgph); #define BMP_PEER_DOWN_NO_RELEVANT_EVENT_CODE 0x00 +enum bmp_vrf_state { + vrf_state_down = -1, + vrf_state_unknown = 0, + vrf_state_up = 1, +}; + struct bmp_bgp { struct bmp_bgph_item bbi; struct bgp *bgp; + + enum bmp_vrf_state vrf_state; + struct bmp_targets_head targets; struct bmp_mirrorq_head mirrorq; @@ -280,12 +289,17 @@ struct bmp_bgp { size_t mirror_qsizelimit; }; +extern bool bmp_bgp_update_vrf_status(struct bmp_bgp *bmpbgp, enum bmp_vrf_state force); + enum { - BMP_PEERDOWN_LOCAL_NOTIFY = 1, - BMP_PEERDOWN_LOCAL_FSM = 2, - BMP_PEERDOWN_REMOTE_NOTIFY = 3, - BMP_PEERDOWN_REMOTE_CLOSE = 4, - BMP_PEERDOWN_ENDMONITOR = 5, + /* RFC7854 - 10.8 */ + BMP_PEERDOWN_LOCAL_NOTIFY = 1, + BMP_PEERDOWN_LOCAL_FSM = 2, + BMP_PEERDOWN_REMOTE_NOTIFY = 3, + BMP_PEERDOWN_REMOTE_CLOSE = 4, + BMP_PEERDOWN_ENDMONITOR = 5, + /* RFC9069 - 8.4 */ + BMP_PEERDOWN_LOCAL_TLV = 6, }; enum { diff --git a/bgpd/bgp_packet.c b/bgpd/bgp_packet.c index 646ab1d95f62..6b116db1075d 100644 --- a/bgpd/bgp_packet.c +++ b/bgpd/bgp_packet.c @@ -641,31 +641,11 @@ void bgp_keepalive_send(struct peer *peer) bgp_writes_on(peer->connection); } -/* - * Creates a BGP Open packet and appends it to the peer's output queue. - * Sets capabilities as necessary. - */ -void bgp_open_send(struct peer_connection *connection) +struct stream *bgp_open_make(struct peer *peer, uint16_t send_holdtime, as_t local_as) { - struct stream *s; - uint16_t send_holdtime; - as_t local_as; - struct peer *peer = connection->peer; + struct stream *s = stream_new(BGP_STANDARD_MESSAGE_MAX_PACKET_SIZE); bool ext_opt_params = false; - if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) - send_holdtime = peer->holdtime; - else - send_holdtime = peer->bgp->default_holdtime; - - /* local-as Change */ - if (peer->change_local_as) - local_as = peer->change_local_as; - else - local_as = peer->local_as; - - s = stream_new(BGP_STANDARD_MESSAGE_MAX_PACKET_SIZE); - /* Make open packet. */ bgp_packet_set_marker(s, BGP_MSG_OPEN); @@ -704,6 +684,33 @@ void bgp_open_send(struct peer_connection *connection) ext_opt_params ? " (Extended)" : "", BGP_VERSION_4, local_as, send_holdtime, &peer->local_id); + return s; +} + +/* + * Creates a BGP Open packet and appends it to the peer's output queue. + * Sets capabilities as necessary. + */ +void bgp_open_send(struct peer_connection *connection) +{ + struct stream *s; + uint16_t send_holdtime; + as_t local_as; + struct peer *peer = connection->peer; + + if (CHECK_FLAG(peer->flags, PEER_FLAG_TIMER)) + send_holdtime = peer->holdtime; + else + send_holdtime = peer->bgp->default_holdtime; + + /* local-as Change */ + if (peer->change_local_as) + local_as = peer->change_local_as; + else + local_as = peer->local_as; + + s = bgp_open_make(peer, send_holdtime, local_as); + /* Dump packet if debug option is set. */ /* bgp_packet_dump (s); */ hook_call(bgp_packet_send, peer, BGP_MSG_OPEN, stream_get_endp(s), s); diff --git a/bgpd/bgp_packet.h b/bgpd/bgp_packet.h index b67acf205593..c266b17266ec 100644 --- a/bgpd/bgp_packet.h +++ b/bgpd/bgp_packet.h @@ -43,6 +43,7 @@ DECLARE_HOOK(bgp_packet_send, /* Packet send and receive function prototypes. */ extern void bgp_keepalive_send(struct peer *peer); +extern struct stream *bgp_open_make(struct peer *peer, uint16_t send_holdtime, as_t local_as); extern void bgp_open_send(struct peer_connection *connection); extern void bgp_notify_send(struct peer_connection *connection, uint8_t code, uint8_t sub_code); diff --git a/bgpd/bgpd.c b/bgpd/bgpd.c index 80b1ae39d4d1..a186243ffcfc 100644 --- a/bgpd/bgpd.c +++ b/bgpd/bgpd.c @@ -85,6 +85,7 @@ DEFINE_QOBJ_TYPE(bgp_master); DEFINE_QOBJ_TYPE(bgp); DEFINE_QOBJ_TYPE(peer); DEFINE_HOOK(bgp_inst_delete, (struct bgp *bgp), (bgp)); +DEFINE_HOOK(bgp_instance_state, (struct bgp *bgp), (bgp)); /* BGP process wide configuration. */ static struct bgp_master bgp_master; @@ -3929,6 +3930,9 @@ void bgp_instance_up(struct bgp *bgp) struct peer *peer; struct listnode *node, *next; + /* notify BMP of instance state changed */ + hook_call(bgp_instance_state, bgp); + bgp_set_redist_vrf_bitmaps(bgp, true); /* Register with zebra. */ @@ -3957,6 +3961,9 @@ void bgp_instance_down(struct bgp *bgp) /* Cleanup evpn instance state */ bgp_evpn_instance_down(bgp); + /* notify BMP of instance state changed */ + hook_call(bgp_instance_state, bgp); + /* Stop timers. */ if (bgp->t_rmap_def_originate_eval) EVENT_OFF(bgp->t_rmap_def_originate_eval); diff --git a/bgpd/bgpd.h b/bgpd/bgpd.h index 3c3655f0a5ad..852efdf19d31 100644 --- a/bgpd/bgpd.h +++ b/bgpd/bgpd.h @@ -880,6 +880,7 @@ DECLARE_HOOK(bgp_snmp_traps_config_write, (struct vty *vty), (vty)); DECLARE_HOOK(bgp_config_end, (struct bgp *bgp), (bgp)); DECLARE_HOOK(bgp_hook_vrf_update, (struct vrf *vrf, bool enabled), (vrf, enabled)); +DECLARE_HOOK(bgp_instance_state, (struct bgp *bgp), (bgp)); /* Thread callback information */ struct afi_safi_info { From 1de5015b09ccf0e7270f73a9876d46f06920bda7 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 28 Feb 2024 13:50:50 +0100 Subject: [PATCH 12/46] topotests: log bmp peer up message type in collector Log "peer up" message type in BMP collector logs. Signed-off-by: Louis Scalbert --- tests/topotests/lib/bmp_collector/bmp.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/topotests/lib/bmp_collector/bmp.py b/tests/topotests/lib/bmp_collector/bmp.py index 237decdd5ecb..8815699bf24d 100644 --- a/tests/topotests/lib/bmp_collector/bmp.py +++ b/tests/topotests/lib/bmp_collector/bmp.py @@ -360,6 +360,7 @@ def dissect(cls, data): msg = { **peer_msg, **{ + "bmp_log_type": "peer up", "local_ip": bin2str_ipaddress(local_addr, peer_msg.get("ipv6")), "local_port": int(local_port), "remote_port": int(remote_port), From d8bfd04e4691ff93f8c108c69dab72e84a6c9ace Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 28 Feb 2024 13:51:50 +0100 Subject: [PATCH 13/46] topotests: add peer down log in bmp collector Add peer down log in bmp collector Signed-off-by: Louis Scalbert --- tests/topotests/lib/bmp_collector/bmp.py | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tests/topotests/lib/bmp_collector/bmp.py b/tests/topotests/lib/bmp_collector/bmp.py index 8815699bf24d..f3c0be49c577 100644 --- a/tests/topotests/lib/bmp_collector/bmp.py +++ b/tests/topotests/lib/bmp_collector/bmp.py @@ -316,7 +316,8 @@ class BMPStatisticsReport: # ------------------------------------------------------------------------------ -class BMPPeerDownNotification: +@BMPMsg.register_msg_type(BMPCodes.BMP_MSG_TYPE_PEER_DOWN_NOTIFICATION) +class BMPPeerDownNotification(BMPPerPeerMessage): """ 0 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 1 2 3 4 5 6 7 8 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ @@ -326,7 +327,20 @@ class BMPPeerDownNotification: +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ """ - pass + @classmethod + def dissect(cls, data): + data, peer_msg = super().dissect(data) + + msg = { + **peer_msg, + **{ + "bmp_log_type": "peer down", + }, + } + + # XXX: dissect the bgp open message + + return msg # ------------------------------------------------------------------------------ From 0430d6f70ba4807564dee993c4cdc132477341df Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 28 Feb 2024 14:07:06 +0100 Subject: [PATCH 14/46] topotests: check for bmp peer up/down messages Check for bmp peer up / down messages Signed-off-by: Louis Scalbert --- tests/topotests/bgp_bmp/test_bgp_bmp.py | 59 +++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/topotests/bgp_bmp/test_bgp_bmp.py b/tests/topotests/bgp_bmp/test_bgp_bmp.py index de5db9eb3e28..1cf6a0e7f826 100644 --- a/tests/topotests/bgp_bmp/test_bgp_bmp.py +++ b/tests/topotests/bgp_bmp/test_bgp_bmp.py @@ -160,6 +160,34 @@ def check_for_prefixes(expected_prefixes, bmp_log_type, policy, labels=None): return True +def check_for_peer_message(expected_peers, bmp_log_type): + """ + Check for the presence of a peer up message for the peer + """ + global SEQ + # we care only about the new messages + messages = [ + m for m in sorted(get_bmp_messages(), key=lambda d: d["seq"]) if m["seq"] > SEQ + ] + + # get the list of pairs (prefix, policy, seq) for the given message type + peers = [ + m["peer_ip"] + for m in messages + if "peer_ip" in m.keys() and m["bmp_log_type"] == bmp_log_type + ] + + # check for prefixes + for ep in expected_peers: + if ep not in peers: + msg = "The peer {} is not present in the {} log messages." + logger.debug(msg.format(ep, bmp_log_type)) + return False + + SEQ = messages[-1]["seq"] + return True + + def set_bmp_policy(tgen, node, asn, target, safi, policy, vrf=None): """ Configure the bmp policy. @@ -276,6 +304,20 @@ def check_for_log_file(): assert success, "The BMP server is not logging" +def test_peer_up(): + """ + Checking for BMP peers up messages + """ + + peers = ["192.168.0.2", "192:168::2"] + + logger.info("checking for BMP peers up messages") + + test_func = partial(check_for_peer_message, peers, "peer up") + success, _ = topotest.run_and_expect(test_func, True, wait=0.5) + assert success, "Checking the updated prefixes has been failed !." + + def test_bmp_bgp_unicast(): """ Add/withdraw bgp unicast prefixes and check the bmp logs. @@ -298,6 +340,23 @@ def test_bmp_bgp_vpn(): vpn_prefixes(LOC_RIB) +def test_peer_down(): + """ + Checking for BMP peers down messages + """ + tgen = get_topogen() + + tgen.gears["r2"].vtysh_cmd("clear bgp *") + + peers = ["192.168.0.2", "192:168::2"] + + logger.info("checking for BMP peers down messages") + + test_func = partial(check_for_peer_message, peers, "peer down") + success, _ = topotest.run_and_expect(test_func, True, wait=0.5) + assert success, "Checking the updated prefixes has been failed !." + + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] sys.exit(pytest.main(args)) From 7bccb8d3802251cd4b1a4d774335c863a12a689e Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Wed, 28 Feb 2024 14:12:19 +0100 Subject: [PATCH 15/46] topotest: add bgp_bmp_vrf topotest Add test to check BMP in VRF. Note that the following configuration works with interface r1-eth0 towards 192.0.2.10 (BMP collector) in the default VRF but not in vrf1. > router bgp 65501 vrf vrf1 > bmp targets bmp1 > bmp connect 192.0.2.10 port 1789 min-retry 100 max-retry 10000 Also, for some reasons, the test works even without "bgpd: bmp loc-rib peer up/down for vrfs" commit. Signed-off-by: Louis Scalbert --- tests/topotests/bgp_bmp_vrf/__init__.py | 0 tests/topotests/bgp_bmp_vrf/r1/bgpd.conf | 23 ++ tests/topotests/bgp_bmp_vrf/r1/zebra.conf | 7 + tests/topotests/bgp_bmp_vrf/r2/bgpd.conf | 19 + tests/topotests/bgp_bmp_vrf/r2/zebra.conf | 8 + .../topotests/bgp_bmp_vrf/test_bgp_bmp_vrf.py | 326 ++++++++++++++++++ 6 files changed, 383 insertions(+) create mode 100644 tests/topotests/bgp_bmp_vrf/__init__.py create mode 100644 tests/topotests/bgp_bmp_vrf/r1/bgpd.conf create mode 100644 tests/topotests/bgp_bmp_vrf/r1/zebra.conf create mode 100644 tests/topotests/bgp_bmp_vrf/r2/bgpd.conf create mode 100644 tests/topotests/bgp_bmp_vrf/r2/zebra.conf create mode 100644 tests/topotests/bgp_bmp_vrf/test_bgp_bmp_vrf.py diff --git a/tests/topotests/bgp_bmp_vrf/__init__.py b/tests/topotests/bgp_bmp_vrf/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/topotests/bgp_bmp_vrf/r1/bgpd.conf b/tests/topotests/bgp_bmp_vrf/r1/bgpd.conf new file mode 100644 index 000000000000..994cdbf68ea7 --- /dev/null +++ b/tests/topotests/bgp_bmp_vrf/r1/bgpd.conf @@ -0,0 +1,23 @@ +router bgp 65501 vrf vrf1 + bgp router-id 192.168.0.1 + bgp log-neighbor-changes + no bgp ebgp-requires-policy + neighbor 192.168.0.2 remote-as 65502 + neighbor 192:168::2 remote-as 65502 +! + bmp targets bmp1 + bmp connect 192.0.2.10 port 1789 min-retry 100 max-retry 10000 + exit +! + + address-family ipv4 unicast + neighbor 192.168.0.2 activate + neighbor 192.168.0.2 soft-reconfiguration inbound + no neighbor 192:168::2 activate + exit-address-family +! + address-family ipv6 unicast + neighbor 192:168::2 activate + neighbor 192:168::2 soft-reconfiguration inbound + exit-address-family +! diff --git a/tests/topotests/bgp_bmp_vrf/r1/zebra.conf b/tests/topotests/bgp_bmp_vrf/r1/zebra.conf new file mode 100644 index 000000000000..0b523c9e18d8 --- /dev/null +++ b/tests/topotests/bgp_bmp_vrf/r1/zebra.conf @@ -0,0 +1,7 @@ +interface r1-eth0 + ip address 192.0.2.1/24 +! +interface r1-eth1 + ip address 192.168.0.1/24 + ipv6 address 192:168::1/64 +! diff --git a/tests/topotests/bgp_bmp_vrf/r2/bgpd.conf b/tests/topotests/bgp_bmp_vrf/r2/bgpd.conf new file mode 100644 index 000000000000..7c8255a17563 --- /dev/null +++ b/tests/topotests/bgp_bmp_vrf/r2/bgpd.conf @@ -0,0 +1,19 @@ +router bgp 65502 + bgp router-id 192.168.0.2 + bgp log-neighbor-changes + no bgp ebgp-requires-policy + no bgp network import-check + neighbor 192.168.0.1 remote-as 65501 + neighbor 192:168::1 remote-as 65501 +! + address-family ipv4 unicast + neighbor 192.168.0.1 activate + no neighbor 192:168::1 activate + redistribute connected + exit-address-family +! + address-family ipv6 unicast + neighbor 192:168::1 activate + redistribute connected + exit-address-family +! diff --git a/tests/topotests/bgp_bmp_vrf/r2/zebra.conf b/tests/topotests/bgp_bmp_vrf/r2/zebra.conf new file mode 100644 index 000000000000..9d82bfe2df5c --- /dev/null +++ b/tests/topotests/bgp_bmp_vrf/r2/zebra.conf @@ -0,0 +1,8 @@ +interface r2-eth0 + ip address 192.168.0.2/24 + ipv6 address 192:168::2/64 +! +interface r2-eth1 + ip address 172.31.0.2/24 + ipv6 address 172:31::2/64 +! diff --git a/tests/topotests/bgp_bmp_vrf/test_bgp_bmp_vrf.py b/tests/topotests/bgp_bmp_vrf/test_bgp_bmp_vrf.py new file mode 100644 index 000000000000..b683920d2e70 --- /dev/null +++ b/tests/topotests/bgp_bmp_vrf/test_bgp_bmp_vrf.py @@ -0,0 +1,326 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright 2023 6WIND S.A. +# Authored by Farid Mihoub +# + +""" +test_bgp_bmp.py: Test BGP BMP functionalities + + +------+ +------+ +------+ + | | | | | | + | BMP1 |------------| R1 |---------------| R2 | + | | | | | | + +------+ +------+ +------+ + +Setup two routers R1 and R2 with one link configured with IPv4 and +IPv6 addresses. +Configure BGP in R1 and R2 to exchange prefixes from +the latter to the first router. +Setup a link between R1 and the BMP server, activate the BMP feature in R1 +and ensure the monitored BGP sessions logs are well present on the BMP server. +""" + +from functools import partial +from ipaddress import ip_network +import json +import os +import platform +import pytest +import sys + +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join("../")) +sys.path.append(os.path.join("../lib/")) + +# pylint: disable=C0413 +# Import topogen and topotest helpers +from lib import topotest +from lib.bgp import verify_bgp_convergence_from_running_config +from lib.topogen import Topogen, TopoRouter, get_topogen +from lib.topolog import logger + +pytestmark = [pytest.mark.bgpd] + +# remember the last sequence number of the logging messages +SEQ = 0 + +PRE_POLICY = "pre-policy" +POST_POLICY = "post-policy" +LOC_RIB = "loc-rib" + + +def build_topo(tgen): + tgen.add_router("r1") + tgen.add_router("r2") + tgen.add_bmp_server("bmp1", ip="192.0.2.10", defaultRoute="via 192.0.2.1") + + switch = tgen.add_switch("s1") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["bmp1"]) + + tgen.add_link(tgen.gears["r1"], tgen.gears["r2"], "r1-eth1", "r2-eth0") + + +def setup_module(mod): + tgen = Topogen(build_topo, mod.__name__) + tgen.start_topology() + + tgen.net["r1"].cmd( + """ +ip link add vrf1 type vrf table 10 +ip link set vrf1 up +ip link set r1-eth1 master vrf1 +""" + ) + + for rname, router in tgen.routers().items(): + router.load_config( + TopoRouter.RD_ZEBRA, os.path.join(CWD, "{}/zebra.conf".format(rname)) + ) + router.load_config( + TopoRouter.RD_BGP, + os.path.join(CWD, "{}/bgpd.conf".format(rname)), + "-M bmp", + ) + + tgen.start_router() + + logger.info("starting BMP servers") + for bmp_name, server in tgen.get_bmp_servers().items(): + server.start(log_file=os.path.join(tgen.logdir, bmp_name, "bmp.log")) + + +def teardown_module(_mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_convergence(): + tgen = get_topogen() + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + result = verify_bgp_convergence_from_running_config(tgen, dut="r1") + assert result is True, "BGP is not converging" + + +def get_bmp_messages(): + """ + Read the BMP logging messages. + """ + messages = [] + tgen = get_topogen() + text_output = tgen.gears["bmp1"].run( + "cat {}".format(os.path.join(tgen.logdir, "bmp1", "bmp.log")) + ) + + for m in text_output.splitlines(): + # some output in the bash can break the message decoding + try: + messages.append(json.loads(m)) + except Exception as e: + logger.warning(str(e) + " message: {}".format(str(m))) + continue + + if not messages: + logger.error("Bad BMP log format, check your BMP server") + + return messages + + +def check_for_prefixes(expected_prefixes, bmp_log_type, policy, labels=None): + """ + Check for the presence of the given prefixes in the BMP server logs with + the given message type and the set policy. + """ + global SEQ + # we care only about the new messages + messages = [ + m for m in sorted(get_bmp_messages(), key=lambda d: d["seq"]) if m["seq"] > SEQ + ] + + # get the list of pairs (prefix, policy, seq) for the given message type + prefixes = [ + m["ip_prefix"] + for m in messages + if "ip_prefix" in m.keys() + and "bmp_log_type" in m.keys() + and m["bmp_log_type"] == bmp_log_type + and m["policy"] == policy + and ( + labels is None + or ( + m["ip_prefix"] in labels.keys() and m["label"] == labels[m["ip_prefix"]] + ) + ) + ] + + # check for prefixes + for ep in expected_prefixes: + if ep not in prefixes: + msg = "The prefix {} is not present in the {} log messages." + logger.debug(msg.format(ep, bmp_log_type)) + return False + + SEQ = messages[-1]["seq"] + return True + + +def check_for_peer_message(expected_peers, bmp_log_type): + """ + Check for the presence of a peer up message for the peer + """ + global SEQ + # we care only about the new messages + messages = [ + m for m in sorted(get_bmp_messages(), key=lambda d: d["seq"]) if m["seq"] > SEQ + ] + + # get the list of pairs (prefix, policy, seq) for the given message type + peers = [ + m["peer_ip"] + for m in messages + if "peer_ip" in m.keys() and m["bmp_log_type"] == bmp_log_type + ] + + # check for prefixes + for ep in expected_peers: + if ep not in peers: + msg = "The peer {} is not present in the {} log messages." + logger.debug(msg.format(ep, bmp_log_type)) + return False + + SEQ = messages[-1]["seq"] + return True + + +def set_bmp_policy(tgen, node, asn, target, safi, policy, vrf=None): + """ + Configure the bmp policy. + """ + vrf = " vrf {}".format(vrf) if vrf else "" + cmd = [ + "con t\n", + "router bgp {}{}\n".format(asn, vrf), + "bmp targets {}\n".format(target), + "bmp monitor ipv4 {} {}\n".format(safi, policy), + "bmp monitor ipv6 {} {}\n".format(safi, policy), + "end\n", + ] + tgen.gears[node].vtysh_cmd("".join(cmd)) + + +def configure_prefixes(tgen, node, asn, safi, prefixes, vrf=None, update=True): + """ + Configure the bgp prefixes. + """ + withdraw = "no " if not update else "" + vrf = " vrf {}".format(vrf) if vrf else "" + for p in prefixes: + ip = ip_network(p) + cmd = [ + "conf t\n", + "router bgp {}{}\n".format(asn, vrf), + "address-family ipv{} {}\n".format(ip.version, safi), + "{}network {}\n".format(withdraw, ip), + "exit-address-family\n", + ] + logger.debug("setting prefix: ipv{} {} {}".format(ip.version, safi, ip)) + tgen.gears[node].vtysh_cmd("".join(cmd)) + + +def unicast_prefixes(policy): + """ + Setup the BMP monitor policy, Add and withdraw ipv4/v6 prefixes. + Check if the previous actions are logged in the BMP server with the right + message type and the right policy. + """ + tgen = get_topogen() + set_bmp_policy(tgen, "r1", 65501, "bmp1", "unicast", policy, vrf="vrf1") + + prefixes = ["172.31.0.15/32", "2111::1111/128"] + # add prefixes + configure_prefixes(tgen, "r2", 65502, "unicast", prefixes) + + logger.info("checking for updated prefixes") + # check + test_func = partial(check_for_prefixes, prefixes, "update", policy) + success, _ = topotest.run_and_expect(test_func, True, wait=0.5) + assert success, "Checking the updated prefixes has been failed !." + + # withdraw prefixes + configure_prefixes(tgen, "r2", 65502, "unicast", prefixes, update=False) + logger.info("checking for withdrawed prefxies") + # check + test_func = partial(check_for_prefixes, prefixes, "withdraw", policy) + success, _ = topotest.run_and_expect(test_func, True, wait=0.5) + assert success, "Checking the withdrawed prefixes has been failed !." + + +def test_bmp_server_logging(): + """ + Assert the logging of the bmp server. + """ + + def check_for_log_file(): + tgen = get_topogen() + output = tgen.gears["bmp1"].run( + "ls {}".format(os.path.join(tgen.logdir, "bmp1")) + ) + if "bmp.log" not in output: + return False + return True + + success, _ = topotest.run_and_expect(check_for_log_file, True, wait=0.5) + assert success, "The BMP server is not logging" + + +def test_peer_up(): + """ + Checking for BMP peers up messages + """ + + peers = ["192.168.0.2", "192:168::2"] + + logger.info("checking for BMP peers up messages") + + test_func = partial(check_for_peer_message, peers, "peer up") + success, _ = topotest.run_and_expect(test_func, True, wait=0.5) + assert success, "Checking the updated prefixes has been failed !." + + +def test_bmp_bgp_unicast(): + """ + Add/withdraw bgp unicast prefixes and check the bmp logs. + """ + logger.info("*** Unicast prefixes pre-policy logging ***") + unicast_prefixes(PRE_POLICY) + logger.info("*** Unicast prefixes post-policy logging ***") + unicast_prefixes(POST_POLICY) + logger.info("*** Unicast prefixes loc-rib logging ***") + unicast_prefixes(LOC_RIB) + + +def test_peer_down(): + """ + Checking for BMP peers down messages + """ + tgen = get_topogen() + + tgen.gears["r2"].vtysh_cmd("clear bgp *") + + peers = ["192.168.0.2", "192:168::2"] + + logger.info("checking for BMP peers down messages") + + test_func = partial(check_for_peer_message, peers, "peer down") + success, _ = topotest.run_and_expect(test_func, True, wait=0.5) + assert success, "Checking the updated prefixes has been failed !." + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From 963792e8c5ba834f1f7bb2555e90aa4b2ff6f33c Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 10 Oct 2024 16:00:08 -0400 Subject: [PATCH 16/46] zebra: Only notify dplane work pthread when needed The fpm_nl_process function was getting the count of the total number of ctx's processed. This leads to after having processed 1 context to always signal the dataplane that there is work to do. Change the code to only notify the dplane worker when a context was actually added to the outgoing context queue. Signed-off-by: Donald Sharp --- zebra/dplane_fpm_nl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c index d594fc2c8640..0e54952eea2c 100644 --- a/zebra/dplane_fpm_nl.c +++ b/zebra/dplane_fpm_nl.c @@ -1525,7 +1525,7 @@ static void fpm_process_queue(struct event *t) * until the dataplane thread gets scheduled for new, * unrelated work. */ - if (dplane_provider_out_ctx_queue_len(fnc->prov) > 0) + if (processed_contexts) dplane_provider_work_ready(); } From 8aa97a439fc21c66132fdaf8ce0113e16801be04 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Thu, 10 Oct 2024 20:08:32 -0400 Subject: [PATCH 17/46] zebra: Slow down fpm_process_queue When the fpm_process_queue has run out of space but has written to the fpm output buffer, schedule it to wake up immediately, as that the write will go out pretty much immediately, since it was scheduled first. If the fpm_process_queue has not written to the output buffer then delay the processing by 10 milliseconds to allow a possibly backed up write processing to have a chance to complete it's work. Signed-off-by: Donald Sharp --- zebra/dplane_fpm_nl.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/zebra/dplane_fpm_nl.c b/zebra/dplane_fpm_nl.c index 0e54952eea2c..4fb57d84d98f 100644 --- a/zebra/dplane_fpm_nl.c +++ b/zebra/dplane_fpm_nl.c @@ -1512,8 +1512,12 @@ static void fpm_process_queue(struct event *t) /* Re-schedule if we ran out of buffer space */ if (no_bufs) { - event_add_event(fnc->fthread->master, fpm_process_queue, fnc, 0, - &fnc->t_dequeue); + if (processed_contexts) + event_add_event(fnc->fthread->master, fpm_process_queue, fnc, 0, + &fnc->t_dequeue); + else + event_add_timer_msec(fnc->fthread->master, fpm_process_queue, fnc, 10, + &fnc->t_dequeue); event_add_timer(fnc->fthread->master, fpm_process_wedged, fnc, DPLANE_FPM_NL_WEDGIE_TIME, &fnc->t_wedged); } else From cf2624a993fd6992fbbce75434a5aefe22ce0bc2 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Fri, 11 Oct 2024 09:33:35 -0400 Subject: [PATCH 18/46] fpm: Allow max fpm message size to float based on ecmp Currently the max message size is 4k. With a 256 way ecmp FRR is seeing message sizes that are in the 6k size. There is desire to allow this to increase as well to 512. Since the multipath size directly effects how big the message may be when sending the routes ecmp let's give a bit of headroom for this value when compiling FRR at greater sizes. Additionally since we know not everyone is using such large ecmp, allow them to build as appropriate for their use cases. Signed-off-by: Donald Sharp --- fpm/fpm.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fpm/fpm.h b/fpm/fpm.h index 70c0df57157e..9003c643b00c 100644 --- a/fpm/fpm.h +++ b/fpm/fpm.h @@ -65,7 +65,7 @@ /* * Largest message that can be sent to or received from the FPM. */ -#define FPM_MAX_MSG_LEN 4096 +#define FPM_MAX_MSG_LEN MAX(MULTIPATH_NUM * 32, 4096) #ifdef __SUNPRO_C #pragma pack(1) From 05e2472de7f5bca8686cb38be042949a51d0c6a4 Mon Sep 17 00:00:00 2001 From: anlan_cs Date: Sun, 13 Oct 2024 21:26:02 +0800 Subject: [PATCH 19/46] zebra: add back one field for debug The `flags` field is removed recently, so add back it for debug. Signed-off-by: anlan_cs --- zebra/interface.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/zebra/interface.c b/zebra/interface.c index d146004781a5..f1f1b17209a7 100644 --- a/zebra/interface.c +++ b/zebra/interface.c @@ -1999,10 +1999,9 @@ static void zebra_if_dplane_ifp_handling(struct zebra_dplane_ctx *ctx) !CHECK_FLAG(ifp->status, ZEBRA_INTERFACE_ACTIVE)) { /* Add interface notification from kernel */ if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug( - "RTM_NEWLINK ADD for %s(%u) vrf_id %u type %d sl_type %d master %u", - name, ifindex, vrf_id, zif_type, - zif_slave_type, master_ifindex); + zlog_debug("RTM_NEWLINK ADD for %s(%u) vrf_id %u type %d sl_type %d master %u flags 0x%llx", + name, ifindex, vrf_id, zif_type, zif_slave_type, + master_ifindex, (unsigned long long)flags); if (ifp == NULL) { /* unknown interface */ @@ -2087,10 +2086,9 @@ static void zebra_if_dplane_ifp_handling(struct zebra_dplane_ctx *ctx) /* Interface update. */ if (IS_ZEBRA_DEBUG_KERNEL) - zlog_debug( - "RTM_NEWLINK update for %s(%u) sl_type %d master %u", - name, ifp->ifindex, zif_slave_type, - master_ifindex); + zlog_debug("RTM_NEWLINK update for %s(%u) sl_type %d master %u flags 0x%llx", + name, ifp->ifindex, zif_slave_type, master_ifindex, + (unsigned long long)flags); set_ifindex(ifp, ifindex, zns); ifp->mtu6 = ifp->mtu = mtu; From a880bfaab28e837a54de5bd88dad1a755aa2e985 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Sun, 13 Oct 2024 19:54:08 +0300 Subject: [PATCH 20/46] isisd: Remove circuit state check for openfabric If we have something like: ``` int eth1 ip router openfabric x ipv6 router openfabric x ``` And eth1 is removed, the first `ip router ...` fails and only `ipv6 router ...` is enabled. If we leave only: ``` int eth1 ipv6 router openfabric x ``` Then also, no interface is going to be enabled, which is weird too. Fixes: https://github.com/FRRouting/frr/issues/17075 Signed-off-by: Donatas Abraitis --- isisd/isis_vty_fabricd.c | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/isisd/isis_vty_fabricd.c b/isisd/isis_vty_fabricd.c index 0d25f6610934..85ef2c40a10a 100644 --- a/isisd/isis_vty_fabricd.c +++ b/isisd/isis_vty_fabricd.c @@ -218,17 +218,9 @@ DEFUN (ip_router_isis, if (!area) isis_area_create(area_tag, VRF_DEFAULT_NAME); - if (!circuit) { + if (!circuit) circuit = isis_circuit_new(ifp, area_tag); - if (circuit->state != C_STATE_CONF - && circuit->state != C_STATE_UP) { - vty_out(vty, - "Couldn't bring up interface, please check log.\n"); - return CMD_WARNING_CONFIG_FAILED; - } - } - bool ip = circuit->ip_router, ipv6 = circuit->ipv6_router; if (af[2] != '\0') ipv6 = true; From 4ae65cc21eccd6d324949d33d36a328ff6b45cfe Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 11 Oct 2024 06:57:14 +0200 Subject: [PATCH 21/46] bgpd: rename reflect in subgroup_announce_check In subgroup_announce_check(), the variable reflect is misleading, as it suggests a relation to route reflection. However, it actually refers to the scenario where an iBGP peer announces a route to another iBGP peer. Rename reflect to ibgp_to_ibgp. Signed-off-by: Louis Scalbert --- bgpd/bgp_route.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 9cefec0706ee..d69c263196c3 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2148,7 +2148,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, struct attr *piattr; route_map_result_t ret; int transparent; - int reflect; + int ibgp_to_ibgp; afi_t afi; safi_t safi; int samepeer_safe = 0; /* for synthetic mplsvpns routes */ @@ -2357,14 +2357,14 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, } } - /* Route-Reflect check. */ + /* iBGP to iBGP check. */ if (from->sort == BGP_PEER_IBGP && peer->sort == BGP_PEER_IBGP) - reflect = 1; + ibgp_to_ibgp = 1; else - reflect = 0; + ibgp_to_ibgp = 0; /* IBGP reflection check. */ - if (reflect && !samepeer_safe) { + if (ibgp_to_ibgp && !samepeer_safe) { /* A route from a Client peer. */ if (CHECK_FLAG(from->af_flags[afi][safi], PEER_FLAG_REFLECTOR_CLIENT)) { @@ -2410,8 +2410,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, /* If originator-id is not set and the route is to be reflected, set the originator id */ - if (reflect - && (!(attr->flag & ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID)))) { + if (ibgp_to_ibgp && (!(attr->flag & ATTR_FLAG_BIT(BGP_ATTR_ORIGINATOR_ID)))) { IPV4_ADDR_COPY(&(attr->originator_id), &(from->remote_id)); SET_FLAG(attr->flag, BGP_ATTR_ORIGINATOR_ID); } @@ -2444,7 +2443,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, * announced to an EBGP peer (and they have the same attributes barring * their nexthop). */ - if (reflect) + if (ibgp_to_ibgp) SET_FLAG(attr->rmap_change_flags, BATTR_REFLECTED); #define NEXTHOP_IS_V6 \ @@ -2472,7 +2471,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, */ if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_local)) global_and_ll = true; - } else if (!reflect && !transparent && + } else if (!ibgp_to_ibgp && !transparent && IN6_IS_ADDR_LINKLOCAL(&peer->nexthop.v6_local) && peer->shared_network && (from == bgp->peer_self || peer->sort == BGP_PEER_EBGP)) global_and_ll = true; @@ -2694,9 +2693,8 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, PEER_FLAG_NEXTHOP_SELF) || CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_FORCE_NEXTHOP_SELF)) { - if (!reflect - || CHECK_FLAG(peer->af_flags[afi][safi], - PEER_FLAG_FORCE_NEXTHOP_SELF)) { + if (!ibgp_to_ibgp || + CHECK_FLAG(peer->af_flags[afi][safi], PEER_FLAG_FORCE_NEXTHOP_SELF)) { subgroup_announce_reset_nhop( (peer_cap_enhe(peer, afi, safi) ? AF_INET6 From c4a82636282c059a32ff9b19396c547c46cbf82b Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Fri, 11 Oct 2024 06:59:16 +0200 Subject: [PATCH 22/46] bgpd, tests: don't send local nexthop from rr client AS 65000 | AS 65001 | RR | | | R1 --- | --- R2 | When r1 peer is an iBGP route reflector client of rr and r2 peer is a eBGP neighbor of rr, and all three routers shares the same network, r2 receives announcements coming from r1 with a IPv6 link-local nexthop from rr. This is incorrect as r2 should send traffic to r1 without involving rr. Do not send an IPv6 link-local nexthop if the originating peer is a route-reflector client. Link: https://github.com/FRRouting/frr/pull/16219#issuecomment-2397425505 Link: https://github.com/FRRouting/frr/pull/17037#discussion_r1792529683 Signed-off-by: Louis Scalbert --- bgpd/bgp_route.c | 1 + .../r4/show_bgp_ipv6_step2.json | 48 +++---------------- .../r5/show_bgp_ipv6_step2.json | 48 +++---------------- 3 files changed, 13 insertions(+), 84 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index d69c263196c3..5fd058b6b597 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -2472,6 +2472,7 @@ bool subgroup_announce_check(struct bgp_dest *dest, struct bgp_path_info *pi, if (IN6_IS_ADDR_LINKLOCAL(&attr->mp_nexthop_local)) global_and_ll = true; } else if (!ibgp_to_ibgp && !transparent && + !CHECK_FLAG(from->af_flags[afi][safi], PEER_FLAG_REFLECTOR_CLIENT) && IN6_IS_ADDR_LINKLOCAL(&peer->nexthop.v6_local) && peer->shared_network && (from == bgp->peer_self || peer->sort == BGP_PEER_EBGP)) global_and_ll = true; diff --git a/tests/topotests/bgp_nexthop_ipv6/r4/show_bgp_ipv6_step2.json b/tests/topotests/bgp_nexthop_ipv6/r4/show_bgp_ipv6_step2.json index 35a31e63f921..5506f07f29c6 100644 --- a/tests/topotests/bgp_nexthop_ipv6/r4/show_bgp_ipv6_step2.json +++ b/tests/topotests/bgp_nexthop_ipv6/r4/show_bgp_ipv6_step2.json @@ -9,13 +9,7 @@ "ip": "fd00:0:2::1", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-sw", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -28,13 +22,7 @@ "ip": "fd00:0:2::2", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-sw", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -47,13 +35,7 @@ "ip": "fd00:0:2::3", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-sw", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -117,13 +99,7 @@ "ip": "fd00:0:2::1", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-sw", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -136,13 +112,7 @@ "ip": "fd00:0:2::2", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-sw", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -155,13 +125,7 @@ "ip": "fd00:0:2::3", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-sw", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] diff --git a/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step2.json b/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step2.json index d0875474ae94..afcf7c3ffc5a 100644 --- a/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step2.json +++ b/tests/topotests/bgp_nexthop_ipv6/r5/show_bgp_ipv6_step2.json @@ -9,13 +9,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -28,13 +22,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -47,13 +35,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -117,13 +99,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -136,13 +112,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] @@ -155,13 +125,7 @@ "ip": "fd00:0:3::9", "hostname": "rr", "afi": "ipv6", - "scope": "global" - }, - { - "ip": "link-local:rr:eth-r5", - "hostname": "rr", - "afi": "ipv6", - "scope": "link-local", + "scope": "global", "used": true } ] From 89c6422585e93f11efebde088cf4d1b5d542ebe6 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Mon, 14 Oct 2024 14:36:54 +0200 Subject: [PATCH 23/46] doc: clarify bgp as-override Clarify bgp as-override Signed-off-by: Louis Scalbert --- doc/user/bgp.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/user/bgp.rst b/doc/user/bgp.rst index 1e9b4f9d2764..6a46128d72d2 100644 --- a/doc/user/bgp.rst +++ b/doc/user/bgp.rst @@ -1850,7 +1850,8 @@ Configuring Peers .. clicmd:: neighbor as-override - Override AS number of the originating router with the local AS number. + Override any AS number in the AS path that matches the neighbor's AS number + with the local AS number. Usually this configuration is used in PEs (Provider Edge) to replace the incoming customer AS number so the connected CE (Customer Edge) From 2ac4c3e58db8a8b8d76588d6b3389b8d75eda830 Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Mon, 14 Oct 2024 14:39:00 +0200 Subject: [PATCH 24/46] tests: fix bgp_as_override number of routers There is only 4 routers not 6. Signed-off-by: Louis Scalbert --- tests/topotests/bgp_as_override/test_bgp_as_override.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/topotests/bgp_as_override/test_bgp_as_override.py b/tests/topotests/bgp_as_override/test_bgp_as_override.py index dbbdf2c88f8e..ab22b4936f7a 100644 --- a/tests/topotests/bgp_as_override/test_bgp_as_override.py +++ b/tests/topotests/bgp_as_override/test_bgp_as_override.py @@ -27,7 +27,7 @@ def build_topo(tgen): - for routern in range(1, 7): + for routern in range(1, 5): tgen.add_router("r{}".format(routern)) switch = tgen.add_switch("s1") From 74e25198e74c4066b630515d2971c3c9381d70f2 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Mon, 14 Oct 2024 11:25:32 -0400 Subject: [PATCH 25/46] zebra: Prevent a kernel route from being there when a connected should There exists a series of events where a kernel route is learned first( that happens to be exactly what a connected route should be ) and FRR ends up with both a kernel route and a connected route, leaving us in a very strange spot. This code change just mirrors the existing code of if there is a connected route drop the kernel route. Here we just do the reverse, if we have a kernel route already and a connected should be created, remove the kernel and keep the connected. Signed-off-by: Donald Sharp --- zebra/connected.c | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/zebra/connected.c b/zebra/connected.c index 974a1c17a292..ce26c14c0525 100644 --- a/zebra/connected.c +++ b/zebra/connected.c @@ -176,6 +176,40 @@ static void connected_update(struct interface *ifp, struct connected *ifc) connected_announce(ifp, ifc); } +/* + * This function goes through and handles the deletion of a kernel route that happened + * to be the exact same as the connected route, so that the connected route wins. + * This can happen during processing if we happen to receive events in a slightly + * unexpected order. This is similiar to code in the other direction where if we + * have a kernel route don't install it if it perfectly matches a connected route. + */ +static void connected_remove_kernel_for_connected(afi_t afi, safi_t safi, struct zebra_vrf *zvrf, + struct prefix *p, struct nexthop *nh) +{ + struct route_node *rn; + struct route_entry *re; + rib_dest_t *dest; + struct route_table *table = zebra_vrf_table(afi, SAFI_UNICAST, zvrf->vrf->vrf_id); + + rn = route_node_match(table, p); + if (!rn) + return; + + if (!prefix_same(&rn->p, p)) + return; + + dest = rib_dest_from_rnode(rn); + if (!dest || !dest->selected_fib) + return; + + re = dest->selected_fib; + if (re->type != ZEBRA_ROUTE_KERNEL) + return; + + rib_delete(afi, SAFI_UNICAST, zvrf->vrf->vrf_id, ZEBRA_ROUTE_KERNEL, 0, 0, p, NULL, nh, 0, + zvrf->table_id, 0, 0, false); +} + /* Called from if_up(). */ void connected_up(struct interface *ifp, struct connected *ifc) { @@ -284,10 +318,13 @@ void connected_up(struct interface *ifp, struct connected *ifc) } if (!CHECK_FLAG(ifc->flags, ZEBRA_IFA_NOPREFIXROUTE)) { + connected_remove_kernel_for_connected(afi, SAFI_UNICAST, zvrf, &p, &nh); + rib_add(afi, SAFI_UNICAST, zvrf->vrf->vrf_id, ZEBRA_ROUTE_CONNECT, 0, flags, &p, NULL, &nh, 0, zvrf->table_id, metric, 0, 0, 0, false); + connected_remove_kernel_for_connected(afi, SAFI_MULTICAST, zvrf, &p, &nh); rib_add(afi, SAFI_MULTICAST, zvrf->vrf->vrf_id, ZEBRA_ROUTE_CONNECT, 0, flags, &p, NULL, &nh, 0, zvrf->table_id, metric, 0, 0, 0, false); From 3f00709a39ab2fe6f4bbae3d8f4b17baaab2e3dd Mon Sep 17 00:00:00 2001 From: Chirag Shah Date: Mon, 7 Oct 2024 19:04:43 -0700 Subject: [PATCH 26/46] bgpd: fix evpn mh esi flap remove local routes In symmetric routing, when local ESI is down, the MH peer learnt local mac-ip prefix is installed into teannt vrf (given l3vni). When ESI is back up and associated to evi/vni then remove the local synced mac-ip imported routes from the tenant vrf as local neigh/arp is present. Ticket: #3878699 Testing: peer advertised mac-ip route: *> [2]:[0]:[48]:[aa:aa:aa:00:00:01]:[32]:[45.0.0.51] RD 27.0.0.4:9 27.0.0.4 (spine-1) 0 64435 65016 i ESI:03:44:38:39:ff:ff:01:00:00:01 RT:65016:1000 RT:65016:4000 ET:8 Rmac:44:38:39:ff:ff:16 When local ESI is flapped torm-11:# ip neigh show 45.0.0.51 45.0.0.51 dev vlan1000 lladdr aa:aa:aa:00:00:01 REACHABLE proto zebra Before fix: (The imported route remained in tenant-vrf) torm-11:# ip route show vrf vrf1 45.0.0.51 45.0.0.51 nhid 257 proto bgp metric 20 After fix: torm-11# ip route show vrf vrf1 45.0.0.51 torm-11# trace: 2024/10/11 18:19:29 BGP: [JMP3T-178G8] route [2]:[0]:[48]:[00:02:00:00:00:08]:[32]:[21.1.0.5] is matched on local esi 03:00:00:00:77:01:04:00:00:0e, uninstall from VRF tenant1 route table Signed-off-by: Chirag Shah --- bgpd/bgp_evpn.c | 11 ++++------- bgpd/bgp_evpn.h | 4 ++++ bgpd/bgp_evpn_mh.c | 36 ++++++++++++++++++++++++++++++++++++ bgpd/bgp_evpn_mh.h | 2 ++ 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/bgpd/bgp_evpn.c b/bgpd/bgp_evpn.c index fb7d2f47fb79..0a8ce6154855 100644 --- a/bgpd/bgp_evpn.c +++ b/bgpd/bgp_evpn.c @@ -3477,9 +3477,8 @@ uninstall_evpn_route_entry_in_vni_mac(struct bgp *bgp, struct bgpevpn *vpn, * Uninstall route entry from the VRF routing table and send message * to zebra, if appropriate. */ -static int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf, - const struct prefix_evpn *evp, - struct bgp_path_info *parent_pi) +int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf, const struct prefix_evpn *evp, + struct bgp_path_info *parent_pi) { struct bgp_dest *dest; struct bgp_path_info *pi; @@ -3846,10 +3845,8 @@ static int bgp_evpn_route_rmac_self_check(struct bgp *bgp_vrf, } /* don't import hosts that are locally attached */ -static inline bool -bgp_evpn_skip_vrf_import_of_local_es(struct bgp *bgp_vrf, - const struct prefix_evpn *evp, - struct bgp_path_info *pi, int install) +bool bgp_evpn_skip_vrf_import_of_local_es(struct bgp *bgp_vrf, const struct prefix_evpn *evp, + struct bgp_path_info *pi, int install) { esi_t *esi; diff --git a/bgpd/bgp_evpn.h b/bgpd/bgp_evpn.h index dc82bcfba9e9..10eff1dcfb2f 100644 --- a/bgpd/bgp_evpn.h +++ b/bgpd/bgp_evpn.h @@ -195,4 +195,8 @@ extern enum zclient_send_status evpn_zebra_uninstall(struct bgp *bgp, struct bgpevpn *vpn, const struct prefix_evpn *p, struct bgp_path_info *pi, bool is_sync); +bool bgp_evpn_skip_vrf_import_of_local_es(struct bgp *bgp_vrf, const struct prefix_evpn *evp, + struct bgp_path_info *pi, int install); +int uninstall_evpn_route_entry_in_vrf(struct bgp *bgp_vrf, const struct prefix_evpn *evp, + struct bgp_path_info *parent_pi); #endif /* _QUAGGA_BGP_EVPN_H */ diff --git a/bgpd/bgp_evpn_mh.c b/bgpd/bgp_evpn_mh.c index 1fb65945572a..ad3625242ee1 100644 --- a/bgpd/bgp_evpn_mh.c +++ b/bgpd/bgp_evpn_mh.c @@ -3807,6 +3807,7 @@ int bgp_evpn_local_es_evi_add(struct bgp *bgp, esi_t *esi, vni_t vni) bgp_evpn_ead_evi_route_update(bgp, es, vpn, &p); } + bgp_evpn_local_es_evi_unistall_local_routes_in_vrfs(es, es_evi); /* update EAD-ES */ if (bgp_evpn_local_es_is_active(es)) bgp_evpn_ead_es_route_update(bgp, es); @@ -5058,3 +5059,38 @@ void bgp_evpn_switch_ead_evi_rx(void) } } } + +void bgp_evpn_local_es_evi_unistall_local_routes_in_vrfs(struct bgp_evpn_es *es, + struct bgp_evpn_es_evi *es_evi) +{ + struct listnode *node; + struct bgp_path_es_info *es_info; + struct bgp_path_info *pi; + const struct prefix_evpn *evp; + struct bgp_evpn_es_vrf *es_vrf = es_evi->es_vrf; + + if (!es_vrf) + return; + + for (ALL_LIST_ELEMENTS_RO(es->macip_global_path_list, node, es_info)) { + pi = es_info->pi; + + if (!bgp_evpn_is_macip_path(pi)) + continue; + + evp = (const struct prefix_evpn *)bgp_dest_get_prefix(pi->net); + + if (!(CHECK_FLAG(pi->flags, BGP_PATH_VALID) && pi->type == ZEBRA_ROUTE_BGP && + pi->sub_type == BGP_ROUTE_NORMAL)) + continue; + + if (BGP_DEBUG(evpn_mh, EVPN_MH_RT)) + zlog_debug("route %pFX is matched on local esi %s, uninstall from %s route table", + evp, es->esi_str, es_vrf->bgp_vrf->name_pretty); + + if (!bgp_evpn_skip_vrf_import_of_local_es(es_vrf->bgp_vrf, evp, pi, 0)) + continue; + + uninstall_evpn_route_entry_in_vrf(es_vrf->bgp_vrf, evp, pi); + } +} diff --git a/bgpd/bgp_evpn_mh.h b/bgpd/bgp_evpn_mh.h index 5d393c37a20d..149bf384b1d8 100644 --- a/bgpd/bgp_evpn_mh.h +++ b/bgpd/bgp_evpn_mh.h @@ -459,5 +459,7 @@ extern void bgp_evpn_path_nh_add(struct bgp *bgp_vrf, struct bgp_path_info *pi); extern void bgp_evpn_path_nh_del(struct bgp *bgp_vrf, struct bgp_path_info *pi); extern void bgp_evpn_mh_config_ead_export_rt(struct bgp *bgp, struct ecommunity *ecom, bool del); +extern void bgp_evpn_local_es_evi_unistall_local_routes_in_vrfs(struct bgp_evpn_es *es, + struct bgp_evpn_es_evi *es_evi); #endif /* _FRR_BGP_EVPN_MH_H */ From 081422e8e71085d3a3d4d2ff0bc1e1abaff0d52e Mon Sep 17 00:00:00 2001 From: Enke Chen Date: Mon, 14 Oct 2024 18:42:15 -0700 Subject: [PATCH 27/46] bgpd: fix route selection with AIGP The nexthop metric should be added to AIGP when calculating the bestpath in bgp_path_info_cmp(). Signed-off-by: Enke Chen --- bgpd/bgp_attr.c | 10 ---------- bgpd/bgp_attr.h | 10 ++++++++++ bgpd/bgp_route.c | 4 ++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 7cdf98cba706..7aa9bbbb91b8 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -480,16 +480,6 @@ static bool bgp_attr_aigp_get_tlv_metric(uint8_t *pnt, int length, return false; } -static uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi) -{ - uint64_t aigp = bgp_attr_get_aigp_metric(bpi->attr); - - if (bpi->nexthop) - return aigp + bpi->nexthop->metric; - else - return aigp; -} - static void stream_put_bgp_aigp_tlv_metric(struct stream *s, struct bgp_path_info *bpi) { diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index a8ca8c1fa638..02ddf88b6242 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -598,6 +598,16 @@ static inline void bgp_attr_set_aigp_metric(struct attr *attr, uint64_t aigp) SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)); } +static inline uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi) +{ + uint64_t aigp = bgp_attr_get_aigp_metric(bpi->attr); + + if (bpi->nexthop) + return aigp + bpi->nexthop->metric; + else + return aigp; +} + static inline struct cluster_list *bgp_attr_get_cluster(const struct attr *attr) { return attr->cluster1; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 9cefec0706ee..0f196157c80c 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1068,8 +1068,8 @@ int bgp_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, if (CHECK_FLAG(newattr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && CHECK_FLAG(existattr->flag, ATTR_FLAG_BIT(BGP_ATTR_AIGP)) && CHECK_FLAG(bgp->flags, BGP_FLAG_COMPARE_AIGP)) { - uint64_t new_aigp = bgp_attr_get_aigp_metric(newattr); - uint64_t exist_aigp = bgp_attr_get_aigp_metric(existattr); + uint64_t new_aigp = bgp_aigp_metric_total(new); + uint64_t exist_aigp = bgp_aigp_metric_total(exist); if (new_aigp < exist_aigp) { *reason = bgp_path_selection_aigp; From 1ee7e63a6c432662ef3a9a2bd0e1c41298bdf196 Mon Sep 17 00:00:00 2001 From: Enke Chen Date: Mon, 14 Oct 2024 18:47:59 -0700 Subject: [PATCH 28/46] tests: fix and adjust topotest/bgp_aigp Fix and adjust the topotest post the fix for route selection with AIGP. When there are multiple IGP domains (OSPF in this case), the nexthop for a BGP route with the AIGP attribute must be resolved in its own IGP domain. The changes in r2/bgpd.conf and r3/bgpd.conf are needed as incorrect IGP metrics are received from NHT for the recursive nexthops. Once the issue is resolved, the changes can be reverted. Signed-off-by: Enke Chen --- tests/topotests/bgp_aigp/r1/ospfd.conf | 2 +- tests/topotests/bgp_aigp/r2/bgpd.conf | 2 + tests/topotests/bgp_aigp/r2/ospfd.conf | 2 +- tests/topotests/bgp_aigp/r3/bgpd.conf | 2 + tests/topotests/bgp_aigp/r3/ospfd.conf | 2 +- tests/topotests/bgp_aigp/r4/bgpd.conf | 16 +++- tests/topotests/bgp_aigp/r4/ospfd.conf | 2 +- tests/topotests/bgp_aigp/r5/bgpd.conf | 16 +++- tests/topotests/bgp_aigp/r5/ospfd.conf | 2 +- tests/topotests/bgp_aigp/r6/bgpd.conf | 8 +- tests/topotests/bgp_aigp/r6/ospfd.conf | 2 +- tests/topotests/bgp_aigp/test_bgp_aigp.py | 90 +++++++++++++++++------ 12 files changed, 111 insertions(+), 35 deletions(-) diff --git a/tests/topotests/bgp_aigp/r1/ospfd.conf b/tests/topotests/bgp_aigp/r1/ospfd.conf index 38aa11d0362a..098bf57b0365 100644 --- a/tests/topotests/bgp_aigp/r1/ospfd.conf +++ b/tests/topotests/bgp_aigp/r1/ospfd.conf @@ -1,6 +1,6 @@ ! interface lo - ip ospf cost 10 + ip ospf passive ! interface r1-eth0 ip ospf dead-interval 4 diff --git a/tests/topotests/bgp_aigp/r2/bgpd.conf b/tests/topotests/bgp_aigp/r2/bgpd.conf index 4db468753642..4539016f916b 100644 --- a/tests/topotests/bgp_aigp/r2/bgpd.conf +++ b/tests/topotests/bgp_aigp/r2/bgpd.conf @@ -1,6 +1,7 @@ router bgp 65001 no bgp ebgp-requires-policy no bgp network import-check + bgp route-reflector allow-outbound-policy neighbor 10.0.0.1 remote-as internal neighbor 10.0.0.1 timers 1 3 neighbor 10.0.0.1 timers connect 1 @@ -11,5 +12,6 @@ router bgp 65001 neighbor 192.168.24.4 aigp address-family ipv4 redistribute connected + neighbor 10.0.0.1 next-hop-self force exit-address-family ! diff --git a/tests/topotests/bgp_aigp/r2/ospfd.conf b/tests/topotests/bgp_aigp/r2/ospfd.conf index ed31941f65c0..106a46251d2e 100644 --- a/tests/topotests/bgp_aigp/r2/ospfd.conf +++ b/tests/topotests/bgp_aigp/r2/ospfd.conf @@ -1,6 +1,6 @@ ! interface lo - ip ospf cost 10 + ip ospf passive ! interface r2-eth0 ip ospf dead-interval 4 diff --git a/tests/topotests/bgp_aigp/r3/bgpd.conf b/tests/topotests/bgp_aigp/r3/bgpd.conf index 5ab712eaba65..bdaa5cf55d1e 100644 --- a/tests/topotests/bgp_aigp/r3/bgpd.conf +++ b/tests/topotests/bgp_aigp/r3/bgpd.conf @@ -1,6 +1,7 @@ router bgp 65001 no bgp ebgp-requires-policy no bgp network import-check + bgp route-reflector allow-outbound-policy neighbor 10.0.0.1 remote-as internal neighbor 10.0.0.1 timers 1 3 neighbor 10.0.0.1 timers connect 1 @@ -11,5 +12,6 @@ router bgp 65001 neighbor 192.168.35.5 aigp address-family ipv4 redistribute connected + neighbor 10.0.0.1 next-hop-self force exit-address-family ! diff --git a/tests/topotests/bgp_aigp/r3/ospfd.conf b/tests/topotests/bgp_aigp/r3/ospfd.conf index f971ad6f8975..9ede3a1fab64 100644 --- a/tests/topotests/bgp_aigp/r3/ospfd.conf +++ b/tests/topotests/bgp_aigp/r3/ospfd.conf @@ -1,6 +1,6 @@ ! interface lo - ip ospf cost 10 + ip ospf passive ! interface r3-eth0 ip ospf dead-interval 4 diff --git a/tests/topotests/bgp_aigp/r4/bgpd.conf b/tests/topotests/bgp_aigp/r4/bgpd.conf index aa88bac91385..e12c45e0bb06 100644 --- a/tests/topotests/bgp_aigp/r4/bgpd.conf +++ b/tests/topotests/bgp_aigp/r4/bgpd.conf @@ -1,6 +1,7 @@ router bgp 65001 no bgp ebgp-requires-policy no bgp network import-check + bgp route-reflector allow-outbound-policy neighbor 192.168.24.2 remote-as internal neighbor 192.168.24.2 timers 1 3 neighbor 192.168.24.2 timers connect 1 @@ -11,7 +12,18 @@ router bgp 65001 neighbor 10.0.0.6 timers connect 1 neighbor 10.0.0.6 update-source lo address-family ipv4 - redistribute connected - redistribute ospf + redistribute connected route-map connected-to-bgp + neighbor 192.168.24.2 route-map set-nexthop out exit-address-family ! +! Two OSPF domains should be isolated - otherwise the connected routes +! on r4 would be advertised to r3 (via r4 -> r6 -> r5 -> r3), and can +! mess up bgp bestpath calculation (igp metrics for the BGP nexthops). +! +route-map connected-to-bgp permit 10 + set community no-advertise +! +route-map set-nexthop permit 10 + set ip next-hop peer-address +exit +! diff --git a/tests/topotests/bgp_aigp/r4/ospfd.conf b/tests/topotests/bgp_aigp/r4/ospfd.conf index c9e6796f6e74..237b5e18abe0 100644 --- a/tests/topotests/bgp_aigp/r4/ospfd.conf +++ b/tests/topotests/bgp_aigp/r4/ospfd.conf @@ -1,6 +1,6 @@ ! interface lo - ip ospf cost 10 + ip ospf passive ! interface r4-eth1 ip ospf dead-interval 4 diff --git a/tests/topotests/bgp_aigp/r5/bgpd.conf b/tests/topotests/bgp_aigp/r5/bgpd.conf index 4fde262053ce..3d1f5e85722b 100644 --- a/tests/topotests/bgp_aigp/r5/bgpd.conf +++ b/tests/topotests/bgp_aigp/r5/bgpd.conf @@ -1,6 +1,7 @@ router bgp 65001 no bgp ebgp-requires-policy no bgp network import-check + bgp route-reflector allow-outbound-policy neighbor 192.168.35.3 remote-as internal neighbor 192.168.35.3 timers 1 3 neighbor 192.168.35.3 timers connect 1 @@ -11,7 +12,18 @@ router bgp 65001 neighbor 10.0.0.6 timers connect 1 neighbor 10.0.0.6 update-source lo address-family ipv4 - redistribute connected - redistribute ospf + redistribute connected route-map connected-to-bgp + neighbor 192.168.35.3 route-map set-nexthop out exit-address-family ! +! Two OSPF domains should be isolated - otherwise the connected routes +! on r5 would be advertised to r2 (via r5 -> r6 -> r4 -> r2), and can +! mess up bgp bestpath calculation (igp metrics for the BGP nexthops). +! +route-map connected-to-bgp permit 10 + set community no-advertise +! +route-map set-nexthop permit 10 + set ip next-hop peer-address +exit +! diff --git a/tests/topotests/bgp_aigp/r5/ospfd.conf b/tests/topotests/bgp_aigp/r5/ospfd.conf index b853c74102a2..65a213df1721 100644 --- a/tests/topotests/bgp_aigp/r5/ospfd.conf +++ b/tests/topotests/bgp_aigp/r5/ospfd.conf @@ -1,6 +1,6 @@ ! interface lo - ip ospf cost 10 + ip ospf passive ! interface r5-eth1 ip ospf dead-interval 4 diff --git a/tests/topotests/bgp_aigp/r6/bgpd.conf b/tests/topotests/bgp_aigp/r6/bgpd.conf index 2faae7720c90..2d5f7a89baf4 100644 --- a/tests/topotests/bgp_aigp/r6/bgpd.conf +++ b/tests/topotests/bgp_aigp/r6/bgpd.conf @@ -1,6 +1,7 @@ router bgp 65001 no bgp ebgp-requires-policy no bgp network import-check + bgp route-reflector allow-outbound-policy neighbor 10.0.0.4 remote-as internal neighbor 10.0.0.4 timers 1 3 neighbor 10.0.0.4 timers connect 1 @@ -15,6 +16,11 @@ router bgp 65001 neighbor 192.168.67.7 timers 1 3 neighbor 192.168.67.7 timers connect 1 address-family ipv4 - redistribute ospf + neighbor 10.0.0.4 route-map set-nexthop out + neighbor 10.0.0.5 route-map set-nexthop out exit-address-family ! +route-map set-nexthop permit 10 + set ip next-hop peer-address +exit +! diff --git a/tests/topotests/bgp_aigp/r6/ospfd.conf b/tests/topotests/bgp_aigp/r6/ospfd.conf index 46b293317866..89cbefa89522 100644 --- a/tests/topotests/bgp_aigp/r6/ospfd.conf +++ b/tests/topotests/bgp_aigp/r6/ospfd.conf @@ -1,6 +1,6 @@ ! interface lo - ip ospf cost 10 + ip ospf passive ! interface r6-eth0 ip ospf dead-interval 4 diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index b81c5432972d..0d859adc53f4 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -12,9 +12,9 @@ r6 receives those routes with aigp-metric TLV. r2 and r3 receives those routes with aigp-metric TLV increased by 20, -and 30 appropriately. +and 10 appropriately. -r1 receives routes with aigp-metric TLV 111,131 and 112,132 appropriately. +r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively. """ import os @@ -108,15 +108,29 @@ def _bgp_converge(): expected = { "paths": [ { - "aigpMetric": 111, + "aigpMetric": 81, "valid": True, - "nexthops": [{"hostname": "r3", "accessible": True}], + "nexthops": [ + { + "ip": "10.0.0.3", + "hostname": "r3", + "metric": 30, + "accessible": True, + } + ], }, { - "aigpMetric": 131, + "aigpMetric": 91, "valid": True, - "bestpath": {"selectionReason": "Neighbor IP"}, - "nexthops": [{"hostname": "r2", "accessible": True}], + "bestpath": {"selectionReason": "IGP Metric"}, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + "metric": 10, + "accessible": True, + } + ], }, ] } @@ -140,30 +154,58 @@ def _bgp_check_aigp_metric_bestpath(): "10.0.0.71/32": { "paths": [ { - "aigpMetric": 111, - "bestpath": {"selectionReason": "AIGP"}, + "aigpMetric": 81, "valid": True, - "nexthops": [{"hostname": "r3", "accessible": True}], + "nexthops": [ + { + "ip": "10.0.0.3", + "hostname": "r3", + "metric": 30, + "accessible": True, + } + ], }, { - "aigpMetric": 131, + "aigpMetric": 91, "valid": True, - "nexthops": [{"hostname": "r2", "accessible": True}], + "bestpath": {"selectionReason": "AIGP"}, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + "metric": 10, + "accessible": True, + } + ], }, ], }, "10.0.0.72/32": { "paths": [ { - "aigpMetric": 112, - "bestpath": {"selectionReason": "AIGP"}, + "aigpMetric": 82, "valid": True, - "nexthops": [{"hostname": "r3", "accessible": True}], + "nexthops": [ + { + "ip": "10.0.0.3", + "hostname": "r3", + "metric": 30, + "accessible": True, + } + ], }, { - "aigpMetric": 132, + "aigpMetric": 92, "valid": True, - "nexthops": [{"hostname": "r2", "accessible": True}], + "bestpath": {"selectionReason": "AIGP"}, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + "metric": 10, + "accessible": True, + } + ], }, ], }, @@ -195,17 +237,17 @@ def _bgp_check_aigp_metric_bestpath(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "aigp-metric for 10.0.0.72/32 is not 72" - # r2, 10.0.0.71/32 with aigp-metric 101 (71 + 30) - test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 101) + # r2, 10.0.0.71/32 with aigp-metric 101 (71 + 20) + test_func = functools.partial(_bgp_check_aigp_metric, r2, "10.0.0.71/32", 91) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) - assert result is None, "aigp-metric for 10.0.0.71/32 is not 101" + assert result is None, "aigp-metric for 10.0.0.71/32 is not 91" - # r3, 10.0.0.72/32 with aigp-metric 92 (72 + 20) - test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 92) + # r3, 10.0.0.72/32 with aigp-metric 92 (72 + 10) + test_func = functools.partial(_bgp_check_aigp_metric, r3, "10.0.0.72/32", 82) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) - assert result is None, "aigp-metric for 10.0.0.72/32 is not 92" + assert result is None, "aigp-metric for 10.0.0.72/32 is not 82" - # r1, check if AIGP is considered in best-path selection (lowest wins) + # r1, check if AIGP is considered in best-path selection (lowest wins: aigp + nexthop-metric) test_func = functools.partial(_bgp_check_aigp_metric_bestpath) _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "AIGP attribute is not considered in best-path selection" From af0d1355c7b0a7213f62cea7852ef3223d38c46f Mon Sep 17 00:00:00 2001 From: Shbinging Date: Tue, 15 Oct 2024 07:26:50 +0000 Subject: [PATCH 29/46] ospfd:fix the bug that the empty area was not free after the command was executed When we use the no area X.X.X.X range A.B.C.D/M command, if the area no longer has an interface to which it belongs, then the area should be deleted from the LSDB. This processing logic is consistent with instructions such as no network area and no area authentication. Signed-off-by: Shbinging --- ospfd/ospf_vty.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index b7261da261ed..34fd804ae8c9 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -781,6 +781,8 @@ DEFUN (no_ospf_area_range_substitute, ospf_area_range_substitute_unset(ospf, area, &p); + ospf_area_check_free(ospf, area_id); + return CMD_SUCCESS; } From a2e226b44e8268c7d2ce148ee566e849aa68479e Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 15 Oct 2024 12:43:38 +0300 Subject: [PATCH 30/46] bgpd: Remove unused BGP_NEXTHOP_CONNECTED_CHANGED flag for nexthop Also reduce the size of change_flags, which is way enough to be 1 byte. Signed-off-by: Donatas Abraitis --- bgpd/bgp_nexthop.c | 13 +++---------- bgpd/bgp_nexthop.h | 5 ++--- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/bgpd/bgp_nexthop.c b/bgpd/bgp_nexthop.c index 564ad118ebc3..401549c4e859 100644 --- a/bgpd/bgp_nexthop.c +++ b/bgpd/bgp_nexthop.c @@ -1377,16 +1377,9 @@ char *bgp_nexthop_dump_bnc_change_flags(struct bgp_nexthop_cache *bnc, return buf; } - snprintfrr(buf, len, "%s%s%s", - CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED) - ? "Changed " - : "", - CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_METRIC_CHANGED) - ? "Metric " - : "", - CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_CONNECTED_CHANGED) - ? "Connected " - : ""); + snprintfrr(buf, len, "%s%s", + CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_CHANGED) ? "Changed " : "", + CHECK_FLAG(bnc->change_flags, BGP_NEXTHOP_METRIC_CHANGED) ? "Metric " : ""); return buf; } diff --git a/bgpd/bgp_nexthop.h b/bgpd/bgp_nexthop.h index 5014eb8f3455..6a4a02dcc824 100644 --- a/bgpd/bgp_nexthop.h +++ b/bgpd/bgp_nexthop.h @@ -45,11 +45,10 @@ struct bgp_nexthop_cache { */ bool is_evpn_gwip_nexthop; - uint16_t change_flags; + uint8_t change_flags; #define BGP_NEXTHOP_CHANGED (1 << 0) #define BGP_NEXTHOP_METRIC_CHANGED (1 << 1) -#define BGP_NEXTHOP_CONNECTED_CHANGED (1 << 2) -#define BGP_NEXTHOP_MACIP_CHANGED (1 << 3) +#define BGP_NEXTHOP_MACIP_CHANGED (1 << 2) struct nexthop *nexthop; time_t last_update; From e15eb6a089ec8f61ab8ec5e887603796d070fb1c Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 15 Oct 2024 12:51:51 +0300 Subject: [PATCH 31/46] bgpd: Check if su_local/su_remote exist before encoding BMP peer state Fixes: 035304c25a3890a040acbe23ca385750b062cdce ("bgpd: bmp loc-rib peer up/down for vrfs") Signed-off-by: Donatas Abraitis --- bgpd/bgp_bmp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index 08f8b8b734ff..bb24d5d69e7d 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -501,14 +501,14 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) } /* Local Port, Remote Port */ - if (is_locrib) + if (!peer->su_local || is_locrib) stream_putw(s, 0); else if (peer->su_local->sa.sa_family == AF_INET6) stream_putw(s, htons(peer->su_local->sin6.sin6_port)); else if (peer->su_local->sa.sa_family == AF_INET) stream_putw(s, htons(peer->su_local->sin.sin_port)); - if (is_locrib) + if (!peer->su_remote || is_locrib) stream_putw(s, 0); else if (peer->su_remote->sa.sa_family == AF_INET6) stream_putw(s, htons(peer->su_remote->sin6.sin6_port)); From 070ce19d0cb8078f0a7f8df0fa95be36c5ae2f42 Mon Sep 17 00:00:00 2001 From: David Lamparter Date: Tue, 15 Oct 2024 13:28:41 +0200 Subject: [PATCH 32/46] tools/gcc-plugins: don't crash on array parameters Need to have arrays as a stop condition in this type normalization function, like pointers and function pointers. Actual arrays as argument types are extremely rare to see because C has this array-decay-to-pointer thing, but it can happen. Signed-off-by: David Lamparter --- tools/gcc-plugins/frr-format.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/gcc-plugins/frr-format.c b/tools/gcc-plugins/frr-format.c index 963741e4798f..d9e38f62cae4 100644 --- a/tools/gcc-plugins/frr-format.c +++ b/tools/gcc-plugins/frr-format.c @@ -2685,7 +2685,8 @@ tree type_normalize (tree type, tree *cousin, tree target = NULL) { while (1) { - if (TREE_CODE (type) == FUNCTION_TYPE || TREE_CODE (type) == POINTER_TYPE) + if (TREE_CODE (type) == FUNCTION_TYPE || TREE_CODE (type) == POINTER_TYPE + || TREE_CODE (type) == ARRAY_TYPE) return type; if (target) /* Strip off any "const" etc. */ From 2a54ddb7fcebc02fdb8e0f77aa4733973147aa3f Mon Sep 17 00:00:00 2001 From: Louis Scalbert Date: Tue, 15 Oct 2024 15:42:02 +0200 Subject: [PATCH 33/46] bgpd: fix bmp coverity issue 1600779 Fix bmp coverity issue 1600779. peer->su_local cannot be NULL. Fixes: 035304c25a ("bgpd: bmp loc-rib peer up/down for vrfs") Signed-off-by: Louis Scalbert --- bgpd/bgp_bmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bgpd/bgp_bmp.c b/bgpd/bgp_bmp.c index bb24d5d69e7d..59efb35235d7 100644 --- a/bgpd/bgp_bmp.c +++ b/bgpd/bgp_bmp.c @@ -489,7 +489,7 @@ static struct stream *bmp_peerstate(struct peer *peer, bool down) &uptime_real); /* Local Address (16 bytes) */ - if (!peer->su_local || is_locrib) + if (is_locrib) stream_put(s, 0, 16); else if (peer->su_local->sa.sa_family == AF_INET6) stream_put(s, &peer->su_local->sin6.sin6_addr, 16); From c8a947e12bbbcc16d6057975bf7c65661db2fc86 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Oct 2024 09:51:08 -0400 Subject: [PATCH 34/46] tests: iproute2_check_path_selection call the actual command For some reason this was missing. Signed-off-by: Donald Sharp --- tests/topotests/lib/common_check.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/topotests/lib/common_check.py b/tests/topotests/lib/common_check.py index 19f02dbadc9f..b04b9de44e89 100644 --- a/tests/topotests/lib/common_check.py +++ b/tests/topotests/lib/common_check.py @@ -58,7 +58,7 @@ def iproute2_check_path_selection(router, ipaddr_str, expected, vrf_name=None): else: cmdstr = f"ip -json route show {ipaddr_str}" try: - output = json.loads(cmdstr) + output = json.loads(router.cmd(cmdstr)) except: output = [] From 5a05dbeb133246431109f451b413d77c74bdb2fc Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 17:15:00 +0300 Subject: [PATCH 35/46] bgpd: Set MED using a helper bgp_attr_set_med() Signed-off-by: Donatas Abraitis --- bgpd/bgp_attr.c | 7 ++----- bgpd/bgp_attr.h | 6 ++++++ bgpd/bgp_route.c | 7 +++---- bgpd/bgp_routemap.c | 3 +-- bgpd/bgp_updgrp_adv.c | 4 ++-- bgpd/rfapi/rfapi.c | 12 ++++-------- bgpd/rfapi/vnc_export_bgp.c | 22 ++++++++++++---------- 7 files changed, 30 insertions(+), 31 deletions(-) diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 7aa9bbbb91b8..596d820f1b50 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1177,8 +1177,7 @@ struct attr *bgp_attr_aggregate_intern( SET_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_ORIGIN)); /* MED */ - attr.med = 0; - SET_FLAG(attr.flag, ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)); + bgp_attr_set_med(&attr, 0); /* AS path attribute. */ if (aspath) @@ -1933,9 +1932,7 @@ static enum bgp_attr_parse_ret bgp_attr_med(struct bgp_attr_parser_args *args) args->total); } - attr->med = stream_getl(peer->curr); - - SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)); + bgp_attr_set_med(attr, stream_getl(peer->curr)); return BGP_ATTR_PARSE_PROCEED; } diff --git a/bgpd/bgp_attr.h b/bgpd/bgp_attr.h index 02ddf88b6242..c10a9dc90a74 100644 --- a/bgpd/bgp_attr.h +++ b/bgpd/bgp_attr.h @@ -608,6 +608,12 @@ static inline uint64_t bgp_aigp_metric_total(struct bgp_path_info *bpi) return aigp; } +static inline void bgp_attr_set_med(struct attr *attr, uint32_t med) +{ + attr->med = med; + SET_FLAG(attr->flag, ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)); +} + static inline struct cluster_list *bgp_attr_get_cluster(const struct attr *attr) { return attr->cluster1; diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 0f196157c80c..a8ca887cdc12 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -6747,8 +6747,8 @@ void bgp_static_update(struct bgp *bgp, const struct prefix *p, bgp_attr_default_set(&attr, bgp, BGP_ORIGIN_IGP); attr.nexthop = bgp_static->igpnexthop; - attr.med = bgp_static->igpmetric; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(&attr, bgp_static->igpmetric); if (afi == AFI_IP) attr.mp_nexthop_len = BGP_ATTR_NHLEN_IPV4; @@ -9015,9 +9015,8 @@ void bgp_redistribute_add(struct bgp *bgp, struct prefix *p, else UNSET_FLAG(attr.nh_flags, BGP_ATTR_NH_IF_OPERSTATE); - attr.med = metric; + bgp_attr_set_med(&attr, metric); attr.distance = distance; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); attr.tag = tag; if (metric) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 4900bb3ce351..0f8adea38db0 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -2222,8 +2222,7 @@ route_set_metric(void *rule, const struct prefix *prefix, void *object) if (path->attr->flag & ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)) med = path->attr->med; - path->attr->med = route_value_adjust(rv, med, path->peer); - path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + bgp_attr_set_med(path->attr, route_value_adjust(rv, med, path->peer)); return RMAP_OKAY; } diff --git a/bgpd/bgp_updgrp_adv.c b/bgpd/bgp_updgrp_adv.c index 250378af6845..1a66df59fc20 100644 --- a/bgpd/bgp_updgrp_adv.c +++ b/bgpd/bgp_updgrp_adv.c @@ -906,8 +906,8 @@ void subgroup_default_originate(struct update_subgroup *subgrp, bool withdraw) assert(attr.aspath); aspath = attr.aspath; - attr.med = 0; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(&attr, 0); if ((afi == AFI_IP6) || peer_cap_enhe(peer, afi, safi)) { /* IPv6 global nexthop must be included. */ diff --git a/bgpd/rfapi/rfapi.c b/bgpd/rfapi/rfapi.c index 23e3eb482308..61d154f1b436 100644 --- a/bgpd/rfapi/rfapi.c +++ b/bgpd/rfapi/rfapi.c @@ -667,10 +667,8 @@ void add_vnc_route(struct rfapi_descriptor *rfd, /* cookie, VPN UN addr, peer */ attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF); } - if (med) { - attr.med = *med; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); - } + if (med) + bgp_attr_set_med(&attr, *med); /* override default weight assigned by bgp_attr_default_set() */ attr.weight = rfd->peer ? rfd->peer->weight[afi][safi] : 0; @@ -863,10 +861,8 @@ void add_vnc_route(struct rfapi_descriptor *rfd, /* cookie, VPN UN addr, peer */ red = bgp_redist_lookup(bgp, afi, type, 0); - if (red && red->redist_metric_flag) { - attr.med = red->redist_metric; - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); - } + if (red && red->redist_metric_flag) + bgp_attr_set_med(&attr, red->redist_metric); bn = bgp_afi_node_get(bgp->rib[afi][safi], afi, safi, p, prd); diff --git a/bgpd/rfapi/vnc_export_bgp.c b/bgpd/rfapi/vnc_export_bgp.c index 4de2306609c9..c9a058ea44c0 100644 --- a/bgpd/rfapi/vnc_export_bgp.c +++ b/bgpd/rfapi/vnc_export_bgp.c @@ -96,15 +96,16 @@ static void encap_attr_export_ce(struct attr *new, struct attr *orig, * neighbor NEIGHBOR attribute-unchanged med */ if (!CHECK_FLAG(new->flag, BGP_ATTR_MULTI_EXIT_DISC)) { + uint32_t med = 255; + if (CHECK_FLAG(new->flag, BGP_ATTR_LOCAL_PREF)) { if (new->local_pref > 255) - new->med = 0; + med = 0; else - new->med = 255 - new->local_pref; - } else { - new->med = 255; /* shouldn't happen */ + med = 255 - new->local_pref; } - new->flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(new, med); } /* @@ -642,15 +643,16 @@ encap_attr_export(struct attr *new, struct attr *orig, * neighbor NEIGHBOR attribute-unchanged med */ if (!CHECK_FLAG(new->flag, BGP_ATTR_MULTI_EXIT_DISC)) { + uint32_t med = 255; + if (CHECK_FLAG(new->flag, BGP_ATTR_LOCAL_PREF)) { if (new->local_pref > 255) - new->med = 0; + med = 0; else - new->med = 255 - new->local_pref; - } else { - new->med = 255; /* shouldn't happen */ + med = 255 - new->local_pref; } - new->flag |= ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC); + + bgp_attr_set_med(new, med); } /* From f677fc8db3c0195dc1bc8d7cd9887cf4e7aa638e Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 20:57:49 +0300 Subject: [PATCH 36/46] bgpd: Implement `set metric igp` command Set metric automatically from the path info (IGP protocol). Signed-off-by: Donatas Abraitis --- bgpd/bgp_routemap.c | 26 +++++++++++++++++--------- doc/user/routemap.rst | 4 +++- lib/routemap_cli.c | 10 ++++++++-- lib/routemap_northbound.c | 21 +++++++++++++++++++++ yang/frr-route-map.yang | 8 ++++++++ 5 files changed, 57 insertions(+), 12 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 0f8adea38db0..0e6ba4ead877 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -125,6 +125,9 @@ o Local extensions #define RMAP_VALUE_ADD 1 #define RMAP_VALUE_SUB 2 +#define RMAP_VALUE_TYPE_RTT 1 +#define RMAP_VALUE_TYPE_IGP 2 + struct rmap_value { uint8_t action; uint8_t variable; @@ -140,14 +143,18 @@ static int route_value_match(struct rmap_value *rv, uint32_t value) } static uint32_t route_value_adjust(struct rmap_value *rv, uint32_t current, - struct peer *peer) + struct bgp_path_info *bpi) { uint32_t value; + struct peer *peer = bpi->peer; switch (rv->variable) { - case 1: + case RMAP_VALUE_TYPE_RTT: value = peer->rtt; break; + case RMAP_VALUE_TYPE_IGP: + value = bpi->extra ? bpi->extra->igpmetric : 0; + break; default: value = rv->value; break; @@ -187,11 +194,12 @@ static void *route_value_compile(const char *arg) larg = strtoul(arg, &endptr, 10); if (*arg == 0 || *endptr != 0 || errno || larg > UINT32_MAX) return NULL; + } else if (strmatch(arg, "rtt")) { + var = RMAP_VALUE_TYPE_RTT; + } else if (strmatch(arg, "igp")) { + var = RMAP_VALUE_TYPE_IGP; } else { - if (strcmp(arg, "rtt") == 0) - var = 1; - else - return NULL; + return NULL; } rv = XMALLOC(MTYPE_ROUTE_MAP_COMPILED, sizeof(struct rmap_value)); @@ -2145,7 +2153,7 @@ route_set_local_pref(void *rule, const struct prefix *prefix, void *object) locpref = path->attr->local_pref; path->attr->flag |= ATTR_FLAG_BIT(BGP_ATTR_LOCAL_PREF); - path->attr->local_pref = route_value_adjust(rv, locpref, path->peer); + path->attr->local_pref = route_value_adjust(rv, locpref, path); return RMAP_OKAY; } @@ -2172,7 +2180,7 @@ route_set_weight(void *rule, const struct prefix *prefix, void *object) path = object; /* Set weight value. */ - path->attr->weight = route_value_adjust(rv, 0, path->peer); + path->attr->weight = route_value_adjust(rv, 0, path); return RMAP_OKAY; } @@ -2222,7 +2230,7 @@ route_set_metric(void *rule, const struct prefix *prefix, void *object) if (path->attr->flag & ATTR_FLAG_BIT(BGP_ATTR_MULTI_EXIT_DISC)) med = path->attr->med; - bgp_attr_set_med(path->attr, route_value_adjust(rv, med, path->peer)); + bgp_attr_set_med(path->attr, route_value_adjust(rv, med, path)); return RMAP_OKAY; } diff --git a/doc/user/routemap.rst b/doc/user/routemap.rst index 9034af39c560..3ee5597f239a 100644 --- a/doc/user/routemap.rst +++ b/doc/user/routemap.rst @@ -305,13 +305,15 @@ Route Map Set Command Set the route's weight. -.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt> +.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt|igp> Set the route metric. When used with BGP, set the BGP attribute MED to a specific value. Use `+`/`-` to add or subtract the specified value to/from the existing/MED. Use `rtt` to set the MED to the round trip time or `+rtt`/`-rtt` to add/subtract the round trip time to/from the MED. + If ``igp`` is specified, then the actual value from the IGP protocol is used. + .. clicmd:: set min-metric <(0-4294967295)> Set the minimum metric for the route. diff --git a/lib/routemap_cli.c b/lib/routemap_cli.c index f64c3c2376a9..3e4893310e7e 100644 --- a/lib/routemap_cli.c +++ b/lib/routemap_cli.c @@ -922,13 +922,14 @@ DEFPY_YANG( DEFPY_YANG( set_metric, set_metric_cmd, - "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt>", + "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt|igp$igp>", SET_STR "Metric value for destination routing protocol\n" "Metric value (use +/- for additions or subtractions)\n" "Assign round trip time\n" "Add round trip time\n" - "Subtract round trip time\n") + "Subtract round trip time\n" + "Metric value from IGP protocol\n") { const char *xpath = "./set-action[action='frr-route-map:set-metric']"; char xpath_value[XPATH_MAXLEN]; @@ -939,6 +940,9 @@ DEFPY_YANG( snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-round-trip-time", xpath); snprintf(value, sizeof(value), "true"); + } else if (igp) { + snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-igp", xpath); + snprintf(value, sizeof(value), "true"); } else if (artt) { snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/add-round-trip-time", xpath); @@ -1148,6 +1152,8 @@ void route_map_action_show(struct vty *vty, const struct lyd_node *dnode, if (yang_dnode_get(dnode, "./rmap-set-action/use-round-trip-time")) { vty_out(vty, " set metric rtt\n"); + } else if (yang_dnode_get(dnode, "./rmap-set-action/use-igp")) { + vty_out(vty, " set metric igp\n"); } else if (yang_dnode_get( dnode, "./rmap-set-action/add-round-trip-time")) { diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index 1bba4dad47a6..a868a80f392f 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -1213,6 +1213,20 @@ static int lib_route_map_entry_set_action_use_round_trip_time_destroy( return lib_route_map_entry_set_action_value_destroy(args); } +/* + * XPath: /frr-route-map:lib/route-map/entry/set-action/use-igp + */ +static int lib_route_map_entry_set_action_use_igp_modify(struct nb_cb_modify_args *args) +{ + return set_action_modify(args->event, args->dnode, args->resource, "igp", args->errmsg, + args->errmsg_len); +} + +static int lib_route_map_entry_set_action_use_igp_destroy(struct nb_cb_destroy_args *args) +{ + return lib_route_map_entry_set_action_value_destroy(args); +} + /* * XPath: /frr-route-map:lib/route-map/entry/set-action/add-round-trip-time */ @@ -1516,6 +1530,13 @@ const struct frr_yang_module_info frr_route_map_info = { .destroy = lib_route_map_entry_set_action_use_round_trip_time_destroy, } }, + { + .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/use-igp", + .cbs = { + .modify = lib_route_map_entry_set_action_use_igp_modify, + .destroy = lib_route_map_entry_set_action_use_igp_destroy, + } + }, { .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/add-round-trip-time", .cbs = { diff --git a/yang/frr-route-map.yang b/yang/frr-route-map.yang index c875a6ec7f05..b83047caee4f 100644 --- a/yang/frr-route-map.yang +++ b/yang/frr-route-map.yang @@ -355,6 +355,14 @@ module frr-route-map { "Subtract round trip time to metric"; } } + + case use-igp { + leaf use-igp { + type boolean; + description + "Use metric from IGP procotol"; + } + } } } From 8d39cfd613944be99671fe28a3d92c6b2cb716ab Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:36:13 +0300 Subject: [PATCH 37/46] tests: Check if MED can be derived from `set metric igp|aigp` Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_aigp/r1/bgpd.conf | 9 +++++++ tests/topotests/bgp_aigp/r1/zebra.conf | 3 +++ tests/topotests/bgp_aigp/r8/bgpd.conf | 4 +++ tests/topotests/bgp_aigp/r8/zebra.conf | 4 +++ tests/topotests/bgp_aigp/test_bgp_aigp.py | 32 ++++++++++++++++++++++- 5 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 tests/topotests/bgp_aigp/r8/bgpd.conf create mode 100644 tests/topotests/bgp_aigp/r8/zebra.conf diff --git a/tests/topotests/bgp_aigp/r1/bgpd.conf b/tests/topotests/bgp_aigp/r1/bgpd.conf index 74a0215bc476..d99192421a39 100644 --- a/tests/topotests/bgp_aigp/r1/bgpd.conf +++ b/tests/topotests/bgp_aigp/r1/bgpd.conf @@ -9,4 +9,13 @@ router bgp 65001 neighbor 10.0.0.3 timers 1 3 neighbor 10.0.0.3 timers connect 1 neighbor 10.0.0.3 update-source lo + neighbor 192.168.18.8 remote-as external + neighbor 192.168.18.8 timers 1 3 + neighbor 192.168.18.8 timers connect 1 + address-family ipv4 + neighbor 192.168.18.8 route-map r8 out + exit-address-family ! +route-map r8 permit 10 + set metric igp +exit diff --git a/tests/topotests/bgp_aigp/r1/zebra.conf b/tests/topotests/bgp_aigp/r1/zebra.conf index 0ed22d37be8b..7ac4bb5de9a3 100644 --- a/tests/topotests/bgp_aigp/r1/zebra.conf +++ b/tests/topotests/bgp_aigp/r1/zebra.conf @@ -8,3 +8,6 @@ interface r1-eth0 interface r1-eth1 ip address 192.168.13.1/24 ! +interface r1-eth2 + ip address 192.168.18.1/24 +! diff --git a/tests/topotests/bgp_aigp/r8/bgpd.conf b/tests/topotests/bgp_aigp/r8/bgpd.conf new file mode 100644 index 000000000000..c50c3ce91a58 --- /dev/null +++ b/tests/topotests/bgp_aigp/r8/bgpd.conf @@ -0,0 +1,4 @@ +router bgp 65008 + no bgp ebgp-requires-policy + neighbor 192.168.18.1 remote-as external +! diff --git a/tests/topotests/bgp_aigp/r8/zebra.conf b/tests/topotests/bgp_aigp/r8/zebra.conf new file mode 100644 index 000000000000..f7545270b2c7 --- /dev/null +++ b/tests/topotests/bgp_aigp/r8/zebra.conf @@ -0,0 +1,4 @@ +! +interface r8-eth0 + ip address 192.168.18.8/24 +! diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index 0d859adc53f4..92b54d0ae102 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -15,6 +15,8 @@ and 10 appropriately. r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively. + +r1 advertises MED from IGP protocol (set metric igp) to r8. """ import os @@ -34,7 +36,7 @@ def build_topo(tgen): - for routern in range(1, 8): + for routern in range(1, 9): tgen.add_router("r{}".format(routern)) switch = tgen.add_switch("s1") @@ -65,6 +67,10 @@ def build_topo(tgen): switch.add_link(tgen.gears["r6"]) switch.add_link(tgen.gears["r7"]) + switch = tgen.add_switch("s8") + switch.add_link(tgen.gears["r1"]) + switch.add_link(tgen.gears["r8"]) + def setup_module(mod): tgen = Topogen(build_topo, mod.__name__) @@ -102,6 +108,7 @@ def test_bgp_aigp(): r3 = tgen.gears["r3"] r4 = tgen.gears["r4"] r5 = tgen.gears["r5"] + r8 = tgen.gears["r8"] def _bgp_converge(): output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast 10.0.0.71/32 json")) @@ -143,6 +150,11 @@ def _bgp_check_aigp_metric(router, prefix, aigp): expected = {"paths": [{"aigpMetric": aigp, "valid": True}]} return topotest.json_cmp(output, expected) + def _bgp_check_received_med(med): + output = json.loads(r8.vtysh_cmd("show bgp ipv4 unicast 10.0.0.71/32 json")) + expected = {"paths": [{"metric": med, "valid": True}]} + return topotest.json_cmp(output, expected) + def _bgp_check_aigp_metric_bestpath(): output = json.loads( r1.vtysh_cmd( @@ -252,6 +264,24 @@ def _bgp_check_aigp_metric_bestpath(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "AIGP attribute is not considered in best-path selection" + # r8, check if MED is set to 20 (derived from `set metric igp`) + test_func = functools.partial(_bgp_check_received_med, 20) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "MED attribute value is not 20" + + r1.vtysh_cmd( + """ +configure terminal +route-map r8 permit 10 + set metric aigp +""" + ) + + # r8, check if MED is set to 111 (derived from `set metric aigp`) + test_func = functools.partial(_bgp_check_received_med, 111) + _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) + assert result is None, "MED attribute value is not 111" + if __name__ == "__main__": args = ["-s"] + sys.argv[1:] From e94f48498d3ec68bd7dec185d7658bdc583a0c32 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:47:40 +0300 Subject: [PATCH 38/46] bgpd: Implement `set metric aigp` command Same as `set metric igp`, but in this case accumulated IGP metric is being sent as MED attribute. Signed-off-by: Donatas Abraitis --- bgpd/bgp_routemap.c | 6 ++++++ lib/routemap_cli.c | 26 ++++++++++++-------------- lib/routemap_northbound.c | 21 +++++++++++++++++++++ yang/frr-route-map.yang | 8 ++++++++ 4 files changed, 47 insertions(+), 14 deletions(-) diff --git a/bgpd/bgp_routemap.c b/bgpd/bgp_routemap.c index 0e6ba4ead877..23e83f9bddac 100644 --- a/bgpd/bgp_routemap.c +++ b/bgpd/bgp_routemap.c @@ -127,6 +127,7 @@ o Local extensions #define RMAP_VALUE_TYPE_RTT 1 #define RMAP_VALUE_TYPE_IGP 2 +#define RMAP_VALUE_TYPE_AIGP 3 struct rmap_value { uint8_t action; @@ -155,6 +156,9 @@ static uint32_t route_value_adjust(struct rmap_value *rv, uint32_t current, case RMAP_VALUE_TYPE_IGP: value = bpi->extra ? bpi->extra->igpmetric : 0; break; + case RMAP_VALUE_TYPE_AIGP: + value = MIN(bpi->attr->aigp_metric, UINT32_MAX); + break; default: value = rv->value; break; @@ -198,6 +202,8 @@ static void *route_value_compile(const char *arg) var = RMAP_VALUE_TYPE_RTT; } else if (strmatch(arg, "igp")) { var = RMAP_VALUE_TYPE_IGP; + } else if (strmatch(arg, "aigp")) { + var = RMAP_VALUE_TYPE_AIGP; } else { return NULL; } diff --git a/lib/routemap_cli.c b/lib/routemap_cli.c index 3e4893310e7e..432805c8d51a 100644 --- a/lib/routemap_cli.c +++ b/lib/routemap_cli.c @@ -922,14 +922,15 @@ DEFPY_YANG( DEFPY_YANG( set_metric, set_metric_cmd, - "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt|igp$igp>", + "set metric <(-4294967295-4294967295)$metric|rtt$rtt|+rtt$artt|-rtt$srtt|igp$igp|aigp$aigp>", SET_STR "Metric value for destination routing protocol\n" "Metric value (use +/- for additions or subtractions)\n" "Assign round trip time\n" "Add round trip time\n" "Subtract round trip time\n" - "Metric value from IGP protocol\n") + "Metric value from IGP protocol\n" + "Metric value from AIGP (Accumulated IGP)\n") { const char *xpath = "./set-action[action='frr-route-map:set-metric']"; char xpath_value[XPATH_MAXLEN]; @@ -943,6 +944,9 @@ DEFPY_YANG( } else if (igp) { snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-igp", xpath); snprintf(value, sizeof(value), "true"); + } else if (aigp) { + snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/use-aigp", xpath); + snprintf(value, sizeof(value), "true"); } else if (artt) { snprintf(xpath_value, sizeof(xpath_value), "%s/rmap-set-action/add-round-trip-time", xpath); @@ -1154,23 +1158,17 @@ void route_map_action_show(struct vty *vty, const struct lyd_node *dnode, vty_out(vty, " set metric rtt\n"); } else if (yang_dnode_get(dnode, "./rmap-set-action/use-igp")) { vty_out(vty, " set metric igp\n"); - } else if (yang_dnode_get( - dnode, - "./rmap-set-action/add-round-trip-time")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/use-aigp")) { + vty_out(vty, " set metric aigp\n"); + } else if (yang_dnode_get(dnode, "./rmap-set-action/add-round-trip-time")) { vty_out(vty, " set metric +rtt\n"); - } else if ( - yang_dnode_get( - dnode, - "./rmap-set-action/subtract-round-trip-time")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/subtract-round-trip-time")) { vty_out(vty, " set metric -rtt\n"); - } else if (yang_dnode_get(dnode, - "./rmap-set-action/add-metric")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/add-metric")) { vty_out(vty, " set metric +%s\n", yang_dnode_get_string( dnode, "./rmap-set-action/add-metric")); - } else if (yang_dnode_get( - dnode, - "./rmap-set-action/subtract-metric")) { + } else if (yang_dnode_get(dnode, "./rmap-set-action/subtract-metric")) { vty_out(vty, " set metric -%s\n", yang_dnode_get_string( dnode, diff --git a/lib/routemap_northbound.c b/lib/routemap_northbound.c index a868a80f392f..0ee055e65304 100644 --- a/lib/routemap_northbound.c +++ b/lib/routemap_northbound.c @@ -1227,6 +1227,20 @@ static int lib_route_map_entry_set_action_use_igp_destroy(struct nb_cb_destroy_a return lib_route_map_entry_set_action_value_destroy(args); } +/* + * XPath: /frr-route-map:lib/route-map/entry/set-action/use-aigp + */ +static int lib_route_map_entry_set_action_use_aigp_modify(struct nb_cb_modify_args *args) +{ + return set_action_modify(args->event, args->dnode, args->resource, "aigp", args->errmsg, + args->errmsg_len); +} + +static int lib_route_map_entry_set_action_use_aigp_destroy(struct nb_cb_destroy_args *args) +{ + return lib_route_map_entry_set_action_value_destroy(args); +} + /* * XPath: /frr-route-map:lib/route-map/entry/set-action/add-round-trip-time */ @@ -1537,6 +1551,13 @@ const struct frr_yang_module_info frr_route_map_info = { .destroy = lib_route_map_entry_set_action_use_igp_destroy, } }, + { + .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/use-aigp", + .cbs = { + .modify = lib_route_map_entry_set_action_use_aigp_modify, + .destroy = lib_route_map_entry_set_action_use_aigp_destroy, + } + }, { .xpath = "/frr-route-map:lib/route-map/entry/set-action/rmap-set-action/add-round-trip-time", .cbs = { diff --git a/yang/frr-route-map.yang b/yang/frr-route-map.yang index b83047caee4f..244c353a6357 100644 --- a/yang/frr-route-map.yang +++ b/yang/frr-route-map.yang @@ -363,6 +363,14 @@ module frr-route-map { "Use metric from IGP procotol"; } } + + case use-aigp { + leaf use-aigp { + type boolean; + description + "Use metric from AIGP (Accumulated IGP)"; + } + } } } From 3105ceaee9a6aab59d0c120fcf28f103bf84d0a3 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:48:23 +0300 Subject: [PATCH 39/46] tests: Check if `set metric aigp` works Signed-off-by: Donatas Abraitis --- tests/topotests/bgp_aigp/r1/bgpd.conf | 7 +++++ tests/topotests/bgp_aigp/test_bgp_aigp.py | 36 +++++++++++------------ 2 files changed, 24 insertions(+), 19 deletions(-) diff --git a/tests/topotests/bgp_aigp/r1/bgpd.conf b/tests/topotests/bgp_aigp/r1/bgpd.conf index d99192421a39..15621999c59f 100644 --- a/tests/topotests/bgp_aigp/r1/bgpd.conf +++ b/tests/topotests/bgp_aigp/r1/bgpd.conf @@ -16,6 +16,13 @@ router bgp 65001 neighbor 192.168.18.8 route-map r8 out exit-address-family ! +ip prefix-list p71 seq 5 permit 10.0.0.71/32 +ip prefix-list p72 seq 5 permit 10.0.0.72/32 +! route-map r8 permit 10 + match ip address prefix-list p71 set metric igp +route-map r8 permit 20 + match ip address prefix-list p72 + set metric aigp exit diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index 92b54d0ae102..a59b71590b12 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -17,6 +17,8 @@ r1 receives routes with aigp-metric TLV 81, 91 and 82, 92 respectively. r1 advertises MED from IGP protocol (set metric igp) to r8. + +r1 advertises MED from AIGP (set metric aigp) to r8. """ import os @@ -150,9 +152,16 @@ def _bgp_check_aigp_metric(router, prefix, aigp): expected = {"paths": [{"aigpMetric": aigp, "valid": True}]} return topotest.json_cmp(output, expected) - def _bgp_check_received_med(med): - output = json.loads(r8.vtysh_cmd("show bgp ipv4 unicast 10.0.0.71/32 json")) - expected = {"paths": [{"metric": med, "valid": True}]} + def _bgp_check_received_med(): + output = json.loads( + r8.vtysh_cmd("show bgp ipv4 unicast 10.0.0.64/28 longer-prefixes json") + ) + expected = { + "routes": { + "10.0.0.71/32": [{"valid": True, "metric": 20}], + "10.0.0.72/32": [{"valid": True, "metric": 112}], + } + } return topotest.json_cmp(output, expected) def _bgp_check_aigp_metric_bestpath(): @@ -264,23 +273,12 @@ def _bgp_check_aigp_metric_bestpath(): _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) assert result is None, "AIGP attribute is not considered in best-path selection" - # r8, check if MED is set to 20 (derived from `set metric igp`) - test_func = functools.partial(_bgp_check_received_med, 20) - _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "MED attribute value is not 20" - - r1.vtysh_cmd( - """ -configure terminal -route-map r8 permit 10 - set metric aigp -""" - ) - - # r8, check if MED is set to 111 (derived from `set metric aigp`) - test_func = functools.partial(_bgp_check_received_med, 111) + # r8, check if MED is set derived from `set metric igp`, and `set metric aigp` + test_func = functools.partial(_bgp_check_received_med) _, result = topotest.run_and_expect(test_func, None, count=30, wait=1) - assert result is None, "MED attribute value is not 111" + assert ( + result is None + ), "MED attribute values are not derived from `set metric [a]igp`" if __name__ == "__main__": From 7cdece8c84bee4694e0679e402b01cd501669d99 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Tue, 8 Oct 2024 21:53:19 +0300 Subject: [PATCH 40/46] doc: Add `set metric aigp` command Signed-off-by: Donatas Abraitis --- doc/user/routemap.rst | 5 ++++- tests/topotests/bgp_aigp/test_bgp_aigp.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/doc/user/routemap.rst b/doc/user/routemap.rst index 3ee5597f239a..02a43bc980b8 100644 --- a/doc/user/routemap.rst +++ b/doc/user/routemap.rst @@ -305,7 +305,7 @@ Route Map Set Command Set the route's weight. -.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt|igp> +.. clicmd:: set metric <[+|-](1-4294967295)|rtt|+rtt|-rtt|igp|aigp> Set the route metric. When used with BGP, set the BGP attribute MED to a specific value. Use `+`/`-` to add or subtract the specified value to/from @@ -314,6 +314,9 @@ Route Map Set Command If ``igp`` is specified, then the actual value from the IGP protocol is used. + If ``aigp`` is specified, then the actual value from the AIGP metric is used + (encoded as MED instead of AIGP attribute). + .. clicmd:: set min-metric <(0-4294967295)> Set the minimum metric for the route. diff --git a/tests/topotests/bgp_aigp/test_bgp_aigp.py b/tests/topotests/bgp_aigp/test_bgp_aigp.py index a59b71590b12..5f253d2ef052 100644 --- a/tests/topotests/bgp_aigp/test_bgp_aigp.py +++ b/tests/topotests/bgp_aigp/test_bgp_aigp.py @@ -158,8 +158,8 @@ def _bgp_check_received_med(): ) expected = { "routes": { - "10.0.0.71/32": [{"valid": True, "metric": 20}], - "10.0.0.72/32": [{"valid": True, "metric": 112}], + "10.0.0.71/32": [{"valid": True, "metric": 10}], + "10.0.0.72/32": [{"valid": True, "metric": 92}], } } return topotest.json_cmp(output, expected) From b924f033875729414d91b7efe5cd4d2003f28390 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 9 Oct 2024 18:20:48 +0300 Subject: [PATCH 41/46] bgpd: Re-announce the routes if the underlay IGP metric changes If the underlay IGP metric changes, we SHOULD re-announce the routes with the correct bpi->extra->igpmetric set. Without this patch if the IGP link cost (metric) changes, we never notice this and the peers do not have the updated metrics, which in turn causes incorrect best path selections on remote peers. Signed-off-by: Donatas Abraitis --- bgpd/bgp_route.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index a8ca887cdc12..93bfa62b7d12 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -3727,11 +3727,10 @@ static void bgp_process_main_one(struct bgp *bgp, struct bgp_dest *dest, /* If there is a change of interest to peers, reannounce the * route. */ - if (CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) - || CHECK_FLAG(old_select->flags, BGP_PATH_LINK_BW_CHG) - || CHECK_FLAG(dest->flags, BGP_NODE_LABEL_CHANGED)) { + if (CHECK_FLAG(old_select->flags, BGP_PATH_ATTR_CHANGED) || + CHECK_FLAG(dest->flags, BGP_NODE_LABEL_CHANGED) || + bgp_zebra_has_route_changed(old_select)) { group_announce_route(bgp, afi, safi, dest, new_select); - /* unicast routes must also be annouced to * labeled-unicast update-groups */ if (safi == SAFI_UNICAST) From a593f156d7711d778c76ab9b43dda6b90ed15712 Mon Sep 17 00:00:00 2001 From: Donatas Abraitis Date: Wed, 9 Oct 2024 18:35:33 +0300 Subject: [PATCH 42/46] tests: Check if underlay IGP metric is reflected into BGP after cost changes Signed-off-by: Donatas Abraitis --- .../topotests/bgp_set_metric_igp/__init__.py | 0 .../topotests/bgp_set_metric_igp/r1/frr.conf | 13 ++ .../topotests/bgp_set_metric_igp/r2/frr.conf | 36 +++++ .../topotests/bgp_set_metric_igp/r3/frr.conf | 38 +++++ .../topotests/bgp_set_metric_igp/r4/frr.conf | 41 +++++ .../topotests/bgp_set_metric_igp/r5/frr.conf | 14 ++ .../test_bgp_set_metric_igp.py | 146 ++++++++++++++++++ 7 files changed, 288 insertions(+) create mode 100644 tests/topotests/bgp_set_metric_igp/__init__.py create mode 100644 tests/topotests/bgp_set_metric_igp/r1/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r2/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r3/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r4/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/r5/frr.conf create mode 100644 tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py diff --git a/tests/topotests/bgp_set_metric_igp/__init__.py b/tests/topotests/bgp_set_metric_igp/__init__.py new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/tests/topotests/bgp_set_metric_igp/r1/frr.conf b/tests/topotests/bgp_set_metric_igp/r1/frr.conf new file mode 100644 index 000000000000..018ea0269eda --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r1/frr.conf @@ -0,0 +1,13 @@ +! +int r1-eth0 + ip address 10.0.0.1/24 +! +int r1-eth1 + ip address 10.0.1.1/24 +! +router bgp 65001 + no bgp ebgp-requires-policy + no bgp network import-check + neighbor 10.0.0.2 remote-as external + neighbor 10.0.1.2 remote-as external +! diff --git a/tests/topotests/bgp_set_metric_igp/r2/frr.conf b/tests/topotests/bgp_set_metric_igp/r2/frr.conf new file mode 100644 index 000000000000..bce06c47f065 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r2/frr.conf @@ -0,0 +1,36 @@ +! +int r2-eth0 + ip address 10.0.0.2/24 + ip router isis n2 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r2-eth1 + ip address 10.0.2.1/24 + ip router isis n2 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.0.1 remote-as external + neighbor 10.0.2.2 remote-as internal + address-family ipv4 unicast + neighbor 10.0.0.1 route-map igp out + exit-address-family +! +router isis n2 + is-type level-2-only + net 49.0001.0000.0000.0002.00 + lsp-mtu 1440 +exit +! +route-map igp permit 10 + set metric igp +exit diff --git a/tests/topotests/bgp_set_metric_igp/r3/frr.conf b/tests/topotests/bgp_set_metric_igp/r3/frr.conf new file mode 100644 index 000000000000..312f97d08a4a --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r3/frr.conf @@ -0,0 +1,38 @@ +! +int r3-eth0 + ip address 10.0.1.2/24 + ip router isis n3 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r3-eth1 + ip address 10.0.3.1/24 + ip router isis n3 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis metric level-1 10 + isis metric level-2 100 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.1.1 remote-as external + neighbor 10.0.3.2 remote-as internal + address-family ipv4 unicast + neighbor 10.0.1.1 route-map igp out + exit-address-family +! +router isis n3 + is-type level-2-only + net 49.0001.0000.0000.0003.00 + lsp-mtu 1440 +exit +! +route-map igp permit 10 + set metric igp +exit diff --git a/tests/topotests/bgp_set_metric_igp/r4/frr.conf b/tests/topotests/bgp_set_metric_igp/r4/frr.conf new file mode 100644 index 000000000000..04165b80b244 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r4/frr.conf @@ -0,0 +1,41 @@ +! +int r4-eth0 + ip address 10.0.2.2/24 + ip router isis n4 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r4-eth1 + ip address 10.0.3.2/24 + ip router isis n4 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis metric level-1 10 + isis metric level-2 100 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +int r4-eth2 + ip address 10.0.4.1/24 + ip router isis n4 + isis circuit-type level-2-only + isis fast-reroute lfa level-2 + isis network point-to-point + isis hello-interval 1 + isis hello-multiplier 10 +! +router bgp 65002 + no bgp ebgp-requires-policy + neighbor 10.0.2.1 remote-as internal + neighbor 10.0.3.1 remote-as internal + neighbor 10.0.4.2 remote-as external +! +router isis n4 + is-type level-2-only + net 49.0001.0000.0000.0004.00 + lsp-mtu 1440 +exit diff --git a/tests/topotests/bgp_set_metric_igp/r5/frr.conf b/tests/topotests/bgp_set_metric_igp/r5/frr.conf new file mode 100644 index 000000000000..af23677b72b5 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/r5/frr.conf @@ -0,0 +1,14 @@ +! +int r5-eth0 + ip address 10.0.4.2/24 +! +router bgp 65005 + no bgp ebgp-requires-policy + no bgp network import-check + neighbor 10.0.4.1 remote-as external + neighbor 10.0.4.1 timers 1 3 + neighbor 10.0.4.1 timers connect 1 + address-family ipv4 unicast + network 10.5.5.5/32 + exit-address-family +! diff --git a/tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py b/tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py new file mode 100644 index 000000000000..8fbe817689d1 --- /dev/null +++ b/tests/topotests/bgp_set_metric_igp/test_bgp_set_metric_igp.py @@ -0,0 +1,146 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2024 by +# Donatas Abraitis +# + +import os +import sys +import json +import pytest +import functools + +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +from lib import topotest +from lib.topogen import Topogen, get_topogen + +pytestmark = [pytest.mark.bgpd] + + +def setup_module(mod): + topodef = { + "s1": ("r1", "r2"), + "s2": ("r1", "r3"), + "s3": ("r2", "r4"), + "s4": ("r3", "r4"), + "s5": ("r4", "r5"), + } + tgen = Topogen(topodef, mod.__name__) + tgen.start_topology() + + router_list = tgen.routers() + + for _, (rname, router) in enumerate(router_list.items(), 1): + router.load_frr_config(os.path.join(CWD, "{}/frr.conf".format(rname))) + + tgen.start_router() + + +def teardown_module(mod): + tgen = get_topogen() + tgen.stop_topology() + + +def test_bgp_set_metric_igp(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + r2 = tgen.gears["r2"] + + def _bgp_converge(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "10.5.5.5/32": [ + { + "valid": True, + "bestpath": True, + "selectionReason": "MED", + "metric": 20, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + } + ], + }, + { + "valid": True, + "bestpath": None, + "metric": 110, + "nexthops": [ + { + "ip": "10.0.1.2", + "hostname": "r3", + } + ], + }, + ] + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge, + ) + _, result = topotest.run_and_expect(test_func, None, count=120, wait=5) + assert result is None, "10.5.5.5/32 best path is not via r2 (MED == 20)" + + r2.vtysh_cmd( + """ +configure terminal +interface r2-eth1 + isis metric level-2 6000 +""" + ) + + def _bgp_converge_after_isis_metric_changes(): + output = json.loads(r1.vtysh_cmd("show bgp ipv4 unicast json")) + expected = { + "routes": { + "10.5.5.5/32": [ + { + "valid": True, + "bestpath": None, + "metric": 6010, + "nexthops": [ + { + "ip": "10.0.0.2", + "hostname": "r2", + } + ], + }, + { + "valid": True, + "bestpath": True, + "selectionReason": "MED", + "metric": 110, + "nexthops": [ + { + "ip": "10.0.1.2", + "hostname": "r3", + } + ], + }, + ] + } + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial( + _bgp_converge_after_isis_metric_changes, + ) + _, result = topotest.run_and_expect(test_func, None, count=120, wait=5) + assert result is None, "10.5.5.5/32 best path is not via r3 (MED == 110)" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) From 38661a6aa50204e7204182bf7eeec45e2a87f14b Mon Sep 17 00:00:00 2001 From: Jafar Al-Gharaibeh Date: Tue, 15 Oct 2024 11:11:03 -0500 Subject: [PATCH 43/46] vtysh: fix SA warning, no need to call getenv() twice Signed-off-by: Jafar Al-Gharaibeh --- vtysh/vtysh_main.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/vtysh/vtysh_main.c b/vtysh/vtysh_main.c index 64198132cc64..297d87ec419d 100644 --- a/vtysh/vtysh_main.c +++ b/vtysh/vtysh_main.c @@ -350,6 +350,7 @@ int main(int argc, char **argv, char **env) char pathspace[MAXPATHLEN] = ""; const char *histfile = NULL; const char *histfile_env = getenv("VTYSH_HISTFILE"); + const char *logpath = getenv("VTYSH_LOG"); /* SUID: drop down to calling user & go back up when needed */ elevuid = geteuid(); @@ -643,9 +644,7 @@ int main(int argc, char **argv, char **env) } } - if (getenv("VTYSH_LOG")) { - const char *logpath = getenv("VTYSH_LOG"); - + if (logpath != NULL) { logfile = fopen(logpath, "a"); if (!logfile) { fprintf(stderr, "Failed to open logfile (%s): %s\n", From 28237d73adee188b9d518a1f7bd9aaeb14f7b1e7 Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Wed, 20 Mar 2024 15:32:30 -0400 Subject: [PATCH 44/46] zebra: Attempt to explain the rnh tracking code better I got asked today what was going on in the rnh code. I had to take time off of what I was doing and rewrap my head around this code, since it's been a long time. As that this question may come up again in the future I am trying to document this better so that someone coming behind us will be able to read this and get a better idea of what the algorithm is attempting to do. Signed-off-by: Donald Sharp --- zebra/zebra_rib.c | 68 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/zebra/zebra_rib.c b/zebra/zebra_rib.c index b8d55012d5fe..72421dc8ee15 100644 --- a/zebra/zebra_rib.c +++ b/zebra/zebra_rib.c @@ -795,13 +795,77 @@ void zebra_rib_evaluate_rn_nexthops(struct route_node *rn, uint32_t seq, struct rnh *rnh; /* - * We are storing the rnh's associated withb - * the tracked nexthop as a list of the rn's. + * We are storing the rnh's associated with + * the tracked nexthop as a list of the rnh's + * on the rn that we have matched to. As an + * example if you have these rnh's: + * rnh 1.1.1.1 + * rnh 1.1.1.2 + * rnh 1.1.3.4 + * rnh 4.5.6.7 + * Now imagine that you have in the tree these + * prefix's: + * 1.1.1.1/32 + * 1.1.1.0/24 + * 1.1.0.0/16 + * 0.0.0.0/0 + * + * The 1.1.1.1 rnh would be stored on 1.1.1.1/32 + * The 1.1.1.2 rnh would be stored on 1.1.1.0/24 + * The 1.1.3.4 rnh would be stored on the 1.1.0.0/16 + * and finally the 4.5.6.7 would be stored on the 0.0.0.0/0 + * prefix. + * * Unresolved rnh's are placed at the top * of the tree list.( 0.0.0.0/0 for v4 and 0::0/0 for v6 ) * As such for each rn we need to walk up the tree * and see if any rnh's need to see if they * would match a more specific route + * + * Now if a 1.1.1.2/32 prefix was added to the tree + * this function would start at this new node and + * see that the 1.1.1.2/32 node has no rnh's and + * there is nothing to do on this node currently, + * so the function would walk the parent pointers, until the + * 1.1.1.0/24 node is hit with the 1.1.1.2 rnh. This function + * would then call zebra_evaluate_rnh() which would then + * do a LPM and match on the 1.1.1.2/32 node. This function + * would then pull the 1.1.1.2 rnh off the 1.1.1.0/24 node + * and place it on the 1.1.1.1/32 node and notify the upper + * level protocols interested about the change( as necessary ). + * At this point in time a sequence number is added to note + * that the rnh has been moved. + * The function would also continue to walk up the tree + * looking at the list of rnh's and moving them around + * as necessary. Since in this example nothing else + * would change no further actions are made. + * + * Another case to consider is a node being deleted + * suppose the 1.1.1.2/32 route is being deleted. + * This function would start at the 1.1.1.1/32 node, + * perform a LPM and settle on the 1.1.1.0/24 node + * as where it belongs. The code would update appropriate + * interested parties and additionally also mark the sequence + * number and walk up the tree. Eventually it would get to + * the 1.1.1.0/24 node and since the seqno matches we would + * know that it is not necessary to reconsider this node + * as it was already moved to this spot. + * + * This all works because each node's parent pointer points + * to a node that has a prefix that contains this node. Eventually + * the parent traversal will hit the 0.0.0.0/0 node and we know + * we are done. We know this is pretty efficient because when + * a more specific is added as we walk the tree we can + * find the rnh's that matched to a less specific very easily + * and move them to a more specific node. Also vice-versa as a + * more specific node is removed. + * + * Long term the rnh code might be improved some as the rnh's + * are stored as a list. This might be transformed to a better + * data structure. This has not proven to be necessary yet as + * that we have not seen any particular case where a rn is + * storing more than a couple rnh's. If we find a case + * where this matters something might need to be done. */ while (rn) { if (IS_ZEBRA_DEBUG_NHT_DETAILED) From 5b6ff51b8ae7f8c7348cea4de9543956f32641a7 Mon Sep 17 00:00:00 2001 From: Enke Chen Date: Tue, 15 Oct 2024 10:23:10 -0700 Subject: [PATCH 45/46] zebra: unlock node only after operation in zebra_free_rnh() Move route_unlock_node() after rnh_list_del(). Signed-off-by: Enke Chen --- zebra/zebra_rnh.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/zebra/zebra_rnh.c b/zebra/zebra_rnh.c index 89317be74d37..7c25fb3ef39d 100644 --- a/zebra/zebra_rnh.c +++ b/zebra/zebra_rnh.c @@ -220,10 +220,9 @@ void zebra_free_rnh(struct rnh *rnh) if (rern) { rib_dest_t *dest; - route_unlock_node(rern); - dest = rib_dest_from_rnode(rern); rnh_list_del(&dest->nht, rnh); + route_unlock_node(rern); } } free_state(rnh->vrf_id, rnh->state, rnh->node); From 573d807adf17ae979df60f999fa0e8e8e6ff985a Mon Sep 17 00:00:00 2001 From: Donald Sharp Date: Tue, 15 Oct 2024 18:32:14 -0400 Subject: [PATCH 46/46] ospfd: Fixup ospf_lsa.[ch] to properly spell out parameters for functions Our standard says when in a .h file and declaring a function all parameters must have their variables spelled out. Let's do this for ospf_lsa.h Modified ospf_lsa.c to also use event instead of thread, and uint8_t instead of unsigned short. Signed-off-by: Donald Sharp --- ospfd/ospf_lsa.c | 17 +++--- ospfd/ospf_lsa.h | 149 ++++++++++++++++++++++------------------------- 2 files changed, 77 insertions(+), 89 deletions(-) diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c index f125fa93b1e4..a8b8de6d5ba0 100644 --- a/ospfd/ospf_lsa.c +++ b/ospfd/ospf_lsa.c @@ -2613,8 +2613,7 @@ void ospf_external_lsa_refresh_default(struct ospf *ospf) } } -void ospf_external_lsa_refresh_type(struct ospf *ospf, uint8_t type, - unsigned short instance, int force) +void ospf_external_lsa_refresh_type(struct ospf *ospf, uint8_t type, uint8_t instance, int force) { struct route_node *rn; struct external_info *ei; @@ -3170,9 +3169,9 @@ int ospf_check_nbr_status(struct ospf *ospf) } -void ospf_maxage_lsa_remover(struct event *thread) +void ospf_maxage_lsa_remover(struct event *event) { - struct ospf *ospf = EVENT_ARG(thread); + struct ospf *ospf = EVENT_ARG(event); struct ospf_lsa *lsa, *old; struct route_node *rn; int reschedule = 0; @@ -3202,7 +3201,7 @@ void ospf_maxage_lsa_remover(struct event *thread) } /* TODO: maybe convert this function to a work-queue */ - if (event_should_yield(thread)) { + if (event_should_yield(event)) { OSPF_TIMER_ON(ospf->t_maxage, ospf_maxage_lsa_remover, 0); route_unlock_node( @@ -3418,9 +3417,9 @@ static int ospf_lsa_maxage_walker_remover(struct ospf *ospf, } /* Periodical check of MaxAge LSA. */ -void ospf_lsa_maxage_walker(struct event *thread) +void ospf_lsa_maxage_walker(struct event *event) { - struct ospf *ospf = EVENT_ARG(thread); + struct ospf *ospf = EVENT_ARG(event); struct route_node *rn; struct ospf_lsa *lsa; struct ospf_area *area; @@ -4157,11 +4156,11 @@ void ospf_refresher_unregister_lsa(struct ospf *ospf, struct ospf_lsa *lsa) } } -void ospf_lsa_refresh_walker(struct event *t) +void ospf_lsa_refresh_walker(struct event *e) { struct list *refresh_list; struct listnode *node, *nnode; - struct ospf *ospf = EVENT_ARG(t); + struct ospf *ospf = EVENT_ARG(e); struct ospf_lsa *lsa; int i; struct list *lsa_to_refresh = list_new(); diff --git a/ospfd/ospf_lsa.h b/ospfd/ospf_lsa.h index d5ca0694ccb2..c751f081fb92 100644 --- a/ospfd/ospf_lsa.h +++ b/ospfd/ospf_lsa.h @@ -225,122 +225,111 @@ enum lsid_status { LSID_AVAILABLE = 0, LSID_CHANGE, LSID_NOT_AVAILABLE }; /* Prototypes. */ /* XXX: Eek, time functions, similar are in lib/thread.c */ extern struct timeval int2tv(int); -extern struct timeval msec2tv(int); +extern struct timeval msec2tv(int a); -extern int get_age(struct ospf_lsa *); -extern uint16_t ospf_lsa_checksum(struct lsa_header *); -extern int ospf_lsa_checksum_valid(struct lsa_header *); -extern int ospf_lsa_refresh_delay(struct ospf_lsa *); +extern int get_age(struct ospf_lsa *lsa); +extern uint16_t ospf_lsa_checksum(struct lsa_header *lsah); +extern int ospf_lsa_checksum_valid(struct lsa_header *lsah); +extern int ospf_lsa_refresh_delay(struct ospf_lsa *lsa); -extern const char *dump_lsa_key(struct ospf_lsa *); -extern uint32_t lsa_seqnum_increment(struct ospf_lsa *); -extern void lsa_header_set(struct stream *, uint8_t, uint8_t, struct in_addr, - struct in_addr); -extern struct ospf_neighbor *ospf_nbr_lookup_ptop(struct ospf_interface *); -extern int ospf_check_nbr_status(struct ospf *); +extern const char *dump_lsa_key(struct ospf_lsa *lsa); +extern uint32_t lsa_seqnum_increment(struct ospf_lsa *lsa); +extern void lsa_header_set(struct stream *s, uint8_t options, uint8_t type, struct in_addr id, + struct in_addr router_id); +extern struct ospf_neighbor *ospf_nbr_lookup_ptop(struct ospf_interface *oi); +extern int ospf_check_nbr_status(struct ospf *ospf); /* Prototype for LSA primitive. */ extern struct ospf_lsa *ospf_lsa_new(void); extern struct ospf_lsa *ospf_lsa_new_and_data(size_t size); -extern struct ospf_lsa *ospf_lsa_dup(struct ospf_lsa *); -extern void ospf_lsa_free(struct ospf_lsa *); -extern struct ospf_lsa *ospf_lsa_lock(struct ospf_lsa *); -extern void ospf_lsa_unlock(struct ospf_lsa **); -extern void ospf_lsa_discard(struct ospf_lsa *); -extern int ospf_lsa_flush_schedule(struct ospf *, struct ospf_lsa *); -extern struct lsa_header *ospf_lsa_data_new(size_t); -extern struct lsa_header *ospf_lsa_data_dup(struct lsa_header *); -extern void ospf_lsa_data_free(struct lsa_header *); +extern struct ospf_lsa *ospf_lsa_dup(struct ospf_lsa *lsa); +extern void ospf_lsa_free(struct ospf_lsa *lsa); +extern struct ospf_lsa *ospf_lsa_lock(struct ospf_lsa *lsa); +extern void ospf_lsa_unlock(struct ospf_lsa **lsa); +extern void ospf_lsa_discard(struct ospf_lsa *lsa); +extern int ospf_lsa_flush_schedule(struct ospf *ospf, struct ospf_lsa *lsa); +extern struct lsa_header *ospf_lsa_data_new(size_t size); +extern struct lsa_header *ospf_lsa_data_dup(struct lsa_header *lsah); +extern void ospf_lsa_data_free(struct lsa_header *lsah); /* Prototype for various LSAs */ extern void ospf_router_lsa_body_set(struct stream **s, struct ospf_area *area); extern uint8_t router_lsa_flags(struct ospf_area *area); -extern int ospf_router_lsa_update(struct ospf *); -extern int ospf_router_lsa_update_area(struct ospf_area *); +extern int ospf_router_lsa_update(struct ospf *ospf); +extern int ospf_router_lsa_update_area(struct ospf_area *area); -extern void ospf_network_lsa_update(struct ospf_interface *); +extern void ospf_network_lsa_update(struct ospf_interface *oi); -extern struct ospf_lsa * -ospf_summary_lsa_originate(struct prefix_ipv4 *, uint32_t, struct ospf_area *); -extern struct ospf_lsa *ospf_summary_asbr_lsa_originate(struct prefix_ipv4 *, - uint32_t, - struct ospf_area *); +extern struct ospf_lsa *ospf_summary_lsa_originate(struct prefix_ipv4 *p, uint32_t metric, + struct ospf_area *area); +extern struct ospf_lsa *ospf_summary_asbr_lsa_originate(struct prefix_ipv4 *p, uint32_t metric, + struct ospf_area *area); -extern struct ospf_lsa *ospf_lsa_install(struct ospf *, struct ospf_interface *, - struct ospf_lsa *); +extern struct ospf_lsa *ospf_lsa_install(struct ospf *ospf, struct ospf_interface *oi, + struct ospf_lsa *lsa); extern void ospf_nssa_lsa_flush(struct ospf *ospf, struct prefix_ipv4 *p); -extern void ospf_external_lsa_flush(struct ospf *, uint8_t, - struct prefix_ipv4 *, +extern void ospf_external_lsa_flush(struct ospf *ospf, uint8_t type, struct prefix_ipv4 *p, ifindex_t /* , struct in_addr nexthop */); -extern struct in_addr ospf_get_ip_from_ifp(struct ospf_interface *); +extern struct in_addr ospf_get_ip_from_ifp(struct ospf_interface *oi); -extern struct ospf_lsa *ospf_external_lsa_originate(struct ospf *, - struct external_info *); +extern struct ospf_lsa *ospf_external_lsa_originate(struct ospf *ospf, struct external_info *ei); extern struct ospf_lsa *ospf_nssa_lsa_originate(struct ospf_area *area, struct external_info *ei); extern struct ospf_lsa *ospf_nssa_lsa_refresh(struct ospf_area *area, struct ospf_lsa *lsa, struct external_info *ei); extern void ospf_external_lsa_rid_change(struct ospf *ospf); -extern struct ospf_lsa *ospf_lsa_lookup(struct ospf *ospf, struct ospf_area *, - uint32_t, struct in_addr, - struct in_addr); -extern struct ospf_lsa *ospf_lsa_lookup_by_id(struct ospf_area *, uint32_t, - struct in_addr); -extern struct ospf_lsa *ospf_lsa_lookup_by_header(struct ospf_area *, - struct lsa_header *); -extern int ospf_lsa_more_recent(struct ospf_lsa *, struct ospf_lsa *); -extern int ospf_lsa_different(struct ospf_lsa *, struct ospf_lsa *, - bool ignore_rcvd_flag); -extern void ospf_flush_self_originated_lsas_now(struct ospf *); - -extern int ospf_lsa_is_self_originated(struct ospf *, struct ospf_lsa *); - -extern struct ospf_lsa *ospf_lsa_lookup_by_prefix(struct ospf_lsdb *, uint8_t, - struct prefix_ipv4 *, - struct in_addr); - -extern void ospf_lsa_maxage(struct ospf *, struct ospf_lsa *); -extern uint32_t get_metric(uint8_t *); - -extern void ospf_lsa_maxage_walker(struct event *thread); -extern struct ospf_lsa *ospf_lsa_refresh(struct ospf *, struct ospf_lsa *); - -extern void ospf_external_lsa_refresh_default(struct ospf *); - -extern void ospf_external_lsa_refresh_type(struct ospf *, uint8_t, - unsigned short, int); -extern struct ospf_lsa *ospf_external_lsa_refresh(struct ospf *, - struct ospf_lsa *, - struct external_info *, int, - bool aggr); +extern struct ospf_lsa *ospf_lsa_lookup(struct ospf *ospf, struct ospf_area *area, uint32_t type, + struct in_addr id, struct in_addr adv_router); +extern struct ospf_lsa *ospf_lsa_lookup_by_id(struct ospf_area *area, uint32_t type, + struct in_addr id); +extern struct ospf_lsa *ospf_lsa_lookup_by_header(struct ospf_area *area, struct lsa_header *lsah); +extern int ospf_lsa_more_recent(struct ospf_lsa *l1, struct ospf_lsa *l2); +extern int ospf_lsa_different(struct ospf_lsa *l1, struct ospf_lsa *l2, bool ignore_rcvd_flag); +extern void ospf_flush_self_originated_lsas_now(struct ospf *ospf); + +extern int ospf_lsa_is_self_originated(struct ospf *ospf, struct ospf_lsa *lsa); + +extern struct ospf_lsa *ospf_lsa_lookup_by_prefix(struct ospf_lsdb *lsdb, uint8_t type, + struct prefix_ipv4 *p, struct in_addr router_id); + +extern void ospf_lsa_maxage(struct ospf *ospf, struct ospf_lsa *lsa); +extern uint32_t get_metric(uint8_t *metric); + +extern void ospf_lsa_maxage_walker(struct event *event); +extern struct ospf_lsa *ospf_lsa_refresh(struct ospf *ospf, struct ospf_lsa *lsa); + +extern void ospf_external_lsa_refresh_default(struct ospf *ospf); + +extern void ospf_external_lsa_refresh_type(struct ospf *ospf, uint8_t type, uint8_t instance, + int force); +extern struct ospf_lsa *ospf_external_lsa_refresh(struct ospf *ospf, struct ospf_lsa *lsa, + struct external_info *ei, int force, bool aggr); extern enum lsid_status ospf_lsa_unique_id(struct ospf *ospf, struct ospf_lsdb *lsdb, uint8_t type, struct prefix_ipv4 *p, struct in_addr *addr); -extern void ospf_schedule_lsa_flood_area(struct ospf_area *, struct ospf_lsa *); -extern void ospf_schedule_lsa_flush_area(struct ospf_area *, struct ospf_lsa *); +extern void ospf_schedule_lsa_flood_area(struct ospf_area *area, struct ospf_lsa *lsa); +extern void ospf_schedule_lsa_flush_area(struct ospf_area *area, struct ospf_lsa *lsa); -extern void ospf_refresher_register_lsa(struct ospf *, struct ospf_lsa *); -extern void ospf_refresher_unregister_lsa(struct ospf *, struct ospf_lsa *); -extern void ospf_lsa_refresh_walker(struct event *thread); +extern void ospf_refresher_register_lsa(struct ospf *ospf, struct ospf_lsa *lsa); +extern void ospf_refresher_unregister_lsa(struct ospf *ospf, struct ospf_lsa *lsa); +extern void ospf_lsa_refresh_walker(struct event *event); -extern void ospf_lsa_maxage_delete(struct ospf *, struct ospf_lsa *); +extern void ospf_lsa_maxage_delete(struct ospf *ospf, struct ospf_lsa *lsa); -extern void ospf_discard_from_db(struct ospf *, struct ospf_lsdb *, - struct ospf_lsa *); +extern void ospf_discard_from_db(struct ospf *ospf, struct ospf_lsdb *lsdb, struct ospf_lsa *lsa); -extern int metric_type(struct ospf *, uint8_t, unsigned short); -extern int metric_value(struct ospf *, uint8_t, unsigned short); +extern int metric_type(struct ospf *ospf, uint8_t src, unsigned short instance); +extern int metric_value(struct ospf *ospf, uint8_t src, unsigned short instance); extern char link_info_set(struct stream **s, struct in_addr id, struct in_addr data, uint8_t type, uint8_t tos, uint16_t cost); -extern struct in_addr ospf_get_nssa_ip(struct ospf_area *); -extern int ospf_translated_nssa_compare(struct ospf_lsa *, struct ospf_lsa *); +extern struct in_addr ospf_get_nssa_ip(struct ospf_area *area); extern struct ospf_lsa *ospf_translated_nssa_refresh(struct ospf *ospf, struct ospf_lsa *type7, struct ospf_lsa *type5); @@ -351,7 +340,7 @@ extern void ospf_check_and_gen_init_seq_lsa(struct ospf_interface *oi, struct ospf_lsa *lsa); extern void ospf_flush_lsa_from_area(struct ospf *ospf, struct in_addr area_id, int type); -extern void ospf_maxage_lsa_remover(struct event *thread); +extern void ospf_maxage_lsa_remover(struct event *event); extern bool ospf_check_dna_lsa(const struct ospf_lsa *lsa); extern void ospf_refresh_area_self_lsas(struct ospf_area *area);