diff options
| author | Jakub Kicinski <kuba@kernel.org> | 2026-04-11 01:52:46 +0300 |
|---|---|---|
| committer | Jakub Kicinski <kuba@kernel.org> | 2026-04-11 01:52:47 +0300 |
| commit | d6aa47704561b2e363821d243fd405da1ac06bcb (patch) | |
| tree | f35b656458e1e7c353f746b5c2939fce315e4738 | |
| parent | 02468f3492bfb901f53fa9a62c85b79df7fd387e (diff) | |
| parent | 20ae6d76e381eb520d0d1db526a41b22390816f6 (diff) | |
| download | linux-d6aa47704561b2e363821d243fd405da1ac06bcb.tar.xz | |
Merge branch 'net-bridge-add-stp_mode-attribute-for-stp-mode-selection'
Andy Roulin says:
====================
net: bridge: add stp_mode attribute for STP mode selection
The bridge-stp usermode helper is currently restricted to the initial
network namespace, preventing userspace STP daemons like mstpd from
operating on bridges in other namespaces. Since commit ff62198553e4
("bridge: Only call /sbin/bridge-stp for the initial network
namespace"), bridges in non-init namespaces silently fall back to
kernel STP with no way to request userspace STP.
This series adds a new IFLA_BR_STP_MODE bridge attribute that allows
explicit per-bridge control over STP mode selection. Three modes are
supported:
- auto (default): existing behavior, try /sbin/bridge-stp in
init_net, fall back to kernel STP otherwise
- user: directly enable BR_USER_STP without invoking the helper,
works in any network namespace
- kernel: directly enable BR_KERNEL_STP without invoking the helper
The user and kernel modes bypass call_usermodehelper() entirely,
addressing the security concerns discussed at [1]. Userspace is
responsible for ensuring an STP daemon manages the bridge, rather
than relying on the kernel to invoke /sbin/bridge-stp.
Patch 1 adds the kernel support. The mode can only be changed while
STP is disabled and is processed before IFLA_BR_STP_STATE in
br_changelink() so both can be set atomically in a single netlink
message.
Patch 2 adds documentation for the new attribute in the bridge docs.
Patch 3 adds a selftest with 9 test cases. The test requires iproute2
with IFLA_BR_STP_MODE support and can be run with virtme-ng:
vng --run arch/x86/boot/bzImage --skip-modules \
--overlay-rwdir /sbin --overlay-rwdir /tmp --overlay-rwdir /bin \
--exec 'cp /path/to/iproute2-next/ip/ip /bin/ip && \
cd tools/testing/selftests/net && \
bash bridge_stp_mode.sh'
iproute2 support can be found here [2].
[1] https://lore.kernel.org/netdev/565B7F7D.80208@nod.at/
[2] https://github.com/aroulin/iproute2-next/tree/bridge-stp-mode
====================
Link: https://patch.msgid.link/20260405205224.3163000-1-aroulin@nvidia.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
| -rw-r--r-- | Documentation/netlink/specs/rt-link.yaml | 12 | ||||
| -rw-r--r-- | Documentation/networking/bridge.rst | 22 | ||||
| -rw-r--r-- | include/uapi/linux/if_link.h | 39 | ||||
| -rw-r--r-- | net/bridge/br_device.c | 1 | ||||
| -rw-r--r-- | net/bridge/br_netlink.c | 24 | ||||
| -rw-r--r-- | net/bridge/br_private.h | 2 | ||||
| -rw-r--r-- | net/bridge/br_stp_if.c | 19 | ||||
| -rw-r--r-- | tools/testing/selftests/net/Makefile | 1 | ||||
| -rwxr-xr-x | tools/testing/selftests/net/bridge_stp_mode.sh | 288 |
9 files changed, 400 insertions, 8 deletions
diff --git a/Documentation/netlink/specs/rt-link.yaml b/Documentation/netlink/specs/rt-link.yaml index fcb5aaf0926f..f23aa5f229c5 100644 --- a/Documentation/netlink/specs/rt-link.yaml +++ b/Documentation/netlink/specs/rt-link.yaml @@ -840,6 +840,14 @@ definitions: entries: - p2p - mp + - + name: br-stp-mode + type: enum + enum-name: br-stp-mode + entries: + - auto + - user + - kernel attribute-sets: - @@ -1550,6 +1558,10 @@ attribute-sets: - name: fdb-max-learned type: u32 + - + name: stp-mode + type: u32 + enum: br-stp-mode - name: linkinfo-brport-attrs name-prefix: ifla-brport- diff --git a/Documentation/networking/bridge.rst b/Documentation/networking/bridge.rst index ef8b73e157b2..c1e6ea52c9e5 100644 --- a/Documentation/networking/bridge.rst +++ b/Documentation/networking/bridge.rst @@ -148,6 +148,28 @@ called by the kernel when STP is enabled/disabled on a bridge stp_state <0|1>``). The kernel enables user_stp mode if that command returns 0, or enables kernel_stp mode if that command returns any other value. +STP mode selection +------------------ + +The ``IFLA_BR_STP_MODE`` bridge attribute allows explicit control over how +STP operates when enabled, bypassing the ``/sbin/bridge-stp`` helper +entirely for the ``user`` and ``kernel`` modes. + +.. kernel-doc:: include/uapi/linux/if_link.h + :doc: Bridge STP mode values + +The default mode is ``BR_STP_MODE_AUTO``, which preserves the traditional +behavior of invoking the ``/sbin/bridge-stp`` helper. The ``user`` and +``kernel`` modes are particularly useful in network namespace environments +where the helper mechanism is not available, as ``call_usermodehelper()`` +is restricted to the initial network namespace. + +Example:: + + ip link set dev br0 type bridge stp_mode user stp_state 1 + +The mode can only be changed while STP is disabled. + VLAN ==== diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h index 280bb1780512..79ce4bc24cba 100644 --- a/include/uapi/linux/if_link.h +++ b/include/uapi/linux/if_link.h @@ -744,6 +744,11 @@ enum in6_addr_gen_mode { * @IFLA_BR_FDB_MAX_LEARNED * Set the number of max dynamically learned FDB entries for the current * bridge. + * + * @IFLA_BR_STP_MODE + * Set the STP mode for the bridge, which controls how the bridge + * selects between userspace and kernel STP. The valid values are + * documented below in the ``BR_STP_MODE_*`` constants. */ enum { IFLA_BR_UNSPEC, @@ -796,11 +801,45 @@ enum { IFLA_BR_MCAST_QUERIER_STATE, IFLA_BR_FDB_N_LEARNED, IFLA_BR_FDB_MAX_LEARNED, + IFLA_BR_STP_MODE, __IFLA_BR_MAX, }; #define IFLA_BR_MAX (__IFLA_BR_MAX - 1) +/** + * DOC: Bridge STP mode values + * + * @BR_STP_MODE_AUTO + * Default. The kernel invokes the ``/sbin/bridge-stp`` helper to hand + * the bridge to a userspace STP daemon (e.g. mstpd). Only attempted in + * the initial network namespace; in other namespaces this falls back to + * kernel STP. + * + * @BR_STP_MODE_USER + * Directly enable userspace STP (``BR_USER_STP``) without invoking the + * ``/sbin/bridge-stp`` helper. Works in any network namespace. + * Userspace is responsible for ensuring an STP daemon manages the + * bridge. + * + * @BR_STP_MODE_KERNEL + * Directly enable kernel STP (``BR_KERNEL_STP``) without invoking the + * helper. + * + * The mode controls how the bridge selects between userspace and kernel + * STP when STP is enabled via ``IFLA_BR_STP_STATE``. It can only be + * changed while STP is disabled (``IFLA_BR_STP_STATE`` == 0), returns + * ``-EBUSY`` otherwise. The default value is ``BR_STP_MODE_AUTO``. + */ +enum br_stp_mode { + BR_STP_MODE_AUTO, + BR_STP_MODE_USER, + BR_STP_MODE_KERNEL, + __BR_STP_MODE_MAX +}; + +#define BR_STP_MODE_MAX (__BR_STP_MODE_MAX - 1) + struct ifla_bridge_id { __u8 prio[2]; __u8 addr[6]; /* ETH_ALEN */ diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c index f7502e62dd35..a35ceae0a6f2 100644 --- a/net/bridge/br_device.c +++ b/net/bridge/br_device.c @@ -518,6 +518,7 @@ void br_dev_setup(struct net_device *dev) ether_addr_copy(br->group_addr, eth_stp_addr); br->stp_enabled = BR_NO_STP; + br->stp_mode = BR_STP_MODE_AUTO; br->group_fwd_mask = BR_GROUPFWD_DEFAULT; br->group_fwd_mask_required = BR_GROUPFWD_DEFAULT; diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c index 0264730938f4..6fd5386a1d64 100644 --- a/net/bridge/br_netlink.c +++ b/net/bridge/br_netlink.c @@ -1270,6 +1270,9 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = { NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)), [IFLA_BR_FDB_N_LEARNED] = { .type = NLA_REJECT }, [IFLA_BR_FDB_MAX_LEARNED] = { .type = NLA_U32 }, + [IFLA_BR_STP_MODE] = NLA_POLICY_RANGE(NLA_U32, + BR_STP_MODE_AUTO, + BR_STP_MODE_MAX), }; static int br_changelink(struct net_device *brdev, struct nlattr *tb[], @@ -1306,6 +1309,23 @@ static int br_changelink(struct net_device *brdev, struct nlattr *tb[], return err; } + if (data[IFLA_BR_STP_MODE]) { + u32 mode = nla_get_u32(data[IFLA_BR_STP_MODE]); + + if (mode != br->stp_mode) { + bool stp_off = br->stp_enabled == BR_NO_STP || + (data[IFLA_BR_STP_STATE] && + !nla_get_u32(data[IFLA_BR_STP_STATE])); + + if (!stp_off) { + NL_SET_ERR_MSG_MOD(extack, + "Can't change STP mode while STP is enabled"); + return -EBUSY; + } + } + br->stp_mode = mode; + } + if (data[IFLA_BR_STP_STATE]) { u32 stp_enabled = nla_get_u32(data[IFLA_BR_STP_STATE]); @@ -1634,6 +1654,7 @@ static size_t br_get_size(const struct net_device *brdev) nla_total_size(sizeof(u8)) + /* IFLA_BR_NF_CALL_ARPTABLES */ #endif nla_total_size(sizeof(struct br_boolopt_multi)) + /* IFLA_BR_MULTI_BOOLOPT */ + nla_total_size(sizeof(u32)) + /* IFLA_BR_STP_MODE */ 0; } @@ -1686,7 +1707,8 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev) nla_put(skb, IFLA_BR_MULTI_BOOLOPT, sizeof(bm), &bm) || nla_put_u32(skb, IFLA_BR_FDB_N_LEARNED, atomic_read(&br->fdb_n_learned)) || - nla_put_u32(skb, IFLA_BR_FDB_MAX_LEARNED, br->fdb_max_learned)) + nla_put_u32(skb, IFLA_BR_FDB_MAX_LEARNED, br->fdb_max_learned) || + nla_put_u32(skb, IFLA_BR_STP_MODE, br->stp_mode)) return -EMSGSIZE; #ifdef CONFIG_BRIDGE_VLAN_FILTERING diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h index 6dbca845e625..361a9b84451e 100644 --- a/net/bridge/br_private.h +++ b/net/bridge/br_private.h @@ -523,6 +523,8 @@ struct net_bridge { unsigned char topology_change; unsigned char topology_change_detected; u16 root_port; + u8 stp_mode; + bool stp_helper_active; unsigned long max_age; unsigned long hello_time; unsigned long forward_delay; diff --git a/net/bridge/br_stp_if.c b/net/bridge/br_stp_if.c index cc4b27ff1b08..28c1d3f7e22f 100644 --- a/net/bridge/br_stp_if.c +++ b/net/bridge/br_stp_if.c @@ -149,7 +149,9 @@ static void br_stp_start(struct net_bridge *br) { int err = -ENOENT; - if (net_eq(dev_net(br->dev), &init_net)) + /* AUTO mode: try bridge-stp helper in init_net only */ + if (br->stp_mode == BR_STP_MODE_AUTO && + net_eq(dev_net(br->dev), &init_net)) err = br_stp_call_user(br, "start"); if (err && err != -ENOENT) @@ -162,8 +164,9 @@ static void br_stp_start(struct net_bridge *br) else if (br->bridge_forward_delay > BR_MAX_FORWARD_DELAY) __br_set_forward_delay(br, BR_MAX_FORWARD_DELAY); - if (!err) { + if (br->stp_mode == BR_STP_MODE_USER || !err) { br->stp_enabled = BR_USER_STP; + br->stp_helper_active = !err; br_debug(br, "userspace STP started\n"); } else { br->stp_enabled = BR_KERNEL_STP; @@ -180,12 +183,14 @@ static void br_stp_start(struct net_bridge *br) static void br_stp_stop(struct net_bridge *br) { - int err; - if (br->stp_enabled == BR_USER_STP) { - err = br_stp_call_user(br, "stop"); - if (err) - br_err(br, "failed to stop userspace STP (%d)\n", err); + if (br->stp_helper_active) { + int err = br_stp_call_user(br, "stop"); + + if (err) + br_err(br, "failed to stop userspace STP (%d)\n", err); + br->stp_helper_active = false; + } /* To start timers on any ports left in blocking */ spin_lock_bh(&br->lock); diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile index cab74ebdaced..231245a95879 100644 --- a/tools/testing/selftests/net/Makefile +++ b/tools/testing/selftests/net/Makefile @@ -15,6 +15,7 @@ TEST_PROGS := \ big_tcp.sh \ bind_bhash.sh \ bpf_offload.py \ + bridge_stp_mode.sh \ bridge_vlan_dump.sh \ broadcast_ether_dst.sh \ broadcast_pmtu.sh \ diff --git a/tools/testing/selftests/net/bridge_stp_mode.sh b/tools/testing/selftests/net/bridge_stp_mode.sh new file mode 100755 index 000000000000..0c81fd029d79 --- /dev/null +++ b/tools/testing/selftests/net/bridge_stp_mode.sh @@ -0,0 +1,288 @@ +#!/bin/bash +# SPDX-License-Identifier: GPL-2.0 +# shellcheck disable=SC2034,SC2154,SC2317,SC2329 +# +# Test for bridge STP mode selection (IFLA_BR_STP_MODE). +# +# Verifies that: +# - stp_mode defaults to auto on new bridges +# - stp_mode can be toggled between user, kernel, and auto +# - stp_mode change is rejected while STP is active (-EBUSY) +# - stp_mode user in a netns yields userspace STP (stp_state=2) +# - stp_mode kernel forces kernel STP (stp_state=1) +# - stp_mode auto preserves traditional fallback to kernel STP +# - stp_mode and stp_state can be set atomically in one message +# - stp_mode persists across STP disable/enable cycles + +source lib.sh + +require_command jq + +ALL_TESTS=" + test_default_auto + test_set_modes + test_reject_change_while_stp_active + test_idempotent_mode_while_stp_active + test_user_mode_in_netns + test_kernel_mode + test_auto_mode + test_atomic_mode_and_state + test_mode_persistence +" + +bridge_info_get() +{ + ip -n "$NS1" -d -j link show "$1" | \ + jq -r ".[0].linkinfo.info_data.$2" +} + +check_stp_mode() +{ + local br=$1; shift + local expected=$1; shift + local msg=$1; shift + local val + + val=$(bridge_info_get "$br" stp_mode) + [ "$val" = "$expected" ] + check_err $? "$msg: expected $expected, got $val" +} + +check_stp_state() +{ + local br=$1; shift + local expected=$1; shift + local msg=$1; shift + local val + + val=$(bridge_info_get "$br" stp_state) + [ "$val" = "$expected" ] + check_err $? "$msg: expected $expected, got $val" +} + +# Create a bridge in NS1, bring it up, and defer its deletion. +bridge_create() +{ + ip -n "$NS1" link add "$1" type bridge + ip -n "$NS1" link set "$1" up + defer ip -n "$NS1" link del "$1" +} + +setup_prepare() +{ + setup_ns NS1 +} + +cleanup() +{ + defer_scopes_cleanup + cleanup_all_ns +} + +# Check that stp_mode defaults to auto when creating a bridge. +test_default_auto() +{ + RET=0 + + ip -n "$NS1" link add br-test type bridge + defer ip -n "$NS1" link del br-test + + check_stp_mode br-test auto "stp_mode default" + + log_test "stp_mode defaults to auto" +} + +# Test setting stp_mode to user, kernel, and back to auto. +test_set_modes() +{ + RET=0 + + ip -n "$NS1" link add br-test type bridge + defer ip -n "$NS1" link del br-test + + ip -n "$NS1" link set dev br-test type bridge stp_mode user + check_err $? "Failed to set stp_mode to user" + check_stp_mode br-test user "after set user" + + ip -n "$NS1" link set dev br-test type bridge stp_mode kernel + check_err $? "Failed to set stp_mode to kernel" + check_stp_mode br-test kernel "after set kernel" + + ip -n "$NS1" link set dev br-test type bridge stp_mode auto + check_err $? "Failed to set stp_mode to auto" + check_stp_mode br-test auto "after set auto" + + log_test "stp_mode set user/kernel/auto" +} + +# Verify that stp_mode cannot be changed while STP is active. +test_reject_change_while_stp_active() +{ + RET=0 + + bridge_create br-test + + ip -n "$NS1" link set dev br-test type bridge stp_mode kernel + check_err $? "Failed to set stp_mode to kernel" + + ip -n "$NS1" link set dev br-test type bridge stp_state 1 + check_err $? "Failed to enable STP" + + # Changing stp_mode while STP is active should fail. + ip -n "$NS1" link set dev br-test type bridge stp_mode auto 2>/dev/null + check_fail $? "Changing stp_mode should fail while STP is active" + + check_stp_mode br-test kernel "mode unchanged after rejected change" + + # Disable STP, then change should succeed. + ip -n "$NS1" link set dev br-test type bridge stp_state 0 + check_err $? "Failed to disable STP" + + ip -n "$NS1" link set dev br-test type bridge stp_mode auto + check_err $? "Changing stp_mode should succeed after STP is disabled" + + log_test "reject stp_mode change while STP is active" +} + +# Verify that re-setting the same stp_mode while STP is active succeeds. +test_idempotent_mode_while_stp_active() +{ + RET=0 + + bridge_create br-test + + ip -n "$NS1" link set dev br-test type bridge stp_mode user stp_state 1 + check_err $? "Failed to enable STP with user mode" + + # Re-setting the same mode while STP is active should succeed. + ip -n "$NS1" link set dev br-test type bridge stp_mode user + check_err $? "Idempotent stp_mode set should succeed while STP is active" + + check_stp_state br-test 2 "stp_state after idempotent set" + + # Changing mode while disabling STP in the same message should succeed. + ip -n "$NS1" link set dev br-test type bridge stp_mode auto stp_state 0 + check_err $? "Mode change with simultaneous STP disable should succeed" + + check_stp_mode br-test auto "mode changed after disable+change" + check_stp_state br-test 0 "stp_state after disable+change" + + log_test "idempotent and simultaneous mode change while STP active" +} + +# Test that stp_mode user in a non-init netns yields userspace STP +# (stp_state == 2). This is the key use case: userspace STP without +# needing /sbin/bridge-stp or being in init_net. +test_user_mode_in_netns() +{ + RET=0 + + bridge_create br-test + + ip -n "$NS1" link set dev br-test type bridge stp_mode user + check_err $? "Failed to set stp_mode to user" + + ip -n "$NS1" link set dev br-test type bridge stp_state 1 + check_err $? "Failed to enable STP" + + check_stp_state br-test 2 "stp_state with user mode" + + log_test "stp_mode user in netns yields userspace STP" +} + +# Test that stp_mode kernel forces kernel STP (stp_state == 1) +# regardless of whether /sbin/bridge-stp exists. +test_kernel_mode() +{ + RET=0 + + bridge_create br-test + + ip -n "$NS1" link set dev br-test type bridge stp_mode kernel + check_err $? "Failed to set stp_mode to kernel" + + ip -n "$NS1" link set dev br-test type bridge stp_state 1 + check_err $? "Failed to enable STP" + + check_stp_state br-test 1 "stp_state with kernel mode" + + log_test "stp_mode kernel forces kernel STP" +} + +# Test that stp_mode auto preserves traditional behavior: in a netns +# (non-init_net), bridge-stp is not called and STP falls back to +# kernel mode (stp_state == 1). +test_auto_mode() +{ + RET=0 + + bridge_create br-test + + # Auto mode is the default; enable STP in a netns. + ip -n "$NS1" link set dev br-test type bridge stp_state 1 + check_err $? "Failed to enable STP" + + # In a netns with auto mode, bridge-stp is skipped (init_net only), + # so STP should fall back to kernel mode (stp_state == 1). + check_stp_state br-test 1 "stp_state with auto mode in netns" + + log_test "stp_mode auto preserves traditional behavior" +} + +# Test that stp_mode and stp_state can be set in a single netlink +# message. This is the intended atomic usage pattern. +test_atomic_mode_and_state() +{ + RET=0 + + bridge_create br-test + + # Set both stp_mode and stp_state in one command. + ip -n "$NS1" link set dev br-test type bridge stp_mode user stp_state 1 + check_err $? "Failed to set stp_mode user and stp_state 1 atomically" + + check_stp_state br-test 2 "stp_state after atomic set" + + log_test "atomic stp_mode user + stp_state 1 in single message" +} + +# Test that stp_mode persists across STP disable/enable cycles. +test_mode_persistence() +{ + RET=0 + + bridge_create br-test + + # Set user mode and enable STP. + ip -n "$NS1" link set dev br-test type bridge stp_mode user + ip -n "$NS1" link set dev br-test type bridge stp_state 1 + check_err $? "Failed to enable STP with user mode" + + # Disable STP. + ip -n "$NS1" link set dev br-test type bridge stp_state 0 + check_err $? "Failed to disable STP" + + # Verify mode is still user. + check_stp_mode br-test user "stp_mode after STP disable" + + # Re-enable STP -- should use user mode again. + ip -n "$NS1" link set dev br-test type bridge stp_state 1 + check_err $? "Failed to re-enable STP" + + check_stp_state br-test 2 "stp_state after re-enable" + + log_test "stp_mode persists across STP disable/enable cycles" +} + +# Check iproute2 support before setting up resources. +if ! ip link add type bridge help 2>&1 | grep -q "stp_mode"; then + echo "SKIP: iproute2 too old, missing stp_mode support" + exit "$ksft_skip" +fi + +trap cleanup EXIT + +setup_prepare +tests_run + +exit "$EXIT_STATUS" |
