From 9d51470ee234ee75bfc3a090b5c8f2bdbb969c45 Mon Sep 17 00:00:00 2001 From: Joachim Wiberg Date: Tue, 10 Dec 2024 13:23:50 +0100 Subject: [PATCH 01/10] utils: add srop, for operating on target sysrepo db Signed-off-by: Joachim Wiberg --- utils/srop | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) create mode 100755 utils/srop diff --git a/utils/srop b/utils/srop new file mode 100755 index 000000000..e2e58a541 --- /dev/null +++ b/utils/srop @@ -0,0 +1,127 @@ +#!/bin/sh +# Interact with sysrepo in a Buildroot setup + +usage() +{ + cat <&2 + exit 1 + fi +fi + +if [ -d "$O/host" ]; then + HOST_DIR="$O/host" +else + echo "*** Error: cannot find Buildroot host binaries dir!" >&2 + exit 1 +fi + +MOD="$O/target/usr/share/yang/modules/confd/" +CTL="$HOST_DIR/bin/sysrepoctl" +CFG="$HOST_DIR/bin/sysrepocfg" + +while [ "$1" != "" ]; do + case $1 in + -e) + shift + features="$features -e $1" + ;; + -g) + shift + group="-g $1" + ;; + -o) + shift + owner="-o $1" + ;; + -m) + shift + perms="-p $1" + ;; + *) + break + ;; + esac + shift +done + +cmd=$1 +if [ -n "$cmd" ]; then + shift +fi + +trap cleanup INT TERM EXIT + +# Trigger separate namespace for sysrepoctl cmds +export SYSREPO_SHM_PREFIX=sr_buildroot + +case $cmd in + cat) + $CFG -X -f json + ;; + help) + usage + ;; + dump) + $CTL -h + $CFG -h + ;; + ls) + $CTL -l + ;; + install) + if [ -f "$1" ]; then + M="$1" + else + M="$MOD/$1" + fi + $CTL -i "$M" -s "$MOD" $perms $owner $group $features -v3 + ;; + modify) + $CTL -c "$1" $perms $owner $group $features -v3 + ;; + uninstall) + $CTL -u "$1" -f -v3 + ;; + *) + echo "NOP" + ;; +esac From 26d06bd13b5cdcbf53b84fca79b2f959d4a8ca2f Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 13:30:03 +0100 Subject: [PATCH 02/10] skeleton: mstpd: Let /sbin/bridge-stp add/remove bridges --- package/skeleton-init-finit/skeleton/etc/bridge-stp.conf | 3 +++ package/skeleton-init-finit/skeleton/etc/default/mstpd | 3 +++ .../skeleton/etc/finit.d/available/mstpd.conf | 8 ++++---- 3 files changed, 10 insertions(+), 4 deletions(-) create mode 100644 package/skeleton-init-finit/skeleton/etc/bridge-stp.conf create mode 100644 package/skeleton-init-finit/skeleton/etc/default/mstpd diff --git a/package/skeleton-init-finit/skeleton/etc/bridge-stp.conf b/package/skeleton-init-finit/skeleton/etc/bridge-stp.conf new file mode 100644 index 000000000..ccfef53b2 --- /dev/null +++ b/package/skeleton-init-finit/skeleton/etc/bridge-stp.conf @@ -0,0 +1,3 @@ +# Daemon is managed by finit(1), /sbin/bridge-stp should assume that +# it is already running and simply add/remove bridges as necessary +MANAGE_MSTPD='n' diff --git a/package/skeleton-init-finit/skeleton/etc/default/mstpd b/package/skeleton-init-finit/skeleton/etc/default/mstpd new file mode 100644 index 000000000..4afe12e9b --- /dev/null +++ b/package/skeleton-init-finit/skeleton/etc/default/mstpd @@ -0,0 +1,3 @@ +# -d: Do _not_ [sic] daemonize +# -s: Log to syslog +MSTPD_ARGS="-d -s" diff --git a/package/skeleton-init-finit/skeleton/etc/finit.d/available/mstpd.conf b/package/skeleton-init-finit/skeleton/etc/finit.d/available/mstpd.conf index bae2749be..d2278d50d 100644 --- a/package/skeleton-init-finit/skeleton/etc/finit.d/available/mstpd.conf +++ b/package/skeleton-init-finit/skeleton/etc/finit.d/available/mstpd.conf @@ -1,4 +1,4 @@ -# Make sure to configure the bridge to run on before starting mstpd -# Note: all 'sysv' type services are handed an extra 'start' or 'stop' -# argument when starting and stopping. -sysv name:mstpd [0123456] pid:!/run/mstpd.pid bridge-stp br0 -- MSTP daemon +# A single mstpd instance can manage multiple bridges, which are +# dynamically added/removed by the kernel via the /sbin/bridge-stp +# usermode helper. +service [S0123456789] env:-/etc/default/mstpd mstpd $MSTPD_ARGS -- Spanning Tree daemon From f041d009b594ddfd9b847ba433bfb30094094962 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 22:08:56 +0100 Subject: [PATCH 03/10] defconfig: all: Build mstpd --- configs/aarch64_defconfig | 1 + configs/aarch64_minimal_defconfig | 1 + configs/riscv64_defconfig | 1 + configs/x86_64_defconfig | 1 + configs/x86_64_minimal_defconfig | 1 + 5 files changed, 5 insertions(+) diff --git a/configs/aarch64_defconfig b/configs/aarch64_defconfig index 62dd557ad..87323b45d 100644 --- a/configs/aarch64_defconfig +++ b/configs/aarch64_defconfig @@ -76,6 +76,7 @@ BR2_PACKAGE_IPROUTE2=y BR2_PACKAGE_IPTABLES_NFTABLES=y BR2_PACKAGE_IPUTILS=y BR2_PACKAGE_LLDPD=y +BR2_PACKAGE_MSTPD=y BR2_PACKAGE_NETCALC=y BR2_PACKAGE_NETCAT_OPENBSD=y BR2_PACKAGE_NETSNMP=y diff --git a/configs/aarch64_minimal_defconfig b/configs/aarch64_minimal_defconfig index abcd8f11e..e76b533bf 100644 --- a/configs/aarch64_minimal_defconfig +++ b/configs/aarch64_minimal_defconfig @@ -71,6 +71,7 @@ BR2_PACKAGE_FRR=y BR2_PACKAGE_IPROUTE2=y BR2_PACKAGE_IPUTILS=y BR2_PACKAGE_LLDPD=y +BR2_PACKAGE_MSTPD=y BR2_PACKAGE_NETCALC=y BR2_PACKAGE_NGINX=y BR2_PACKAGE_NGINX_HTTP_SSL_MODULE=y diff --git a/configs/riscv64_defconfig b/configs/riscv64_defconfig index 0618f6581..d72086540 100644 --- a/configs/riscv64_defconfig +++ b/configs/riscv64_defconfig @@ -88,6 +88,7 @@ BR2_PACKAGE_IPROUTE2=y BR2_PACKAGE_IPTABLES_NFTABLES=y BR2_PACKAGE_IPUTILS=y BR2_PACKAGE_LLDPD=y +BR2_PACKAGE_MSTPD=y BR2_PACKAGE_NETCALC=y BR2_PACKAGE_NETCAT_OPENBSD=y BR2_PACKAGE_NETSNMP=y diff --git a/configs/x86_64_defconfig b/configs/x86_64_defconfig index c9b88924e..a4cb735f2 100644 --- a/configs/x86_64_defconfig +++ b/configs/x86_64_defconfig @@ -76,6 +76,7 @@ BR2_PACKAGE_IPROUTE2=y BR2_PACKAGE_IPTABLES_NFTABLES=y BR2_PACKAGE_IPUTILS=y BR2_PACKAGE_LLDPD=y +BR2_PACKAGE_MSTPD=y BR2_PACKAGE_NETCALC=y BR2_PACKAGE_NETCAT_OPENBSD=y BR2_PACKAGE_NETSNMP=y diff --git a/configs/x86_64_minimal_defconfig b/configs/x86_64_minimal_defconfig index eac7370a4..6de8b9a9e 100644 --- a/configs/x86_64_minimal_defconfig +++ b/configs/x86_64_minimal_defconfig @@ -71,6 +71,7 @@ BR2_PACKAGE_FRR=y BR2_PACKAGE_IPROUTE2=y BR2_PACKAGE_IPUTILS=y BR2_PACKAGE_LLDPD=y +BR2_PACKAGE_MSTPD=y BR2_PACKAGE_NETCALC=y BR2_PACKAGE_NGINX=y BR2_PACKAGE_NGINX_HTTP_SSL_MODULE=y From b425edffce74f1c7f9ff51b111f831a6231edaae Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Fri, 20 Dec 2024 16:34:57 +0100 Subject: [PATCH 04/10] libsrx: Add lyx versions of lyd_find_xpath - Helper to get a set of nodes, using a formatted path - Helper to get a single node, using a formatted path --- src/libsrx/src/lyx.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ src/libsrx/src/lyx.h | 6 ++++++ 2 files changed, 50 insertions(+) diff --git a/src/libsrx/src/lyx.c b/src/libsrx/src/lyx.c index b669c2adb..e7ca28336 100644 --- a/src/libsrx/src/lyx.c +++ b/src/libsrx/src/lyx.c @@ -147,6 +147,50 @@ struct lyd_node *lydx_find_by_name(struct lyd_node *from, const char *by, const return NULL; } +struct ly_set *lydx_find_vxpathf(struct lyd_node *ctx, const char *xpfmt, va_list ap) +{ + struct ly_set *set; + char *xpath; + int err; + + vasprintf(&xpath, xpfmt, ap); + + err = lyd_find_xpath(ctx, xpath, &set); + free(xpath); + + return err ? NULL : set; +} + +struct ly_set *lydx_find_xpathf(struct lyd_node *ctx, const char *xpfmt, ...) +{ + struct ly_set *set; + va_list ap; + + va_start(ap, xpfmt); + set = lydx_find_vxpathf(ctx, xpfmt, ap); + va_end(ap); + return set; +} + +struct lyd_node *lydx_get_xpathf(struct lyd_node *ctx, const char *xpfmt, ...) +{ + struct lyd_node *node = NULL; + struct ly_set *set; + va_list ap; + + va_start(ap, xpfmt); + set = lydx_find_vxpathf(ctx, xpfmt, ap); + va_end(ap); + if (!set) + return NULL; + + if (set->count == 1) + node = *set->dnodes; + + ly_set_free(set, NULL); + return node; +} + const char *lydx_get_mattr(struct lyd_node *node, const char *name) { struct lyd_meta *meta; diff --git a/src/libsrx/src/lyx.h b/src/libsrx/src/lyx.h index e4ea6edcf..da0f3f7df 100644 --- a/src/libsrx/src/lyx.h +++ b/src/libsrx/src/lyx.h @@ -39,6 +39,12 @@ struct lyd_node *lydx_vdescend(struct lyd_node *from, va_list ap); struct lyd_node *lydx_get_descendant(struct lyd_node *from, ...); struct lyd_node *lydx_find_by_name(struct lyd_node *from, const char *by, const char *name); +struct ly_set *lydx_find_vxpathf(struct lyd_node *ctx, const char *xpfmt, va_list ap); +struct ly_set *lydx_find_xpathf(struct lyd_node *ctx, const char *xpfmt, ...) + __attribute__ ((format (printf, 2, 3))); +struct lyd_node *lydx_get_xpathf(struct lyd_node *ctx, const char *xpfmt, ...) + __attribute__ ((format (printf, 2, 3))); + const char *lydx_get_mattr(struct lyd_node *node, const char *name); const char *lydx_get_attr(struct lyd_node *sibling, const char *name); const char *lydx_get_cattr(struct lyd_node *parent, const char *name); From bfeeade43be62edefffa3fb9061e33b0c441d633 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Thu, 19 Dec 2024 14:38:40 +0100 Subject: [PATCH 05/10] confd: bridge: Add STP/RSTP support --- .../rootfs/etc/finit.d/available/mstpd.conf | 8 + .../rootfs/etc/finit.d/enabled/mstpd.conf | 1 + doc/ChangeLog.md | 1 + src/confd/bin/Makefile.am | 3 +- src/confd/bin/mstpd-wait-online | 16 ++ src/confd/src/dagger.h | 5 +- src/confd/src/ietf-interfaces.c | 5 + src/confd/src/ietf-interfaces.h | 1 + src/confd/src/infix-if-bridge-port.c | 76 +++++++ src/confd/src/infix-if-bridge.c | 89 ++++++++ src/confd/yang/confd.inc | 2 +- src/confd/yang/containers.inc | 2 +- src/confd/yang/infix-if-bridge.yang | 190 +++++++++++++++++- ...8.yang => infix-if-bridge@2025-01-08.yang} | 0 src/confd/yang/infix-interfaces.yang | 5 + ....yang => infix-interfaces@2025-01-08.yang} | 0 16 files changed, 399 insertions(+), 5 deletions(-) create mode 100644 board/common/rootfs/etc/finit.d/available/mstpd.conf create mode 120000 board/common/rootfs/etc/finit.d/enabled/mstpd.conf create mode 100644 src/confd/bin/mstpd-wait-online rename src/confd/yang/{infix-if-bridge@2024-11-28.yang => infix-if-bridge@2025-01-08.yang} (100%) rename src/confd/yang/{infix-interfaces@2024-11-27.yang => infix-interfaces@2025-01-08.yang} (100%) diff --git a/board/common/rootfs/etc/finit.d/available/mstpd.conf b/board/common/rootfs/etc/finit.d/available/mstpd.conf new file mode 100644 index 000000000..211aaa8eb --- /dev/null +++ b/board/common/rootfs/etc/finit.d/available/mstpd.conf @@ -0,0 +1,8 @@ +# A single mstpd instance can manage multiple bridges, which are +# dynamically added/removed by the kernel via the /sbin/bridge-stp +# usermode helper. We depend on usr/mstpd so that confd can +# enable/disable the service without an initctl barrier, since it +# needs to already be running when a bridge interface with spanning +# tree enabled is created. +service env:-/etc/default/mstpd \ + [S0123456789] mstpd $MSTPD_ARGS -- Spanning Tree daemon diff --git a/board/common/rootfs/etc/finit.d/enabled/mstpd.conf b/board/common/rootfs/etc/finit.d/enabled/mstpd.conf new file mode 120000 index 000000000..e55e364d2 --- /dev/null +++ b/board/common/rootfs/etc/finit.d/enabled/mstpd.conf @@ -0,0 +1 @@ +../available/mstpd.conf \ No newline at end of file diff --git a/doc/ChangeLog.md b/doc/ChangeLog.md index 7507412eb..f176a5d17 100644 --- a/doc/ChangeLog.md +++ b/doc/ChangeLog.md @@ -18,6 +18,7 @@ All notable changes to the project are documented in this file. - SSH Server is now configurable, issue #441 SSH Server and NETCONF Server now uses the same SSH hostkey in factory-config - Add support for GRE/GRETAP tunnels + - Support for STP/RSTP on bridges ### Fixes diff --git a/src/confd/bin/Makefile.am b/src/confd/bin/Makefile.am index fd7c12d37..1748757e7 100644 --- a/src/confd/bin/Makefile.am +++ b/src/confd/bin/Makefile.am @@ -1,3 +1,4 @@ pkglibexec_SCRIPTS = bootstrap error load gen-service gen-hostname \ - gen-interfaces gen-motd gen-hardware gen-version + gen-interfaces gen-motd gen-hardware gen-version \ + mstpd-wait-online sbin_SCRIPTS = dagger migrate diff --git a/src/confd/bin/mstpd-wait-online b/src/confd/bin/mstpd-wait-online new file mode 100644 index 000000000..cb4139964 --- /dev/null +++ b/src/confd/bin/mstpd-wait-online @@ -0,0 +1,16 @@ +#!/bin/sh +# +# Block until mstpd has opened its control socket + +set -e + +timeout=${1:-10} + +for _ in $(seq $((timeout * 10))); do + ss -Haxn src '@.mstp_server*' | grep -q mstp_server && exit 0 + sleep .1 +done + +logger -I $$ -p user.error -t mstpd-wait-online \ + "Timeout waiting for mstpd to start" +exit 1 diff --git a/src/confd/src/dagger.h b/src/confd/src/dagger.h index f60408bf0..4720657b1 100644 --- a/src/confd/src/dagger.h +++ b/src/confd/src/dagger.h @@ -73,8 +73,11 @@ enum netdag_init { /* Interface specific setup (bridge VLAN PVID) of lowers */ NETDAG_INIT_LOWERS_PROTO = 65, - /* Start daemons running on interface (zcip etc.) */ + /* Start/configure daemons on interface (mstpd, zcip etc.) */ NETDAG_INIT_DAEMON = 70, + + /* Inject lower-specific configuration to daemon (mstpd) */ + NETDAG_INIT_DAEMON_LOWERS = 75, }; FILE *dagger_fopen_net_init(struct dagger *d, const char *node, enum netdag_init order, diff --git a/src/confd/src/ietf-interfaces.c b/src/confd/src/ietf-interfaces.c index 4cd6028fa..2ced2d960 100644 --- a/src/confd/src/ietf-interfaces.c +++ b/src/confd/src/ietf-interfaces.c @@ -662,6 +662,11 @@ static sr_error_t ifchange_post(sr_session_ctx_t *session, struct dagger *net, */ err = bridge_mcd_gen(cifs); + /* Whenever at least one bridge has spanning tree enabled, + * start mstpd; otherwise, stop it. + */ + err = err ? : bridge_mstpd_gen(cifs); + return err ? SR_ERR_INTERNAL : SR_ERR_OK; } diff --git a/src/confd/src/ietf-interfaces.h b/src/confd/src/ietf-interfaces.h index 7c7339505..4b74a2beb 100644 --- a/src/confd/src/ietf-interfaces.h +++ b/src/confd/src/ietf-interfaces.h @@ -57,6 +57,7 @@ int netdag_gen_ip_addrs(struct dagger *net, FILE *ip, const char *proto, struct lyd_node *cif, struct lyd_node *dif); /* infix-if-bridge.c */ +int bridge_mstpd_gen(struct lyd_node *cifs); int bridge_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip, int add); /* infix-if-bridge-mcd.c */ int bridge_mcd_gen(struct lyd_node *cifs); diff --git a/src/confd/src/infix-if-bridge-port.c b/src/confd/src/infix-if-bridge-port.c index 08103b768..42f77f2a8 100644 --- a/src/confd/src/infix-if-bridge-port.c +++ b/src/confd/src/infix-if-bridge-port.c @@ -12,6 +12,78 @@ #include "ietf-interfaces.h" +static const char *mstpd_cost_str(struct lyd_node *cost) +{ + const char *val = lyd_get_value(cost); + + if (!strcmp(val, "auto")) + return "0"; + + return val; +} + +static int gen_stp_tree(FILE *mstpctl, const char *iface, + const char *brname, struct lyd_node *tree) +{ + int mstid = 0; + + fprintf(mstpctl, "settreeportcost %s %s %d %s\n", brname, iface, mstid, + mstpd_cost_str(lydx_get_child(tree, "internal-path-cost"))); + + fprintf(mstpctl, "settreeportprio %s %s %d %s\n", brname, iface, mstid, + lydx_get_cattr(tree, "priority")); + + return 0; +} + +static int gen_stp(struct lyd_node *cif) +{ + const char *brname, *edge, *iface; + struct lyd_node *stp, *cist; + FILE *mstpctl; + int err; + + brname = lydx_get_cattr(lydx_get_child(cif, "bridge-port"), "bridge"); + if (!brname) + return 0; + + if (!lydx_get_xpathf(cif, "../interface[name='%s']/bridge/stp", brname)) + return 0; + + iface = lydx_get_cattr(cif, "name"); + stp = lydx_get_descendant(lyd_child(cif), "bridge-port", "stp", NULL); + + mstpctl = dagger_fopen_net_init(&confd.netdag, brname, + NETDAG_INIT_DAEMON_LOWERS, "init-ports.mstpctl"); + if (!mstpctl) + return -EIO; + + fputs("#!/sbin/mstpctl -b\n", mstpctl); + + edge = lydx_get_cattr(stp, "edge"); + if (!strcmp(edge, "true")) { + fprintf(mstpctl, "setportautoedge %s %s no\n", brname, iface); + fprintf(mstpctl, "setportadminedge %s %s yes\n", brname, iface); + } else if (!strcmp(edge, "false")) { + fprintf(mstpctl, "setportautoedge %s %s no\n", brname, iface); + fprintf(mstpctl, "setportadminedge %s %s no\n", brname, iface); + } else if (!strcmp(edge, "auto")) { + fprintf(mstpctl, "setportautoedge %s %s yes\n", brname, iface); + } + + cist = lydx_get_child(stp, "cist"); + err = gen_stp_tree(mstpctl, iface, brname, cist); + if (err) + goto out_close; + + fprintf(mstpctl, "setportpathcost %s %s %s\n", brname, iface, + mstpd_cost_str(lydx_get_child(cist, "external-path-cost"))); + +out_close: + fclose(mstpctl); + return err; +} + static const char *get_port_egress_mode(const char *iface, int vid, const char *brname) { static const char *modes[] = { "tagged", "untagged", NULL }; @@ -237,5 +309,9 @@ int bridge_port_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip) if (err) return err; + err = gen_stp(cif); + if (err) + return err; + return 0; } diff --git a/src/confd/src/infix-if-bridge.c b/src/confd/src/infix-if-bridge.c index cbe3cacf0..cc9d2e6fa 100644 --- a/src/confd/src/infix-if-bridge.c +++ b/src/confd/src/infix-if-bridge.c @@ -308,6 +308,91 @@ static int gen_vlan(struct ixif_br *br) /* VLAN */ +/* STP */ + +int bridge_mstpd_gen(struct lyd_node *cifs) +{ + struct ly_set *stp_brs; + int n, err = 0; + FILE *cond; + + if (lyd_find_xpath(cifs, "../interface/bridge/stp", &stp_brs)) + return -EINVAL; + + n = stp_brs->count; + ly_set_free(stp_brs, NULL); + + if (n) { + cond = dagger_fopen_next(&confd.netdag, "init", "@pre", 50, "enable-mstpd.sh"); + if (!cond) + return -EIO; + + fputs("initctl -bnq cond set mstpd\n" + "/usr/libexec/confd/mstpd-wait-online || exit 1\n", cond); + fclose(cond); + } else if (!dagger_is_bootstrap(&confd.netdag)) { + cond = dagger_fopen_current(&confd.netdag, "exit", "@post", 50, "disable-mstpd.sh"); + if (!cond) + return -EIO; + + fputs("initctl -bnq cond clear mstpd\n", cond); + fclose(cond); + } + + return err; +} + +static int gen_stp(struct ixif_br *br) +{ + struct lyd_node *stp; + FILE *mstpctl; + + stp = lydx_get_descendant(lyd_child(br->cif), "bridge", "stp", NULL); + if (!stp) { + fputs(" stp_state 0", br->bropts.fp); + return 0; + } + + fputs(" stp_state 1", br->bropts.fp); + + if (lydx_get_descendant(lyd_child(br->cif), "bridge", "vlans", NULL)) + /* Use the MSTP compatible mode of managing port + * states, rather then the legacy (and default) + * per-VLAN mode. */ + fputs(" mst_enabled 1", br->bropts.fp); + + mstpctl = dagger_fopen_net_init(&confd.netdag, br->name, + NETDAG_INIT_DAEMON, "init-br.mstpctl"); + if (!mstpctl) + return -EIO; + + fputs("#!/sbin/mstpctl -b\n", mstpctl); + + fprintf(mstpctl, "setforcevers %s %s\n", br->name, + lydx_get_cattr(stp, "force-protocol")); + + fprintf(mstpctl, "setfdelay %s %s\n", br->name, + lydx_get_cattr(stp, "forward-delay")); + + fprintf(mstpctl, "setmaxage %s %s\n", br->name, + lydx_get_cattr(stp, "max-age")); + + fprintf(mstpctl, "settxholdcount %s %s\n", br->name, + lydx_get_cattr(stp, "transmit-hold-count")); + + fprintf(mstpctl, "setmaxhops %s %s\n", br->name, + lydx_get_cattr(stp, "max-hops")); + + fprintf(mstpctl, "settreeprio %s 0 %s\n", br->name, + lydx_get_cattr(lydx_get_child(stp, "cist"), "priority")); + + fclose(mstpctl); + return 0; + +} + +/* STP */ + /* BR */ static void gen_phys_address(struct ixif_br *br) @@ -420,6 +505,10 @@ int bridge_gen(struct lyd_node *dif, struct lyd_node *cif, FILE *ip, int add) fputs(" mcast_flood_always 1", br.bropts.fp); fputs(" vlan_default_pvid 0", br.bropts.fp); + err = gen_stp(&br); + if (err) + goto out; + err = gen_vlan(&br); if (err) goto out; diff --git a/src/confd/yang/confd.inc b/src/confd/yang/confd.inc index b0a0c77be..4f87d649b 100644 --- a/src/confd/yang/confd.inc +++ b/src/confd/yang/confd.inc @@ -37,7 +37,7 @@ MODULES=( "ieee802-ethernet-interface@2019-06-21.yang" "infix-ethernet-interface@2024-02-27.yang" "infix-factory-default@2023-06-28.yang" - "infix-interfaces@2024-11-27.yang -e vlan-filtering" + "infix-interfaces@2025-01-08.yang -e vlan-filtering" # from rousette "ietf-restconf@2017-01-26.yang" diff --git a/src/confd/yang/containers.inc b/src/confd/yang/containers.inc index 6ef7e607e..6c863c96f 100644 --- a/src/confd/yang/containers.inc +++ b/src/confd/yang/containers.inc @@ -1,6 +1,6 @@ # -*- sh -*- # REMEMBER TO UPDATE infix-interfaces ALSO IN confd.inc MODULES=( - "infix-interfaces@2024-11-27.yang -e vlan-filtering -e containers" + "infix-interfaces@2025-01-08.yang -e vlan-filtering -e containers" "infix-containers@2024-11-15.yang" ) diff --git a/src/confd/yang/infix-if-bridge.yang b/src/confd/yang/infix-if-bridge.yang index 4a594b411..11adce17d 100644 --- a/src/confd/yang/infix-if-bridge.yang +++ b/src/confd/yang/infix-if-bridge.yang @@ -26,6 +26,11 @@ submodule infix-if-bridge { contact "kernelkit@googlegroups.com"; description "Linux bridge extension for ietf-interfaces."; + revision 2025-01-08 { + description "Add Spanning Tree Protocol (STP) support."; + reference "internal"; + } + revision 2024-11-28 { description "Drop must() expressions for IP addres on VLAN bridges. If a bridge is untagged member of a VLAN it should be @@ -148,6 +153,26 @@ submodule infix-if-bridge { reserved link-local multicast groups, in 01:80:C2:00:00:0X."; } + typedef stp-protocol { + description "Spanning Tree Protocol version."; + type enumeration { + enum stp { + value 0; + description "Spanning Tree Protocol."; + } + enum rstp { + value 2; + description "Rapid Spanning Tree Protocol."; + } + // enum mstp { + // value 3; + // description "Multiple Spanning Tree Protocol"; + // } + } + + reference "IEEE 802.1Q-2018 13.7.2"; + } + typedef stp-state { description "User-friendly enumeration of different bridge port operational states."; type enumeration { @@ -165,7 +190,7 @@ submodule infix-if-bridge { } enum forwarding { value 3; - description "Port is in STP FORWARDING state. This is the default vlan state."; + description "Port is in STP FORWARDING state."; } enum blocking { value 4; @@ -174,6 +199,36 @@ submodule infix-if-bridge { } } + typedef stp-priority { + type uint8 { + range "0..15"; + } + + default 8; + } + + typedef stp-bool-or-auto { + type union { + type enumeration { + enum auto; + } + type boolean; + } + default auto; + } + + typedef stp-path-cost { + type union { + type enumeration { + enum auto; + } + type uint32 { + range "0..200000000"; + } + } + default auto; + } + typedef querier-mode { description "Type of IGMP/MLD querier, recommend using 'auto'."; type enumeration { @@ -224,6 +279,7 @@ submodule infix-if-bridge { } } + /* * Shared settings */ @@ -321,6 +377,34 @@ submodule infix-if-bridge { } } + grouping stp-tree { + leaf priority { + description "Priority."; + type stp-priority; + reference "IEEE 802.1Q-2018 13.18"; + } + } + + grouping stp-tree-port { + leaf internal-path-cost { + description "Internal path cost."; + type stp-path-cost; + reference "IEEE 802.1Q-2018 13.27.33"; + } + + leaf priority { + description "Priority."; + type stp-priority; + reference "IEEE 802.1Q-2018 13.18"; + } + + leaf state { + description "Operational state."; + type stp-state; + config false; + } + } + /* * Data Nodes */ @@ -392,6 +476,77 @@ submodule infix-if-bridge { } } } + + container stp { + presence stp; + description "Spanning Tree Protocol"; + + leaf force-protocol { + description "Protocol version."; + type stp-protocol; + default rstp; + reference "IEEE 802.1Q-2018 13.7.2"; + } + + leaf forward-delay { + description "Forward delay."; + type uint8 { + range "4..30"; + } + default 15; + reference "IEEE 802.1Q-2018 13.25"; + } + + leaf max-age { + description "Max age."; + type uint8 { + range "6..40"; + } + default 20; + reference "IEEE 802.1Q-2018 13.25"; + } + + leaf transmit-hold-count { + description "Transmit hold count."; + type uint8 { + range "1..10"; + } + default 6; + reference "IEEE 802.1Q-2018 13.25"; + } + + leaf max-hops { + description "Max hops."; + type uint8 { + range "6..40"; + } + default 20; + reference "IEEE 802.1Q-2018 13.25"; + } + + container cist { + description "Common and Internal Spanning Tree."; + uses stp-tree; + } + + // list mst { + // description "Multiple Spanning Tree Instance."; + // key "mstid"; + + // leaf mstid { + // description "The MSTI identifier to which this entry applies."; + // type dot1q-types:mstid-type; + // } + + // uses stp-tree; + + // leaf-list vlan { + // type leafref { + // path "/if:interfaces/interface/bridge/vlans/vlan/vid"; + // } + // } + // } + } } } deviation "/if:interfaces/if:interface/infix-if:bridge/type/ieee8021d/multicast-filters/multicast-filter/ports/port" { @@ -506,6 +661,39 @@ submodule infix-if-bridge { } } + container stp { + description "Spanning Tree Protocol"; + reference "IEEE 802.1Q-2018 13.27"; + + leaf edge { + description "Edge."; + type stp-bool-or-auto; + } + + container cist { + description "Common and Internal Spanning Tree."; + uses stp-tree-port; + + leaf external-path-cost { + description "External path cost."; + type stp-path-cost; + reference "IEEE 802.1Q-2018 13.27.25"; + } + } + + // list mst { + // description "Multiple Spanning Tree Instance."; + // key "mstid"; + + // leaf mstid { + // description "The MSTI identifier to which this entry applies."; + // type dot1q-types:mstid-type; + // } + + // uses stp-tree-port; + // } + } + leaf stp-state { type stp-state; config false; diff --git a/src/confd/yang/infix-if-bridge@2024-11-28.yang b/src/confd/yang/infix-if-bridge@2025-01-08.yang similarity index 100% rename from src/confd/yang/infix-if-bridge@2024-11-28.yang rename to src/confd/yang/infix-if-bridge@2025-01-08.yang diff --git a/src/confd/yang/infix-interfaces.yang b/src/confd/yang/infix-interfaces.yang index 90978e07f..f339091a1 100644 --- a/src/confd/yang/infix-interfaces.yang +++ b/src/confd/yang/infix-interfaces.yang @@ -27,6 +27,11 @@ module infix-interfaces { contact "kernelkit@googlegroups.com"; description "Linux bridge and lag extensions for ietf-interfaces."; + revision 2025-01-08 { + description "Add Spanning Tree Protocol (STP) support to bridges."; + reference "internal"; + } + revision 2024-11-27 { description "Allow IP addresses directly on VLAN filtering bridges."; reference "internal"; diff --git a/src/confd/yang/infix-interfaces@2024-11-27.yang b/src/confd/yang/infix-interfaces@2025-01-08.yang similarity index 100% rename from src/confd/yang/infix-interfaces@2024-11-27.yang rename to src/confd/yang/infix-interfaces@2025-01-08.yang From 343400c5fe8da2f74946809ce1c202eb6032bdea Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 17:03:13 +0100 Subject: [PATCH 06/10] test: infamy: Default to attaching to DUTs via the "mgmt" port This convention is now well established, so it seems silly to have to repeat it in every test. --- test/infamy/env.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/infamy/env.py b/test/infamy/env.py index e8a50caa5..c7cb071f5 100644 --- a/test/infamy/env.py +++ b/test/infamy/env.py @@ -74,7 +74,7 @@ def attr(self, name, default=None): def get_password(self, node): return self.ptop.get_password(node) - def attach(self, node, port, protocol=None, test_reset=True, username = None, password = None): + def attach(self, node, port="mgmt", protocol=None, test_reset=True, username = None, password = None): """Attach to node on port using protocol.""" name = node From e4535aa856754dbda36479486f8959fffd2f305c Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 18:46:44 +0100 Subject: [PATCH 07/10] test: infamy: Make it easier to resolve mapped port names The somewhat clumsy construct of resolving a logical port name to its physical counterpart has been with us from Infamy's inception: _, physical_port = env.ltop.xlate(logical_node, logical_port) This is needlessly verbose. Since devices already have access to the mappings which applies to it, make them easy to access: target = env.attach("target") # Get physical port names via the subscript operator target["data0"] --- test/infamy/restconf.py | 1 + test/infamy/transport.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/test/infamy/restconf.py b/test/infamy/restconf.py index b1ebeed1b..67e1d1e08 100644 --- a/test/infamy/restconf.py +++ b/test/infamy/restconf.py @@ -119,6 +119,7 @@ def __init__(self, self.name = name self.location = location + self.mapping = mapping self.url_base = f"https://[{location.host}]:{location.port}" self.restconf_url = f"{self.url_base}/restconf" self.yang_url = f"{self.url_base}/yang" diff --git a/test/infamy/transport.py b/test/infamy/transport.py index 3c754a94a..922a9de2e 100644 --- a/test/infamy/transport.py +++ b/test/infamy/transport.py @@ -55,6 +55,9 @@ def call_dict(self, module, call): def call_action(self, xpath): pass + def __getitem__(self, key): + return self.mapping[key] + def get_iface(self, name): """Fetch target dict for iface and extract param from JSON""" content = self.get_data(infamy.iface.get_xpath(name)) From 44b60956a89543f93712305924feb69c150f4a14 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 15:20:12 +0100 Subject: [PATCH 08/10] test: upgrade: Add .gitignore for package symlink --- test/case/ietf_system/upgrade/bundles/.gitignore | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 test/case/ietf_system/upgrade/bundles/.gitignore diff --git a/test/case/ietf_system/upgrade/bundles/.gitignore b/test/case/ietf_system/upgrade/bundles/.gitignore new file mode 100644 index 000000000..22cb1386b --- /dev/null +++ b/test/case/ietf_system/upgrade/bundles/.gitignore @@ -0,0 +1,2 @@ +# This gets symlinked to whichever bundle we're told to upgrade to +/package From 3ef40031dd088b9a11df2a70d2472690aecb0552 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Tue, 10 Dec 2024 18:51:31 +0100 Subject: [PATCH 09/10] test: bridge_stp_basic: Add --- test/case/ietf_interfaces/Readme.adoc | 2 + .../bridge_stp_basic/Readme.adoc | 31 ++++ .../ietf_interfaces/bridge_stp_basic/test.py | 89 ++++++++++ .../bridge_stp_basic/topology.dot | 55 ++++++ .../bridge_stp_basic/topology.svg | 168 ++++++++++++++++++ .../case/ietf_interfaces/ietf_interfaces.yaml | 3 + 6 files changed, 348 insertions(+) create mode 100644 test/case/ietf_interfaces/bridge_stp_basic/Readme.adoc create mode 100755 test/case/ietf_interfaces/bridge_stp_basic/test.py create mode 100644 test/case/ietf_interfaces/bridge_stp_basic/topology.dot create mode 100644 test/case/ietf_interfaces/bridge_stp_basic/topology.svg diff --git a/test/case/ietf_interfaces/Readme.adoc b/test/case/ietf_interfaces/Readme.adoc index 61b861fa4..97e48bc58 100644 --- a/test/case/ietf_interfaces/Readme.adoc +++ b/test/case/ietf_interfaces/Readme.adoc @@ -23,6 +23,8 @@ include::bridge_fwd_dual_dut/Readme.adoc[] include::bridge_fwd_sgl_dut/Readme.adoc[] +include::bridge_stp_basic/Readme.adoc[] + include::bridge_veth/Readme.adoc[] include::bridge_vlan/Readme.adoc[] diff --git a/test/case/ietf_interfaces/bridge_stp_basic/Readme.adoc b/test/case/ietf_interfaces/bridge_stp_basic/Readme.adoc new file mode 100644 index 000000000..e017893f1 --- /dev/null +++ b/test/case/ietf_interfaces/bridge_stp_basic/Readme.adoc @@ -0,0 +1,31 @@ +=== Bridge STP Basic +==== Description +Verify that a fully connected mesh of 4 DUTs is pruned to a spanning +tree. + +Since the mesh contains 3 redundant paths, can infer that a spanning +tree has been created if all host interfaces can reach each other +while exactly three links are in the blocking state. + +==== Topology +ifdef::topdoc[] +image::../../test/case/ietf_interfaces/bridge_stp_basic/topology.svg[Bridge STP Basic topology] +endif::topdoc[] +ifndef::topdoc[] +ifdef::testgroup[] +image::bridge_stp_basic/topology.svg[Bridge STP Basic topology] +endif::testgroup[] +ifndef::testgroup[] +image::topology.svg[Bridge STP Basic topology] +endif::testgroup[] +endif::topdoc[] +==== Test sequence +. Set up topology and attach to target DUT +. Configure a bridge with spanning tree eneabled on dut a, b, c, and d +. Add an IP address to each host interface in the 10.0.0.0/24 subnet +. Verify that exactly three links are blocking +. Verify that host:a can reach host:{b,c,d} + + +<<< + diff --git a/test/case/ietf_interfaces/bridge_stp_basic/test.py b/test/case/ietf_interfaces/bridge_stp_basic/test.py new file mode 100755 index 000000000..f4a74e9cd --- /dev/null +++ b/test/case/ietf_interfaces/bridge_stp_basic/test.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +r"""Bridge STP Basic + +Verify that a fully connected mesh of 4 DUTs is pruned to a spanning +tree. + +Since the mesh contains 3 redundant paths, can infer that a spanning +tree has been created if all host interfaces can reach each other +while exactly three links are in the blocking state. + +""" +import infamy +from infamy.util import parallel, until + +def addbr(dut): + ip = { + "A": "10.0.0.101", + "B": "10.0.0.102", + "C": "10.0.0.103", + "D": "10.0.0.104" + }[dut.name] + + brports = [ + { + "name": dut[n], + "infix-interfaces:bridge-port": { + "bridge": "br0", + } + } for n in ("a", "b", "c", "d", "h") if n != dut.name.lower() + ] + + dut.put_config_dicts({ + "ietf-interfaces": { + "interfaces": { + "interface": [ + { + "name": "br0", + "type": "infix-if-type:bridge", + "enabled": True, + "bridge": { + "stp": {}, + }, + "ipv4": { + "address": [ + { + "ip": ip, + "prefix-length": 24, + } + ] + }, + } + ] + brports, + } + } + }) + +def num_blocking(dut): + num = 0 + for iface in dut.get_data("/ietf-interfaces:interfaces")["interfaces"]["interface"]: + if iface.get("bridge-port", {}).get("stp-state") == "blocking": + num += 1 + + return num + +with infamy.Test() as test: + with test.step("Set up topology and attach to target DUT"): + env = infamy.Env() + a, b, c, d = parallel(lambda: env.attach("A"), lambda: env.attach("B"), + lambda: env.attach("C"), lambda: env.attach("D")) + + host = { p: env.ltop.xlate("host", p)[1] for p in ("a", "b", "c", "d") } + + with test.step("Configure a bridge with spanning tree eneabled on dut a, b, c, and d"): + parallel(lambda: addbr(a), lambda: addbr(b), lambda: addbr(c), lambda: addbr(d)) + + with test.step("Add an IP address to each host interface in the 10.0.0.0/24 subnet"): + ns = { p: infamy.IsolatedMacVlan(host[p]).start() for p in ("a", "b", "c", "d") } + parallel(lambda: ns["a"].addip("10.0.0.1"), lambda: ns["b"].addip("10.0.0.2"), + lambda: ns["c"].addip("10.0.0.3"), lambda: ns["d"].addip("10.0.0.4")) + + with test.step("Verify that exactly three links are blocking"): + until(lambda: sum(map(num_blocking, (a, b, c, d))) == 3, 60) + + with test.step("Verify that host:a can reach host:{b,c,d}"): + parallel(lambda: ns["a"].must_reach("10.0.0.2"), + lambda: ns["a"].must_reach("10.0.0.3"), + lambda: ns["a"].must_reach("10.0.0.4")) + + test.succeed() diff --git a/test/case/ietf_interfaces/bridge_stp_basic/topology.dot b/test/case/ietf_interfaces/bridge_stp_basic/topology.dot new file mode 100644 index 000000000..b467d13c0 --- /dev/null +++ b/test/case/ietf_interfaces/bridge_stp_basic/topology.dot @@ -0,0 +1,55 @@ +graph "stp" { + layout="neato"; + overlap="false"; + esep="+80"; + + node [shape=record, fontname="DejaVu Sans Mono, Book"]; + edge [penwidth="2", fontname="DejaVu Serif, Book"]; + + host [ + label="{ { mgmtd | d | mgmta | a | b | mgmtb | c | mgmtc } | host }", + color="grey",fontcolor="grey",pos="9,0!", + kind="controller", + ]; + + A [ + label="{ A | { mgmt | h } } | { b | c | d }", + pos="6,6!", + kind="infix", + ]; + B [ + label="{ a | d | c } | { B | { h | mgmt } }", + pos="12,6!", + kind="infix", + ]; + C [ + label="{ b | a | d } | { C | { h | mgmt } }", + pos="12,3!", + kind="infix", + ]; + D [ + label="{ D | { mgmt | h } } | { a | b | c }", + pos="6,3!", + kind="infix", + ]; + + host:mgmta -- A:mgmt [kind=mgmt, color="lightgrey"] + host:mgmtb -- B:mgmt [kind=mgmt, color="lightgrey"] + host:mgmtc -- C:mgmt [kind=mgmt, color="lightgrey"] + host:mgmtd -- D:mgmt [kind=mgmt, color="lightgrey"] + + host:a -- A:h [color="cornflowerblue"] + host:b -- B:h [color="cornflowerblue"] + host:c -- C:h [color="cornflowerblue"] + host:d -- D:h [color="cornflowerblue"] + + # Ring + A:b -- B:a + B:c -- C:b + C:d -- D:c + D:a -- A:d + + # Cross-links + A:c -- C:a + B:d -- D:b +} diff --git a/test/case/ietf_interfaces/bridge_stp_basic/topology.svg b/test/case/ietf_interfaces/bridge_stp_basic/topology.svg new file mode 100644 index 000000000..aa67633c8 --- /dev/null +++ b/test/case/ietf_interfaces/bridge_stp_basic/topology.svg @@ -0,0 +1,168 @@ + + + + + + +stp + + + +host + +mgmtd + +d + +mgmta + +a + +b + +mgmtb + +c + +mgmtc + +host + + + +A + +A + +mgmt + +h + +b + +c + +d + + + +host:mgmta--A:mgmt + + + + +host:a--A:h + + + + +B + +a + +d + +c + +B + +h + +mgmt + + + +host:mgmtb--B:mgmt + + + + +host:b--B:h + + + + +C + +b + +a + +d + +C + +h + +mgmt + + + +host:mgmtc--C:mgmt + + + + +host:c--C:h + + + + +D + +D + +mgmt + +h + +a + +b + +c + + + +host:mgmtd--D:mgmt + + + + +host:d--D:h + + + + +A:b--B:a + + + + +A:c--C:a + + + + +B:c--C:b + + + + +B:d--D:b + + + + +C:d--D:c + + + + +D:a--A:d + + + + diff --git a/test/case/ietf_interfaces/ietf_interfaces.yaml b/test/case/ietf_interfaces/ietf_interfaces.yaml index 532a8de52..b5e4b846c 100644 --- a/test/case/ietf_interfaces/ietf_interfaces.yaml +++ b/test/case/ietf_interfaces/ietf_interfaces.yaml @@ -41,6 +41,9 @@ - name: bridge_fwd_dual_dut case: bridge_fwd_dual_dut/test.py +- name: bridge_stp_basic + case: bridge_stp_basic/test.py + - name: bridge_vlan_separation case: bridge_vlan_separation/test.py From f3f313bb98a00e8ee26000f5a5bd3654a734b2e2 Mon Sep 17 00:00:00 2001 From: Tobias Waldekranz Date: Fri, 20 Dec 2024 23:05:07 +0100 Subject: [PATCH 10/10] infamy: Fix default transport selection Using `sys.argv[0]` does not quite achieve the effect we want, namely that any (test-case, $PYTHONHASHSEED) tuple should always default to the same transport. For example, the following two examples should always use the same transport: $ make PYTHONHASHSEED=1337 test-sh infamy0:test$ ./case/ietf_system/hostname/test.py $ make PYTHONHASHSEED=1337 test-sh infamy0:test$ cd ./case/ietf_system/hostname infamy0:hostname$ ./test.py But, since their argv[0]'s are different, they don't. Therefore use the _last two components_ of argv[0]'s full path instead. This should: - Spread the allocation over an entire suite run, since the penultimate component is usually the test name. - Keep the allocation stable when test code is modified (which is common during debugging of failing tests) - Avoid instabilities stemming from the local storage location (i.e., home directory names etc.) --- test/infamy/env.py | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/test/infamy/env.py b/test/infamy/env.py index c7cb071f5..06f84216c 100644 --- a/test/infamy/env.py +++ b/test/infamy/env.py @@ -18,6 +18,20 @@ def attr(self, _, default=None): class ArgumentParser(argparse.ArgumentParser): + def DefaultTransport(): + """Pick pseudo-random transport + + If the user does not specify a particular transport, make sure + that any (test, $PYTHONHASHSEED) tuple will always map to the + same transport. + + """ + name = "/".join(os.path.realpath(sys.argv[0]).split(os.sep)[-2:]) + seed = os.environ.get('PYTHONHASHSEED', 0) + random.seed(f"{name}-{seed}") + + return random.choice(["netconf", "restconf"]) + def __init__(self, top): super().__init__() @@ -25,7 +39,7 @@ def __init__(self, top): self.add_argument("-l", "--logical-topology", dest="ltop", default=top) self.add_argument("-p", "--package", default=None) self.add_argument("-y", "--yangdir", default=None) - self.add_argument("-t", "--transport", default=None) + self.add_argument("-t", "--transport", default=ArgumentParser.DefaultTransport()) self.add_argument("ptop", nargs=1, metavar="topology") @@ -84,15 +98,12 @@ def attach(self, node, port="mgmt", protocol=None, test_reset=True, username = N else: mapping = None - # Test protocol always highest prio, followed by command line, - # then environment (detected from defconfig), lastly random. + # Precedence: + # 1. Caller specifies `protocol` + # 2. User specifies `-t` when executing test + # 3. One is pseudo-randomly picked based on $PYTHONHASHSEED if protocol is None: - if self.args.transport is not None: - protocol = self.args.transport - else: - hseed = os.environ.get('PYTHONHASHSEED', 0) - random.seed(f"{sys.argv[0]}-{hseed}") - protocol = random.choice(["netconf", "restconf"]) + protocol = self.args.transport if password is None: password = self.get_password(node)