diff options
64 files changed, 1062 insertions, 192 deletions
diff --git a/.gitignore b/.gitignore index 3dd6a44409..a66e3ccd3c 100644 --- a/.gitignore +++ b/.gitignore @@ -106,6 +106,7 @@ GSYMS GRTAGS GPATH compile_commands.json +.ccls .ccls-cache .dirstamp refix diff --git a/bfdd/bfd_packet.c b/bfdd/bfd_packet.c index 311ce4d379..ea7a1038ae 100644 --- a/bfdd/bfd_packet.c +++ b/bfdd/bfd_packet.c @@ -878,7 +878,7 @@ void bfd_recv_cb(struct event *t) /* * We may have a situation where received packet is on wrong vrf */ - if (bfd && bfd->vrf && bfd->vrf != bvrf->vrf) { + if (bfd && bfd->vrf && bfd->vrf->vrf_id != vrfid) { cp_debug(is_mhop, &peer, &local, ifindex, vrfid, "wrong vrfid."); return; diff --git a/bfdd/bfdd.c b/bfdd/bfdd.c index 5fbe2f48f6..95066b97ce 100644 --- a/bfdd/bfdd.c +++ b/bfdd/bfdd.c @@ -28,8 +28,8 @@ * FRR related code. */ DEFINE_MGROUP(BFDD, "Bidirectional Forwarding Detection Daemon"); -DEFINE_MTYPE(BFDD, BFDD_CONTROL, "long-lived control socket memory"); -DEFINE_MTYPE(BFDD, BFDD_NOTIFICATION, "short-lived control notification data"); +DEFINE_MTYPE(BFDD, BFDD_CONTROL, "control socket memory"); +DEFINE_MTYPE(BFDD, BFDD_NOTIFICATION, "control notification data"); /* Master of threads. */ struct event_loop *master; diff --git a/bgpd/bgp_attr.c b/bgpd/bgp_attr.c index 195298c815..d5223a1e6e 100644 --- a/bgpd/bgp_attr.c +++ b/bgpd/bgp_attr.c @@ -1082,9 +1082,6 @@ struct attr *bgp_attr_aggregate_intern( attr.aspath = aspath_empty(bgp->asnotation); attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_AS_PATH); - /* Next hop attribute. */ - attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP); - if (community) { uint32_t gshut = COMMUNITY_GSHUT; @@ -1121,6 +1118,21 @@ struct attr *bgp_attr_aggregate_intern( attr.aggregator_as = bgp->as; attr.aggregator_addr = bgp->router_id; + /* Aggregate are done for IPv4/IPv6 so checking ipv4 family, + * This should only be set for IPv4 AFI type + * based on RFC-4760: + * "An UPDATE message that carries no NLRI, + * other than the one encoded in + * the MP_REACH_NLRI attribute, + * SHOULD NOT carry the NEXT_HOP + * attribute" + */ + if (p->family == AF_INET) { + /* Next hop attribute. */ + attr.flag |= ATTR_FLAG_BIT(BGP_ATTR_NEXT_HOP); + attr.mp_nexthop_len = IPV4_MAX_BYTELEN; + } + /* Apply route-map */ if (aggregate->rmap.name) { struct attr attr_tmp = attr; diff --git a/bgpd/bgp_evpn_vty.c b/bgpd/bgp_evpn_vty.c index 811856bfed..66079cad22 100644 --- a/bgpd/bgp_evpn_vty.c +++ b/bgpd/bgp_evpn_vty.c @@ -3217,7 +3217,14 @@ int bgp_evpn_show_all_routes(struct vty *vty, struct bgp *bgp, int type, evpn_show_all_routes(vty, bgp, type, json, detail, false); if (use_json) - vty_json(vty, json); + /* + * We are using no_pretty here because under extremely high + * settings (lots of routes with many different paths) this can + * save several minutes of output when FRR is run on older cpu's + * or more underperforming routers out there. So for route + * scale, we need to use no_pretty json. + */ + vty_json_no_pretty(vty, json); return CMD_SUCCESS; } diff --git a/bgpd/bgp_route.c b/bgpd/bgp_route.c index 4e4dce84fe..85f08bbed5 100644 --- a/bgpd/bgp_route.c +++ b/bgpd/bgp_route.c @@ -1466,7 +1466,7 @@ int bgp_evpn_path_info_cmp(struct bgp *bgp, struct bgp_path_info *new, struct bgp_path_info *exist, int *paths_eq) { enum bgp_path_selection_reason reason; - char pfx_buf[PREFIX2STR_BUFFER]; + char pfx_buf[PREFIX2STR_BUFFER] = {}; return bgp_path_info_cmp(bgp, new, exist, paths_eq, NULL, 0, pfx_buf, AFI_L2VPN, SAFI_EVPN, &reason); @@ -2653,7 +2653,7 @@ void bgp_best_selection(struct bgp *bgp, struct bgp_dest *dest, struct bgp_path_info *nextpi = NULL; int paths_eq, do_mpath, debug; struct list mp_list; - char pfx_buf[PREFIX2STR_BUFFER]; + char pfx_buf[PREFIX2STR_BUFFER] = {}; char path_buf[PATH_ADDPATH_STR_BUFFER]; bgp_mp_list_init(&mp_list); @@ -7418,7 +7418,7 @@ static bool bgp_aggregate_info_same(struct bgp_path_info *pi, uint8_t origin, asnotation = bgp_get_asnotation(NULL); - if (!ae) + if (!aspath) ae = aspath_empty(asnotation); if (!pi) @@ -7477,8 +7477,8 @@ static void bgp_aggregate_install( * If the aggregate information has not changed * no need to re-install it again. */ - if (bgp_aggregate_info_same(orig, origin, aspath, community, - ecommunity, lcommunity)) { + if (pi && bgp_aggregate_info_same(pi, origin, aspath, community, + ecommunity, lcommunity)) { bgp_dest_unlock_node(dest); if (aspath) @@ -13068,6 +13068,15 @@ DEFPY(show_ip_bgp, show_ip_bgp_cmd, get_afi_safi_str(afi, safi, true)); + + /* Adding 'routes' key to make + * the json output format valid + * for evpn + */ + if (safi == SAFI_EVPN) + vty_out(vty, + "\"routes\":"); + } else vty_out(vty, "\nFor address family: %s\n", diff --git a/bgpd/rfapi/rfapi_import.c b/bgpd/rfapi/rfapi_import.c index 4b8e07a9c4..27f7c88d7b 100644 --- a/bgpd/rfapi/rfapi_import.c +++ b/bgpd/rfapi/rfapi_import.c @@ -1939,7 +1939,7 @@ static void rfapiBgpInfoAttachSorted(struct agg_node *rn, struct bgp *bgp; struct bgp_path_info *prev; struct bgp_path_info *next; - char pfx_buf[PREFIX2STR_BUFFER]; + char pfx_buf[PREFIX2STR_BUFFER] = {}; bgp = bgp_get_default(); /* assume 1 instance for now */ diff --git a/configure.ac b/configure.ac index b9af768641..0120c517c6 100644 --- a/configure.ac +++ b/configure.ac @@ -794,6 +794,9 @@ if test "$ac_cv_lib_json_c_json_object_get" = "no"; then fi ]) +AC_ARG_ENABLE([ccls], +AS_HELP_STRING([--enable-ccls], [Write .ccls config for this build])) + AC_ARG_ENABLE([dev_build], AS_HELP_STRING([--enable-dev-build], [build for development])) @@ -2820,6 +2823,42 @@ AC_CONFIG_FILES([tools/frrcommon.sh]) AC_CONFIG_FILES([tools/frr.service]) AC_CONFIG_FILES([tools/frr@.service]) +# dnl write out a ccls file with our compile configuration +# dnl have to add -Wno-unused-function otherwise foobar_cmd_magic causes +# dnl all DEFPY(), et al., macros to flag as errors. +AS_IF([test "$enable_ccls" = "yes"], [ + AC_CONFIG_COMMANDS([gen-dot-ccls], [ + cat > "${srcdir}/.ccls" <<EOF +clang +-DHAVE_CONFIG_H +-I. +-I./include +-I./lib +-I./lib/assert +-DSYSCONFDIR="${ac_frr_sysconfdir}" +-DCONFDATE=${ac_frr_confdate} +EOF + if test "$ac_abs_top_builddir" != "$ac_abs_top_srcdir"; then + echo "-I${ac_abs_top_builddir}" >> "${srcdir}/.ccls" + fi + if test -n "$FRR_ALL_CCLS_FLAGS"; then + echo ${FRR_ALL_CCLS_FLAGS} | tr ' ' '\n' >> "${srcdir}/.ccls" + fi + if test -n "$FRR_ALL_CCLS_CFLAGS"; then + cat >> "${srcdir}/.ccls" <<EOF +%c $(echo ${FRR_ALL_CCLS_CFLAGS} | sed -e 's/ */\n%c /g') +%c -Wno-unused-function +EOF +fi + ], [ + FRR_ALL_CCLS_FLAGS="$(echo ${LIBYANG_CFLAGS} ${LUA_INCLUDE} ${SQLITE3_CFLAGS} | sed -e 's/ */ /g')" + FRR_ALL_CCLS_CFLAGS="$(echo ${CFLAGS} ${WERROR} ${AC_CFLAGS} ${SAN_FLAGS} | sed -e 's/ */ /g')" + ac_frr_confdate="${CONFDATE}" + ac_frr_sysconfdir="${sysconfdir}/" + ]) +]) + + AS_IF([test "$with_pkg_git_version" = "yes"], [ AC_CONFIG_COMMANDS([lib/gitversion.h], [ dst="${ac_abs_top_builddir}/lib/gitversion.h" diff --git a/doc/developer/include-compile.rst b/doc/developer/include-compile.rst index 513cac6179..b98d237e68 100644 --- a/doc/developer/include-compile.rst +++ b/doc/developer/include-compile.rst @@ -17,7 +17,6 @@ obtained by running ``./configure -h``. The options shown below are examples. --localstatedir=/var/run/frr \ --sysconfdir=/etc/frr \ --with-moduledir=\${prefix}/lib/frr/modules \ - --with-libyang-pluginsdir=\${prefix}/lib/frr/libyang_plugins \ --enable-configfile-mask=0640 \ --enable-logfile-mask=0640 \ --enable-snmp=agentx \ diff --git a/doc/developer/topotests.rst b/doc/developer/topotests.rst index 7b1fde7b53..f44cf9df98 100644 --- a/doc/developer/topotests.rst +++ b/doc/developer/topotests.rst @@ -17,13 +17,23 @@ Tested with Ubuntu 20.04,Ubuntu 18.04, and Debian 11. Instructions are the same for all setups (i.e. ExaBGP is only used for BGP tests). +Tshark is only required if you enable any packet captures on test runs. + +Valgrind is only required if you enable valgrind on test runs. + Installing Topotest Requirements ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ .. code:: shell - apt-get install gdb iproute2 net-tools python3-pip \ - iputils-ping valgrind + apt-get install \ + gdb \ + iproute2 \ + net-tools \ + python3-pip \ + iputils-ping \ + tshark \ + valgrind python3 -m pip install wheel python3 -m pip install 'pytest>=6.2.4' python3 -m pip install 'pytest-xdist>=2.3.0' diff --git a/doc/user/installation.rst b/doc/user/installation.rst index 2310d397cd..8e8fb24608 100644 --- a/doc/user/installation.rst +++ b/doc/user/installation.rst @@ -390,6 +390,18 @@ and the configuration files in :file:`/usr/local/etc`. The :file:`/usr/local/` installation prefix and other directories may be changed using the following options to the configuration script. +.. option:: --enable-ccls + + Enable the creation of a :file:`.ccls` file in the top level source + directory. + + Some development environments (e.g., LSP server within emacs, et al.) can + utilize :clicmd:`ccls` to provide highly sophisticated IDE features (e.g., + semantically accurate jump-to definition/reference, and even code + refactoring). The `--enable-ccls` causes :file:`configure` to generate a + configuration for the :clicmd:`ccls` command, based on the configured + FRR build environment. + .. option:: --prefix <prefix> Install architecture-independent files in `prefix` [/usr/local]. diff --git a/doc/user/ripngd.rst b/doc/user/ripngd.rst index df7a0e249e..4c9b734d88 100644 --- a/doc/user/ripngd.rst +++ b/doc/user/ripngd.rst @@ -38,6 +38,10 @@ Currently ripngd supports the following commands: Set RIPng static routing announcement of NETWORK. +.. clicmd:: allow-ecmp [1-MULTIPATH_NUM] + + Control how many ECMP paths RIPng can inject for the same prefix. If specified + without a number, a maximum is taken (compiled with ``--enable-multipath``). .. _ripngd-terminal-mode-commands: diff --git a/lib/grammar_sandbox.c b/lib/grammar_sandbox.c index fc7ebebbe5..24833e08ec 100644 --- a/lib/grammar_sandbox.c +++ b/lib/grammar_sandbox.c @@ -19,7 +19,7 @@ #define GRAMMAR_STR "CLI grammar sandbox\n" -DEFINE_MTYPE_STATIC(LIB, CMD_TOKENS, "Command desc"); +DEFINE_MTYPE_STATIC(LIB, CMD_DESCRIPTIONS, "Command desc"); /** headers **/ void grammar_sandbox_init(void); @@ -53,7 +53,7 @@ DEFUN (grammar_test, // create cmd_element for parser struct cmd_element *cmd = - XCALLOC(MTYPE_CMD_TOKENS, sizeof(struct cmd_element)); + XCALLOC(MTYPE_CMD_DESCRIPTIONS, sizeof(struct cmd_element)); cmd->string = command; cmd->doc = "0\n1\n2\n3\n4\n5\n6\n7\n8\n9\n10\n11\n12\n13\n14\n15\n16\n17\n18\n19\n"; @@ -207,11 +207,11 @@ DEFUN (grammar_test_doc, // create cmd_element with docstring struct cmd_element *cmd = - XCALLOC(MTYPE_CMD_TOKENS, sizeof(struct cmd_element)); + XCALLOC(MTYPE_CMD_DESCRIPTIONS, sizeof(struct cmd_element)); cmd->string = XSTRDUP( - MTYPE_CMD_TOKENS, + MTYPE_CMD_DESCRIPTIONS, "test docstring <example|selector follow> (1-255) end VARIABLE [OPTION|set lol] . VARARG"); - cmd->doc = XSTRDUP(MTYPE_CMD_TOKENS, + cmd->doc = XSTRDUP(MTYPE_CMD_DESCRIPTIONS, "Test stuff\n" "docstring thing\n" "first example\n" diff --git a/lib/mgmt_be_client.c b/lib/mgmt_be_client.c index 9427f7cf3d..f74cf6ba09 100644 --- a/lib/mgmt_be_client.c +++ b/lib/mgmt_be_client.c @@ -26,9 +26,8 @@ #define MGMTD_DBG_BE_CLIENT_CHECK() \ DEBUG_MODE_CHECK(&mgmt_dbg_be_client, DEBUG_MODE_ALL) -DEFINE_MTYPE_STATIC(LIB, MGMTD_BE_BATCH, - "MGMTD backend transaction batch data"); -DEFINE_MTYPE_STATIC(LIB, MGMTD_BE_TXN, "MGMTD backend transaction data"); +DEFINE_MTYPE_STATIC(LIB, MGMTD_BE_BATCH, "backend transaction batch data"); +DEFINE_MTYPE_STATIC(LIB, MGMTD_BE_TXN, "backend transaction data"); enum mgmt_be_txn_event { MGMTD_BE_TXN_PROC_SETCFG = 1, diff --git a/lib/plist.c b/lib/plist.c index e286a32f92..d8ce83d219 100644 --- a/lib/plist.c +++ b/lib/plist.c @@ -336,6 +336,22 @@ prefix_list_entry_lookup(struct prefix_list *plist, struct prefix *prefix, return NULL; } +static bool +prefix_list_entry_lookup_prefix(struct prefix_list *plist, + struct prefix_list_entry *plist_entry) +{ + struct prefix_list_entry *pentry = NULL; + + for (pentry = plist->head; pentry; pentry = pentry->next) { + if (pentry == plist_entry) + continue; + if (prefix_same(&pentry->prefix, &plist_entry->prefix)) + return true; + } + + return false; +} + static void trie_walk_affected(size_t validbits, struct pltrie_table *table, uint8_t byte, struct prefix_list_entry *object, void (*fn)(struct prefix_list_entry *object, @@ -404,12 +420,16 @@ static void prefix_list_trie_del(struct prefix_list *plist, void prefix_list_entry_delete(struct prefix_list *plist, - struct prefix_list_entry *pentry, - int update_list) + struct prefix_list_entry *pentry, int update_list) { + bool duplicate = false; + if (plist == NULL || pentry == NULL) return; + if (prefix_list_entry_lookup_prefix(plist, pentry)) + duplicate = true; + prefix_list_trie_del(plist, pentry); if (pentry->prev) @@ -421,8 +441,10 @@ void prefix_list_entry_delete(struct prefix_list *plist, else plist->tail = pentry->prev; - route_map_notify_pentry_dependencies(plist->name, pentry, - RMAP_EVENT_PLIST_DELETED); + if (!duplicate) + route_map_notify_pentry_dependencies(plist->name, pentry, + RMAP_EVENT_PLIST_DELETED); + prefix_list_entry_free(pentry); plist->count--; @@ -557,11 +579,15 @@ static void prefix_list_entry_add(struct prefix_list *plist, void prefix_list_entry_update_start(struct prefix_list_entry *ple) { struct prefix_list *pl = ple->pl; + bool duplicate = false; /* Not installed, nothing to do. */ if (!ple->installed) return; + if (prefix_list_entry_lookup_prefix(pl, ple)) + duplicate = true; + prefix_list_trie_del(pl, ple); /* List manipulation: shameless copy from `prefix_list_entry_delete`. */ @@ -574,8 +600,9 @@ void prefix_list_entry_update_start(struct prefix_list_entry *ple) else pl->tail = ple->prev; - route_map_notify_pentry_dependencies(pl->name, ple, - RMAP_EVENT_PLIST_DELETED); + if (!duplicate) + route_map_notify_pentry_dependencies(pl->name, ple, + RMAP_EVENT_PLIST_DELETED); pl->count--; route_map_notify_dependencies(pl->name, RMAP_EVENT_PLIST_DELETED); diff --git a/lib/printf/glue.c b/lib/printf/glue.c index 25e2ff37a3..f799378af3 100644 --- a/lib/printf/glue.c +++ b/lib/printf/glue.c @@ -273,6 +273,7 @@ static ssize_t printfrr_va(struct fbuf *buf, struct printfrr_eargs *ea, { const struct va_format *vaf = ptr; va_list ap; + ssize_t s; if (!vaf || !vaf->fmt || !vaf->va) return bputs(buf, "NULL"); @@ -285,6 +286,9 @@ static ssize_t printfrr_va(struct fbuf *buf, struct printfrr_eargs *ea, #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat-nonliteral" /* can't format check this */ - return vbprintfrr(buf, vaf->fmt, ap); + s = vbprintfrr(buf, vaf->fmt, ap); #pragma GCC diagnostic pop + va_end(ap); + + return s; } diff --git a/mgmtd/mgmt_ds.c b/mgmtd/mgmt_ds.c index b5eaf7bff6..b8cb5b2b15 100644 --- a/mgmtd/mgmt_ds.c +++ b/mgmtd/mgmt_ds.c @@ -542,7 +542,7 @@ int mgmt_ds_iter_data(struct mgmt_ds_ctx *ds_ctx, char *base_xpath, void *ctx), void *ctx, bool alloc_xp_copy) { - int ret; + int ret = 0; char xpath[MGMTD_MAX_XPATH_LEN]; struct lyd_node *base_dnode = NULL; struct lyd_node *node; diff --git a/mgmtd/mgmt_history.c b/mgmtd/mgmt_history.c index a49718a490..ab84b1efcf 100644 --- a/mgmtd/mgmt_history.c +++ b/mgmtd/mgmt_history.c @@ -35,23 +35,18 @@ DECLARE_DLIST(mgmt_cmt_infos, struct mgmt_cmt_info_t, cmts); */ static struct vty *rollback_vty; -static bool mgmt_history_record_exists(char *file_path) +static bool file_exists(const char *path) { - int exist; - - exist = access(file_path, F_OK); - if (exist == 0) - return true; - else - return false; + return !access(path, F_OK); } -static void mgmt_history_remove_file(char *name) +static void remove_file(const char *path) { - if (remove(name) == 0) - zlog_debug("Old commit info deletion succeeded"); - else - zlog_err("Old commit info deletion failed"); + if (!file_exists(path)) + return; + if (unlink(path)) + zlog_err("Failed to remove commit history file %s: %s", path, + safe_strerror(errno)); } static struct mgmt_cmt_info_t *mgmt_history_new_cmt_info(void) @@ -84,7 +79,7 @@ static struct mgmt_cmt_info_t *mgmt_history_create_cmt_rec(void) last_cmt_info = cmt_info; if (last_cmt_info) { - mgmt_history_remove_file(last_cmt_info->cmt_json_file); + remove_file(last_cmt_info->cmt_json_file); mgmt_cmt_infos_del(&mm->cmts, last_cmt_info); XFREE(MTYPE_MGMTD_CMT_INFO, last_cmt_info); } @@ -114,20 +109,21 @@ static bool mgmt_history_read_cmt_record_index(void) struct mgmt_cmt_info_t *new; int cnt = 0; + if (!file_exists(MGMTD_COMMIT_FILE_PATH)) + return false; + fp = fopen(MGMTD_COMMIT_INDEX_FILE_NAME, "rb"); if (!fp) { - zlog_err("Failed to open file %s rb mode", - MGMTD_COMMIT_INDEX_FILE_NAME); + zlog_err("Failed to open commit history %s for reading: %s", + MGMTD_COMMIT_INDEX_FILE_NAME, safe_strerror(errno)); return false; } while ((fread(&cmt_info, sizeof(cmt_info), 1, fp)) > 0) { if (cnt < MGMTD_MAX_COMMIT_LIST) { - if (!mgmt_history_record_exists( - cmt_info.cmt_json_file)) { - zlog_err( - "Commit record present in index_file, but commit file %s missing", - cmt_info.cmt_json_file); + if (!file_exists(cmt_info.cmt_json_file)) { + zlog_err("Commit in index, but file %s missing", + cmt_info.cmt_json_file); continue; } @@ -136,8 +132,9 @@ static bool mgmt_history_read_cmt_record_index(void) memcpy(new, &cmt_info, sizeof(struct mgmt_cmt_info_t)); mgmt_cmt_infos_add_tail(&mm->cmts, new); } else { - zlog_err("More records found in index file %s", - MGMTD_COMMIT_INDEX_FILE_NAME); + zlog_warn( + "More records found in commit history file %s than expected", + MGMTD_COMMIT_INDEX_FILE_NAME); fclose(fp); return false; } @@ -157,11 +154,10 @@ static bool mgmt_history_dump_cmt_record_index(void) struct mgmt_cmt_info_t cmt_info_set[10]; int cnt = 0; - mgmt_history_remove_file((char *)MGMTD_COMMIT_INDEX_FILE_NAME); - fp = fopen(MGMTD_COMMIT_INDEX_FILE_NAME, "ab"); + fp = fopen(MGMTD_COMMIT_INDEX_FILE_NAME, "wb"); if (!fp) { - zlog_err("Failed to open file %s ab mode", - MGMTD_COMMIT_INDEX_FILE_NAME); + zlog_err("Failed to open commit history %s for writing: %s", + MGMTD_COMMIT_INDEX_FILE_NAME, safe_strerror(errno)); return false; } @@ -179,11 +175,11 @@ static bool mgmt_history_dump_cmt_record_index(void) ret = fwrite(&cmt_info_set, sizeof(struct mgmt_cmt_info_t), cnt, fp); fclose(fp); if (ret != cnt) { - zlog_err("Write record failed"); + zlog_err("Failed to write full commit history, removing file"); + remove_file(MGMTD_COMMIT_INDEX_FILE_NAME); return false; - } else { - return true; } + return true; } static int mgmt_history_rollback_to_cmt(struct vty *vty, @@ -281,7 +277,7 @@ int mgmt_history_rollback_by_id(struct vty *vty, const char *cmtid_str) return ret; } - mgmt_history_remove_file(cmt_info->cmt_json_file); + remove_file(cmt_info->cmt_json_file); mgmt_cmt_infos_del(&mm->cmts, cmt_info); XFREE(MTYPE_MGMTD_CMT_INFO, cmt_info); } @@ -322,7 +318,7 @@ int mgmt_history_rollback_n(struct vty *vty, int num_cmts) } cnt++; - mgmt_history_remove_file(cmt_info->cmt_json_file); + remove_file(cmt_info->cmt_json_file); mgmt_cmt_infos_del(&mm->cmts, cmt_info); XFREE(MTYPE_MGMTD_CMT_INFO, cmt_info); } diff --git a/mgmtd/mgmt_memory.c b/mgmtd/mgmt_memory.c index 2858bc7e45..920ab9363c 100644 --- a/mgmtd/mgmt_memory.c +++ b/mgmtd/mgmt_memory.c @@ -18,19 +18,15 @@ */ DEFINE_MGROUP(MGMTD, "mgmt"); -DEFINE_MTYPE(MGMTD, MGMTD, "MGMTD instance"); -DEFINE_MTYPE(MGMTD, MGMTD_BE_ADPATER, "MGMTD backend adapter"); -DEFINE_MTYPE(MGMTD, MGMTD_FE_ADPATER, "MGMTD Frontend adapter"); -DEFINE_MTYPE(MGMTD, MGMTD_FE_SESSION, "MGMTD Frontend Client Session"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN, "MGMTD Transaction"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN_REQ, "MGMTD Transaction Requests"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN_SETCFG_REQ, - "MGMTD Transaction Set-Config Requests"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN_COMMCFG_REQ, - "MGMTD Transaction Commit-Config Requests"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN_GETDATA_REQ, - "MGMTD Transaction Get-Data Requests"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN_GETDATA_REPLY, - "MGMTD Transaction Get-Data Replies"); -DEFINE_MTYPE(MGMTD, MGMTD_TXN_CFG_BATCH, "MGMTD Transaction Gonfig Batches"); -DEFINE_MTYPE(MGMTD, MGMTD_CMT_INFO, "MGMTD commit info for tracking commits"); +DEFINE_MTYPE(MGMTD, MGMTD, "instance"); +DEFINE_MTYPE(MGMTD, MGMTD_BE_ADPATER, "backend adapter"); +DEFINE_MTYPE(MGMTD, MGMTD_FE_ADPATER, "Frontend adapter"); +DEFINE_MTYPE(MGMTD, MGMTD_FE_SESSION, "Frontend Client Session"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN, "Trnsction"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN_REQ, "Trnsction Requests"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN_SETCFG_REQ, "Trnsction Set-Config Requests"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN_COMMCFG_REQ, "Trnsction Commit-Config Requests"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN_GETDATA_REQ, "Trnsction Get-Data Requests"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN_GETDATA_REPLY, "Trnsction Get-Data Replies"); +DEFINE_MTYPE(MGMTD, MGMTD_TXN_CFG_BATCH, "Trnsction Gonfig Batches"); +DEFINE_MTYPE(MGMTD, MGMTD_CMT_INFO, "info for tracking commits"); diff --git a/ripd/rip_cli.c b/ripd/rip_cli.c index 6f45bb5d9e..097c708ab1 100644 --- a/ripd/rip_cli.c +++ b/ripd/rip_cli.c @@ -91,11 +91,11 @@ DEFUN_YANG (rip_allow_ecmp, "Allow Equal Cost MultiPath\n" "Number of paths\n") { - int idx_number = 1; + int idx_number = 0; char mpaths[3] = {}; uint32_t paths = MULTIPATH_NUM; - if (argv[idx_number]) + if (argv_find(argv, argc, CMD_RANGE_STR(1, MULTIPATH_NUM), &idx_number)) paths = strtol(argv[idx_number]->arg, NULL, 10); snprintf(mpaths, sizeof(mpaths), "%u", paths); diff --git a/ripngd/ripng_cli.c b/ripngd/ripng_cli.c index 5e59dfd2c4..9a96e29313 100644 --- a/ripngd/ripng_cli.c +++ b/ripngd/ripng_cli.c @@ -85,14 +85,32 @@ void cli_show_router_ripng(struct vty *vty, const struct lyd_node *dnode, /* * XPath: /frr-ripngd:ripngd/instance/allow-ecmp */ -DEFPY_YANG (ripng_allow_ecmp, - ripng_allow_ecmp_cmd, - "[no] allow-ecmp", - NO_STR - "Allow Equal Cost MultiPath\n") +DEFUN_YANG (ripng_allow_ecmp, + ripng_allow_ecmp_cmd, + "allow-ecmp [" CMD_RANGE_STR(1, MULTIPATH_NUM) "]", + "Allow Equal Cost MultiPath\n" + "Number of paths\n") +{ + int idx_number = 0; + char mpaths[3] = {}; + uint32_t paths = MULTIPATH_NUM; + + if (argv_find(argv, argc, CMD_RANGE_STR(1, MULTIPATH_NUM), &idx_number)) + paths = strtol(argv[idx_number]->arg, NULL, 10); + snprintf(mpaths, sizeof(mpaths), "%u", paths); + + nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY, mpaths); + + return nb_cli_apply_changes(vty, NULL); +} + +DEFUN_YANG (no_ripng_allow_ecmp, + no_ripng_allow_ecmp_cmd, + "no allow-ecmp [" CMD_RANGE_STR(1, MULTIPATH_NUM) "]", NO_STR + "Allow Equal Cost MultiPath\n" + "Number of paths\n") { - nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY, - no ? "false" : "true"); + nb_cli_enqueue_change(vty, "./allow-ecmp", NB_OP_MODIFY, 0); return nb_cli_apply_changes(vty, NULL); } @@ -100,10 +118,14 @@ DEFPY_YANG (ripng_allow_ecmp, void cli_show_ripng_allow_ecmp(struct vty *vty, const struct lyd_node *dnode, bool show_defaults) { - if (!yang_dnode_get_bool(dnode, NULL)) - vty_out(vty, " no"); + uint8_t paths; + + paths = yang_dnode_get_uint8(dnode, NULL); - vty_out(vty, " allow-ecmp\n"); + if (!paths) + vty_out(vty, " no allow-ecmp\n"); + else + vty_out(vty, " allow-ecmp %d\n", paths); } /* @@ -547,6 +569,7 @@ void ripng_cli_init(void) install_element(RIPNG_NODE, &ripng_no_ipv6_distribute_list_cmd); install_element(RIPNG_NODE, &ripng_allow_ecmp_cmd); + install_element(RIPNG_NODE, &no_ripng_allow_ecmp_cmd); install_element(RIPNG_NODE, &ripng_default_information_originate_cmd); install_element(RIPNG_NODE, &ripng_default_metric_cmd); install_element(RIPNG_NODE, &no_ripng_default_metric_cmd); diff --git a/ripngd/ripng_main.c b/ripngd/ripng_main.c index 1d392efdde..9933dae5cd 100644 --- a/ripngd/ripng_main.c +++ b/ripngd/ripng_main.c @@ -32,6 +32,8 @@ struct option longopts[] = {{0}}; /* ripngd privileges */ zebra_capabilities_t _caps_p[] = {ZCAP_NET_RAW, ZCAP_BIND, ZCAP_SYS_ADMIN}; +uint32_t zebra_ecmp_count = MULTIPATH_NUM; + struct zebra_privs_t ripngd_privs = { #if defined(FRR_USER) .user = FRR_USER, diff --git a/ripngd/ripng_nb_config.c b/ripngd/ripng_nb_config.c index 30f707e061..0b1bd68eca 100644 --- a/ripngd/ripng_nb_config.c +++ b/ripngd/ripng_nb_config.c @@ -129,9 +129,14 @@ int ripngd_instance_allow_ecmp_modify(struct nb_cb_modify_args *args) return NB_OK; ripng = nb_running_get_entry(args->dnode, NULL, true); - ripng->ecmp = yang_dnode_get_bool(args->dnode, NULL); - if (!ripng->ecmp) + ripng->ecmp = + MIN(yang_dnode_get_uint8(args->dnode, NULL), zebra_ecmp_count); + if (!ripng->ecmp) { ripng_ecmp_disable(ripng); + return NB_OK; + } + + ripng_ecmp_change(ripng); return NB_OK; } diff --git a/ripngd/ripng_zebra.c b/ripngd/ripng_zebra.c index d974d65df5..49b8a197ad 100644 --- a/ripngd/ripng_zebra.c +++ b/ripngd/ripng_zebra.c @@ -30,7 +30,7 @@ static void ripng_zebra_ipv6_send(struct ripng *ripng, struct agg_node *rp, struct zapi_nexthop *api_nh; struct listnode *listnode = NULL; struct ripng_info *rinfo = NULL; - int count = 0; + uint32_t count = 0; const struct prefix *p = agg_node_get_prefix(rp); memset(&api, 0, sizeof(api)); @@ -41,7 +41,7 @@ static void ripng_zebra_ipv6_send(struct ripng *ripng, struct agg_node *rp, SET_FLAG(api.message, ZAPI_MESSAGE_NEXTHOP); for (ALL_LIST_ELEMENTS_RO(list, listnode, rinfo)) { - if (count >= MULTIPATH_NUM) + if (count >= zebra_ecmp_count) break; api_nh = &api.nexthops[count]; api_nh->vrf_id = ripng->vrf->vrf_id; @@ -227,6 +227,11 @@ static zclient_handler *const ripng_handlers[] = { [ZEBRA_REDISTRIBUTE_ROUTE_DEL] = ripng_zebra_read_route, }; +static void ripng_zebra_capabilities(struct zclient_capabilities *cap) +{ + zebra_ecmp_count = MIN(cap->ecmp, zebra_ecmp_count); +} + /* Initialize zebra structure and it's commands. */ void zebra_init(struct event_loop *master) { @@ -236,6 +241,7 @@ void zebra_init(struct event_loop *master) zclient_init(zclient, ZEBRA_ROUTE_RIPNG, 0, &ripngd_privs); zclient->zebra_connected = ripng_zebra_connected; + zclient->zebra_capabilities = ripng_zebra_capabilities; } void ripng_zebra_stop(void) diff --git a/ripngd/ripngd.c b/ripngd/ripngd.c index 2f6409a70d..7269e76656 100644 --- a/ripngd/ripngd.c +++ b/ripngd/ripngd.c @@ -443,7 +443,10 @@ struct ripng_info *ripng_ecmp_add(struct ripng *ripng, { struct agg_node *rp = rinfo_new->rp; struct ripng_info *rinfo = NULL; + struct ripng_info *rinfo_exist = NULL; struct list *list = NULL; + struct listnode *node = NULL; + struct listnode *nnode = NULL; if (rp->info == NULL) rp->info = list_new(); @@ -454,6 +457,33 @@ struct ripng_info *ripng_ecmp_add(struct ripng *ripng, if (listcount(list) && !ripng->ecmp) return NULL; + /* Add or replace an existing ECMP path with lower neighbor IP */ + if (listcount(list) && listcount(list) >= ripng->ecmp) { + struct ripng_info *from_highest = NULL; + + /* Find the rip_info struct that has the highest nexthop IP */ + for (ALL_LIST_ELEMENTS(list, node, nnode, rinfo_exist)) + if (!from_highest || + (from_highest && + IPV6_ADDR_CMP(&rinfo_exist->from, + &from_highest->from) > 0)) { + from_highest = rinfo_exist; + } + + /* If we have a route in ECMP group, delete the old + * one that has a higher next-hop address. Lower IP is + * preferred. + */ + if (ripng->ecmp > 1 && from_highest && + IPV6_ADDR_CMP(&from_highest->from, &rinfo_new->from) > 0) { + ripng_ecmp_delete(ripng, from_highest); + goto add_or_replace; + } + + return NULL; + } + +add_or_replace: rinfo = ripng_info_new(); memcpy(rinfo, rinfo_new, sizeof(struct ripng_info)); listnode_add(list, rinfo); @@ -475,6 +505,36 @@ struct ripng_info *ripng_ecmp_add(struct ripng *ripng, return rinfo; } +/* Update ECMP routes to zebra when `allow-ecmp` changed. */ +void ripng_ecmp_change(struct ripng *ripng) +{ + struct agg_node *rp; + struct ripng_info *rinfo; + struct list *list; + struct listnode *node, *nextnode; + + for (rp = agg_route_top(ripng->table); rp; rp = agg_route_next(rp)) { + list = rp->info; + if (list && listcount(list) > 1) { + while (listcount(list) > ripng->ecmp) { + struct ripng_info *from_highest = NULL; + + for (ALL_LIST_ELEMENTS(list, node, nextnode, + rinfo)) { + if (!from_highest || + (from_highest && + IPV6_ADDR_CMP( + &rinfo->from, + &from_highest->from) > 0)) + from_highest = rinfo; + } + + ripng_ecmp_delete(ripng, from_highest); + } + } + } +} + /* Replace the ECMP list with the new route. * RETURN: the new entry added in the list */ @@ -1814,7 +1874,7 @@ struct ripng *ripng_create(const char *vrf_name, struct vrf *vrf, int socket) "%s/timers/flush-interval", RIPNG_INSTANCE); ripng->default_metric = yang_get_default_uint8("%s/default-metric", RIPNG_INSTANCE); - ripng->ecmp = yang_get_default_bool("%s/allow-ecmp", RIPNG_INSTANCE); + ripng->ecmp = yang_get_default_uint8("%s/allow-ecmp", RIPNG_INSTANCE); /* Make buffer. */ ripng->ibuf = stream_new(RIPNG_MAX_PACKET_SIZE * 5); diff --git a/ripngd/ripngd.h b/ripngd/ripngd.h index eefcb0ee69..c7468b6317 100644 --- a/ripngd/ripngd.h +++ b/ripngd/ripngd.h @@ -130,7 +130,7 @@ struct ripng { struct event *t_triggered_interval; /* RIPng ECMP flag */ - bool ecmp; + uint8_t ecmp; /* RIPng redistribute configuration. */ struct { @@ -429,9 +429,12 @@ extern struct ripng_info *ripng_ecmp_replace(struct ripng *ripng, struct ripng_info *rinfo); extern struct ripng_info *ripng_ecmp_delete(struct ripng *ripng, struct ripng_info *rinfo); +extern void ripng_ecmp_change(struct ripng *ripng); extern void ripng_vrf_init(void); extern void ripng_vrf_terminate(void); extern void ripng_cli_init(void); +extern uint32_t zebra_ecmp_count; + #endif /* _ZEBRA_RIPNG_RIPNGD_H */ diff --git a/tests/topotests/analyze.py b/tests/topotests/analyze.py index 360c9cf1e9..9c9bfda1ed 100755 --- a/tests/topotests/analyze.py +++ b/tests/topotests/analyze.py @@ -161,6 +161,8 @@ def main(): logging.critical('No "/tmp/topotests" directory to save') sys.exit(1) subprocess.run(["mv", "/tmp/topotests", args.results]) + if "SUDO_USER" in os.environ: + subprocess.run(["chown", "-R", os.environ["SUDO_USER"], args.results]) # # Old location for results # if os.path.exists("/tmp/topotests.xml", args.results): # subprocess.run(["mv", "/tmp/topotests.xml", args.results]) diff --git a/tests/topotests/bgp_auth/test_bgp_auth1.py b/tests/topotests/bgp_auth/test_bgp_auth1.py index 566d391f7a..9d47106c07 100644 --- a/tests/topotests/bgp_auth/test_bgp_auth1.py +++ b/tests/topotests/bgp_auth/test_bgp_auth1.py @@ -158,8 +158,8 @@ def setup_module(mod): # For all registered routers, load the zebra configuration file for rname, router in router_list.items(): router.load_config(TopoRouter.RD_ZEBRA, "zebra.conf") - router.load_config(TopoRouter.RD_OSPF) - router.load_config(TopoRouter.RD_BGP) + router.load_config(TopoRouter.RD_OSPF, "") + router.load_config(TopoRouter.RD_BGP, "") # After copying the configurations, this function loads configured daemons. tgen.start_router() diff --git a/tests/topotests/bgp_auth/test_bgp_auth2.py b/tests/topotests/bgp_auth/test_bgp_auth2.py index 0e9942a227..6b92036727 100644 --- a/tests/topotests/bgp_auth/test_bgp_auth2.py +++ b/tests/topotests/bgp_auth/test_bgp_auth2.py @@ -158,8 +158,8 @@ def setup_module(mod): # For all registered routers, load the zebra configuration file for rname, router in router_list.items(): router.load_config(TopoRouter.RD_ZEBRA, "zebra.conf") - router.load_config(TopoRouter.RD_OSPF) - router.load_config(TopoRouter.RD_BGP) + router.load_config(TopoRouter.RD_OSPF, "") + router.load_config(TopoRouter.RD_BGP, "") # After copying the configurations, this function loads configured daemons. tgen.start_router() diff --git a/tests/topotests/bgp_auth/test_bgp_auth3.py b/tests/topotests/bgp_auth/test_bgp_auth3.py index 99a8953b3f..2237c6b1b6 100644 --- a/tests/topotests/bgp_auth/test_bgp_auth3.py +++ b/tests/topotests/bgp_auth/test_bgp_auth3.py @@ -157,8 +157,8 @@ def setup_module(mod): # For all registered routers, load the zebra configuration file for rname, router in router_list.items(): router.load_config(TopoRouter.RD_ZEBRA, "zebra.conf") - router.load_config(TopoRouter.RD_OSPF) - router.load_config(TopoRouter.RD_BGP) + router.load_config(TopoRouter.RD_OSPF, "") + router.load_config(TopoRouter.RD_BGP, "") # After copying the configurations, this function loads configured daemons. tgen.start_router() diff --git a/tests/topotests/bgp_auth/test_bgp_auth4.py b/tests/topotests/bgp_auth/test_bgp_auth4.py index dffef0eef7..d6fe42504b 100644 --- a/tests/topotests/bgp_auth/test_bgp_auth4.py +++ b/tests/topotests/bgp_auth/test_bgp_auth4.py @@ -157,8 +157,8 @@ def setup_module(mod): # For all registered routers, load the zebra configuration file for rname, router in router_list.items(): router.load_config(TopoRouter.RD_ZEBRA, "zebra.conf") - router.load_config(TopoRouter.RD_OSPF) - router.load_config(TopoRouter.RD_BGP) + router.load_config(TopoRouter.RD_OSPF, "") + router.load_config(TopoRouter.RD_BGP, "") # After copying the configurations, this function loads configured daemons. tgen.start_router() diff --git a/tests/topotests/bgp_default_originate/test_bgp_default_originate_2links.py b/tests/topotests/bgp_default_originate/test_bgp_default_originate_2links.py index 50c34d45fa..8058823baf 100644 --- a/tests/topotests/bgp_default_originate/test_bgp_default_originate_2links.py +++ b/tests/topotests/bgp_default_originate/test_bgp_default_originate_2links.py @@ -19,6 +19,7 @@ import pytest import datetime from copy import deepcopy from lib.topolog import logger +from time import sleep # pylint: disable=C0413 # Import topogen and topotest helpers @@ -592,6 +593,7 @@ def test_verify_bgp_default_originate_with_default_static_route_p1(request): step("Taking uptime snapshot before configuring default - originate") uptime_before_ipv4 = get_rib_route_uptime(tgen, "ipv4", "r2", ipv4_uptime_dict) uptime_before_ipv6 = get_rib_route_uptime(tgen, "ipv6", "r2", ipv6_uptime_dict) + sleep(1) step( "Configure default-originate on R1 link-1 again for IPv4 and IPv6 address family" @@ -1031,6 +1033,7 @@ def test_verify_bgp_default_originate_with_default_static_route_p1(request): step("Taking uptime snapshot before removing redisctribute static ") uptime_before_ipv4 = get_rib_route_uptime(tgen, "ipv4", "r2", ipv4_uptime_dict) uptime_before_ipv6 = get_rib_route_uptime(tgen, "ipv6", "r2", ipv6_uptime_dict) + sleep(1) step("Remove redistribute static from IPv4 and IPv6 address family ") input_dict_1 = { diff --git a/tests/topotests/bgp_disable_addpath_rx/r1/bgpd.conf b/tests/topotests/bgp_disable_addpath_rx/r1/bgpd.conf index af1353e0e0..44b009e9ca 100644 --- a/tests/topotests/bgp_disable_addpath_rx/r1/bgpd.conf +++ b/tests/topotests/bgp_disable_addpath_rx/r1/bgpd.conf @@ -1,6 +1,6 @@ ! router bgp 65001 - timers 3 10 + timers bgp 3 10 no bgp ebgp-requires-policy neighbor 192.168.1.2 remote-as external neighbor 192.168.1.2 timers connect 5 diff --git a/tests/topotests/bgp_disable_addpath_rx/r2/bgpd.conf b/tests/topotests/bgp_disable_addpath_rx/r2/bgpd.conf index db68e554d4..8274e3f96d 100644 --- a/tests/topotests/bgp_disable_addpath_rx/r2/bgpd.conf +++ b/tests/topotests/bgp_disable_addpath_rx/r2/bgpd.conf @@ -1,5 +1,5 @@ router bgp 65002 - timers 3 10 + timers bgp 3 10 no bgp ebgp-requires-policy neighbor 192.168.1.1 remote-as external neighbor 192.168.1.1 timers connect 5 diff --git a/tests/topotests/bgp_disable_addpath_rx/r3/bgpd.conf b/tests/topotests/bgp_disable_addpath_rx/r3/bgpd.conf index 3ac6a08e47..98eb2e1711 100644 --- a/tests/topotests/bgp_disable_addpath_rx/r3/bgpd.conf +++ b/tests/topotests/bgp_disable_addpath_rx/r3/bgpd.conf @@ -1,5 +1,5 @@ router bgp 65003 - timers 3 10 + timers bgp 3 10 no bgp ebgp-requires-policy neighbor 192.168.2.2 remote-as external neighbor 192.168.2.2 timers connect 5 diff --git a/tests/topotests/bgp_disable_addpath_rx/r4/bgpd.conf b/tests/topotests/bgp_disable_addpath_rx/r4/bgpd.conf index 8ab405fbd8..68245c4a21 100644 --- a/tests/topotests/bgp_disable_addpath_rx/r4/bgpd.conf +++ b/tests/topotests/bgp_disable_addpath_rx/r4/bgpd.conf @@ -1,5 +1,5 @@ router bgp 65004 - timers 3 10 + timers bgp 3 10 no bgp ebgp-requires-policy neighbor 192.168.2.2 remote-as external neighbor 192.168.2.2 timers connect 5 diff --git a/tests/topotests/bgp_prefix_list_topo1/prefix_modify.json b/tests/topotests/bgp_prefix_list_topo1/prefix_modify.json new file mode 100644 index 0000000000..4a066ae535 --- /dev/null +++ b/tests/topotests/bgp_prefix_list_topo1/prefix_modify.json @@ -0,0 +1,120 @@ +{ + "address_types": ["ipv4"], + "ipv4base":"192.120.1.0", + "ipv4mask":24, + "link_ip_start":{"ipv4":"192.120.1.0", "v4mask":30, "ipv6":"fd00::", "v6mask":64}, + "routers":{ + "r1":{ + "links":{ + "lo": {"ipv4": "auto", "type": "loopback", "add_static_route":"yes"}, + "r2":{"ipv4":"auto"}, + "r3":{"ipv4":"auto"} + }, + "bgp":{ + "local_as":"100", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r2": { + "dest_link": { + "r1": {} + } + }, + "r3": { + "dest_link": { + "r1": {} + } + } + } + } + } + } + } + }, + "r2":{ + "links":{ + "lo": {"ipv4": "auto", "ipv6": "auto", "type": "loopback", "add_static_route":"yes"}, + "r1":{"ipv4":"auto", "ipv6":"auto"}, + "r3":{"ipv4":"auto", "ipv6":"auto"} + }, + "bgp":{ + "local_as":"100", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r1": { + "dest_link": { + "r2": {} + } + }, + "r3": { + "dest_link": { + "r2": {} + } + } + } + } + } + } + } + }, + "r3":{ + "links":{ + "lo": {"ipv4": "auto", "type": "loopback", "add_static_route":"yes"}, + "r1":{"ipv4":"auto"}, + "r2":{"ipv4":"auto"}, + "r4":{"ipv4":"auto"} + }, + "bgp":{ + "local_as":"100", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r1": { + "dest_link": { + "r3": {} + } + }, + "r2": { + "dest_link": { + "r3": {} + } + }, + "r4": { + "dest_link": { + "r3": {} + } + } + } + } + } + } + } + }, + "r4":{ + "links":{ + "lo": {"ipv4": "auto", "type": "loopback", "add_static_route":"yes"}, + "r3":{"ipv4":"auto"} + }, + "bgp":{ + "local_as":"200", + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r3": { + "dest_link": { + "r4": {} + } + } + } + } + } + } + } + } + } +} diff --git a/tests/topotests/bgp_prefix_list_topo1/test_prefix_modify.py b/tests/topotests/bgp_prefix_list_topo1/test_prefix_modify.py new file mode 100644 index 0000000000..541b9de342 --- /dev/null +++ b/tests/topotests/bgp_prefix_list_topo1/test_prefix_modify.py @@ -0,0 +1,404 @@ +#!/usr/bin/python +# SPDX-License-Identifier: ISC + +# +# Copyright (c) 2019 by VMware, Inc. ("VMware") +# Used Copyright (c) 2018 by Network Device Education Foundation, +# Inc. ("NetDEF") in this file. +# + +""" +Following tests are covered to test prefix-list functionality: + +Test steps +- Create topology (setup module) + Creating 4 routers topology, r1, r2, r3 are in IBGP and + r3, r4 are in EBGP +- Bring up topology +- Verify for bgp to converge + +IP prefix-list tests +- Test modify prefix-list action +""" + +import sys +import time +import os +import pytest + +# Save the Current Working Directory to find configuration files. +CWD = os.path.dirname(os.path.realpath(__file__)) +sys.path.append(os.path.join(CWD, "../")) + +# pylint: disable=C0413 +# Import topogen and topotest helpers +from lib.topogen import Topogen, get_topogen + +# Import topoJson from lib, to create topology and initial configuration +from lib.common_config import ( + start_topology, + write_test_header, + write_test_footer, + reset_config_on_routers, + verify_rib, + create_static_routes, + create_prefix_lists, + step, + create_route_maps, + check_router_status, +) +from lib.topolog import logger +from lib.bgp import verify_bgp_convergence, create_router_bgp, clear_bgp + +from lib.topojson import build_config_from_json + +pytestmark = [pytest.mark.bgpd] + + +# Global variables +bgp_convergence = False + +IPV4_PF3 = "192.168.0.0/18" +IPV4_PF4 = "192.150.10.0/24" +IPV4_PF5 = "192.168.10.1/32" +IPV4_PF6 = "192.168.10.10/32" +IPV4_PF7 = "192.168.10.0/24" + + +def setup_module(mod): + """ + Sets up the pytest environment + + * `mod`: module name + """ + + testsuite_run_time = time.asctime(time.localtime(time.time())) + logger.info("Testsuite start time: {}".format(testsuite_run_time)) + logger.info("=" * 40) + + logger.info("Running setup_module to create topology") + + # This function initiates the topology build with Topogen... + json_file = "{}/prefix_lists.json".format(CWD) + tgen = Topogen(json_file, mod.__name__) + global topo + topo = tgen.json_topo + # ... and here it calls Mininet initialization functions. + + # Starting topology, create tmp files which are loaded to routers + # to start daemons and then start routers + start_topology(tgen) + + # Creating configuration from JSON + build_config_from_json(tgen, topo) + + # Checking BGP convergence + global BGP_CONVERGENCE + + # Don't run this test if we have any failure. + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + # Api call verify whether BGP is converged + BGP_CONVERGENCE = verify_bgp_convergence(tgen, topo) + assert BGP_CONVERGENCE is True, "setup_module :Failed \n Error:" " {}".format( + BGP_CONVERGENCE + ) + + logger.info("Running setup_module() done") + + +def teardown_module(mod): + """ + Teardown the pytest environment + + * `mod`: module name + """ + + logger.info("Running teardown_module to delete topology") + + tgen = get_topogen() + + # Stop toplogy and Remove tmp files + tgen.stop_topology() + + logger.info( + "Testsuite end time: {}".format(time.asctime(time.localtime(time.time()))) + ) + logger.info("=" * 40) + + +##################################################### +# +# Tests starting +# +##################################################### + + +def test_bug_prefix_lists_deny_to_permit_p1(request): + """ + Verify modification of prefix-list action + """ + + tgen = get_topogen() + if BGP_CONVERGENCE is not True: + pytest.skip("skipped because of BGP Convergence failure") + + # test case name + tc_name = request.node.name + write_test_header(tc_name) + + # Creating configuration from JSON + if tgen.routers_have_failure(): + check_router_status(tgen) + reset_config_on_routers(tgen) + + # base config + step("Configure IPV4 and IPv6 IBGP and EBGP session as mentioned in setup") + step("Configure static routes on R2 with Null 0 nexthop") + input_dict_1 = { + "r2": { + "static_routes": [ + {"network": IPV4_PF7, "no_of_ip": 1, "next_hop": "Null0"}, + {"network": IPV4_PF6, "no_of_ip": 1, "next_hop": "Null0"}, + ] + } + } + result = create_static_routes(tgen, input_dict_1) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step("Advertise static route in BGP using redistribute static command") + input_dict_4 = { + "r2": { + "bgp": { + "address_family": { + "ipv4": { + "unicast": { + "redistribute": [ + {"redist_type": "static"}, + {"redist_type": "connected"}, + ] + } + } + } + } + } + } + result = create_router_bgp(tgen, topo, input_dict_4) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step( + "All the static route advertised in R4 as BGP " + "routes verify using 'show ip bgp'and 'show bgp'" + ) + dut = "r4" + protocol = "bgp" + + input_dict_route = { + "r4": {"static_routes": [{"network": IPV4_PF7}, {"network": IPV4_PF6}]} + } + + result = verify_rib(tgen, "ipv4", dut, input_dict_route) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step("Configure IPv4 and IPv6 prefix-list") + input_dict_3 = { + "r3": { + "prefix_lists": { + "ipv4": { + "pf_list_1_ipv4": [ + {"seqid": "5", "network": IPV4_PF7, "action": "deny"} + ], + } + } + } + } + result = create_prefix_lists(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + input_dict_3 = { + "r3": { + "prefix_lists": { + "ipv4": { + "pf_list_1": [ + {"seqid": "10", "network": IPV4_PF7, "action": "permit"} + ] + } + } + } + } + result = create_prefix_lists(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step( + "configure route-map seq to permit IPV4 prefix list and seq" + "2 to permit IPV6 prefix list and apply it to out direction on R3" + ) + + input_dict_3 = { + "r3": { + "route_maps": { + "rmap_match_pf_1": [ + { + "action": "permit", + "match": {"ipv4": {"prefix_lists": "pf_list_1"}}, + } + ] + } + } + } + result = create_route_maps(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + input_dict_7 = { + "r3": { + "bgp": { + "address_family": { + "ipv4": { + "unicast": { + "neighbor": { + "r4": { + "dest_link": { + "r3": { + "route_maps": [ + { + "name": "rmap_match_pf_1", + "direction": "out", + } + ] + } + } + } + } + } + } + } + } + } + } + + result = create_router_bgp(tgen, topo, input_dict_7) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step( + "Verify on R4 should not have any IPv4 and IPv6 BGP routes using " + "show ip bgp show bgp" + ) + + dut = "r4" + protocol = "bgp" + + result = verify_rib( + tgen, "ipv4", dut, input_dict_route, protocol=protocol, expected=False + ) + assert ( + result is not True + ), "Testcase {} : Failed \n" "Error : Routes are still present \n {}".format( + tc_name, result + ) + + step("Modify IPv4/IPv6 prefix-list sequence 5 to another value on R3") + input_dict_3 = { + "r3": { + "prefix_lists": { + "ipv4": { + "pf_list_1": [ + {"seqid": "5", "network": IPV4_PF4, "action": "deny"} + ], + } + } + } + } + result = create_prefix_lists(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step( + "Verify /24 and /120 routes present on" + "R4 BGP table using show ip bgp show bgp" + ) + input_dict = {"r4": {"static_routes": [{"network": IPV4_PF7}]}} + + dut = "r4" + protocol = "bgp" + + result = verify_rib(tgen, "ipv4", dut, input_dict, protocol=protocol) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step("Change prefix-list to same as original on R3") + input_dict_3 = { + "r3": { + "prefix_lists": { + "ipv4": { + "pf_list_1": [ + {"seqid": "5", "network": IPV4_PF7, "action": "deny"} + ], + } + } + } + } + result = create_prefix_lists(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step( + "Verify /24 and /120 routes removed on" + "R4 BGP table using show ip bgp show bgp" + ) + + result = verify_rib( + tgen, "ipv4", dut, input_dict, protocol=protocol, expected=False + ) + assert ( + result is not True + ), "Testcase {} : Failed \n" "Error : Routes are still present \n {}".format( + tc_name, result + ) + + step("Modify IPv4/IPv6 prefix-list sequence 5 to another value") + input_dict_3 = { + "r3": { + "prefix_lists": { + "ipv4": { + "pf_list_1": [ + {"seqid": "5", "network": IPV4_PF4, "action": "deny"} + ], + } + } + } + } + result = create_prefix_lists(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + result = verify_rib(tgen, "ipv4", dut, input_dict, protocol=protocol) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step("Clear BGP on R3 and verify the routes") + clear_bgp(tgen, "ipv4", "r3") + + result = verify_rib(tgen, "ipv4", dut, input_dict, protocol=protocol) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step("On R3 add prefix-list permit any for IPv4 and IPv6 seq 15") + input_dict_3 = { + "r3": { + "prefix_lists": { + "ipv4": { + "pf_list_1": [ + {"seqid": "15", "network": "any", "action": "permit"} + ], + } + } + } + } + result = create_prefix_lists(tgen, input_dict_3) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + step("verify /24 and /32 /120 and /128 routes are present on R4") + result = verify_rib(tgen, "ipv4", dut, input_dict_route) + assert result is True, "Testcase {} : Failed \n Error: {}".format(tc_name, result) + + write_test_footer(tc_name) + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py b/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py index 1134237447..e48f81c53d 100644 --- a/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py +++ b/tests/topotests/bgp_remove_private_as/test_bgp_remove_private_as.py @@ -409,8 +409,6 @@ def test_bgp_remove_private_as(): # the old flag after each iteration so we only test the flags we expect. _change_remove_type(rmv_type, "del") - return True - if __name__ == "__main__": args = ["-s"] + sys.argv[1:] diff --git a/tests/topotests/lib/bgp.py b/tests/topotests/lib/bgp.py index a0b12b2d64..0bd9408c28 100644 --- a/tests/topotests/lib/bgp.py +++ b/tests/topotests/lib/bgp.py @@ -164,7 +164,6 @@ def create_router_bgp(tgen, topo=None, input_dict=None, build=False, load_config router, ) else: - ipv4_data = bgp_addr_data.setdefault("ipv4", {}) ipv6_data = bgp_addr_data.setdefault("ipv6", {}) l2vpn_data = bgp_addr_data.setdefault("l2vpn", {}) @@ -530,6 +529,7 @@ def __create_bgp_unicast_neighbor( config_data.extend(neigh_addr_data) + config_data.append("exit") logger.debug("Exiting lib API: {}".format(sys._getframe().f_code.co_name)) return config_data @@ -645,7 +645,6 @@ def __create_l2vpn_evpn_address_family( if advertise_data: for address_type, unicast_type in advertise_data.items(): - if type(unicast_type) is dict: for key, value in unicast_type.items(): cmd = "advertise {} {}".format(address_type, key) @@ -1651,7 +1650,6 @@ def modify_as_number(tgen, topo, input_dict): logger.debug("Entering lib API: {}".format(sys._getframe().f_code.co_name)) try: - new_topo = deepcopy(topo["routers"]) router_dict = {} for router in input_dict.keys(): @@ -1953,7 +1951,6 @@ def clear_bgp_and_verify(tgen, topo, router, rid=None): total_peer = 0 for addr_type in bgp_addr_type.keys(): - if not check_address_types(addr_type): continue @@ -2022,7 +2019,6 @@ def clear_bgp_and_verify(tgen, topo, router, rid=None): peer_uptime_after_clear_bgp = {} # Verifying BGP convergence after bgp clear command for retry in range(50): - # Waiting for BGP to converge logger.info( "Waiting for %s sec for BGP to converge on router" " %s...", @@ -3278,7 +3274,6 @@ def verify_graceful_restart( if lmode is None: if "graceful-restart" in input_dict[dut]["bgp"]: - if ( "graceful-restart" in input_dict[dut]["bgp"]["graceful-restart"] and input_dict[dut]["bgp"]["graceful-restart"][ @@ -3326,7 +3321,6 @@ def verify_graceful_restart( if rmode is None: if "graceful-restart" in input_dict[peer]["bgp"]: - if ( "graceful-restart" in input_dict[peer]["bgp"]["graceful-restart"] @@ -3628,7 +3622,6 @@ def verify_eor(tgen, topo, addr_type, input_dict, dut, peer, expected=True): eor_json = show_bgp_graceful_json_out[afi]["endOfRibStatus"] if "endOfRibSend" in eor_json: - if eor_json["endOfRibSend"]: logger.info( "[DUT: %s]: EOR Send true for %s " "%s", dut, neighbor_ip, afi @@ -3918,7 +3911,6 @@ def verify_graceful_restart_timers(tgen, topo, addr_type, input_dict, dut, peer) "timer" ].items(): if rs_timer == "restart-time": - receivedTimer = value if ( show_bgp_graceful_json_out["timers"][ @@ -4777,7 +4769,6 @@ def get_prefix_count_route( tgen = get_topogen() for router, rnode in tgen.routers().items(): if router == dut: - if vrf: ipv4_cmd = "sh ip bgp vrf {} summary json".format(vrf) show_bgp_json_ipv4 = run_frr_cmd(rnode, ipv4_cmd, isjson=True) @@ -4871,7 +4862,6 @@ def verify_rib_default_route( connected_routes = {} for router, rnode in tgen.routers().items(): if router == dut: - ipv4_routes = run_frr_cmd(rnode, "sh ip bgp json", isjson=True) ipv6_routes = run_frr_cmd(rnode, "sh ip bgp ipv6 unicast json", isjson=True) is_ipv4_default_attrib_found = False diff --git a/tests/topotests/lib/bgprib.py b/tests/topotests/lib/bgprib.py index cd449cb50c..699c7a4da6 100644 --- a/tests/topotests/lib/bgprib.py +++ b/tests/topotests/lib/bgprib.py @@ -25,6 +25,7 @@ from lib.lutil import luCommand, luResult, LUtil import json import re + # gpz: get rib in json form and compare against desired routes class BgpRib: def log(self, str): diff --git a/tests/topotests/lib/common_config.py b/tests/topotests/lib/common_config.py index d19d8db75c..a85b86668c 100644 --- a/tests/topotests/lib/common_config.py +++ b/tests/topotests/lib/common_config.py @@ -243,7 +243,6 @@ def run_frr_cmd(rnode, cmd, isjson=False): def apply_raw_config(tgen, input_dict): - """ API to configure raw configuration on device. This can be used for any cli which has not been implemented in JSON. @@ -447,7 +446,6 @@ def check_router_status(tgen): try: router_list = tgen.routers() for router, rnode in router_list.items(): - result = rnode.check_router_running() if result != "": daemons = [] @@ -1914,7 +1912,6 @@ def interface_status(tgen, topo, input_dict): rlist = [] for router in input_dict.keys(): - interface_list = input_dict[router]["interface_list"] status = input_dict[router].setdefault("status", "up") for intf in interface_list: @@ -2189,6 +2186,7 @@ def create_interfaces_cfg(tgen, topo, build=False): interface_data += _create_interfaces_ospf_cfg( "ospf6", c_data, data, ospf_keywords + ["area"] ) + interface_data.append("exit") if interface_data: interface_data_dict[c_router] = interface_data @@ -2529,7 +2527,6 @@ def create_route_maps(tgen, input_dict, build=False): continue rmap_data = [] for rmap_name, rmap_value in input_dict[router]["route_maps"].items(): - for rmap_dict in rmap_value: del_action = rmap_dict.setdefault("delete", False) @@ -3002,7 +2999,6 @@ def addKernelRoute( group_addr_range = [group_addr_range] for grp_addr in group_addr_range: - addr_type = validate_ip_address(grp_addr) if addr_type == "ipv4": if next_hop is not None: @@ -3193,7 +3189,6 @@ def configure_brctl(tgen, topo, input_dict): if "brctl" in input_dict[dut]: for brctl_dict in input_dict[dut]["brctl"]: - brctl_names = brctl_dict.setdefault("brctl_name", []) addvxlans = brctl_dict.setdefault("addvxlan", []) stp_values = brctl_dict.setdefault("stp", []) @@ -3203,7 +3198,6 @@ def configure_brctl(tgen, topo, input_dict): for brctl_name, vxlan, vrf, stp in zip( brctl_names, addvxlans, vrfs, stp_values ): - ip_cmd_list = [] cmd = "ip link add name {} type bridge stp_state {}".format( brctl_name, stp @@ -3614,7 +3608,6 @@ def verify_rib( for static_route in static_routes: if "vrf" in static_route and static_route["vrf"] is not None: - logger.info( "[DUT: {}]: Verifying routes for VRF:" " {}".format(router, static_route["vrf"]) @@ -4053,7 +4046,6 @@ def verify_fib_routes(tgen, addr_type, dut, input_dict, next_hop=None, protocol= for static_route in static_routes: if "vrf" in static_route and static_route["vrf"] is not None: - logger.info( "[DUT: {}]: Verifying routes for VRF:" " {}".format(router, static_route["vrf"]) diff --git a/tests/topotests/lib/lutil.py b/tests/topotests/lib/lutil.py index 9aef41cd1c..1eb88f26d4 100644 --- a/tests/topotests/lib/lutil.py +++ b/tests/topotests/lib/lutil.py @@ -177,7 +177,17 @@ Total %-4d %-4d %d\n\ self.log("unable to read: " + tstFile) sys.exit(1) - def command(self, target, command, regexp, op, result, returnJson, startt=None, force_result=False): + def command( + self, + target, + command, + regexp, + op, + result, + returnJson, + startt=None, + force_result=False, + ): global net if op == "jsoncmp_pass" or op == "jsoncmp_fail": returnJson = True @@ -326,7 +336,9 @@ Total %-4d %-4d %d\n\ if strict and (wait_count == 1): force_result = True - found = self.command(target, command, regexp, op, result, returnJson, startt, force_result) + found = self.command( + target, command, regexp, op, result, returnJson, startt, force_result + ) if found is not False: break @@ -342,6 +354,7 @@ Total %-4d %-4d %d\n\ # initialized by luStart LUtil = None + # entry calls def luStart( baseScriptDir=".", @@ -455,6 +468,7 @@ def luShowFail(): if printed > 0: logger.error("See %s for details of errors" % LUtil.fout_name) + # # Sets default wait type for luCommand(op="wait) (may be overridden by # specifying luCommand(op="wait-strict") or luCommand(op="wait-nostrict")). diff --git a/tests/topotests/lib/ospf.py b/tests/topotests/lib/ospf.py index ffe81fbd99..5486e904df 100644 --- a/tests/topotests/lib/ospf.py +++ b/tests/topotests/lib/ospf.py @@ -337,6 +337,7 @@ def __create_ospf_global(tgen, input_dict, router, build, load_config, ospf): cmd = "no {}".format(cmd) config_data.append(cmd) + config_data.append("exit") logger.debug("Exiting lib API: create_ospf_global()") return config_data diff --git a/tests/topotests/lib/pim.py b/tests/topotests/lib/pim.py index 925890b324..e26bdb3af3 100644 --- a/tests/topotests/lib/pim.py +++ b/tests/topotests/lib/pim.py @@ -593,7 +593,6 @@ def find_rp_details(tgen, topo): topo_data = topo["routers"] for router in router_list.keys(): - if "pim" not in topo_data[router]: continue @@ -1495,7 +1494,6 @@ def verify_mroutes( and data["outboundInterface"] in oil ): if return_uptime: - uptime_dict[grp_addr][src_address] = data["upTime"] logger.info( @@ -1917,7 +1915,6 @@ def get_pim_interface_traffic(tgen, input_dict): for intf, data in input_dict[dut].items(): interface_json = show_pim_intf_traffic_json[intf] for state in data: - # Verify Tx/Rx if state in interface_json: output_dict[dut][state] = interface_json[state] @@ -1990,7 +1987,6 @@ def get_pim6_interface_traffic(tgen, input_dict): for intf, data in input_dict[dut].items(): interface_json = show_pim_intf_traffic_json[intf] for state in data: - # Verify Tx/Rx if state in interface_json: output_dict[dut][state] = interface_json[state] @@ -3007,7 +3003,6 @@ def verify_pim_upstream_rpf( logger.debug("Entering lib API: {}".format(sys._getframe().f_code.co_name)) if "pim" in topo["routers"][dut]: - logger.info("[DUT: %s]: Verifying ip pim upstream rpf:", dut) rnode = tgen.routers()[dut] @@ -3245,7 +3240,6 @@ def verify_pim_join( grp_addr = grp_addr.split("/")[0] for source, data in interface_json[grp_addr].items(): - # Verify pim join if pim_join: if data["group"] == grp_addr and data["channelJoinName"] == "JOIN": @@ -3338,7 +3332,6 @@ def verify_igmp_config(tgen, input_dict, stats_return=False, expected=True): rnode = tgen.routers()[dut] for interface, data in input_dict[dut]["igmp"]["interfaces"].items(): - statistics = False report = False if "statistics" in input_dict[dut]["igmp"]["interfaces"][interface]["igmp"]: @@ -3623,7 +3616,6 @@ def verify_pim_config(tgen, input_dict, expected=True): rnode = tgen.routers()[dut] for interface, data in input_dict[dut]["pim"]["interfaces"].items(): - logger.info("[DUT: %s]: Verifying PIM interface %s detail:", dut, interface) show_ip_igmp_intf_json = run_frr_cmd( @@ -3772,7 +3764,6 @@ def verify_multicast_traffic(tgen, input_dict, return_traffic=False, expected=Tr elif ( interface_json["pktsIn"] != 0 and interface_json["bytesIn"] != 0 ): - traffic_dict[traffic_type][interface][ "pktsIn" ] = interface_json["pktsIn"] @@ -3836,7 +3827,6 @@ def verify_multicast_traffic(tgen, input_dict, return_traffic=False, expected=Tr interface_json["pktsOut"] != 0 and interface_json["bytesOut"] != 0 ): - traffic_dict[traffic_type][interface][ "pktsOut" ] = interface_json["pktsOut"] @@ -4232,7 +4222,6 @@ def verify_local_igmp_groups(tgen, dut, interface, group_addresses): group_addresses = [group_addresses] if interface not in show_ip_local_igmp_json: - errormsg = ( "[DUT %s]: Verifying local IGMP group received" " from interface %s [FAILED]!! " % (dut, interface) @@ -4319,7 +4308,6 @@ def verify_pim_interface_traffic(tgen, input_dict, return_stats=True, addr_type= for intf, data in input_dict[dut].items(): interface_json = show_pim_intf_traffic_json[intf] for state in data: - # Verify Tx/Rx if state in interface_json: output_dict[dut][state] = interface_json[state] @@ -4525,7 +4513,6 @@ def verify_mld_config(tgen, input_dict, stats_return=False, expected=True): for dut in input_dict.keys(): rnode = tgen.routers()[dut] for interface, data in input_dict[dut]["mld"]["interfaces"].items(): - statistics = False report = False if "statistics" in input_dict[dut]["mld"]["interfaces"][interface]["mld"]: @@ -5040,7 +5027,6 @@ def verify_pim6_config(tgen, input_dict, expected=True): rnode = tgen.routers()[dut] for interface, data in input_dict[dut]["pim6"]["interfaces"].items(): - logger.info( "[DUT: %s]: Verifying PIM6 interface %s detail:", dut, interface ) @@ -5158,7 +5144,6 @@ def verify_local_mld_groups(tgen, dut, interface, group_addresses): group_addresses = [group_addresses] if interface not in show_ipv6_local_mld_json["default"]: - errormsg = ( "[DUT %s]: Verifying local MLD group received" " from interface %s [FAILED]!! " % (dut, interface) diff --git a/tests/topotests/lib/test/test_json.py b/tests/topotests/lib/test/test_json.py index ae58d9b2d1..230c2bd7f7 100755 --- a/tests/topotests/lib/test/test_json.py +++ b/tests/topotests/lib/test/test_json.py @@ -616,7 +616,6 @@ def test_json_object_asterisk_matching(): def test_json_list_nested_with_objects(): - dcomplete = [{"key": 1, "list": [123]}, {"key": 2, "list": [123]}] dsub1 = [{"key": 2, "list": [123]}, {"key": 1, "list": [123]}] diff --git a/tests/topotests/lib/topojson.py b/tests/topotests/lib/topojson.py index 53e6945bee..901e4f623a 100644 --- a/tests/topotests/lib/topojson.py +++ b/tests/topotests/lib/topojson.py @@ -206,7 +206,6 @@ def build_topo_from_json(tgen, topo=None): for destRouterLink, data in sorted( topo["switches"][curSwitch]["links"].items() ): - # Loopback interfaces if "dst_node" in data: destRouter = data["dst_node"] @@ -220,7 +219,6 @@ def build_topo_from_json(tgen, topo=None): destRouter = destRouterLink if destRouter in listAllRouters: - topo["routers"][destRouter]["links"][curSwitch] = deepcopy( topo["switches"][curSwitch]["links"][destRouterLink] ) @@ -316,6 +314,7 @@ def build_config_from_json(tgen, topo=None, save_bkup=True): func_dict = OrderedDict( [ ("vrfs", create_vrf_cfg), + ("ospf", create_router_ospf), ("links", create_interfaces_cfg), ("static_routes", create_static_routes), ("prefix_lists", create_prefix_lists), @@ -325,7 +324,6 @@ def build_config_from_json(tgen, topo=None, save_bkup=True): ("igmp", create_igmp_config), ("mld", create_mld_config), ("bgp", create_router_bgp), - ("ospf", create_router_ospf), ] ) @@ -342,7 +340,7 @@ def build_config_from_json(tgen, topo=None, save_bkup=True): result = load_config_to_routers(tgen, routers, save_bkup) if not result: logger.info("build_config_from_json: failed to configure topology") - pytest.exit(1) + assert False logger.info( "Built config now clearing ospf neighbors as that router-id might not be what is used" @@ -398,7 +396,7 @@ def setup_module_from_json(testfile, json_file=None): tgen = create_tgen_from_json(testfile, json_file) # Start routers (and their daemons) - start_topology(tgen, topo_daemons(tgen)) + start_topology(tgen) # Configure routers build_config_from_json(tgen) diff --git a/tests/topotests/lib/topotest.py b/tests/topotests/lib/topotest.py index 9c594d3c8e..aeb83d4290 100644 --- a/tests/topotests/lib/topotest.py +++ b/tests/topotests/lib/topotest.py @@ -1527,6 +1527,9 @@ class Router(Node): """ # Unfortunately this API allowsfor source to not exist for any and all routers. + if source is None: + source = f"{daemon}.conf" + if source: head, tail = os.path.split(source) if not head and not self.path_exists(tail): @@ -1558,7 +1561,7 @@ class Router(Node): self.cmd_raises("cp {} {}".format(source, conf_file_mgmt)) self.cmd_raises("cp {} {}".format(source, conf_file)) - if not self.unified_config or daemon == "frr": + if not (self.unified_config or daemon == "frr"): self.cmd_raises("chown {0}:{0} {1}".format(self.routertype, conf_file)) self.cmd_raises("chmod 664 {}".format(conf_file)) @@ -1952,7 +1955,7 @@ class Router(Node): tail_log_files.append("{}/{}/frr.log".format(self.logdir, self.name)) for tailf in tail_log_files: - self.run_in_window("tail -f " + tailf, title=tailf, background=True) + self.run_in_window("tail -n10000 -F " + tailf, title=tailf, background=True) return "" diff --git a/tests/topotests/munet/cli.py b/tests/topotests/munet/cli.py index f58ea99d67..f631073bb1 100644 --- a/tests/topotests/munet/cli.py +++ b/tests/topotests/munet/cli.py @@ -93,7 +93,7 @@ def spawn(unet, host, cmd, iow, ns_only): elif master_fd in r: o = os.read(master_fd, 10240) if o: - iow.write(o.decode("utf-8")) + iow.write(o.decode("utf-8", "ignore")) iow.flush() finally: # restore tty settings back @@ -281,7 +281,6 @@ async def async_input(prompt, histfile): def make_help_str(unet): - w = sorted([x if x else "" for x in unet.cli_in_window_cmds]) ww = unet.cli_in_window_cmds u = sorted([x if x else "" for x in unet.cli_run_cmds]) @@ -516,7 +515,6 @@ class Completer: async def doline( unet, line, outf, background=False, notty=False ): # pylint: disable=R0911 - line = line.strip() m = re.fullmatch(r"^(\S+)(?:\s+(.*))?$", line) if not m: @@ -670,7 +668,7 @@ async def cli_client(sockpath, prompt="munet> "): rb = rb[: -len(ENDMARKER)] # Write the output - sys.stdout.write(rb.decode("utf-8")) + sys.stdout.write(rb.decode("utf-8", "ignore")) async def local_cli(unet, outf, prompt, histfile, background): @@ -729,7 +727,7 @@ async def cli_client_connected(unet, background, reader, writer): self.writer = writer def write(self, x): - self.writer.write(x.encode("utf-8")) + self.writer.write(x.encode("utf-8", "ignore")) def flush(self): self.writer.flush() diff --git a/tests/topotests/ospf_basic_functionality/test_ospf_asbr_summary_topo1.py b/tests/topotests/ospf_basic_functionality/test_ospf_asbr_summary_topo1.py index cf7d95b65a..0531e81d44 100644 --- a/tests/topotests/ospf_basic_functionality/test_ospf_asbr_summary_topo1.py +++ b/tests/topotests/ospf_basic_functionality/test_ospf_asbr_summary_topo1.py @@ -1579,10 +1579,6 @@ def test_ospf_type5_summary_tc45_p0(request): result = create_interfaces_cfg(tgen, input_dict) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) - # clear neighbor state on both routers to avoid stale state - tgen.net["r0"].cmd("clear ip ospf neighbor") - tgen.net["r1"].cmd("clear ip ospf neighbor") - ospf_covergence = verify_ospf_neighbor(tgen, topo) assert ospf_covergence is True, "setup_module :Failed \n Error {}".format( ospf_covergence diff --git a/tests/topotests/ospfv3_basic_functionality/test_ospfv3_asbr_summary_topo1.py b/tests/topotests/ospfv3_basic_functionality/test_ospfv3_asbr_summary_topo1.py index 8cc0ed6090..49c25ab8f6 100644 --- a/tests/topotests/ospfv3_basic_functionality/test_ospfv3_asbr_summary_topo1.py +++ b/tests/topotests/ospfv3_basic_functionality/test_ospfv3_asbr_summary_topo1.py @@ -1428,10 +1428,6 @@ def ospfv3_type5_summary_tc45_p0(request): result = create_interfaces_cfg(tgen, input_dict) assert result is True, "Testcase {} :Failed \n Error: {}".format(tc_name, result) - # restart interface state machine on both routers to avoid stale state - tgen.net["r0"].cmd("clear ipv6 ospf6 interface") - tgen.net["r1"].cmd("clear ipv6 ospf6 interface") - ospf_covergence = verify_ospf6_neighbor(tgen, topo) assert ospf_covergence is True, "Testcase {} :Failed \n Error: {}".format( tc_name, ospf_covergence diff --git a/tests/topotests/pytest.ini b/tests/topotests/pytest.ini index f779bf0a74..0062cf3de2 100644 --- a/tests/topotests/pytest.ini +++ b/tests/topotests/pytest.ini @@ -1,7 +1,7 @@ # Skip pytests example directory [pytest] -asyncio_mode = auto +# asyncio_mode = auto # We always turn this on inside conftest.py, default shown # addopts = --junitxml=<rundir>/topotests.xml diff --git a/tests/topotests/ripng_allow_ecmp/__init__.py b/tests/topotests/ripng_allow_ecmp/__init__.py new file mode 100644 index 0000000000..e69de29bb2 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/__init__.py diff --git a/tests/topotests/ripng_allow_ecmp/r1/frr.conf b/tests/topotests/ripng_allow_ecmp/r1/frr.conf new file mode 100644 index 0000000000..effb5df1ea --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r1/frr.conf @@ -0,0 +1,9 @@ +! +int r1-eth0 + ipv6 address 2001:db8:1::1/64 +! +router ripng + allow-ecmp + network 2001:db8:1::/64 + timers basic 5 15 10 +exit diff --git a/tests/topotests/ripng_allow_ecmp/r2/frr.conf b/tests/topotests/ripng_allow_ecmp/r2/frr.conf new file mode 100644 index 0000000000..71da101c0d --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r2/frr.conf @@ -0,0 +1,13 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r2-eth0 + ipv6 address 2001:db8:1::2/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit diff --git a/tests/topotests/ripng_allow_ecmp/r3/frr.conf b/tests/topotests/ripng_allow_ecmp/r3/frr.conf new file mode 100644 index 0000000000..fe9594dcd8 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r3/frr.conf @@ -0,0 +1,14 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r3-eth0 + ipv6 address 2001:db8:1::3/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit + diff --git a/tests/topotests/ripng_allow_ecmp/r4/frr.conf b/tests/topotests/ripng_allow_ecmp/r4/frr.conf new file mode 100644 index 0000000000..0d3ea0bdea --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r4/frr.conf @@ -0,0 +1,14 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r4-eth0 + ipv6 address 2001:db8:1::4/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit + diff --git a/tests/topotests/ripng_allow_ecmp/r5/frr.conf b/tests/topotests/ripng_allow_ecmp/r5/frr.conf new file mode 100644 index 0000000000..6d6ca56571 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/r5/frr.conf @@ -0,0 +1,14 @@ +! +int lo + ipv6 address 2001:db8:2::1/64 +! +int r5-eth0 + ipv6 address 2001:db8:1::5/64 +! +router ripng + redistribute connected + network 2001:db8:1::/64 + network 2001:db8:2::/64 + timers basic 5 15 10 +exit + diff --git a/tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py b/tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py new file mode 100644 index 0000000000..08bb999928 --- /dev/null +++ b/tests/topotests/ripng_allow_ecmp/test_ripng_allow_ecmp.py @@ -0,0 +1,93 @@ +#!/usr/bin/env python +# SPDX-License-Identifier: ISC + +# Copyright (c) 2023 by +# Donatas Abraitis <donatas@opensourcerouting.org> +# + +""" +Test if RIPng `allow-ecmp` command works correctly. +""" + +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, TopoRouter, get_topogen +from lib.common_config import step + +pytestmark = [pytest.mark.ripngd] + + +def setup_module(mod): + topodef = {"s1": ("r1", "r2", "r3", "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_ripng_allow_ecmp(): + tgen = get_topogen() + + if tgen.routers_have_failure(): + pytest.skip(tgen.errors) + + r1 = tgen.gears["r1"] + + def _show_routes(nh_num): + output = json.loads(r1.vtysh_cmd("show ipv6 route json")) + expected = { + "2001:db8:2::/64": [ + { + "internalNextHopNum": nh_num, + "internalNextHopActiveNum": nh_num, + } + ] + } + return topotest.json_cmp(output, expected) + + test_func = functools.partial(_show_routes, 4) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert ( + result is None + ), "Can't see 2001:db8:2::/64 as multipath (4) in `show ipv6 route`" + + step( + "Configure allow-ecmp 2, ECMP group routes SHOULD have next-hops with the lowest IPs" + ) + r1.vtysh_cmd( + """ + configure terminal + router ripng + allow-ecmp 2 + """ + ) + + test_func = functools.partial(_show_routes, 2) + _, result = topotest.run_and_expect(test_func, None, count=60, wait=1) + assert ( + result is None + ), "Can't see 2001:db8:2::/64 as multipath (2) in `show ipv6 route`" + + +if __name__ == "__main__": + args = ["-s"] + sys.argv[1:] + sys.exit(pytest.main(args)) diff --git a/yang/frr-ripngd.yang b/yang/frr-ripngd.yang index 7b2b135fb5..4aeaf36400 100644 --- a/yang/frr-ripngd.yang +++ b/yang/frr-ripngd.yang @@ -86,8 +86,8 @@ module frr-ripngd { "VRF name."; } leaf allow-ecmp { - type boolean; - default "false"; + type uint8; + default 0; description "Allow equal-cost multi-path."; } diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c index 3653f71527..a768c33a30 100644 --- a/zebra/zebra_dplane.c +++ b/zebra/zebra_dplane.c @@ -3799,8 +3799,7 @@ tc_qdisc_update_internal(enum dplane_op_e op, } else { atomic_fetch_add_explicit(&zdplane_info.dg_tcs_errors, 1, memory_order_relaxed); - if (ctx) - dplane_ctx_free(&ctx); + dplane_ctx_free(&ctx); } return result; @@ -3830,8 +3829,7 @@ tc_class_update_internal(enum dplane_op_e op, struct zebra_tc_class *class) } else { atomic_fetch_add_explicit(&zdplane_info.dg_tcs_errors, 1, memory_order_relaxed); - if (ctx) - dplane_ctx_free(&ctx); + dplane_ctx_free(&ctx); } return result; @@ -3861,8 +3859,7 @@ tc_filter_update_internal(enum dplane_op_e op, struct zebra_tc_filter *filter) } else { atomic_fetch_add_explicit(&zdplane_info.dg_tcs_errors, 1, memory_order_relaxed); - if (ctx) - dplane_ctx_free(&ctx); + dplane_ctx_free(&ctx); } return result; @@ -4230,8 +4227,7 @@ done: else { atomic_fetch_add_explicit(&zdplane_info.dg_lsp_errors, 1, memory_order_relaxed); - if (ctx) - dplane_ctx_free(&ctx); + dplane_ctx_free(&ctx); } return result; } diff --git a/zebra/zebra_fpm.c b/zebra/zebra_fpm.c index e379a5868c..699f3ed110 100644 --- a/zebra/zebra_fpm.c +++ b/zebra/zebra_fpm.c @@ -497,6 +497,11 @@ static inline void zfpm_connect_off(void) EVENT_OFF(zfpm_g->t_connect); } +static inline void zfpm_conn_down_off(void) +{ + EVENT_OFF(zfpm_g->t_conn_down); +} + /* * zfpm_conn_up_thread_cb * @@ -635,8 +640,6 @@ static void zfpm_conn_down_thread_cb(struct event *thread) while ((mac = TAILQ_FIRST(&zfpm_g->mac_q)) != NULL) zfpm_mac_info_del(mac); - zfpm_g->t_conn_down = NULL; - iter = &zfpm_g->t_conn_down_state.iter; while ((rnode = zfpm_rnodes_iter_next(iter))) { @@ -667,7 +670,6 @@ static void zfpm_conn_down_thread_cb(struct event *thread) zfpm_g->stats.t_conn_down_yields++; zfpm_rnodes_iter_pause(iter); - zfpm_g->t_conn_down = NULL; event_add_timer_msec(zfpm_g->master, zfpm_conn_down_thread_cb, NULL, 0, &zfpm_g->t_conn_down); return; @@ -712,7 +714,7 @@ static void zfpm_connection_down(const char *detail) */ assert(!zfpm_g->t_conn_down); zfpm_rnodes_iter_init(&zfpm_g->t_conn_down_state.iter); - zfpm_g->t_conn_down = NULL; + zfpm_conn_down_off(); event_add_timer_msec(zfpm_g->master, zfpm_conn_down_thread_cb, NULL, 0, &zfpm_g->t_conn_down); zfpm_g->stats.t_conn_down_starts++; @@ -2042,10 +2044,13 @@ static int zfpm_fini(void) zfpm_write_off(); zfpm_read_off(); zfpm_connect_off(); + zfpm_conn_down_off(); zfpm_stop_stats_timer(); hook_unregister(rib_update, zfpm_trigger_update); + hook_unregister(zebra_rmac_update, zfpm_trigger_rmac_update); + return 0; } diff --git a/zebra/zebra_fpm_netlink.c b/zebra/zebra_fpm_netlink.c index 5adca14d71..ba34951e76 100644 --- a/zebra/zebra_fpm_netlink.c +++ b/zebra/zebra_fpm_netlink.c @@ -252,20 +252,15 @@ static int netlink_route_info_fill(struct netlink_route_info *ri, int cmd, rib_dest_t *dest, struct route_entry *re) { struct nexthop *nexthop; - struct rib_table_info *table_info = - rib_table_info(rib_dest_table(dest)); - struct zebra_vrf *zvrf = table_info->zvrf; memset(ri, 0, sizeof(*ri)); ri->prefix = rib_dest_prefix(dest); ri->af = rib_dest_af(dest); - if (zvrf && zvrf->zns) - ri->nlmsg_pid = zvrf->zns->netlink_dplane_out.snl.nl_pid; + ri->nlmsg_pid = pid; ri->nlmsg_type = cmd; - ri->rtm_table = table_info->table_id; ri->rtm_protocol = RTPROT_UNSPEC; /* @@ -280,6 +275,8 @@ static int netlink_route_info_fill(struct netlink_route_info *ri, int cmd, return 0; } + ri->rtm_table = re->table; + ri->rtm_protocol = netlink_proto_from_route_type(re->type); ri->rtm_type = RTN_UNICAST; ri->metric = &re->metric; diff --git a/zebra/zebra_ptm.c b/zebra/zebra_ptm.c index a1fee840df..a678e71734 100644 --- a/zebra/zebra_ptm.c +++ b/zebra/zebra_ptm.c @@ -16,6 +16,7 @@ #include "ptm_lib.h" #include "rib.h" #include "stream.h" +#include "lib/version.h" #include "vrf.h" #include "vty.h" #include "lib_errors.h" @@ -1184,7 +1185,7 @@ struct ptm_process { TAILQ_HEAD(ppqueue, ptm_process) ppqueue; DEFINE_MTYPE_STATIC(ZEBRA, ZEBRA_PTM_BFD_PROCESS, - "PTM BFD process registration table."); + "PTM BFD process reg table"); /* * Prototypes. |