* Re: [PATCH net-next 01/10] net: stmmac: fix TSO support when some channels have TBS available
From: Russell King (Oracle) @ 2026-03-29 9:40 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
Ong Boon Leong, Paolo Abeni
In-Reply-To: <E1w6bKj-0000000ELtf-3md9@rmk-PC.armlinux.org.uk>
On Sat, Mar 28, 2026 at 09:36:41PM +0000, Russell King (Oracle) wrote:
> According to the STM32MP25xx manual, which is dwmac v5.3, TBS (time
> based scheduling) is not permitted for channels which have hardware
> TSO enabled. Intel's commit 5e6038b88a57 ("net: stmmac: fix TSO and
> TBS feature enabling during driver open") concurs with this, but it
> is incomplete.
>
> This commit avoids enabling TSO support on the channels which have
> TBS available, which, as far as the hardware is concerned, means we
> do not set the TSE bit in the DMA channel's transmit control register.
>
> However, the net device's features apply to all queues(channels), which
> means these channels may still be handed TSO skbs to transmit, and the
> driver will pass them to stmmac_tso_xmit(). This will generate the
> descriptors for TSO, even though the channel has the TSE bit clear.
>
> Fix this by checking whether the queue(channel) has TBS available,
> and if it does, fall back to software GSO support.
This is sufficient for the immediate issue of fixing the patch below,
but I think there's another issue that also needs fixing here.
TSO requires the hardware to support checksum offload, and there is
a comment in the driver:
/* DWMAC IPs can be synthesized to support tx coe only for a few tx
* queues. In that case, checksum offloading for those queues that don't
* support tx coe needs to fallback to software checksum calculation.
*
* Packets that won't trigger the COE e.g. most DSA-tagged packets will
* also have to be checksummed in software.
*/
So, it seems at the very least we need to add a check (in a subsequent
patch) for priv->plat->tx_queues_cfg[queue].coe_unsupported to
stmmac_channel_tso_permitted().
I'm also wondering about the stmmac_has_ip_ethertype() thing, which
checks whether the skb can be checksummed by the hardware, and how that
interacts with TSO, and whether that's yet another hole that needs
plugging.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net-next 06/10] net: stmmac: simplify GSO/TSO test in stmmac_xmit()
From: Russell King (Oracle) @ 2026-03-29 9:17 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
Ong Boon Leong, Paolo Abeni
In-Reply-To: <E1w6bL9-0000000ELu9-1quN@rmk-PC.armlinux.org.uk>
On Sat, Mar 28, 2026 at 09:37:07PM +0000, Russell King (Oracle) wrote:
> +static void stmmac_set_gso_types(struct stmmac_priv *priv, bool tso)
> +{
> + if (!tso) {
> + priv->gso_enabled_types = 0;
> + } else {
> + /* Manage oversized TCP frames for GMAC4 device */
> + priv->gso_enabled_types = SKB_GSO_TCPV4 | SKB_GSO_TCPV6;
> + if (priv->plat->core_type == DWMAC_CORE_GMAC4)
> + priv->gso_enabled_types |= SKB_GSO_UDP_L4;
I've been wondering whether keying all three of these off NETIF_F_TSO
is correct. Shouldn't SKB_GSP_UDP_L4 be dependent on NETIF_F_GSO_UDP_L4?
(The above code doesn't change the current driver behaviour, so this
would be a separate fix.)
> + }
> +}
> +
> /**
> * stmmac_tso_xmit - Tx entry point of the driver for oversized frames (TSO)
> * @skb : the socket buffer
> @@ -4671,7 +4683,6 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
> u32 queue = skb_get_queue_mapping(skb);
> int nfrags = skb_shinfo(skb)->nr_frags;
> unsigned int first_entry, tx_packets;
> - int gso = skb_shinfo(skb)->gso_type;
> struct stmmac_txq_stats *txq_stats;
> struct dma_desc *desc, *first_desc;
> struct stmmac_tx_queue *tx_q;
> @@ -4683,14 +4694,9 @@ static netdev_tx_t stmmac_xmit(struct sk_buff *skb, struct net_device *dev)
> if (priv->tx_path_in_lpi_mode && priv->eee_sw_timer_en)
> stmmac_stop_sw_lpi(priv);
>
> - /* Manage oversized TCP frames for GMAC4 device */
> - if (skb_is_gso(skb) && priv->tso) {
> - if (gso & (SKB_GSO_TCPV4 | SKB_GSO_TCPV6))
> - return stmmac_tso_xmit(skb, dev);
> - if (priv->plat->core_type == DWMAC_CORE_GMAC4 &&
> - (gso & SKB_GSO_UDP_L4))
> - return stmmac_tso_xmit(skb, dev);
> - }
> + if (skb_is_gso(skb) &&
> + skb_shinfo(skb)->gso_type & priv->gso_enabled_types)
> + return stmmac_tso_xmit(skb, dev);
I'm also wondering whether we should check gso_type in our
.ndo_features_check() method rather than here - if we get a GSO skb at
this point for a type that we don't recognise, surely it is wrong to
pass it via the normal skb transmission flow.
Yet more worms in the stmmac can... :/
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH net-next 03/10] net: stmmac: move TSO VLAN tag insertion to core code
From: Russell King (Oracle) @ 2026-03-29 9:09 UTC (permalink / raw)
To: Andrew Lunn
Cc: Alexandre Torgue, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, linux-arm-kernel, linux-stm32, netdev,
Ong Boon Leong, Paolo Abeni
In-Reply-To: <E1w6bKu-0000000ELtr-0U6v@rmk-PC.armlinux.org.uk>
On Sat, Mar 28, 2026 at 09:36:52PM +0000, Russell King (Oracle) wrote:
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index e21ca1c70c6d..ed3e9515cf25 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -4419,19 +4419,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
> u8 proto_hdr_len, hdr;
> dma_addr_t des;
>
> - /* Always insert VLAN tag to SKB payload for TSO frames.
> - *
> - * Never insert VLAN tag by HW, since segments split by
> - * TSO engine will be un-tagged by mistake.
> - */
> - if (skb_vlan_tag_present(skb)) {
> - skb = __vlan_hwaccel_push_inside(skb);
> - if (unlikely(!skb)) {
> - priv->xstats.tx_dropped++;
> - return NETDEV_TX_OK;
> - }
> - }
> -
> nfrags = skb_shinfo(skb)->nr_frags;
> queue = skb_get_queue_mapping(skb);
>
> @@ -4932,6 +4919,14 @@ static netdev_features_t stmmac_features_check(struct sk_buff *skb,
> features = vlan_features_check(skb, features);
>
> if (skb_is_gso(skb)) {
> + /* Always insert VLAN tag to SKB payload for TSO frames.
> + *
> + * Never insert VLAN tag by HW, since segments split by
> + * TSO engine will be un-tagged by mistake.
> + */
> + features &= ~(NETIF_F_HW_VLAN_STAG_TX |
> + NETIF_F_HW_VLAN_CTAG_TX);
> +
I'm wondering whether this is the correct place to do this. If as a
result of the following tests we fallback to software GSO, then we
will be submitting "normal" frames to the driver to transmit, which
means it can insert the VLAN tag in hardware.
So, I'm thinking this isn't the correct place for the test, but it
should be after the tests that disable NETIF_F_GSO_MASK and only be
masked out when the features mask still contains any of the
NETIF_F_GSO_MASK features. Anyone concur?
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
^ permalink raw reply
* Re: [PATCH] ethtool: don't touch the parent device of a net device being unregistered
From: Alexander Popov @ 2026-03-29 8:47 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Andrew Lunn, David Miller, Eric Dumazet, Paolo Abeni,
Simon Horman, Maxime Chevallier, Michal Kubecek, Gal Pressman,
Kory Maincent, Oleksij Rempel, Ido Schimmel, Heiner Kallweit,
Greg KH, netdev, linux-kernel, security, notify
In-Reply-To: <1c41e408-15f2-4c83-8a0f-24b46b1ec1c1@linux.com>
On 25 March 2026 03:46:20 GMT+09:00, Alexander Popov <alex.popov@linux.com> wrote:
>On 3/24/26 01:08, Jakub Kicinski wrote:
>> On Mon, 23 Mar 2026 02:08:53 +0300 Alexander Popov wrote:
>>> Hello Andrew, let me describe the scenario that I see:
>>>
>>> - The netdev_run_todo() function handles the net devices in net_todo_list
>>> in a loop and moves each of them into the NETREG_UNREGISTERED state:
>>> netdev_lock(dev);
>>> WRITE_ONCE(dev->reg_state, NETREG_UNREGISTERED);
>>> netdev_unlock(dev);
>>>
>>> - Then netdev_run_todo() frees these net devices in another loop.
>>> On each iteration, it chooses a device for freeing:
>>> dev = netdev_wait_allrefs_any(&list);
>>>
>>> - At the same time, the ethnl_set_features() function calls
>>> ethnl_parse_header_dev_get() for the child net device.
>>>
>>> - If the race condition succeeds, ethnl_set_features() takes the reference
>>> to the child net device being unregistered. That makes netdev_run_todo()
>>> free the parent first.
>>
>> That's not sufficient detail. ethnl_parse_header_dev_get() is under RCU
>> and unregistration does an RCU sync after delisting the device. Also
>> not sure you're distinguishing struct net_device and struct device.
>>
>> How did you hit this issue? What are the net devices involved?
>
>I've provided additional details about the reproducer of this vulnerability to Jakub and to security@kernel.org.
Hello! May I ask about the decision on this patch?
At patchwork.kernel.org, it is marked as "Changes Requested":
<https://patchwork.kernel.org/project/netdevbpf/patch/20260322075917.254874-1-alex.popov@linux.com/>
However, I don't have any instructions on what to change in it.
Thanks!
Alexander
^ permalink raw reply
* Re: [PATCH RFC 2/8] clk: sunxi-ng: sdm: Add dual patterns support
From: Chen-Yu Tsai @ 2026-03-29 7:56 UTC (permalink / raw)
To: Junhui Liu
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jernej Skrabec, Samuel Holland, Philipp Zabel,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Richard Cochran, linux-clk, devicetree, linux-arm-kernel,
linux-sunxi, linux-kernel, linux-riscv, netdev
In-Reply-To: <20260310-a733-clk-v1-2-36b4e9b24457@pigmoral.tech>
On Tue, Mar 10, 2026 at 4:42 PM Junhui Liu <junhui.liu@pigmoral.tech> wrote:
>
> On newer Allwinner platforms like the A733, the Sigma-Delta Modulation
> (SDM) control logic is more complex. The SDM enable bit, which was
> previously located in the PLL register, is now moved to a second
> pattern register (PATTERN1).
>
> To support this, rename the existing "tuning" members to "pattern0" to
> align with the datasheet, and introduce the _SUNXI_CCU_SDM_DUAL_PAT
> macro to provide pattern1 register support. Related operations are also
> updated.
>
> Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech>
> ---
> drivers/clk/sunxi-ng/ccu_sdm.c | 51 +++++++++++++++++++++++++++++-------------
> drivers/clk/sunxi-ng/ccu_sdm.h | 32 +++++++++++++++++---------
> 2 files changed, 57 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/clk/sunxi-ng/ccu_sdm.c b/drivers/clk/sunxi-ng/ccu_sdm.c
> index c564e5f9e610..204e25feaa36 100644
> --- a/drivers/clk/sunxi-ng/ccu_sdm.c
> +++ b/drivers/clk/sunxi-ng/ccu_sdm.c
> @@ -18,7 +18,10 @@ bool ccu_sdm_helper_is_enabled(struct ccu_common *common,
> if (sdm->enable && !(readl(common->base + common->reg) & sdm->enable))
> return false;
>
> - return !!(readl(common->base + sdm->tuning_reg) & sdm->tuning_enable);
> + if (sdm->pat1_enable && !(readl(common->base + sdm->pat1_reg) & sdm->pat1_enable))
> + return false;
> +
> + return !!(readl(common->base + sdm->pat0_reg) & sdm->pat0_enable);
> }
> EXPORT_SYMBOL_NS_GPL(ccu_sdm_helper_is_enabled, "SUNXI_CCU");
>
> @@ -37,18 +40,27 @@ void ccu_sdm_helper_enable(struct ccu_common *common,
> for (i = 0; i < sdm->table_size; i++)
> if (sdm->table[i].rate == rate)
> writel(sdm->table[i].pattern,
> - common->base + sdm->tuning_reg);
> + common->base + sdm->pat0_reg);
>
> /* Make sure SDM is enabled */
> spin_lock_irqsave(common->lock, flags);
> - reg = readl(common->base + sdm->tuning_reg);
> - writel(reg | sdm->tuning_enable, common->base + sdm->tuning_reg);
> + reg = readl(common->base + sdm->pat0_reg);
> + writel(reg | sdm->pat0_enable, common->base + sdm->pat0_reg);
> spin_unlock_irqrestore(common->lock, flags);
>
> - spin_lock_irqsave(common->lock, flags);
> - reg = readl(common->base + common->reg);
> - writel(reg | sdm->enable, common->base + common->reg);
> - spin_unlock_irqrestore(common->lock, flags);
> + if (sdm->enable) {
> + spin_lock_irqsave(common->lock, flags);
> + reg = readl(common->base + common->reg);
> + writel(reg | sdm->enable, common->base + common->reg);
> + spin_unlock_irqrestore(common->lock, flags);
> + }
> +
> + if (sdm->pat1_enable) {
> + spin_lock_irqsave(common->lock, flags);
> + reg = readl(common->base + sdm->pat1_reg);
> + writel(reg | sdm->pat1_enable, common->base + sdm->pat1_reg);
> + spin_unlock_irqrestore(common->lock, flags);
> + }
> }
> EXPORT_SYMBOL_NS_GPL(ccu_sdm_helper_enable, "SUNXI_CCU");
>
> @@ -61,14 +73,23 @@ void ccu_sdm_helper_disable(struct ccu_common *common,
> if (!(common->features & CCU_FEATURE_SIGMA_DELTA_MOD))
> return;
>
> - spin_lock_irqsave(common->lock, flags);
> - reg = readl(common->base + common->reg);
> - writel(reg & ~sdm->enable, common->base + common->reg);
> - spin_unlock_irqrestore(common->lock, flags);
> + if (sdm->enable) {
> + spin_lock_irqsave(common->lock, flags);
> + reg = readl(common->base + common->reg);
> + writel(reg & ~sdm->enable, common->base + common->reg);
> + spin_unlock_irqrestore(common->lock, flags);
> + }
> +
> + if (sdm->pat1_enable) {
> + spin_lock_irqsave(common->lock, flags);
> + reg = readl(common->base + sdm->pat1_reg);
> + writel(reg & ~sdm->pat1_enable, common->base + sdm->pat1_reg);
> + spin_unlock_irqrestore(common->lock, flags);
> + }
>
> spin_lock_irqsave(common->lock, flags);
> - reg = readl(common->base + sdm->tuning_reg);
> - writel(reg & ~sdm->tuning_enable, common->base + sdm->tuning_reg);
> + reg = readl(common->base + sdm->pat0_reg);
> + writel(reg & ~sdm->pat0_enable, common->base + sdm->pat0_reg);
> spin_unlock_irqrestore(common->lock, flags);
> }
> EXPORT_SYMBOL_NS_GPL(ccu_sdm_helper_disable, "SUNXI_CCU");
> @@ -123,7 +144,7 @@ unsigned long ccu_sdm_helper_read_rate(struct ccu_common *common,
> pr_debug("%s: clock is sigma-delta modulated\n",
> clk_hw_get_name(&common->hw));
>
> - reg = readl(common->base + sdm->tuning_reg);
> + reg = readl(common->base + sdm->pat0_reg);
>
> pr_debug("%s: pattern reg is 0x%x",
> clk_hw_get_name(&common->hw), reg);
> diff --git a/drivers/clk/sunxi-ng/ccu_sdm.h b/drivers/clk/sunxi-ng/ccu_sdm.h
> index c1a7159b89c3..c289be28e1b4 100644
> --- a/drivers/clk/sunxi-ng/ccu_sdm.h
> +++ b/drivers/clk/sunxi-ng/ccu_sdm.h
> @@ -33,21 +33,31 @@ struct ccu_sdm_internal {
> u32 table_size;
> /* early SoCs don't have the SDM enable bit in the PLL register */
> u32 enable;
> - /* second enable bit in tuning register */
> - u32 tuning_enable;
> - u16 tuning_reg;
> + /* second enable bit in pattern0 register */
> + u32 pat0_enable;
> + u16 pat0_reg;
> + /* on some platforms, the sdm enable bit in pattern1 register */
> + u32 pat1_enable;
> + u16 pat1_reg;
> };
>
> -#define _SUNXI_CCU_SDM(_table, _enable, \
> - _reg, _reg_enable) \
> - { \
> - .table = _table, \
> - .table_size = ARRAY_SIZE(_table), \
> - .enable = _enable, \
> - .tuning_enable = _reg_enable, \
> - .tuning_reg = _reg, \
> +#define __SUNXI_CCU_SDM(_table, _enable, _pat0, _pat0_enable, _pat1, _pat1_enable) \
> + { \
> + .table = _table, \
> + .table_size = ARRAY_SIZE(_table), \
> + .enable = _enable, \
> + .pat0_enable = _pat0_enable, \
> + .pat0_reg = _pat0, \
> + .pat1_enable = _pat1_enable, \
> + .pat1_reg = _pat1, \
> }
>
> +#define _SUNXI_CCU_SDM(_table, _enable, _pat0, _pat0_enable) \
> + __SUNXI_CCU_SDM(_table, _enable, _pat0, _pat0_enable, 0, 0)
> +
> +#define _SUNXI_CCU_SDM_DUAL_PAT(_table, _pat0, _pat0_enable, _pat1, _pat1_enable) \
> + __SUNXI_CCU_SDM(_table, 0, _pat0, _pat0_enable, _pat1, _pat1_enable)
> +
Don't introduce an intermediate macro that looks _almost_ the same as the
macro the driver is actually supposed to use.
Just declare _SUNXI_CCU_SDM_DUAL_PAT() to expand to the full entry, and
_SUNXI_CCU_SDM() to _SUNXI_CCU_SDM_DUAL_PAT() with the last two parameters
as zero. That takes less lines.
ChenYu
> bool ccu_sdm_helper_is_enabled(struct ccu_common *common,
> struct ccu_sdm_internal *sdm);
> void ccu_sdm_helper_enable(struct ccu_common *common,
>
> --
> 2.52.0
>
>
^ permalink raw reply
* [PATCH iproute2-next v2] dpll: Send object per event in JSON monitor mode
From: Vitaly Grinberg @ 2026-03-29 7:51 UTC (permalink / raw)
To: netdev; +Cc: stephen, Vitaly Grinberg
Previously, monitor mode wrapped all events in a single JSON array
inside a top-level object, which made piping the output to external
tools (such as `jq`) impossible.
Send a separate JSON object for each event in monitor mode,
making the output suitable for line-by-line consumers. Skip the
global JSON wrapper for monitor mode.
Signed-off-by: Vitaly Grinberg <vgrinber@redhat.com>
---
v2:
- Fixed indentation
---
dpll/dpll.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/dpll/dpll.c b/dpll/dpll.c
index 9893ca8c..49576d23 100644
--- a/dpll/dpll.c
+++ b/dpll/dpll.c
@@ -571,8 +571,15 @@ int main(int argc, char **argv)
argc -= optind;
argv += optind;
- new_json_obj_plain(json);
- open_json_object(NULL);
+ /* Monitor emits one JSON object per event for streaming;
+ * other commands use a single JSON wrapper object.
+ */
+ bool is_monitor = argc > 0 && strcmp(argv[0], "monitor") == 0;
+
+ if (!is_monitor) {
+ new_json_obj_plain(json);
+ open_json_object(NULL);
+ }
/* Skip netlink init for help commands */
bool need_nl = true;
@@ -602,8 +609,10 @@ dpll_fini:
if (need_nl)
dpll_fini(dpll);
json_cleanup:
- close_json_object();
- delete_json_obj_plain();
+ if (!is_monitor) {
+ close_json_object();
+ delete_json_obj_plain();
+ }
dpll_free:
dpll_free(dpll);
return ret;
@@ -2173,6 +2182,7 @@ static int cmd_monitor_cb(const struct nlmsghdr *nlh, void *data)
mnl_attr_parse(nlh, sizeof(struct genlmsghdr), attr_cb, tb);
+ new_json_obj_plain(json);
open_json_object(NULL);
print_string(PRINT_JSON, "name", NULL, json_name);
open_json_object("msg");
@@ -2182,6 +2192,7 @@ static int cmd_monitor_cb(const struct nlmsghdr *nlh, void *data)
close_json_object();
close_json_object();
+ delete_json_obj_plain();
break;
}
case DPLL_CMD_PIN_CREATE_NTF:
@@ -2200,6 +2211,7 @@ static int cmd_monitor_cb(const struct nlmsghdr *nlh, void *data)
json_name = "pin-delete-ntf";
}
+ new_json_obj_plain(json);
open_json_object(NULL);
print_string(PRINT_JSON, "name", NULL, json_name);
open_json_object("msg");
@@ -2209,6 +2221,7 @@ static int cmd_monitor_cb(const struct nlmsghdr *nlh, void *data)
close_json_object();
close_json_object();
+ delete_json_obj_plain();
break;
}
default:
@@ -2273,8 +2286,6 @@ static int cmd_monitor(struct dpll *dpll)
goto err_signalfd;
}
- open_json_array(PRINT_JSON, "monitor");
-
pfds[0].fd = signal_fd;
pfds[0].events = POLLIN;
pfds[1].fd = netlink_fd;
@@ -2307,8 +2318,6 @@ static int cmd_monitor(struct dpll *dpll)
}
}
- close_json_array(PRINT_JSON, NULL);
-
err_signalfd:
if (signal_fd >= 0)
close(signal_fd);
---
base-commit: 36252727bfc653905bb39bec115a32869452beb1
change-id: 20260328-dpll-mon-j-d88eb48c1f09
Best regards,
--
Vitaly Grinberg <vgrinber@redhat.com>
^ permalink raw reply related
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Lucien.Jheng @ 2026-03-29 5:09 UTC (permalink / raw)
To: Andrew Lunn, Russell King (Oracle)
Cc: Eric Woudstra, hkallweit1, davem, edumazet, kuba, pabeni, netdev,
linux-kernel, bjorn, frank-w, daniel, lucien.jheng
In-Reply-To: <e1a3ca7c-99f0-4452-bd5e-ca3662fd8d9a@lunn.ch>
Andrew Lunn 於 2026/3/28 下午 10:15 寫道:
> On Sat, Mar 28, 2026 at 10:25:06AM +0000, Russell King (Oracle) wrote:
>> On Sat, Mar 28, 2026 at 11:21:59AM +0100, Eric Woudstra wrote:
>>> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
>>> u32 pbus_data)
>>> {
>>> int ret;
>>>
>>> mutex_lock(&mdio->bus->mdio_lock);
>> I wonder why we have phy_lock_mdio_bus() but not mdio_bus_lock()...
>> Should a helper be provided at mdio bus level to complement the PHY
>> level helper if we're going to have MDIO drivers directly manipulating
>> the lock?
> DSA is pretty much the only class of mdio driver. And they mostly need
> to use:
>
> mutex_lock_nested(&bus->mdio_lock, MDIO_MUTEX_NESTED);
>
> Not the plain mutex_lock().
>
> Hence it has not been added up until now.
>
> But yes, such a helper would make sense for a PHY driver having to do
> odd things.
>
> Andrew
Thank you for your feedback
I will implement the helper you suggested for this in the next iteration.
^ permalink raw reply
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Lucien.Jheng @ 2026-03-29 5:05 UTC (permalink / raw)
To: Eric Woudstra, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn
Cc: frank-w, daniel, lucien.jheng
In-Reply-To: <6f42d92b-a26d-49a4-bef7-2227c252eea7@gmail.com>
Eric Woudstra 於 2026/3/28 下午 06:21 寫道:
>
> On 3/27/26 5:41 AM, Eric Woudstra wrote:
>> static int __air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
>> u32 pbus_data)
>> {
>> int ret;
>>
>> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
>> (u16)(pbus_address >> 6));
>> if (ret < 0)
>> return ret;
>>
>> ret = __mdiobus_write(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf,
>> (u16)pbus_data);
>> if (ret < 0)
>> return ret;
>>
>> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
>> (u16)(pbus_data >> 16));
>> if (ret < 0)
>> return ret;
>>
>> return 0;
>> }
>>
>> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
>> u32 pbus_data)
>> {
>> int ret;
>>
>> mutex_lock(&mdio->bus->mdio_lock);
>>
>> ret = __air_pbus_reg_write(mdio, pbus_address, pbus_data);
>> if (ret < 0)
>> dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
>> pbus_address, ret);
>>
>> mutex_unlock(&mdio->bus->mdio_lock);
>>
>> return ret;
>> }
>>
>> static int __air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
>> u32 *pbus_data)
>> {
>> int ret, pbus_data_low;
>>
>> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
>> (u16)(pbus_address >> 6));
>> if (ret < 0)
>> return ret;
>>
>> ret = __mdiobus_read(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf);
>> if (ret < 0)
>> return ret;
>> pbus_data_low = ret;
>>
>> ret = __mdiobus_read(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH);
>> if (ret < 0)
>> return ret;
>>
>> *pbus_data = (ret << 16) + pbus_data_low;
>>
>> return 0;
>> }
>>
>> static int air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
>> u32 *pbus_data)
>> {
>> int ret;
>>
>> mutex_lock(&mdio->bus->mdio_lock);
>>
>> ret = __air_pbus_reg_read(mdio, pbus_address, pbus_data);
>> if (ret < 0)
>> dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
>> pbus_address, ret);
>>
>> mutex_unlock(&mdio->bus->mdio_lock);
>>
>> return ret;
>> }
>>
>> With:
>>
>> #define AIR_PBUS_DATA_HIGH 0x10
> Small correction using upper_16_bits() and lower_16_bits():
>
> static int __air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> u32 pbus_data)
> {
> int ret;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
> (u16)(pbus_address >> 6));
> if (ret < 0)
> return ret;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf,
> lower_16_bits(pbus_data));
> if (ret < 0)
> return ret;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
> upper_16_bits(pbus_data));
> if (ret < 0)
> return ret;
>
> return 0;
> }
>
> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> u32 pbus_data)
> {
> int ret;
>
> mutex_lock(&mdio->bus->mdio_lock);
>
> ret = __air_pbus_reg_write(mdio, pbus_address, pbus_data);
> if (ret < 0)
> dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
> pbus_address, ret);
>
> mutex_unlock(&mdio->bus->mdio_lock);
>
> return ret;
> }
Thank you for your feedback.
I will include this in the next version.
^ permalink raw reply
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Lucien.Jheng @ 2026-03-29 5:04 UTC (permalink / raw)
To: Eric Woudstra, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn
Cc: frank-w, daniel, lucien.jheng
In-Reply-To: <edeee8e5-0de3-48ea-aeb5-cc705fdcd469@gmail.com>
Eric Woudstra 於 2026/3/27 下午 12:41 寫道:
>
> On 3/26/26 4:35 PM, Lucien.Jheng wrote:
>> AN8811HB needs a MCU soft-reset cycle before firmware loading begins.
>> Assert the MCU (hold it in reset) and immediately deassert (release)
>> via a dedicated PBUS register pair (0x5cf9f8 / 0x5cf9fc), accessed
>> through the PHY-addr+8 MDIO bus node rather than the BUCKPBUS indirect
>> path. This clears the MCU state before the firmware loading sequence
>> starts.
>>
>> Add __air_pbus_reg_write() as a low-level helper for this access, then
>> implement an8811hb_mcu_assert() / _deassert() on top of it. Wire both
>> into an8811hb_load_firmware() and en8811h_restart_mcu() so every
>> firmware load or MCU restart on AN8811HB correctly sequences the reset
>> control registers.
>>
>> Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB support")
>> Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
>> ---
>> Changes in v2:
>> - Rewrite commit message: The previous wording was ambiguous,
>> as it suggested the MCU remains asserted during the entire
>> firmware loading process, rather than a quick reset cycle
>> before it starts.
>> Clarify that assert and deassert is an immediate soft-reset
>> cycle to clear MCU state before firmware loading begins.
>> - Add Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB
>> support").
>> - Change phydev_info() to phydev_dbg() in an8811hb_mcu_assert() and
>> an8811hb_mcu_deassert() to avoid noise during normal boot.
>>
>> drivers/net/phy/air_en8811h.c | 105 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 105 insertions(+)
>>
>> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
>> index 29ae73e65caa..01fce1b93618 100644
>> --- a/drivers/net/phy/air_en8811h.c
>> +++ b/drivers/net/phy/air_en8811h.c
>> @@ -170,6 +170,16 @@
>> #define AN8811HB_CLK_DRV_CKO_LDPWD BIT(13)
>> #define AN8811HB_CLK_DRV_CKO_LPPWD BIT(14)
>>
>> +#define AN8811HB_MCU_SW_RST 0x5cf9f8
>> +#define AN8811HB_MCU_SW_RST_HOLD BIT(16)
>> +#define AN8811HB_MCU_SW_RST_RUN (BIT(16) | BIT(0))
>> +#define AN8811HB_MCU_SW_START 0x5cf9fc
>> +#define AN8811HB_MCU_SW_START_EN BIT(16)
>> +
>> +/* MII register constants for PBUS access (PHY addr + 8) */
>> +#define AIR_PBUS_ADDR_HIGH 0x1c
>> +#define AIR_PBUS_DATA_HIGH 0x10
>> +
>> /* Led definitions */
>> #define EN8811H_LED_COUNT 3
>>
>> @@ -254,6 +264,36 @@ static int air_phy_write_page(struct phy_device *phydev, int page)
>> return __phy_write(phydev, AIR_EXT_PAGE_ACCESS, page);
>> }
>>
>> +static int __air_pbus_reg_write(struct phy_device *phydev,
>> + u32 pbus_reg, u32 pbus_data)
>> +{
>> + struct mii_bus *bus = phydev->mdio.bus;
>> + int pbus_addr = phydev->mdio.addr + 8;
>> + int ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, AIR_EXT_PAGE_ACCESS,
>> + upper_16_bits(pbus_reg));
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_ADDR_HIGH,
>> + (pbus_reg & GENMASK(15, 6)) >> 6);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, (pbus_reg & GENMASK(5, 2)) >> 2,
>> + lower_16_bits(pbus_data));
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_DATA_HIGH,
>> + upper_16_bits(pbus_data));
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
> Maybe you can use mutex_lock() and mutex_unlock() here also, like so:
>
> static int __air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> u32 pbus_data)
> {
> int ret;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
> (u16)(pbus_address >> 6));
> if (ret < 0)
> return ret;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf,
> (u16)pbus_data);
> if (ret < 0)
> return ret;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH,
> (u16)(pbus_data >> 16));
> if (ret < 0)
> return ret;
>
> return 0;
> }
>
> static int air_pbus_reg_write(struct mdio_device *mdio, u32 pbus_address,
> u32 pbus_data)
> {
> int ret;
>
> mutex_lock(&mdio->bus->mdio_lock);
>
> ret = __air_pbus_reg_write(mdio, pbus_address, pbus_data);
> if (ret < 0)
> dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
> pbus_address, ret);
>
> mutex_unlock(&mdio->bus->mdio_lock);
>
> return ret;
> }
>
> static int __air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
> u32 *pbus_data)
> {
> int ret, pbus_data_low;
>
> ret = __mdiobus_write(mdio->bus, mdio->addr, AIR_EXT_PAGE_ACCESS,
> (u16)(pbus_address >> 6));
> if (ret < 0)
> return ret;
>
> ret = __mdiobus_read(mdio->bus, mdio->addr, (pbus_address >> 2) & 0xf);
> if (ret < 0)
> return ret;
> pbus_data_low = ret;
>
> ret = __mdiobus_read(mdio->bus, mdio->addr, AIR_PBUS_DATA_HIGH);
> if (ret < 0)
> return ret;
>
> *pbus_data = (ret << 16) + pbus_data_low;
>
> return 0;
> }
>
> static int air_pbus_reg_read(struct mdio_device *mdio, u32 pbus_address,
> u32 *pbus_data)
> {
> int ret;
>
> mutex_lock(&mdio->bus->mdio_lock);
>
> ret = __air_pbus_reg_read(mdio, pbus_address, pbus_data);
> if (ret < 0)
> dev_err(&mdio->dev, "%s 0x%08x failed: %d\n", __func__,
> pbus_address, ret);
>
> mutex_unlock(&mdio->bus->mdio_lock);
>
> return ret;
> }
>
> With:
>
> #define AIR_PBUS_DATA_HIGH 0x10
>
Thank you for your feedback.
I should add mutex_lock() and mutex_unlock() to protect these operations.
I will include this in the next version.
>> static int __air_buckpbus_reg_write(struct phy_device *phydev,
>> u32 pbus_address, u32 pbus_data)
>> {
>> @@ -570,10 +610,65 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
>> return ret;
>> }
>>
>> +static int an8811hb_mcu_assert(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + phy_lock_mdio_bus(phydev);
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
>> + AN8811HB_MCU_SW_RST_HOLD);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + msleep(50);
>> + phydev_dbg(phydev, "MCU asserted\n");
>> +
>> +unlock:
>> + phy_unlock_mdio_bus(phydev);
>> + return ret;
>> +}
>> +
>> +static int an8811hb_mcu_deassert(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + phy_lock_mdio_bus(phydev);
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START,
>> + AN8811HB_MCU_SW_START_EN);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
>> + AN8811HB_MCU_SW_RST_RUN);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + msleep(50);
>> + phydev_dbg(phydev, "MCU deasserted\n");
>> +
>> +unlock:
>> + phy_unlock_mdio_bus(phydev);
>> + return ret;
>> +}
>> +
>> static int an8811hb_load_firmware(struct phy_device *phydev)
>> {
>> int ret;
>>
>> + ret = an8811hb_mcu_assert(phydev);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = an8811hb_mcu_deassert(phydev);
>> + if (ret < 0)
>> + return ret;
>> +
>> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
>> EN8811H_FW_CTRL_1_START);
>> if (ret < 0)
>> @@ -662,6 +757,16 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
>> {
>> int ret;
>>
>> + if (phy_id_compare_model(phydev->phy_id, AN8811HB_PHY_ID)) {
>> + ret = an8811hb_mcu_assert(phydev);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = an8811hb_mcu_deassert(phydev);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
>> EN8811H_FW_CTRL_1_START);
>> if (ret < 0)
>> --
>> 2.34.1
>>
^ permalink raw reply
* Re: [PATCH v2] net: phy: air_en8811h: add AN8811HB MCU assert/deassert support
From: Lucien.Jheng @ 2026-03-29 4:59 UTC (permalink / raw)
To: Eric Woudstra, andrew, hkallweit1, linux, davem, edumazet, kuba,
pabeni, netdev, linux-kernel, bjorn
Cc: frank-w, daniel, lucien.jheng
In-Reply-To: <d59bfdca-08cb-4595-8583-eec61e282599@gmail.com>
Eric Woudstra 於 2026/3/27 上午 04:00 寫道:
>
> On 3/26/26 4:35 PM, Lucien.Jheng wrote:
>> AN8811HB needs a MCU soft-reset cycle before firmware loading begins.
>> Assert the MCU (hold it in reset) and immediately deassert (release)
>> via a dedicated PBUS register pair (0x5cf9f8 / 0x5cf9fc), accessed
>> through the PHY-addr+8 MDIO bus node rather than the BUCKPBUS indirect
>> path. This clears the MCU state before the firmware loading sequence
>> starts.
> Perhapse you can register this extra device in the probe() function:
>
> struct mdio_device *mdiodev;
>
> mdiodev = mdio_device_create(phydev->mdio.bus,
> phydev->mdio.addr + EN8811H_PBUS_ADDR_OFFS);
> if (IS_ERR(mdiodev))
> return PTR_ERR(mdiodev);
>
> ret = mdio_device_register(mdiodev);
> if (ret) {
> mdio_device_free(mdiodev);
> return ret;
> }
>
> priv->pbusdev = mdiodev;
>
> In the define part add:
>
> #define EN8811H_PBUS_ADDR_OFFS 8
>
> Add to struct en8811h_priv:
>
> struct mdio_device *pbusdev;
>
> And in the remove function:
>
> mdio_device_remove(priv->pbusdev);
> mdio_device_free(priv->pbusdev);
>
> Then you can use the device:
>
> ret = en8811hb_some_neat_function(priv->pbusdev);
Thank you for the feedback
I will handle the registration of this extra device within the probe
function in the next version.
>> Add __air_pbus_reg_write() as a low-level helper for this access, then
>> implement an8811hb_mcu_assert() / _deassert() on top of it. Wire both
>> into an8811hb_load_firmware() and en8811h_restart_mcu() so every
>> firmware load or MCU restart on AN8811HB correctly sequences the reset
>> control registers.
>>
>> Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB support")
>> Signed-off-by: Lucien Jheng <lucienzx159@gmail.com>
>> ---
>> Changes in v2:
>> - Rewrite commit message: The previous wording was ambiguous,
>> as it suggested the MCU remains asserted during the entire
>> firmware loading process, rather than a quick reset cycle
>> before it starts.
>> Clarify that assert and deassert is an immediate soft-reset
>> cycle to clear MCU state before firmware loading begins.
>> - Add Fixes: 0a55766b7711 ("net: phy: air_en8811h: add Airoha AN8811HB
>> support").
>> - Change phydev_info() to phydev_dbg() in an8811hb_mcu_assert() and
>> an8811hb_mcu_deassert() to avoid noise during normal boot.
>>
>> drivers/net/phy/air_en8811h.c | 105 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 105 insertions(+)
>>
>> diff --git a/drivers/net/phy/air_en8811h.c b/drivers/net/phy/air_en8811h.c
>> index 29ae73e65caa..01fce1b93618 100644
>> --- a/drivers/net/phy/air_en8811h.c
>> +++ b/drivers/net/phy/air_en8811h.c
>> @@ -170,6 +170,16 @@
>> #define AN8811HB_CLK_DRV_CKO_LDPWD BIT(13)
>> #define AN8811HB_CLK_DRV_CKO_LPPWD BIT(14)
>>
>> +#define AN8811HB_MCU_SW_RST 0x5cf9f8
>> +#define AN8811HB_MCU_SW_RST_HOLD BIT(16)
>> +#define AN8811HB_MCU_SW_RST_RUN (BIT(16) | BIT(0))
>> +#define AN8811HB_MCU_SW_START 0x5cf9fc
>> +#define AN8811HB_MCU_SW_START_EN BIT(16)
>> +
>> +/* MII register constants for PBUS access (PHY addr + 8) */
>> +#define AIR_PBUS_ADDR_HIGH 0x1c
>> +#define AIR_PBUS_DATA_HIGH 0x10
>> +
>> /* Led definitions */
>> #define EN8811H_LED_COUNT 3
>>
>> @@ -254,6 +264,36 @@ static int air_phy_write_page(struct phy_device *phydev, int page)
>> return __phy_write(phydev, AIR_EXT_PAGE_ACCESS, page);
>> }
>>
>> +static int __air_pbus_reg_write(struct phy_device *phydev,
>> + u32 pbus_reg, u32 pbus_data)
>> +{
>> + struct mii_bus *bus = phydev->mdio.bus;
>> + int pbus_addr = phydev->mdio.addr + 8;
>> + int ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, AIR_EXT_PAGE_ACCESS,
>> + upper_16_bits(pbus_reg));
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_ADDR_HIGH,
>> + (pbus_reg & GENMASK(15, 6)) >> 6);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, (pbus_reg & GENMASK(5, 2)) >> 2,
>> + lower_16_bits(pbus_data));
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = __mdiobus_write(bus, pbus_addr, AIR_PBUS_DATA_HIGH,
>> + upper_16_bits(pbus_data));
>> + if (ret < 0)
>> + return ret;
>> +
>> + return 0;
>> +}
>> +
>> static int __air_buckpbus_reg_write(struct phy_device *phydev,
>> u32 pbus_address, u32 pbus_data)
>> {
>> @@ -570,10 +610,65 @@ static int an8811hb_load_file(struct phy_device *phydev, const char *name,
>> return ret;
>> }
>>
>> +static int an8811hb_mcu_assert(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + phy_lock_mdio_bus(phydev);
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
>> + AN8811HB_MCU_SW_RST_HOLD);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START, 0);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + msleep(50);
>> + phydev_dbg(phydev, "MCU asserted\n");
>> +
>> +unlock:
>> + phy_unlock_mdio_bus(phydev);
>> + return ret;
>> +}
>> +
>> +static int an8811hb_mcu_deassert(struct phy_device *phydev)
>> +{
>> + int ret;
>> +
>> + phy_lock_mdio_bus(phydev);
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_START,
>> + AN8811HB_MCU_SW_START_EN);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + ret = __air_pbus_reg_write(phydev, AN8811HB_MCU_SW_RST,
>> + AN8811HB_MCU_SW_RST_RUN);
>> + if (ret < 0)
>> + goto unlock;
>> +
>> + msleep(50);
>> + phydev_dbg(phydev, "MCU deasserted\n");
>> +
>> +unlock:
>> + phy_unlock_mdio_bus(phydev);
>> + return ret;
>> +}
>> +
>> static int an8811hb_load_firmware(struct phy_device *phydev)
>> {
>> int ret;
>>
>> + ret = an8811hb_mcu_assert(phydev);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = an8811hb_mcu_deassert(phydev);
>> + if (ret < 0)
>> + return ret;
>> +
>> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
>> EN8811H_FW_CTRL_1_START);
>> if (ret < 0)
>> @@ -662,6 +757,16 @@ static int en8811h_restart_mcu(struct phy_device *phydev)
>> {
>> int ret;
>>
>> + if (phy_id_compare_model(phydev->phy_id, AN8811HB_PHY_ID)) {
>> + ret = an8811hb_mcu_assert(phydev);
>> + if (ret < 0)
>> + return ret;
>> +
>> + ret = an8811hb_mcu_deassert(phydev);
>> + if (ret < 0)
>> + return ret;
>> + }
>> +
>> ret = air_buckpbus_reg_write(phydev, EN8811H_FW_CTRL_1,
>> EN8811H_FW_CTRL_1_START);
>> if (ret < 0)
>> --
>> 2.34.1
>>
^ permalink raw reply
* [syzbot] [wireless?] WARNING in cfg80211_chandef_dfs_required (2)
From: syzbot @ 2026-03-29 4:32 UTC (permalink / raw)
To: johannes, linux-kernel, linux-wireless, netdev, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: b1c803d5c816 net: airoha: Rework the code flow in airoha_r..
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=17ad21d6580000
kernel config: https://syzkaller.appspot.com/x/.config?x=71d49d824b43a0d9
dashboard link: https://syzkaller.appspot.com/bug?extid=02a1a03b8622d3c7d1c9
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/09cf620a7914/disk-b1c803d5.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/72906e01e665/vmlinux-b1c803d5.xz
kernel image: https://storage.googleapis.com/syzbot-assets/e1bab2219b98/bzImage-b1c803d5.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+02a1a03b8622d3c7d1c9@syzkaller.appspotmail.com
------------[ cut here ]------------
!cfg80211_chandef_valid(chandef)
WARNING: net/wireless/chan.c:796 at cfg80211_chandef_dfs_required+0xe3e/0xf00 net/wireless/chan.c:796, CPU#0: syz.0.4705/23179
Modules linked in:
CPU: 0 UID: 0 PID: 23179 Comm: syz.0.4705 Not tainted syzkaller #0 PREEMPT(full)
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 02/12/2026
RIP: 0010:cfg80211_chandef_dfs_required+0xe3e/0xf00 net/wireless/chan.c:796
Code: de e8 26 ed bb f6 48 83 fb 3f 0f 87 aa 00 00 00 e8 d7 e8 bb f6 b8 01 00 00 00 89 d9 48 d3 e0 e9 0a f4 ff ff e8 c3 e8 bb f6 90 <0f> 0b 90 b8 ea ff ff ff e9 f7 f3 ff ff e8 10 d9 a5 00 89 d1 80 e1
RSP: 0018:ffffc900039c7680 EFLAGS: 00010283
RAX: ffffffff8b09b83d RBX: 1ffff92000738f10 RCX: 0000000000080000
RDX: ffffc90005477000 RSI: 00000000000005cf RDI: 00000000000005d0
RBP: ffffc900039c7810 R08: ffffffff822422df R09: ffff8880b863f428
R10: dffffc0000000000 R11: ffffed10165f8539 R12: ffffc900039c7a00
R13: dffffc0000000000 R14: 0000000000000006 R15: 1ffff92000738ee8
FS: 00007f6f79eb16c0(0000) GS:ffff888125461000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f6f7904eddd CR3: 0000000054d9e000 CR4: 00000000003526f0
Call Trace:
<TASK>
_ieee80211_link_use_channel+0x29c/0xd50 net/mac80211/chan.c:2034
ieee80211_link_use_channel net/mac80211/ieee80211_i.h:2754 [inline]
ieee80211_set_monitor_channel+0x692/0x8e0 net/mac80211/cfg.c:1084
rdev_set_monitor_channel net/wireless/rdev-ops.h:453 [inline]
cfg80211_set_monitor_channel+0x22d/0x650 net/wireless/chan.c:1600
cfg80211_wext_siwfreq+0x730/0x870 net/wireless/wext-compat.c:792
ioctl_standard_call+0xcb/0x1b0 net/wireless/wext-core.c:1042
wireless_process_ioctl net/wireless/wext-core.c:-1 [inline]
wext_ioctl_dispatch+0xee/0x410 net/wireless/wext-core.c:1013
wext_handle_ioctl+0x10f/0x1d0 net/wireless/wext-core.c:1074
sock_ioctl+0x159/0x7f0 net/socket.c:1300
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:597 [inline]
__se_sys_ioctl+0xfc/0x170 fs/ioctl.c:583
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x14d/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f6f78f9c799
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f6f79eb1028 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007f6f79215fa0 RCX: 00007f6f78f9c799
RDX: 0000200000000040 RSI: 0000000000008b04 RDI: 0000000000000003
RBP: 00007f6f79032c99 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007f6f79216038 R14: 00007f6f79215fa0 R15: 00007ffe8253b778
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* [PATCH v8 net-next 5/5] selftest/net: psp: Add test for dev-assoc/disassoc
From: Wei Wang @ 2026-03-29 4:19 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Daniel Zahka, Willem de Bruijn, David Wei,
Andrew Lunn, David S . Miller, Eric Dumazet, Simon Horman
Cc: Wei Wang
In-Reply-To: <20260329041922.3850747-1-weibunny.kernel@gmail.com>
From: Wei Wang <weibunny@fb.com>
Add a new param to NetDrvContEnv to add an additional bpf redirect
program on nk_host to redirect traffic to the psp_dev_local.
The topology looks like this:
Host NS: psp_dev_local <---> nk_host
| |
| | (netkit pair)
| |
Remote NS: psp_dev_peer Guest NS: nk_guest
(responder) (PSP tests)
Add following tests for dev-assoc/dev-disassoc functionality:
1. Test the output of `./tools/net/ynl/pyynl/cli.py --spec
Documentation/netlink/specs/psp.yaml --dump dev-get` in both default and
the guest netns.
2. Test the case where we associate netkit with psp_dev_local, and
send PSP traffic from nk_guest to psp_dev_peer in 2 different netns.
3. Test to make sure the key rotation notification is sent to the netns
for associated dev as well
4. Test to make sure the dev change notification is sent to the netns
for associated dev as well
5. Test for dev-assoc/dev-disassoc without nsid parameter.
6. Test the deletion of nk_guest in client netns, and proper cleanup in
the assoc-list for psp dev.
Signed-off-by: Wei Wang <weibunny@fb.com>
---
tools/testing/selftests/drivers/net/config | 1 +
.../selftests/drivers/net/lib/py/env.py | 54 +-
tools/testing/selftests/drivers/net/psp.py | 492 ++++++++++++++++--
3 files changed, 513 insertions(+), 34 deletions(-)
diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config
index 77ccf83d87e0..cdde8234dc07 100644
--- a/tools/testing/selftests/drivers/net/config
+++ b/tools/testing/selftests/drivers/net/config
@@ -7,4 +7,5 @@ CONFIG_NETCONSOLE=m
CONFIG_NETCONSOLE_DYNAMIC=y
CONFIG_NETCONSOLE_EXTENDED_LOG=y
CONFIG_NETDEVSIM=m
+CONFIG_NETKIT=y
CONFIG_XDP_SOCKETS=y
diff --git a/tools/testing/selftests/drivers/net/lib/py/env.py b/tools/testing/selftests/drivers/net/lib/py/env.py
index ccff345fe1c1..2b88e4738fae 100644
--- a/tools/testing/selftests/drivers/net/lib/py/env.py
+++ b/tools/testing/selftests/drivers/net/lib/py/env.py
@@ -2,6 +2,7 @@
import ipaddress
import os
+import re
import time
import json
from pathlib import Path
@@ -327,7 +328,7 @@ class NetDrvContEnv(NetDrvEpEnv):
+---------------+
"""
- def __init__(self, src_path, rxqueues=1, **kwargs):
+ def __init__(self, src_path, rxqueues=1, install_tx_redirect_bpf=False, **kwargs):
self.netns = None
self._nk_host_ifname = None
self._nk_guest_ifname = None
@@ -338,6 +339,8 @@ class NetDrvContEnv(NetDrvEpEnv):
self._init_ns_attached = False
self._old_fwd = None
self._old_accept_ra = None
+ self._nk_host_tc_attached = False
+ self._nk_host_bpf_prog_pref = None
super().__init__(src_path, **kwargs)
@@ -388,7 +391,13 @@ class NetDrvContEnv(NetDrvEpEnv):
self._setup_ns()
self._attach_bpf()
+ if install_tx_redirect_bpf:
+ self._attach_tx_redirect_bpf()
+
def __del__(self):
+ if self._nk_host_tc_attached:
+ cmd(f"tc filter del dev {self._nk_host_ifname} ingress pref {self._nk_host_bpf_prog_pref}", fail=False)
+ self._nk_host_tc_attached = False
if self._tc_attached:
cmd(f"tc filter del dev {self.ifname} ingress pref {self._bpf_prog_pref}")
self._tc_attached = False
@@ -496,3 +505,46 @@ class NetDrvContEnv(NetDrvEpEnv):
value = ipv6_bytes + ifindex_bytes
value_hex = ' '.join(f'{b:02x}' for b in value)
bpftool(f"map update id {bss_map_id} key hex 00 00 00 00 value hex {value_hex}")
+
+ def _attach_tx_redirect_bpf(self):
+ """
+ Attach BPF program on nk_host ingress to redirect TX traffic.
+
+ Packets from nk_guest destined for the nsim network arrive at nk_host
+ via the netkit pair. This BPF program redirects them to the physical
+ interface so they can reach the remote peer.
+ """
+ bpf_obj = self.test_dir / "nk_redirect.bpf.o"
+ if not bpf_obj.exists():
+ raise KsftSkipEx("BPF prog nk_redirect.bpf.o not found")
+
+ cmd(f"tc qdisc add dev {self._nk_host_ifname} clsact")
+
+ cmd(f"tc filter add dev {self._nk_host_ifname} ingress bpf obj {bpf_obj} sec tc/ingress direct-action")
+ self._nk_host_tc_attached = True
+
+ tc_info = cmd(f"tc filter show dev {self._nk_host_ifname} ingress").stdout
+ match = re.search(r'pref (\d+).*nk_redirect\.bpf.*id (\d+)', tc_info)
+ if not match:
+ raise Exception("Failed to get TX redirect BPF prog ID")
+ self._nk_host_bpf_prog_pref = int(match.group(1))
+ nk_host_bpf_prog_id = int(match.group(2))
+
+ prog_info = bpftool(f"prog show id {nk_host_bpf_prog_id}", json=True)
+ map_ids = prog_info.get("map_ids", [])
+
+ bss_map_id = None
+ for map_id in map_ids:
+ map_info = bpftool(f"map show id {map_id}", json=True)
+ if map_info.get("name").endswith("bss"):
+ bss_map_id = map_id
+
+ if bss_map_id is None:
+ raise Exception("Failed to find TX redirect BPF .bss map")
+
+ ipv6_addr = ipaddress.IPv6Address(self.nsim_v6_pfx)
+ ipv6_bytes = ipv6_addr.packed
+ ifindex_bytes = self.ifindex.to_bytes(4, byteorder='little')
+ value = ipv6_bytes + ifindex_bytes
+ value_hex = ' '.join(f'{b:02x}' for b in value)
+ bpftool(f"map update id {bss_map_id} key hex 00 00 00 00 value hex {value_hex}")
diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 864d9fce1094..0ef6a522769e 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -5,6 +5,7 @@
import errno
import fcntl
+import os
import socket
import struct
import termios
@@ -14,9 +15,12 @@ from lib.py import defer
from lib.py import ksft_run, ksft_exit, ksft_pr
from lib.py import ksft_true, ksft_eq, ksft_ne, ksft_gt, ksft_raises
from lib.py import ksft_not_none
-from lib.py import KsftSkipEx
-from lib.py import NetDrvEpEnv, PSPFamily, NlError
+from lib.py import ksft_variants, KsftNamedVariant
+from lib.py import KsftSkipEx, KsftFailEx
+from lib.py import NetDrvEpEnv, NetDrvContEnv, PSPFamily, NlError
+from lib.py import NetNSEnter
from lib.py import bkg, rand_port, wait_port_listen
+from lib.py import ip
def _get_outq(s):
@@ -117,11 +121,13 @@ def _get_stat(cfg, key):
# Test case boiler plate
#
-def _init_psp_dev(cfg):
+def _init_psp_dev(cfg, use_psp_ifindex=False):
if not hasattr(cfg, 'psp_dev_id'):
# Figure out which local device we are testing against
+ # For NetDrvContEnv: use psp_ifindex instead of ifindex
+ target_ifindex = cfg.psp_ifindex if use_psp_ifindex else cfg.ifindex
for dev in cfg.pspnl.dev_get({}, dump=True):
- if dev['ifindex'] == cfg.ifindex:
+ if dev['ifindex'] == target_ifindex:
cfg.psp_info = dev
cfg.psp_dev_id = cfg.psp_info['id']
break
@@ -394,6 +400,297 @@ def _data_basic_send(cfg, version, ipver):
_close_psp_conn(cfg, s)
+def _data_basic_send_netkit_psp_assoc(cfg, version, ipver):
+ """
+ Test basic data send with netkit interface associated with PSP dev.
+ """
+
+ _init_psp_dev(cfg, True)
+ psp_dev_id_for_assoc = cfg.psp_dev_id
+
+ # Associate PSP device with nk_guest interface (in guest namespace)
+ nk_guest_dev = ip(f"link show dev {cfg._nk_guest_ifname}", json=True, ns=cfg.netns)[0]
+ nk_guest_ifindex = nk_guest_dev['ifindex']
+
+ cfg.pspnl.dev_assoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ # Check if assoc-list contains nk_guest
+ dev_info = cfg.pspnl.dev_get({'id': psp_dev_id_for_assoc})
+
+ if 'assoc-list' in dev_info:
+ found = False
+ for assoc in dev_info['assoc-list']:
+ if assoc['ifindex'] == nk_guest_ifindex and assoc['nsid'] == cfg.psp_dev_peer_nsid:
+ found = True
+ break
+ ksft_true(found, "Associated device not found in dev_get() response")
+ else:
+ raise RuntimeError("No assoc-list in dev_get() response after association")
+
+ # Enter guest namespace (netns) to run PSP test
+ with NetNSEnter(cfg.netns.name):
+ cfg.pspnl = PSPFamily()
+
+ s = _make_psp_conn(cfg, version, ipver)
+
+ rx_assoc = cfg.pspnl.rx_assoc({"version": version,
+ "dev-id": cfg.psp_dev_id,
+ "sock-fd": s.fileno()})
+ rx = rx_assoc['rx-key']
+ tx = _spi_xchg(s, rx)
+
+ cfg.pspnl.tx_assoc({"dev-id": cfg.psp_dev_id,
+ "version": version,
+ "tx-key": tx,
+ "sock-fd": s.fileno()})
+
+ data_len = _send_careful(cfg, s, 100)
+ _check_data_rx(cfg, data_len)
+ _close_psp_conn(cfg, s)
+
+ # Clean up - back in host namespace
+ cfg.pspnl = PSPFamily()
+ cfg.pspnl.dev_disassoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ del cfg.psp_dev_id
+ del cfg.psp_info
+
+
+def _key_rotation_notify_multi_ns_netkit(cfg, version, ipver):
+ """ Test key rotation notifications across multiple namespaces using netkit """
+ _init_psp_dev(cfg, True)
+ psp_dev_id_for_assoc = cfg.psp_dev_id
+
+ # Associate PSP device with nk_guest interface (in guest namespace)
+ nk_guest_dev = ip(f"link show dev {cfg._nk_guest_ifname}", json=True, ns=cfg.netns)[0]
+ nk_guest_ifindex = nk_guest_dev['ifindex']
+
+ cfg.pspnl.dev_assoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ # Create listener in guest namespace; socket stays bound to that ns
+ with NetNSEnter(cfg.netns.name):
+ peer_pspnl = PSPFamily()
+ peer_pspnl.ntf_subscribe('use')
+
+ # Create listener in main namespace
+ main_pspnl = PSPFamily()
+ main_pspnl.ntf_subscribe('use')
+
+ # Trigger key rotation on the PSP device
+ cfg.pspnl.key_rotate({"id": psp_dev_id_for_assoc})
+
+ # Poll both sockets from main thread
+ for pspnl, label in [(main_pspnl, "main"), (peer_pspnl, "guest")]:
+ for i in range(100):
+ pspnl.check_ntf()
+
+ try:
+ msg = pspnl.async_msg_queue.get_nowait()
+ break
+ except Exception:
+ pass
+
+ time.sleep(0.1)
+ else:
+ raise KsftFailEx(f"No key rotation notification received in {label} namespace")
+
+ ksft_true(msg['msg'].get('id') == psp_dev_id_for_assoc,
+ f"Key rotation notification for correct device not found in {label} namespace")
+
+ # Clean up
+ cfg.pspnl.dev_disassoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+ del cfg.psp_dev_id
+ del cfg.psp_info
+
+
+def _dev_change_notify_multi_ns_netkit(cfg, version, ipver):
+ """ Test dev_change notifications across multiple namespaces using netkit """
+ _init_psp_dev(cfg, True)
+ psp_dev_id_for_assoc = cfg.psp_dev_id
+
+ # Associate PSP device with nk_guest interface (in guest namespace)
+ nk_guest_dev = ip(f"link show dev {cfg._nk_guest_ifname}", json=True, ns=cfg.netns)[0]
+ nk_guest_ifindex = nk_guest_dev['ifindex']
+
+ cfg.pspnl.dev_assoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ # Create listener in guest namespace; socket stays bound to that ns
+ with NetNSEnter(cfg.netns.name):
+ peer_pspnl = PSPFamily()
+ peer_pspnl.ntf_subscribe('mgmt')
+
+ # Create listener in main namespace
+ main_pspnl = PSPFamily()
+ main_pspnl.ntf_subscribe('mgmt')
+
+ # Trigger dev_change by calling dev_set (notification is always sent)
+ cfg.pspnl.dev_set({'id': psp_dev_id_for_assoc, 'psp-versions-ena': cfg.psp_info['psp-versions-cap']})
+
+ # Poll both sockets from main thread
+ for pspnl, label in [(main_pspnl, "main"), (peer_pspnl, "guest")]:
+ for i in range(100):
+ pspnl.check_ntf()
+
+ try:
+ msg = pspnl.async_msg_queue.get_nowait()
+ break
+ except Exception:
+ pass
+
+ time.sleep(0.1)
+ else:
+ raise KsftFailEx(f"No dev_change notification received in {label} namespace")
+
+ ksft_true(msg['msg'].get('id') == psp_dev_id_for_assoc,
+ f"Dev_change notification for correct device not found in {label} namespace")
+
+ # Clean up
+ cfg.pspnl.dev_disassoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+ del cfg.psp_dev_id
+ del cfg.psp_info
+
+
+def _psp_dev_get_check_netkit_psp_assoc(cfg, version, ipver):
+ """ Check psp dev-get output with netkit interface associated with PSP dev """
+
+ _init_psp_dev(cfg, True)
+ psp_dev_id_for_assoc = cfg.psp_dev_id
+
+ # Associate PSP device with nk_guest interface (in guest namespace)
+ nk_guest_dev = ip(f"link show dev {cfg._nk_guest_ifname}", json=True, ns=cfg.netns)[0]
+ nk_guest_ifindex = nk_guest_dev['ifindex']
+
+ cfg.pspnl.dev_assoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ # Check 1: In default netns, verify dev-get has correct ifindex and assoc-list
+ dev_info = cfg.pspnl.dev_get({'id': psp_dev_id_for_assoc})
+
+ # Verify the PSP device has the correct ifindex
+ ksft_eq(dev_info['ifindex'], cfg.psp_ifindex)
+
+ # Verify assoc-list exists and contains the associated nk_guest with correct ifindex and nsid
+ ksft_true('assoc-list' in dev_info, "No assoc-list in dev_get() response after association")
+ found = False
+ for assoc in dev_info['assoc-list']:
+ if assoc['ifindex'] == nk_guest_ifindex and assoc['nsid'] == cfg.psp_dev_peer_nsid:
+ found = True
+ break
+ ksft_true(found, "Associated device not found in assoc-list with correct ifindex and nsid")
+
+ # Check 2: In guest netns, verify dev-get has assoc-list with nk_guest device
+ with NetNSEnter(cfg.netns.name):
+ peer_pspnl = PSPFamily()
+
+ # Dump all devices in the guest namespace
+ peer_devices = peer_pspnl.dev_get({}, dump=True)
+
+ # Find the device with by-association flag
+ peer_dev = None
+ for dev in peer_devices:
+ if dev.get('by-association'):
+ peer_dev = dev
+ break
+
+ ksft_not_none(peer_dev, "No PSP device found with by-association flag in guest netns")
+
+ # Verify assoc-list contains the nk_guest device
+ ksft_true('assoc-list' in peer_dev and len(peer_dev['assoc-list']) > 0,
+ "Guest device should have assoc-list with local devices")
+
+ # Verify the assoc-list contains nk_guest ifindex with nsid=-1 (same namespace)
+ found = False
+ for assoc in peer_dev['assoc-list']:
+ if assoc['ifindex'] == nk_guest_ifindex:
+ ksft_eq(assoc['nsid'], -1,
+ "nsid should be -1 (NETNSA_NSID_NOT_ASSIGNED) for same-namespace device")
+ found = True
+ break
+ ksft_true(found, "nk_guest ifindex not found in assoc-list")
+
+ # Clean up
+ cfg.pspnl.dev_disassoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ del cfg.psp_dev_id
+ del cfg.psp_info
+
+
+def _dev_assoc_no_nsid(cfg):
+ """ Test dev-assoc and dev-disassoc without nsid attribute """
+ _init_psp_dev(cfg, True)
+ psp_dev_id = cfg.psp_dev_id
+
+ # Get nk_host's ifindex (in host namespace, same as caller)
+ nk_host_dev = ip(f"link show dev {cfg._nk_host_ifname}", json=True)[0]
+ nk_host_ifindex = nk_host_dev['ifindex']
+
+ # Associate without nsid - should look up ifindex in caller's netns
+ cfg.pspnl.dev_assoc({'id': psp_dev_id, 'ifindex': nk_host_ifindex})
+
+ # Verify assoc-list contains the device
+ dev_info = cfg.pspnl.dev_get({'id': psp_dev_id})
+ ksft_true('assoc-list' in dev_info, "No assoc-list after association")
+ found = False
+ for assoc in dev_info['assoc-list']:
+ if assoc['ifindex'] == nk_host_ifindex:
+ found = True
+ break
+ ksft_true(found, "Associated device not found in assoc-list")
+
+ # Disassociate without nsid - should also use caller's netns
+ cfg.pspnl.dev_disassoc({'id': psp_dev_id, 'ifindex': nk_host_ifindex})
+
+ # Verify assoc-list no longer contains the device
+ dev_info = cfg.pspnl.dev_get({'id': psp_dev_id})
+ found = False
+ if 'assoc-list' in dev_info:
+ for assoc in dev_info['assoc-list']:
+ if assoc['ifindex'] == nk_host_ifindex:
+ found = True
+ break
+ ksft_true(not found, "Device should not be in assoc-list after disassociation")
+
+ del cfg.psp_dev_id
+ del cfg.psp_info
+
+
+def _psp_dev_assoc_cleanup_on_netkit_del(cfg):
+ """ Test that assoc-list is cleared when associated netkit interface is deleted """
+ _init_psp_dev(cfg, True)
+ psp_dev_id_for_assoc = cfg.psp_dev_id
+
+ # Associate PSP device with nk_guest interface (in guest namespace)
+ nk_guest_dev = ip(f"link show dev {cfg._nk_guest_ifname}", json=True, ns=cfg.netns)[0]
+ nk_guest_ifindex = nk_guest_dev['ifindex']
+
+ cfg.pspnl.dev_assoc({'id': psp_dev_id_for_assoc, 'ifindex': nk_guest_ifindex, 'nsid': cfg.psp_dev_peer_nsid})
+
+ # Verify assoc-list exists in default netns
+ dev_info = cfg.pspnl.dev_get({'id': psp_dev_id_for_assoc})
+ ksft_true('assoc-list' in dev_info, "No assoc-list after association")
+ found = False
+ for assoc in dev_info['assoc-list']:
+ if assoc['ifindex'] == nk_guest_ifindex and assoc['nsid'] == cfg.psp_dev_peer_nsid:
+ found = True
+ break
+ ksft_true(found, "Associated device not found in assoc-list")
+
+ # Delete the netkit interface in the guest namespace
+ ip(f"link del {cfg._nk_guest_ifname}", ns=cfg.netns)
+
+ # Mark netkit as already deleted so cleanup won't try to delete it again
+ # (deleting nk_guest also removes nk_host since they're a pair)
+ cfg._nk_host_ifname = None
+ cfg._nk_guest_ifname = None
+
+ # Verify assoc-list is gone in default netns after netkit deletion
+ dev_info = cfg.pspnl.dev_get({'id': psp_dev_id_for_assoc})
+ ksft_true('assoc-list' not in dev_info or len(dev_info['assoc-list']) == 0,
+ "assoc-list should be empty after netkit deletion")
+
+ del cfg.psp_dev_id
+ del cfg.psp_info
+
+
def __bad_xfer_do(cfg, s, tx, version='hdr0-aes-gcm-128'):
# Make sure we accept the ACK for the SPI before we seal with the bad assoc
_check_data_outq(s, 0)
@@ -571,33 +868,162 @@ def removal_device_bi(cfg):
_close_conn(cfg, s)
-def psp_ip_ver_test_builder(name, test_func, psp_ver, ipver):
- """Build test cases for each combo of PSP version and IP version"""
- def test_case(cfg):
- cfg.require_ipver(ipver)
- test_func(cfg, psp_ver, ipver)
-
- test_case.__name__ = f"{name}_v{psp_ver}_ip{ipver}"
- return test_case
+@ksft_variants([
+ KsftNamedVariant(f"v{v}_ip{ip}", v, ip)
+ for v in range(4) for ip in ("4", "6")
+])
+def data_basic_send(cfg, version, ipver):
+ cfg.require_ipver(ipver)
+ _data_basic_send(cfg, version, ipver)
+
+
+@ksft_variants([
+ KsftNamedVariant(f"ip{ip}", ip)
+ for ip in ("4", "6")
+])
+def data_mss_adjust(cfg, ipver):
+ cfg.require_ipver(ipver)
+ _data_mss_adjust(cfg, ipver)
+
+
+@ksft_variants([
+ KsftNamedVariant(f"v{v}_ip6", v, "6")
+ for v in range(4)
+])
+def data_basic_send_netkit_psp_assoc(cfg, version, ipver):
+ cfg.require_ipver(ipver)
+ _data_basic_send_netkit_psp_assoc(cfg, version, ipver)
+
+
+@ksft_variants([
+ KsftNamedVariant(f"v{v}_ip6", v, "6")
+ for v in range(4)
+])
+def key_rotation_notify_multi_ns_netkit(cfg, version, ipver):
+ cfg.require_ipver(ipver)
+ _key_rotation_notify_multi_ns_netkit(cfg, version, ipver)
+
+
+@ksft_variants([
+ KsftNamedVariant(f"v{v}_ip6", v, "6")
+ for v in range(4)
+])
+def dev_change_notify_multi_ns_netkit(cfg, version, ipver):
+ cfg.require_ipver(ipver)
+ _dev_change_notify_multi_ns_netkit(cfg, version, ipver)
+
+
+@ksft_variants([
+ KsftNamedVariant(f"v{v}_ip6", v, "6")
+ for v in range(4)
+])
+def psp_dev_get_check_netkit_psp_assoc(cfg, version, ipver):
+ cfg.require_ipver(ipver)
+ _psp_dev_get_check_netkit_psp_assoc(cfg, version, ipver)
+
+
+@ksft_variants([
+ KsftNamedVariant(f"v{v}_ip6", v, "6")
+ for v in range(4)
+])
+def dev_assoc_no_nsid(cfg, version, ipver):
+ cfg.require_ipver(ipver)
+ _dev_assoc_no_nsid(cfg)
+
+
+def _get_nsid(ns_name):
+ """Get the nsid for a namespace."""
+ for entry in ip("netns list-id", json=True):
+ if entry.get("name") == str(ns_name):
+ return entry["nsid"]
+ raise KsftSkipEx(f"nsid not found for namespace {ns_name}")
+
+
+def _setup_psp_attributes(cfg):
+ """
+ Set up PSP-specific attributes on the environment.
+
+ This sets attributes needed for PSP tests based on whether we're using
+ netdevsim or a real NIC.
+ """
+ if cfg._ns is not None:
+ # netdevsim case: PSP device is the local dev (in host namespace)
+ cfg.psp_dev = cfg._ns.nsims[0].dev
+ cfg.psp_ifname = cfg.psp_dev['ifname']
+ cfg.psp_ifindex = cfg.psp_dev['ifindex']
+
+ # PSP peer device is the remote dev (in _netns, where psp_responder runs)
+ cfg.psp_dev_peer = cfg._ns_peer.nsims[0].dev
+ cfg.psp_dev_peer_ifname = cfg.psp_dev_peer['ifname']
+ cfg.psp_dev_peer_ifindex = cfg.psp_dev_peer['ifindex']
+ else:
+ # Real NIC case: PSP device is the local interface
+ cfg.psp_dev = cfg.dev
+ cfg.psp_ifname = cfg.ifname
+ cfg.psp_ifindex = cfg.ifindex
+
+ # PSP peer device is the remote interface
+ cfg.psp_dev_peer = cfg.remote_dev
+ cfg.psp_dev_peer_ifname = cfg.remote_ifname
+ cfg.psp_dev_peer_ifindex = cfg.remote_ifindex
+
+ # Get nsid for the guest namespace (netns) where nk_guest is
+ cfg.psp_dev_peer_nsid = _get_nsid(cfg.netns.name)
+
+
+def _setup_psp_routes(cfg):
+ """
+ Set up routes for cross-namespace connectivity.
+
+ Traffic flows:
+ 1. remote (_netns) -> nk_guest (netns):
+ psp_dev_peer -> psp_dev_local -> BPF redirect -> nk_host -> nk_guest
+ Needs: route in _netns to nk_v6_pfx/64 via psp_dev_local
+
+ 2. nk_guest (netns) -> remote (_netns):
+ nk_guest -> nk_host -> psp_dev_local -> psp_dev_peer
+ Needs: route in netns to dev_v6_pfx/64 via nk_host
+ """
+ # In _netns (remote namespace): add route to nk_guest prefix via psp_dev_local
+ # psp_dev_peer can reach psp_dev_local via the link, then traffic goes through BPF
+ ip(f"-6 route add {cfg.nk_v6_pfx}/64 via {cfg.nsim_v6_pfx}1 dev {cfg.psp_dev_peer_ifname}",
+ ns=cfg._netns)
+
+ # In netns (guest namespace): add route to remote peer prefix
+ # nk_guest default route goes to nk_host, but we need explicit route to dev_v6_pfx/64
+ ip(f"-6 route add {cfg.nsim_v6_pfx}/64 via fe80::1 dev {cfg._nk_guest_ifname}",
+ ns=cfg.netns)
-def ipver_test_builder(name, test_func, ipver):
- """Build test cases for each IP version"""
- def test_case(cfg):
- cfg.require_ipver(ipver)
- test_func(cfg, ipver)
+def main() -> None:
+ """ Ksft boiler plate main """
- test_case.__name__ = f"{name}_ip{ipver}"
- return test_case
+ # Use a different prefix for netkit guest to avoid conflict with dev prefix
+ nk_v6_pfx = "2001:db9::"
+ # Set LOCAL_PREFIX_V6 to a DIFFERENT prefix than the dev prefix to avoid BPF
+ # redirecting psp_responder traffic. The BPF only redirects traffic
+ # matching LOCAL_PREFIX_V6, so dev traffic (2001:db8::) won't be affected.
+ if "LOCAL_PREFIX_V6" not in os.environ:
+ os.environ["LOCAL_PREFIX_V6"] = nk_v6_pfx
-def main() -> None:
- """ Ksft boiler plate main """
+ try:
+ env = NetDrvContEnv(__file__, install_tx_redirect_bpf=True)
+ has_cont = True
+ except KsftSkipEx:
+ env = NetDrvEpEnv(__file__)
+ has_cont = False
- with NetDrvEpEnv(__file__) as cfg:
+ with env as cfg:
cfg.pspnl = PSPFamily()
+ if has_cont:
+ cfg.nk_v6_pfx = nk_v6_pfx
+ _setup_psp_attributes(cfg)
+ _setup_psp_routes(cfg)
+
# Set up responder and communication sock
+ # psp_responder runs in _netns (remote namespace with psp_dev_peer)
responder = cfg.remote.deploy("psp_responder")
cfg.comm_port = rand_port()
@@ -611,17 +1037,17 @@ def main() -> None:
cfg.comm_port),
timeout=1)
- cases = [
- psp_ip_ver_test_builder(
- "data_basic_send", _data_basic_send, version, ipver
- )
- for version in range(0, 4)
- for ipver in ("4", "6")
- ]
- cases += [
- ipver_test_builder("data_mss_adjust", _data_mss_adjust, ipver)
- for ipver in ("4", "6")
- ]
+ cases = [data_basic_send, data_mss_adjust]
+
+ if has_cont:
+ cases += [
+ data_basic_send_netkit_psp_assoc,
+ key_rotation_notify_multi_ns_netkit,
+ dev_change_notify_multi_ns_netkit,
+ psp_dev_get_check_netkit_psp_assoc,
+ dev_assoc_no_nsid,
+ _psp_dev_assoc_cleanup_on_netkit_del,
+ ]
ksft_run(cases=cases, globs=globals(),
case_pfx={"dev_", "data_", "assoc_", "removal_"},
--
2.52.0
^ permalink raw reply related
* [PATCH v8 net-next 4/5] selftests/net: Add bpf skb forwarding program
From: Wei Wang @ 2026-03-29 4:19 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Daniel Zahka, Willem de Bruijn, David Wei,
Andrew Lunn, David S . Miller, Eric Dumazet, Simon Horman
Cc: Wei Wang, Bobby Eshleman
In-Reply-To: <20260329041922.3850747-1-weibunny.kernel@gmail.com>
From: Wei Wang <weibunny@fb.com>
Add nk_redirect.bpf.c, a BPF program that forwards skbs matching some IPv6
prefix received on eth0 ifindex to a specified dev ifindex.
bpf_redirect_neigh() is used to make sure neighbor lookup is performed
and proper MAC addr is being used.
Signed-off-by: Wei Wang <weibunny@fb.com>
Reviewed-by: Bobby Eshleman <bobbyeshleman@meta.com>
Tested-by: Bobby Eshleman <bobbyeshleman@meta.com>
---
.../drivers/net/hw/nk_redirect.bpf.c | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 tools/testing/selftests/drivers/net/hw/nk_redirect.bpf.c
diff --git a/tools/testing/selftests/drivers/net/hw/nk_redirect.bpf.c b/tools/testing/selftests/drivers/net/hw/nk_redirect.bpf.c
new file mode 100644
index 000000000000..7ac9ffd50f15
--- /dev/null
+++ b/tools/testing/selftests/drivers/net/hw/nk_redirect.bpf.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * BPF program for redirecting traffic using bpf_redirect_neigh().
+ * Unlike bpf_redirect() which preserves L2 headers, bpf_redirect_neigh()
+ * performs neighbor lookup and fills in the correct L2 addresses for the
+ * target interface. This is necessary when redirecting across different
+ * device types (e.g., from netdevsim to netkit).
+ */
+#include <linux/bpf.h>
+#include <linux/pkt_cls.h>
+#include <linux/if_ether.h>
+#include <linux/ipv6.h>
+#include <linux/in6.h>
+#include <bpf/bpf_endian.h>
+#include <bpf/bpf_helpers.h>
+
+#define TC_ACT_OK 0
+#define ETH_P_IPV6 0x86DD
+
+#define ctx_ptr(field) ((void *)(long)(field))
+
+#define v6_p64_equal(a, b) (a.s6_addr32[0] == b.s6_addr32[0] && \
+ a.s6_addr32[1] == b.s6_addr32[1])
+
+volatile __u32 redirect_ifindex;
+volatile __u8 ipv6_prefix[16];
+
+SEC("tc/ingress")
+int tc_redirect(struct __sk_buff *skb)
+{
+ void *data_end = ctx_ptr(skb->data_end);
+ void *data = ctx_ptr(skb->data);
+ struct in6_addr *match_prefix;
+ struct ipv6hdr *ip6h;
+ struct ethhdr *eth;
+
+ match_prefix = (struct in6_addr *)ipv6_prefix;
+
+ if (skb->protocol != bpf_htons(ETH_P_IPV6))
+ return TC_ACT_OK;
+
+ eth = data;
+ if ((void *)(eth + 1) > data_end)
+ return TC_ACT_OK;
+
+ ip6h = data + sizeof(struct ethhdr);
+ if ((void *)(ip6h + 1) > data_end)
+ return TC_ACT_OK;
+
+ if (!v6_p64_equal(ip6h->daddr, (*match_prefix)))
+ return TC_ACT_OK;
+
+ /*
+ * Use bpf_redirect_neigh() to perform neighbor lookup and fill in
+ * correct L2 addresses for the target interface.
+ */
+ return bpf_redirect_neigh(redirect_ifindex, NULL, 0, 0);
+}
+
+char __license[] SEC("license") = "GPL";
--
2.52.0
^ permalink raw reply related
* [PATCH v8 net-next 3/5] psp: add a new netdev event for dev unregister
From: Wei Wang @ 2026-03-29 4:19 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Daniel Zahka, Willem de Bruijn, David Wei,
Andrew Lunn, David S . Miller, Eric Dumazet, Simon Horman
Cc: Wei Wang
In-Reply-To: <20260329041922.3850747-1-weibunny.kernel@gmail.com>
From: Wei Wang <weibunny@fb.com>
Add a new netdev event for dev unregister and handle the removal of this
dev from psp->assoc_dev_list, upon the first dev-assoc operation.
Signed-off-by: Wei Wang <weibunny@fb.com>
---
Documentation/netlink/specs/psp.yaml | 2 +-
net/psp/psp-nl-gen.c | 2 +-
net/psp/psp-nl-gen.h | 3 ++
net/psp/psp.h | 1 +
net/psp/psp_main.c | 70 ++++++++++++++++++++++++++++
net/psp/psp_nl.c | 8 ++++
6 files changed, 84 insertions(+), 2 deletions(-)
diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
index 336ef19155ff..d7c1fc62de1c 100644
--- a/Documentation/netlink/specs/psp.yaml
+++ b/Documentation/netlink/specs/psp.yaml
@@ -320,7 +320,7 @@ operations:
- nsid
reply:
attributes: []
- pre: psp-device-get-locked
+ pre: psp-device-get-locked-dev-assoc
post: psp-device-unlock
-
name: dev-disassoc
diff --git a/net/psp/psp-nl-gen.c b/net/psp/psp-nl-gen.c
index 114299c64423..389a8480cc3d 100644
--- a/net/psp/psp-nl-gen.c
+++ b/net/psp/psp-nl-gen.c
@@ -135,7 +135,7 @@ static const struct genl_split_ops psp_nl_ops[] = {
},
{
.cmd = PSP_CMD_DEV_ASSOC,
- .pre_doit = psp_device_get_locked,
+ .pre_doit = psp_device_get_locked_dev_assoc,
.doit = psp_nl_dev_assoc_doit,
.post_doit = psp_device_unlock,
.policy = psp_dev_assoc_nl_policy,
diff --git a/net/psp/psp-nl-gen.h b/net/psp/psp-nl-gen.h
index 4dd0f0f23053..24d51bff997f 100644
--- a/net/psp/psp-nl-gen.h
+++ b/net/psp/psp-nl-gen.h
@@ -21,6 +21,9 @@ int psp_device_get_locked_admin(const struct genl_split_ops *ops,
struct sk_buff *skb, struct genl_info *info);
int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
struct sk_buff *skb, struct genl_info *info);
+int psp_device_get_locked_dev_assoc(const struct genl_split_ops *ops,
+ struct sk_buff *skb,
+ struct genl_info *info);
void
psp_device_unlock(const struct genl_split_ops *ops, struct sk_buff *skb,
struct genl_info *info);
diff --git a/net/psp/psp.h b/net/psp/psp.h
index 0f9c4e4e52cb..fd7457dedd30 100644
--- a/net/psp/psp.h
+++ b/net/psp/psp.h
@@ -15,6 +15,7 @@ extern struct mutex psp_devs_lock;
void psp_dev_free(struct psp_dev *psd);
int psp_dev_check_access(struct psp_dev *psd, struct net *net, bool admin);
+void psp_attach_netdev_notifier(void);
void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd);
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index 178b848989f1..160eaf3e7229 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -375,10 +375,80 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
}
EXPORT_SYMBOL(psp_dev_rcv);
+static void psp_dev_disassoc_one(struct psp_dev *psd, struct net_device *dev)
+{
+ struct psp_assoc_dev *entry, *tmp;
+
+ list_for_each_entry_safe(entry, tmp, &psd->assoc_dev_list, dev_list) {
+ if (entry->assoc_dev == dev) {
+ list_del(&entry->dev_list);
+ rcu_assign_pointer(entry->assoc_dev->psp_dev, NULL);
+ netdev_put(entry->assoc_dev, &entry->dev_tracker);
+ kfree(entry);
+ return;
+ }
+ }
+}
+
+static int psp_netdev_event(struct notifier_block *nb, unsigned long event,
+ void *ptr)
+{
+ struct net_device *dev = netdev_notifier_info_to_dev(ptr);
+ struct psp_dev *psd;
+
+ if (event != NETDEV_UNREGISTER)
+ return NOTIFY_DONE;
+
+ rcu_read_lock();
+ psd = rcu_dereference(dev->psp_dev);
+ if (psd && psp_dev_tryget(psd)) {
+ rcu_read_unlock();
+ mutex_lock(&psd->lock);
+ psp_dev_disassoc_one(psd, dev);
+ mutex_unlock(&psd->lock);
+ psp_dev_put(psd);
+ } else {
+ rcu_read_unlock();
+ }
+
+ return NOTIFY_DONE;
+}
+
+static struct notifier_block psp_netdev_notifier = {
+ .notifier_call = psp_netdev_event,
+};
+
+static bool psp_notifier_registered;
+
+/**
+ * psp_attach_netdev_notifier() - register netdev notifier on first use
+ *
+ * Register the netdevice notifier when the first device association
+ * is created. In many installations no associations will be created and
+ * the notifier won't be needed.
+ *
+ * Must be called without psd->lock held, due to lock ordering:
+ * rtnl_lock -> psd->lock (the notifier callback runs under rtnl_lock
+ * and takes psd->lock).
+ */
+void psp_attach_netdev_notifier(void)
+{
+ if (READ_ONCE(psp_notifier_registered))
+ return;
+
+ mutex_lock(&psp_devs_lock);
+ if (!psp_notifier_registered) {
+ if (!register_netdevice_notifier(&psp_netdev_notifier))
+ WRITE_ONCE(psp_notifier_registered, true);
+ }
+ mutex_unlock(&psp_devs_lock);
+}
+
static int __init psp_init(void)
{
mutex_init(&psp_devs_lock);
return genl_register_family(&psp_nl_family);
}
subsys_initcall(psp_init);
diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
--- a/net/psp/psp_nl.c
+++ b/net/psp/psp_nl.c
@@ -159,6 +159,14 @@ int psp_device_get_locked(const struct genl_split_ops *ops,
return __psp_device_get_locked(ops, skb, info, false);
}
+int psp_device_get_locked_dev_assoc(const struct genl_split_ops *ops,
+ struct sk_buff *skb, struct genl_info *info)
+{
+ psp_attach_netdev_notifier();
+
+ return __psp_device_get_locked(ops, skb, info, false);
+}
+
void
psp_device_unlock(const struct genl_split_ops *ops, struct sk_buff *skb,
struct genl_info *info)
--
2.52.0
^ permalink raw reply related
* [PATCH v8 net-next 2/5] psp: add new netlink cmd for dev-assoc and dev-disassoc
From: Wei Wang @ 2026-03-29 4:19 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Daniel Zahka, Willem de Bruijn, David Wei,
Andrew Lunn, David S . Miller, Eric Dumazet, Simon Horman
Cc: Wei Wang
In-Reply-To: <20260329041922.3850747-1-weibunny.kernel@gmail.com>
From: Wei Wang <weibunny@fb.com>
The main purpose of this cmd is to be able to associate a
non-psp-capable device (e.g. veth or netkit) with a psp device.
One use case is if we create a pair of veth/netkit, and assign 1 end
inside a netns, while leaving the other end within the default netns,
with a real PSP device, e.g. netdevsim or a physical PSP-capable NIC.
With this command, we could associate the veth/netkit inside the netns
with PSP device, so the virtual device could act as PSP-capable device
to initiate PSP connections, and performs PSP encryption/decryption on
the real PSP device.
Signed-off-by: Wei Wang <weibunny@fb.com>
---
Documentation/netlink/specs/psp.yaml | 67 +++++-
include/net/psp/types.h | 15 ++
include/uapi/linux/psp.h | 13 ++
net/psp/psp-nl-gen.c | 32 +++
net/psp/psp-nl-gen.h | 2 +
net/psp/psp_main.c | 21 +-
net/psp/psp_nl.c | 311 ++++++++++++++++++++++++++-
7 files changed, 449 insertions(+), 12 deletions(-)
diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
index fe2cdc966604..336ef19155ff 100644
--- a/Documentation/netlink/specs/psp.yaml
+++ b/Documentation/netlink/specs/psp.yaml
@@ -13,6 +13,17 @@ definitions:
hdr0-aes-gmac-128, hdr0-aes-gmac-256]
attribute-sets:
+ -
+ name: assoc-dev-info
+ attributes:
+ -
+ name: ifindex
+ doc: ifindex of an associated network device.
+ type: u32
+ -
+ name: nsid
+ doc: Network namespace ID of the associated device.
+ type: s32
-
name: dev
attributes:
@@ -24,7 +35,9 @@ attribute-sets:
min: 1
-
name: ifindex
- doc: ifindex of the main netdevice linked to the PSP device.
+ doc: |
+ ifindex of the main netdevice linked to the PSP device,
+ or the ifindex to associate with the PSP device.
type: u32
-
name: psp-versions-cap
@@ -38,6 +51,28 @@ attribute-sets:
type: u32
enum: version
enum-as-flags: true
+ -
+ name: assoc-list
+ doc: List of associated virtual devices.
+ type: nest
+ nested-attributes: assoc-dev-info
+ multi-attr: true
+ -
+ name: nsid
+ doc: |
+ Network namespace ID for the device to associate/disassociate.
+ Optional for dev-assoc and dev-disassoc; if not present, the
+ device is looked up in the caller's network namespace.
+ type: s32
+ -
+ name: by-association
+ doc: |
+ Flag indicating the PSP device is an associated device from a
+ different network namespace.
+ Present when in associated namespace, absent when in primary/host
+ namespace.
+ type: flag
+
-
name: assoc
attributes:
@@ -170,6 +205,8 @@ operations:
- ifindex
- psp-versions-cap
- psp-versions-ena
+ - assoc-list
+ - by-association
pre: psp-device-get-locked
post: psp-device-unlock
dump:
@@ -271,6 +308,34 @@ operations:
post: psp-device-unlock
dump:
reply: *stats-all
+ -
+ name: dev-assoc
+ doc: Associate a network device with a PSP device.
+ attribute-set: dev
+ do:
+ request:
+ attributes:
+ - id
+ - ifindex
+ - nsid
+ reply:
+ attributes: []
+ pre: psp-device-get-locked
+ post: psp-device-unlock
+ -
+ name: dev-disassoc
+ doc: Disassociate a network device from a PSP device.
+ attribute-set: dev
+ do:
+ request:
+ attributes:
+ - id
+ - ifindex
+ - nsid
+ reply:
+ attributes: []
+ pre: psp-device-get-locked
+ post: psp-device-unlock
mcast-groups:
list:
diff --git a/include/net/psp/types.h b/include/net/psp/types.h
index 25a9096d4e7d..4bd432ed107a 100644
--- a/include/net/psp/types.h
+++ b/include/net/psp/types.h
@@ -5,6 +5,7 @@
#include <linux/mutex.h>
#include <linux/refcount.h>
+#include <net/net_trackers.h>
struct netlink_ext_ack;
@@ -43,9 +44,22 @@ struct psp_dev_config {
u32 versions;
};
+/**
+ * struct psp_assoc_dev - wrapper for associated net_device
+ * @dev_list: list node for psp_dev::assoc_dev_list
+ * @assoc_dev: the associated net_device
+ * @dev_tracker: tracker for the net_device reference
+ */
+struct psp_assoc_dev {
+ struct list_head dev_list;
+ struct net_device *assoc_dev;
+ netdevice_tracker dev_tracker;
+};
+
/**
* struct psp_dev - PSP device struct
* @main_netdev: original netdevice of this PSP device
+ * @assoc_dev_list: list of psp_assoc_dev entries associated with this PSP device
* @ops: driver callbacks
* @caps: device capabilities
* @drv_priv: driver priv pointer
@@ -67,6 +81,7 @@ struct psp_dev_config {
*/
struct psp_dev {
struct net_device *main_netdev;
+ struct list_head assoc_dev_list;
struct psp_dev_ops *ops;
struct psp_dev_caps *caps;
diff --git a/include/uapi/linux/psp.h b/include/uapi/linux/psp.h
index a3a336488dc3..1c8899cd4da5 100644
--- a/include/uapi/linux/psp.h
+++ b/include/uapi/linux/psp.h
@@ -17,11 +17,22 @@ enum psp_version {
PSP_VERSION_HDR0_AES_GMAC_256,
};
+enum {
+ PSP_A_ASSOC_DEV_INFO_IFINDEX = 1,
+ PSP_A_ASSOC_DEV_INFO_NSID,
+
+ __PSP_A_ASSOC_DEV_INFO_MAX,
+ PSP_A_ASSOC_DEV_INFO_MAX = (__PSP_A_ASSOC_DEV_INFO_MAX - 1)
+};
+
enum {
PSP_A_DEV_ID = 1,
PSP_A_DEV_IFINDEX,
PSP_A_DEV_PSP_VERSIONS_CAP,
PSP_A_DEV_PSP_VERSIONS_ENA,
+ PSP_A_DEV_ASSOC_LIST,
+ PSP_A_DEV_NSID,
+ PSP_A_DEV_BY_ASSOCIATION,
__PSP_A_DEV_MAX,
PSP_A_DEV_MAX = (__PSP_A_DEV_MAX - 1)
@@ -74,6 +85,8 @@ enum {
PSP_CMD_RX_ASSOC,
PSP_CMD_TX_ASSOC,
PSP_CMD_GET_STATS,
+ PSP_CMD_DEV_ASSOC,
+ PSP_CMD_DEV_DISASSOC,
__PSP_CMD_MAX,
PSP_CMD_MAX = (__PSP_CMD_MAX - 1)
diff --git a/net/psp/psp-nl-gen.c b/net/psp/psp-nl-gen.c
index 1f5e73e7ccc1..114299c64423 100644
--- a/net/psp/psp-nl-gen.c
+++ b/net/psp/psp-nl-gen.c
@@ -53,6 +53,20 @@ static const struct nla_policy psp_get_stats_nl_policy[PSP_A_STATS_DEV_ID + 1] =
[PSP_A_STATS_DEV_ID] = NLA_POLICY_MIN(NLA_U32, 1),
};
+/* PSP_CMD_DEV_ASSOC - do */
+static const struct nla_policy psp_dev_assoc_nl_policy[PSP_A_DEV_NSID + 1] = {
+ [PSP_A_DEV_ID] = NLA_POLICY_MIN(NLA_U32, 1),
+ [PSP_A_DEV_IFINDEX] = { .type = NLA_U32, },
+ [PSP_A_DEV_NSID] = { .type = NLA_S32, },
+};
+
+/* PSP_CMD_DEV_DISASSOC - do */
+static const struct nla_policy psp_dev_disassoc_nl_policy[PSP_A_DEV_NSID + 1] = {
+ [PSP_A_DEV_ID] = NLA_POLICY_MIN(NLA_U32, 1),
+ [PSP_A_DEV_IFINDEX] = { .type = NLA_U32, },
+ [PSP_A_DEV_NSID] = { .type = NLA_S32, },
+};
+
/* Ops table for psp */
static const struct genl_split_ops psp_nl_ops[] = {
{
@@ -119,6 +133,24 @@ static const struct genl_split_ops psp_nl_ops[] = {
.dumpit = psp_nl_get_stats_dumpit,
.flags = GENL_CMD_CAP_DUMP,
},
+ {
+ .cmd = PSP_CMD_DEV_ASSOC,
+ .pre_doit = psp_device_get_locked,
+ .doit = psp_nl_dev_assoc_doit,
+ .post_doit = psp_device_unlock,
+ .policy = psp_dev_assoc_nl_policy,
+ .maxattr = PSP_A_DEV_NSID,
+ .flags = GENL_CMD_CAP_DO,
+ },
+ {
+ .cmd = PSP_CMD_DEV_DISASSOC,
+ .pre_doit = psp_device_get_locked,
+ .doit = psp_nl_dev_disassoc_doit,
+ .post_doit = psp_device_unlock,
+ .policy = psp_dev_disassoc_nl_policy,
+ .maxattr = PSP_A_DEV_NSID,
+ .flags = GENL_CMD_CAP_DO,
+ },
};
static const struct genl_multicast_group psp_nl_mcgrps[] = {
diff --git a/net/psp/psp-nl-gen.h b/net/psp/psp-nl-gen.h
index 977355455395..4dd0f0f23053 100644
--- a/net/psp/psp-nl-gen.h
+++ b/net/psp/psp-nl-gen.h
@@ -33,6 +33,8 @@ int psp_nl_rx_assoc_doit(struct sk_buff *skb, struct genl_info *info);
int psp_nl_tx_assoc_doit(struct sk_buff *skb, struct genl_info *info);
int psp_nl_get_stats_doit(struct sk_buff *skb, struct genl_info *info);
int psp_nl_get_stats_dumpit(struct sk_buff *skb, struct netlink_callback *cb);
+int psp_nl_dev_assoc_doit(struct sk_buff *skb, struct genl_info *info);
+int psp_nl_dev_disassoc_doit(struct sk_buff *skb, struct genl_info *info);
enum {
PSP_NLGRP_MGMT,
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index 82de78a1d6bd..178b848989f1 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -37,8 +37,18 @@ struct mutex psp_devs_lock;
*/
int psp_dev_check_access(struct psp_dev *psd, struct net *net, bool admin)
{
+ struct psp_assoc_dev *entry;
+
if (dev_net(psd->main_netdev) == net)
return 0;
+
+ if (!admin) {
+ list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
+ if (dev_net(entry->assoc_dev) == net)
+ return 0;
+ }
+ }
+
return -ENOENT;
}
@@ -74,6 +84,7 @@ psp_dev_create(struct net_device *netdev,
return ERR_PTR(-ENOMEM);
psd->main_netdev = netdev;
+ INIT_LIST_HEAD(&psd->assoc_dev_list);
psd->ops = psd_ops;
psd->caps = psd_caps;
psd->drv_priv = priv_ptr;
@@ -121,6 +132,7 @@ void psp_dev_free(struct psp_dev *psd)
*/
void psp_dev_unregister(struct psp_dev *psd)
{
+ struct psp_assoc_dev *entry, *entry_tmp;
struct psp_assoc *pas, *next;
mutex_lock(&psp_devs_lock);
@@ -140,6 +152,14 @@ void psp_dev_unregister(struct psp_dev *psd)
list_for_each_entry_safe(pas, next, &psd->stale_assocs, assocs_list)
psp_dev_tx_key_del(psd, pas);
+ list_for_each_entry_safe(entry, entry_tmp, &psd->assoc_dev_list,
+ dev_list) {
+ list_del(&entry->dev_list);
+ rcu_assign_pointer(entry->assoc_dev->psp_dev, NULL);
+ netdev_put(entry->assoc_dev, &entry->dev_tracker);
+ kfree(entry);
+ }
+
rcu_assign_pointer(psd->main_netdev->psp_dev, NULL);
psd->ops = NULL;
@@ -361,5 +381,4 @@ static int __init psp_init(void)
return genl_register_family(&psp_nl_family);
}
-
subsys_initcall(psp_init);
diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
index eb47a9ee4438..1e4cbc9d87af 100644
--- a/net/psp/psp_nl.c
+++ b/net/psp/psp_nl.c
@@ -2,6 +2,7 @@
#include <linux/ethtool.h>
#include <linux/skbuff.h>
+#include <linux/net_namespace.h>
#include <linux/xarray.h>
#include <net/genetlink.h>
#include <net/psp.h>
@@ -38,6 +39,73 @@ static int psp_nl_reply_send(struct sk_buff *rsp, struct genl_info *info)
return genlmsg_reply(rsp, info);
}
+/**
+ * psp_nl_multicast_per_ns() - multicast a notification to each unique netns
+ * @psd: PSP device (must be locked)
+ * @group: multicast group
+ * @build_ntf: callback to build an skb for a given netns, or NULL on failure
+ * @ctx: opaque context passed to @build_ntf
+ *
+ * Iterates all unique network namespaces from the associated device list
+ * plus the main device's netns. For each unique netns, calls @build_ntf
+ * to construct a notification skb and multicasts it.
+ */
+static void psp_nl_multicast_per_ns(struct psp_dev *psd, unsigned int group,
+ struct sk_buff *(*build_ntf)(struct psp_dev *,
+ struct net *,
+ void *),
+ void *ctx)
+{
+ struct psp_assoc_dev *entry;
+ struct xarray sent_nets;
+ struct net *main_net;
+ struct sk_buff *ntf;
+
+ main_net = dev_net(psd->main_netdev);
+ xa_init(&sent_nets);
+
+ list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
+ struct net *assoc_net = dev_net(entry->assoc_dev);
+ int ret;
+
+ if (net_eq(assoc_net, main_net))
+ continue;
+
+ ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
+ GFP_KERNEL);
+ if (ret == -EBUSY)
+ continue;
+
+ ntf = build_ntf(psd, assoc_net, ctx);
+ if (!ntf)
+ continue;
+
+ genlmsg_multicast_netns(&psp_nl_family, assoc_net, ntf, 0,
+ group, GFP_KERNEL);
+ }
+ xa_destroy(&sent_nets);
+
+ /* Send to main device netns */
+ ntf = build_ntf(psd, main_net, ctx);
+ if (!ntf)
+ return;
+ genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
+ GFP_KERNEL);
+}
+
+static struct sk_buff *psp_nl_clone_ntf(struct psp_dev *psd, struct net *net,
+ void *ctx)
+{
+ return skb_clone(ctx, GFP_KERNEL);
+}
+
+static void psp_nl_multicast_all_ns(struct psp_dev *psd, struct sk_buff *ntf,
+ unsigned int group)
+{
+ psp_nl_multicast_per_ns(psd, group, psp_nl_clone_ntf, ntf);
+ nlmsg_free(ntf);
+}
+
/* Device stuff */
static struct psp_dev *
@@ -103,11 +171,74 @@ psp_device_unlock(const struct genl_split_ops *ops, struct sk_buff *skb,
sockfd_put(socket);
}
+static bool psp_has_assoc_dev_in_ns(struct psp_dev *psd, struct net *net)
+{
+ struct psp_assoc_dev *entry;
+
+ list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
+ if (dev_net(entry->assoc_dev) == net)
+ return true;
+ }
+
+ return false;
+}
+
+static int psp_nl_fill_assoc_dev_list(struct psp_dev *psd, struct sk_buff *rsp,
+ struct net *cur_net,
+ struct net *filter_net)
+{
+ struct psp_assoc_dev *entry;
+ struct net *dev_net_ns;
+ struct nlattr *nest;
+ int nsid;
+
+ list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
+ dev_net_ns = dev_net(entry->assoc_dev);
+
+ if (filter_net && dev_net_ns != filter_net)
+ continue;
+
+ /* When filtering by namespace, all devices are in the caller's
+ * namespace so nsid is always NETNSA_NSID_NOT_ASSIGNED (-1).
+ * Otherwise, calculate the nsid relative to cur_net.
+ */
+ nsid = filter_net ? NETNSA_NSID_NOT_ASSIGNED :
+ peernet2id_alloc(cur_net, dev_net_ns,
+ GFP_KERNEL);
+
+ nest = nla_nest_start(rsp, PSP_A_DEV_ASSOC_LIST);
+ if (!nest)
+ return -1;
+
+ if (nla_put_u32(rsp, PSP_A_ASSOC_DEV_INFO_IFINDEX,
+ entry->assoc_dev->ifindex) ||
+ nla_put_s32(rsp, PSP_A_ASSOC_DEV_INFO_NSID, nsid)) {
+ nla_nest_cancel(rsp, nest);
+ return -1;
+ }
+
+ nla_nest_end(rsp, nest);
+ }
+
+ return 0;
+}
+
static int
psp_nl_dev_fill(struct psp_dev *psd, struct sk_buff *rsp,
const struct genl_info *info)
{
+ struct net *cur_net;
void *hdr;
+ int err;
+
+ cur_net = genl_info_net(info);
+
+ /* Skip this device if we're in an associated netns but have no
+ * associated devices in cur_net
+ */
+ if (cur_net != dev_net(psd->main_netdev) &&
+ !psp_has_assoc_dev_in_ns(psd, cur_net))
+ return 0;
hdr = genlmsg_iput(rsp, info);
if (!hdr)
@@ -119,6 +250,22 @@ psp_nl_dev_fill(struct psp_dev *psd, struct sk_buff *rsp,
nla_put_u32(rsp, PSP_A_DEV_PSP_VERSIONS_ENA, psd->config.versions))
goto err_cancel_msg;
+ if (cur_net == dev_net(psd->main_netdev)) {
+ /* Primary device - dump assoc list */
+ err = psp_nl_fill_assoc_dev_list(psd, rsp, cur_net, NULL);
+ if (err)
+ goto err_cancel_msg;
+ } else {
+ /* In netns: set by-association flag and dump filtered
+ * assoc list containing only devices in cur_net
+ */
+ if (nla_put_flag(rsp, PSP_A_DEV_BY_ASSOCIATION))
+ goto err_cancel_msg;
+ err = psp_nl_fill_assoc_dev_list(psd, rsp, cur_net, cur_net);
+ if (err)
+ goto err_cancel_msg;
+ }
+
genlmsg_end(rsp, hdr);
return 0;
@@ -127,27 +274,34 @@ psp_nl_dev_fill(struct psp_dev *psd, struct sk_buff *rsp,
return -EMSGSIZE;
}
-void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd)
+static struct sk_buff *psp_nl_build_dev_ntf(struct psp_dev *psd,
+ struct net *net, void *ctx)
{
+ u32 cmd = *(u32 *)ctx;
struct genl_info info;
struct sk_buff *ntf;
- if (!genl_has_listeners(&psp_nl_family, dev_net(psd->main_netdev),
- PSP_NLGRP_MGMT))
- return;
+ if (!genl_has_listeners(&psp_nl_family, net, PSP_NLGRP_MGMT))
+ return NULL;
ntf = genlmsg_new(GENLMSG_DEFAULT_SIZE, GFP_KERNEL);
if (!ntf)
- return;
+ return NULL;
genl_info_init_ntf(&info, &psp_nl_family, cmd);
+ genl_info_net_set(&info, net);
if (psp_nl_dev_fill(psd, ntf, &info)) {
nlmsg_free(ntf);
- return;
+ return NULL;
}
- genlmsg_multicast_netns(&psp_nl_family, dev_net(psd->main_netdev), ntf,
- 0, PSP_NLGRP_MGMT, GFP_KERNEL);
+ return ntf;
+}
+
+void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd)
+{
+ psp_nl_multicast_per_ns(psd, PSP_NLGRP_MGMT,
+ psp_nl_build_dev_ntf, &cmd);
}
int psp_nl_dev_get_doit(struct sk_buff *req, struct genl_info *info)
@@ -281,8 +435,9 @@ int psp_nl_key_rotate_doit(struct sk_buff *skb, struct genl_info *info)
psd->stats.rotations++;
nlmsg_end(ntf, (struct nlmsghdr *)ntf->data);
- genlmsg_multicast_netns(&psp_nl_family, dev_net(psd->main_netdev), ntf,
- 0, PSP_NLGRP_USE, GFP_KERNEL);
+
+ psp_nl_multicast_all_ns(psd, ntf, PSP_NLGRP_USE);
+
return psp_nl_reply_send(rsp, info);
err_free_ntf:
@@ -292,6 +447,140 @@ int psp_nl_key_rotate_doit(struct sk_buff *skb, struct genl_info *info)
return err;
}
+int psp_nl_dev_assoc_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct psp_dev *psd = info->user_ptr[0];
+ struct psp_assoc_dev *psp_assoc_dev;
+ struct net_device *assoc_dev;
+ u32 assoc_ifindex;
+ struct sk_buff *rsp;
+ struct net *net;
+ int nsid;
+
+ if (GENL_REQ_ATTR_CHECK(info, PSP_A_DEV_IFINDEX))
+ return -EINVAL;
+
+ if (info->attrs[PSP_A_DEV_NSID]) {
+ nsid = nla_get_s32(info->attrs[PSP_A_DEV_NSID]);
+
+ net = get_net_ns_by_id(genl_info_net(info), nsid);
+ if (!net) {
+ NL_SET_BAD_ATTR(info->extack,
+ info->attrs[PSP_A_DEV_NSID]);
+ return -EINVAL;
+ }
+ } else {
+ net = get_net(genl_info_net(info));
+ }
+
+ psp_assoc_dev = kzalloc(sizeof(*psp_assoc_dev), GFP_KERNEL);
+ if (!psp_assoc_dev) {
+ put_net(net);
+ return -ENOMEM;
+ }
+
+ assoc_ifindex = nla_get_u32(info->attrs[PSP_A_DEV_IFINDEX]);
+ assoc_dev = netdev_get_by_index(net, assoc_ifindex,
+ &psp_assoc_dev->dev_tracker,
+ GFP_KERNEL);
+ if (!assoc_dev) {
+ put_net(net);
+ kfree(psp_assoc_dev);
+ NL_SET_BAD_ATTR(info->extack, info->attrs[PSP_A_DEV_IFINDEX]);
+ return -ENODEV;
+ }
+
+ /* Check if device is already associated with a PSP device */
+ if (cmpxchg(&assoc_dev->psp_dev, NULL, psd)) {
+ NL_SET_ERR_MSG(info->extack,
+ "Device already associated with a PSP device");
+ netdev_put(assoc_dev, &psp_assoc_dev->dev_tracker);
+ put_net(net);
+ kfree(psp_assoc_dev);
+ return -EBUSY;
+ }
+
+ psp_assoc_dev->assoc_dev = assoc_dev;
+ rsp = psp_nl_reply_new(info);
+ if (!rsp) {
+ rcu_assign_pointer(assoc_dev->psp_dev, NULL);
+ netdev_put(assoc_dev, &psp_assoc_dev->dev_tracker);
+ put_net(net);
+ kfree(psp_assoc_dev);
+ return -ENOMEM;
+ }
+
+ list_add_tail(&psp_assoc_dev->dev_list, &psd->assoc_dev_list);
+
+ put_net(net);
+
+ psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
+
+ return psp_nl_reply_send(rsp, info);
+}
+
+int psp_nl_dev_disassoc_doit(struct sk_buff *skb, struct genl_info *info)
+{
+ struct psp_assoc_dev *entry, *found = NULL;
+ struct psp_dev *psd = info->user_ptr[0];
+ u32 assoc_ifindex;
+ struct sk_buff *rsp;
+ struct net *net;
+ int nsid;
+
+ if (GENL_REQ_ATTR_CHECK(info, PSP_A_DEV_IFINDEX))
+ return -EINVAL;
+
+ if (info->attrs[PSP_A_DEV_NSID]) {
+ nsid = nla_get_s32(info->attrs[PSP_A_DEV_NSID]);
+
+ net = get_net_ns_by_id(genl_info_net(info), nsid);
+ if (!net) {
+ NL_SET_BAD_ATTR(info->extack,
+ info->attrs[PSP_A_DEV_NSID]);
+ return -EINVAL;
+ }
+ } else {
+ net = get_net(genl_info_net(info));
+ }
+
+ assoc_ifindex = nla_get_u32(info->attrs[PSP_A_DEV_IFINDEX]);
+
+ /* Search the association list by ifindex and netns */
+ list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
+ if (entry->assoc_dev->ifindex == assoc_ifindex &&
+ dev_net(entry->assoc_dev) == net) {
+ found = entry;
+ break;
+ }
+ }
+
+ if (!found) {
+ put_net(net);
+ NL_SET_BAD_ATTR(info->extack, info->attrs[PSP_A_DEV_IFINDEX]);
+ return -ENODEV;
+ }
+
+ rsp = psp_nl_reply_new(info);
+ if (!rsp) {
+ put_net(net);
+ return -ENOMEM;
+ }
+
+ /* Notify before removal */
+ psp_nl_notify_dev(psd, PSP_CMD_DEV_CHANGE_NTF);
+
+ /* Remove from the association list */
+ list_del(&found->dev_list);
+ rcu_assign_pointer(found->assoc_dev->psp_dev, NULL);
+ netdev_put(found->assoc_dev, &found->dev_tracker);
+ kfree(found);
+
+ put_net(net);
+
+ return psp_nl_reply_send(rsp, info);
+}
+
/* Key etc. */
int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
@@ -320,7 +609,9 @@ int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
psd = psp_dev_get_for_sock(socket->sk);
if (psd) {
+ mutex_lock(&psd->lock);
err = psp_dev_check_access(psd, genl_info_net(info), false);
+ mutex_unlock(&psd->lock);
if (err) {
psp_dev_put(psd);
psd = NULL;
--
2.52.0
^ permalink raw reply related
* [PATCH v8 net-next 1/5] psp: add admin/non-admin version of psp_device_get_locked
From: Wei Wang @ 2026-03-29 4:19 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Daniel Zahka, Willem de Bruijn, David Wei,
Andrew Lunn, David S . Miller, Eric Dumazet, Simon Horman
Cc: Wei Wang
In-Reply-To: <20260329041922.3850747-1-weibunny.kernel@gmail.com>
From: Wei Wang <weibunny@fb.com>
Introduce 2 versions of psp_device_get_locked:
1. psp_device_get_locked_admin(): This version is used for operations
that would change the status of the psd, and are currently used for
dev-set nad key-rotation.
2. psp_device_get_locked(): This is the non-admin version, which are
used for broader user issued operations including: dev-get, rx-assoc,
tx-assoc, get-stats.
Following commit will be implementing both of the checks.
Signed-off-by: Wei Wang <weibunny@fb.com>
---
Documentation/netlink/specs/psp.yaml | 4 ++--
net/psp/psp-nl-gen.c | 4 ++--
net/psp/psp-nl-gen.h | 2 ++
net/psp/psp.h | 2 +-
net/psp/psp_main.c | 7 +++++-
net/psp/psp_nl.c | 33 ++++++++++++++++++++--------
6 files changed, 37 insertions(+), 15 deletions(-)
diff --git a/Documentation/netlink/specs/psp.yaml b/Documentation/netlink/specs/psp.yaml
index f3a57782d2cf..fe2cdc966604 100644
--- a/Documentation/netlink/specs/psp.yaml
+++ b/Documentation/netlink/specs/psp.yaml
@@ -195,7 +195,7 @@ operations:
- psp-versions-ena
reply:
attributes: []
- pre: psp-device-get-locked
+ pre: psp-device-get-locked-admin
post: psp-device-unlock
-
name: dev-change-ntf
@@ -214,7 +214,7 @@ operations:
reply:
attributes:
- id
- pre: psp-device-get-locked
+ pre: psp-device-get-locked-admin
post: psp-device-unlock
-
name: key-rotate-ntf
diff --git a/net/psp/psp-nl-gen.c b/net/psp/psp-nl-gen.c
index 22a48d0fa378..1f5e73e7ccc1 100644
--- a/net/psp/psp-nl-gen.c
+++ b/net/psp/psp-nl-gen.c
@@ -71,7 +71,7 @@ static const struct genl_split_ops psp_nl_ops[] = {
},
{
.cmd = PSP_CMD_DEV_SET,
- .pre_doit = psp_device_get_locked,
+ .pre_doit = psp_device_get_locked_admin,
.doit = psp_nl_dev_set_doit,
.post_doit = psp_device_unlock,
.policy = psp_dev_set_nl_policy,
@@ -80,7 +80,7 @@ static const struct genl_split_ops psp_nl_ops[] = {
},
{
.cmd = PSP_CMD_KEY_ROTATE,
- .pre_doit = psp_device_get_locked,
+ .pre_doit = psp_device_get_locked_admin,
.doit = psp_nl_key_rotate_doit,
.post_doit = psp_device_unlock,
.policy = psp_key_rotate_nl_policy,
diff --git a/net/psp/psp-nl-gen.h b/net/psp/psp-nl-gen.h
index 599c5f1c82f2..977355455395 100644
--- a/net/psp/psp-nl-gen.h
+++ b/net/psp/psp-nl-gen.h
@@ -17,6 +17,8 @@ extern const struct nla_policy psp_keys_nl_policy[PSP_A_KEYS_SPI + 1];
int psp_device_get_locked(const struct genl_split_ops *ops,
struct sk_buff *skb, struct genl_info *info);
+int psp_device_get_locked_admin(const struct genl_split_ops *ops,
+ struct sk_buff *skb, struct genl_info *info);
int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
struct sk_buff *skb, struct genl_info *info);
void
diff --git a/net/psp/psp.h b/net/psp/psp.h
index 9f19137593a0..0f9c4e4e52cb 100644
--- a/net/psp/psp.h
+++ b/net/psp/psp.h
@@ -14,7 +14,7 @@ extern struct xarray psp_devs;
extern struct mutex psp_devs_lock;
void psp_dev_free(struct psp_dev *psd);
-int psp_dev_check_access(struct psp_dev *psd, struct net *net);
+int psp_dev_check_access(struct psp_dev *psd, struct net *net, bool admin);
void psp_nl_notify_dev(struct psp_dev *psd, u32 cmd);
diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
index 9508b6c38003..82de78a1d6bd 100644
--- a/net/psp/psp_main.c
+++ b/net/psp/psp_main.c
@@ -27,10 +27,15 @@ struct mutex psp_devs_lock;
* psp_dev_check_access() - check if user in a given net ns can access PSP dev
* @psd: PSP device structure user is trying to access
* @net: net namespace user is in
+ * @admin: If true, only allow access from @psd's main device's netns,
+ * for admin operations like config changes and key rotation.
+ * If false, also allow access from network namespaces that have
+ * an associated device with @psd, for read-only and association
+ * management operations.
*
* Return: 0 if PSP device should be visible in @net, errno otherwise.
*/
-int psp_dev_check_access(struct psp_dev *psd, struct net *net)
+int psp_dev_check_access(struct psp_dev *psd, struct net *net, bool admin)
{
if (dev_net(psd->main_netdev) == net)
return 0;
diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
index 6afd7707ec12..eb47a9ee4438 100644
--- a/net/psp/psp_nl.c
+++ b/net/psp/psp_nl.c
@@ -41,7 +41,8 @@ static int psp_nl_reply_send(struct sk_buff *rsp, struct genl_info *info)
/* Device stuff */
static struct psp_dev *
-psp_device_get_and_lock(struct net *net, struct nlattr *dev_id)
+psp_device_get_and_lock(struct net *net, struct nlattr *dev_id,
+ bool admin)
{
struct psp_dev *psd;
int err;
@@ -56,7 +57,7 @@ psp_device_get_and_lock(struct net *net, struct nlattr *dev_id)
mutex_lock(&psd->lock);
mutex_unlock(&psp_devs_lock);
- err = psp_dev_check_access(psd, net);
+ err = psp_dev_check_access(psd, net, admin);
if (err) {
mutex_unlock(&psd->lock);
return ERR_PTR(err);
@@ -65,17 +66,31 @@ psp_device_get_and_lock(struct net *net, struct nlattr *dev_id)
return psd;
}
-int psp_device_get_locked(const struct genl_split_ops *ops,
- struct sk_buff *skb, struct genl_info *info)
+static int __psp_device_get_locked(const struct genl_split_ops *ops,
+ struct sk_buff *skb, struct genl_info *info,
+ bool admin)
{
if (GENL_REQ_ATTR_CHECK(info, PSP_A_DEV_ID))
return -EINVAL;
info->user_ptr[0] = psp_device_get_and_lock(genl_info_net(info),
- info->attrs[PSP_A_DEV_ID]);
+ info->attrs[PSP_A_DEV_ID],
+ admin);
return PTR_ERR_OR_ZERO(info->user_ptr[0]);
}
+int psp_device_get_locked_admin(const struct genl_split_ops *ops,
+ struct sk_buff *skb, struct genl_info *info)
+{
+ return __psp_device_get_locked(ops, skb, info, true);
+}
+
+int psp_device_get_locked(const struct genl_split_ops *ops,
+ struct sk_buff *skb, struct genl_info *info)
+{
+ return __psp_device_get_locked(ops, skb, info, false);
+}
+
void
psp_device_unlock(const struct genl_split_ops *ops, struct sk_buff *skb,
struct genl_info *info)
@@ -160,7 +175,7 @@ static int
psp_nl_dev_get_dumpit_one(struct sk_buff *rsp, struct netlink_callback *cb,
struct psp_dev *psd)
{
- if (psp_dev_check_access(psd, sock_net(rsp->sk)))
+ if (psp_dev_check_access(psd, sock_net(rsp->sk), false))
return 0;
return psp_nl_dev_fill(psd, rsp, genl_info_dump(cb));
@@ -305,7 +320,7 @@ int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
psd = psp_dev_get_for_sock(socket->sk);
if (psd) {
- err = psp_dev_check_access(psd, genl_info_net(info));
+ err = psp_dev_check_access(psd, genl_info_net(info), false);
if (err) {
psp_dev_put(psd);
psd = NULL;
@@ -330,7 +345,7 @@ int psp_assoc_device_get_locked(const struct genl_split_ops *ops,
psp_dev_put(psd);
} else {
- psd = psp_device_get_and_lock(genl_info_net(info), id);
+ psd = psp_device_get_and_lock(genl_info_net(info), id, false);
if (IS_ERR(psd)) {
err = PTR_ERR(psd);
goto err_sock_put;
@@ -573,7 +588,7 @@ static int
psp_nl_stats_get_dumpit_one(struct sk_buff *rsp, struct netlink_callback *cb,
struct psp_dev *psd)
{
- if (psp_dev_check_access(psd, sock_net(rsp->sk)))
+ if (psp_dev_check_access(psd, sock_net(rsp->sk), false))
return 0;
return psp_nl_stats_fill(psd, rsp, genl_info_dump(cb));
--
2.52.0
^ permalink raw reply related
* [PATCH v8 net-next 0/5] psp: Add support for dev-assoc/disassoc
From: Wei Wang @ 2026-03-29 4:19 UTC (permalink / raw)
To: netdev, Jakub Kicinski, Daniel Zahka, Willem de Bruijn, David Wei,
Andrew Lunn, David S . Miller, Eric Dumazet, Simon Horman
Cc: Wei Wang
From: Wei Wang <weibunny@fb.com>
The main purpose of this feature is to associate virtual devices like
veth or netkit with a real PSP device, so we could provide PSP
functionality to the application running with virtual devices.
A typical deployment that works with this feature is as follows:
Host Namespace:
psp_dev_local ←──physically linked──→ psp_dev_peer
(PSP device)
│
│ BPF on psp_dev_local ingress: bpf_redirect_peer() to nk_guest
│
nk_host / veth_host
│
│ BPF on nk_host ingress: bpf_redirect_neigh() to psp_dev_local
│
Guest Namespace (netns):
│
nk_guest / veth_guest
★ PSP application run here
Remote Namespace (_netns):
psp_dev_peer
★ PSP server application runs here
Note:
The general requirement for this feature to work:
For PSP to work correctly, the egress device at validate_xmit_skb()
time must have psp_dev matching the association's psd. Any device
stacking or traffic redirection that changes the egress device will
cause either:
1. TX validation failure (SKB_DROP_REASON_PSP_OUTPUT) - fail-safe
2. RX policy failure after tx-assoc - packets without PSP extension
are rejected by receiver expecting encrypted traffic
Here are a few examples that this feature would not work:
- Bonding with load balancing in round-robin, XOR, 802.3ad mode across
multiple PSP devices, or mixed PSP and non-PSP devices
- Bonding with active-backup mode might work without PSP migration for
failover case.
- ipvlan/macvlan in bridge mode would not work given packets are
loopbacked locally without going through the PSP device.
Changes since v7:
- Refactor in patch 1 to have a common helper for
psp_device_get_locked_admin() and psp_device_get_locked()
- Take psd->lock in psp_assoc_device_get_locked() before
psp_dev_check_access() in patch 2
- Use cmpxchg() for assoc_dev->psp_dev assignment when doing dev-assoc
in patch 2
- Check for err for register_netdevice_notifier() in patch 3
- Call psp_attach_netdev_notifier() in pre_doit handler for dev-assoc to
avoid releasing of psd->lock in patch 3
Changes since v6:
- Remove the unused remote_addr, nk_guest_addr and import cmd in patch 5
Changes since v5:
- Remove module_exit() in patch 3
Changes since v4:
- Address compilation warning in patch 3
- Removed the call to psp_nl_has_listeners_any_ns() and check listeners
when looping through netns in psp_nl_notify_dev() in patch 2. This
makes sure we only send notification to netns that has listeners.
Changes since v3:
- Make nsid optional for dev-assoc/dev-disassoc operation, and use
the ns user is in when it's not specified. Also added a test for this.
- Fix psp_nl_notify_dev() to compute the correct nsid relative to the
listener's netns.
- Only register the new netdev event for psp dev cleanup upon the first
successful dev-assoc operation.
- Change the following in selftest:
- Add CONFIG_NETKIT to driver/net's config
- Fall back to NetDrvEpEnv and run basic test cases if NetDrvContEnv
does not load
- Use ksft_variants instead of psp_ip_ver_test_builder
Changes since v2:
- Change the newly added parameter to psp_device_get_and_lock() to
admin in patch 1. Introduce 2 device check functions:
- psp_device_get_locked_admin() for dev-set and key-rotate
- psp_device_get_locked() for all other operations
Flip the logic for checking the dev_assoc_list accordingly in patch 2.
- Move psp_nl_notify_dev() before removing the dev from assoc_dev_list
in psp_nl_dev_disassoc_doit() and correct the typo in commit msg in
patch 2.
- Remove the threading and subprocess and some comment updates in patch 5.
Changes since v1:
- Update the first 4 patches to reflect the latest changes in
https://lore.kernel.org/netdev/20260302053315.1919859-1-dw@davidwei.uk/
- Update patch 9 to add a param to NetDrvContEnv to control the loading
of the tx forwarding bpf program
Wei Wang (5):
psp: add admin/non-admin version of psp_device_get_locked
psp: add new netlink cmd for dev-assoc and dev-disassoc
psp: add a new netdev event for dev unregister
selftests/net: Add bpf skb forwarding program
selftest/net: psp: Add test for dev-assoc/disassoc
Documentation/netlink/specs/psp.yaml | 71 ++-
include/net/psp/types.h | 15 +
include/uapi/linux/psp.h | 13 +
net/psp/psp-nl-gen.c | 36 +-
net/psp/psp-nl-gen.h | 7 +
net/psp/psp.h | 3 +-
net/psp/psp_main.c | 96 +++-
net/psp/psp_nl.c | 352 ++++++++++++-
tools/testing/selftests/drivers/net/config | 1 +
.../drivers/net/hw/nk_redirect.bpf.c | 60 +++
.../selftests/drivers/net/lib/py/env.py | 54 +-
tools/testing/selftests/drivers/net/psp.py | 492 ++++++++++++++++--
12 files changed, 1140 insertions(+), 60 deletions(-)
create mode 100644 tools/testing/selftests/drivers/net/hw/nk_redirect.bpf.c
--
2.52.0
^ permalink raw reply
* Re: [PATCH v1 net-next] tun: Ignore tun in netdev_lock_pos().
From: kernel test robot @ 2026-03-29 4:06 UTC (permalink / raw)
To: Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Willem de Bruijn, Jason Wang, Andrew Lunn
Cc: oe-kbuild-all, netdev, Simon Horman, Kuniyuki Iwashima
In-Reply-To: <20260327213441.3781433-1-kuniyu@google.com>
Hi Kuniyuki,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/tun-Ignore-tun-in-netdev_lock_pos/20260328-220059
base: net-next/main
patch link: https://lore.kernel.org/r/20260327213441.3781433-1-kuniyu%40google.com
patch subject: [PATCH v1 net-next] tun: Ignore tun in netdev_lock_pos().
config: s390-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260329/202603290603.7d9NDw4e-lkp@intel.com/config)
compiler: s390x-linux-gnu-gcc (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603290603.7d9NDw4e-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603290603.7d9NDw4e-lkp@intel.com/
All errors (new ones prefixed by >>):
s390x-linux-gnu-ld: net/core/dev.o: in function `netdev_lock_pos':
net/core/dev.c:540:(.text+0x86a6): undefined reference to `tun_link_ops'
>> s390x-linux-gnu-ld: net/core/dev.c:540:(.text+0x2b5ac): undefined reference to `tun_link_ops'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v1 net-next] tun: Ignore tun in netdev_lock_pos().
From: kernel test robot @ 2026-03-29 3:36 UTC (permalink / raw)
To: Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Willem de Bruijn, Jason Wang, Andrew Lunn
Cc: oe-kbuild-all, netdev, Simon Horman, Kuniyuki Iwashima
In-Reply-To: <20260327213441.3781433-1-kuniyu@google.com>
Hi Kuniyuki,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/tun-Ignore-tun-in-netdev_lock_pos/20260328-220059
base: net-next/main
patch link: https://lore.kernel.org/r/20260327213441.3781433-1-kuniyu%40google.com
patch subject: [PATCH v1 net-next] tun: Ignore tun in netdev_lock_pos().
config: arm64-allnoconfig-bpf (https://download.01.org/0day-ci/archive/20260329/202603290553.sJzkZoGo-lkp@intel.com/config)
compiler: aarch64-linux-gnu-gcc (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260329/202603290553.sJzkZoGo-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202603290553.sJzkZoGo-lkp@intel.com/
All errors (new ones prefixed by >>):
aarch64-linux-gnu-ld: net/core/dev.o: in function `netdev_lock_pos':
net/core/dev.c:540:(.text+0x8088): undefined reference to `tun_link_ops'
aarch64-linux-gnu-ld: net/core/dev.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `tun_link_ops' which may bind externally can not be used when making a shared object; recompile with -fPIC
>> net/core/dev.c:540:(.text+0x8088): dangerous relocation: unsupported relocation
>> aarch64-linux-gnu-ld: net/core/dev.c:540:(.text+0x808c): undefined reference to `tun_link_ops'
aarch64-linux-gnu-ld: net/core/dev.c:540:(.text+0x29d70): undefined reference to `tun_link_ops'
aarch64-linux-gnu-ld: net/core/dev.o: relocation R_AARCH64_ADR_PREL_PG_HI21 against symbol `tun_link_ops' which may bind externally can not be used when making a shared object; recompile with -fPIC
net/core/dev.c:540:(.text+0x29d70): dangerous relocation: unsupported relocation
aarch64-linux-gnu-ld: net/core/dev.c:540:(.text+0x29d74): undefined reference to `tun_link_ops'
vim +540 net/core/dev.c
529
530 static inline unsigned short netdev_lock_pos(const struct net_device *dev)
531 {
532 unsigned short dev_type = dev->type;
533 int i;
534
535 for (i = 0; i < ARRAY_SIZE(netdev_lock_type); i++)
536 if (netdev_lock_type[i] == dev_type)
537 return i;
538
539 /* the last key is used by default */
> 540 WARN_ONCE(!netdev_is_tun(dev),
541 "netdev_lock_pos() could not find dev_type=%u\n", dev_type);
542 return ARRAY_SIZE(netdev_lock_type) - 1;
543 }
544
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH net v7] bnxt_en: set backing store type from query type
From: Michael Chan @ 2026-03-29 3:27 UTC (permalink / raw)
To: Pengpeng Hou
Cc: pavan.chebbi, andrew+netdev, davem, edumazet, kuba, pabeni,
netdev, linux-kernel
In-Reply-To: <20260328234357.43669-1-pengpeng@iscas.ac.cn>
[-- Attachment #1: Type: text/plain, Size: 1135 bytes --]
On Sat, Mar 28, 2026 at 4:44 PM Pengpeng Hou <pengpeng@iscas.ac.cn> wrote:
>
> bnxt_hwrm_func_backing_store_qcaps_v2() stores resp->type from the
> firmware response in ctxm->type and later uses that value to index
> fixed backing-store metadata arrays such as ctx_arr[] and
> bnxt_bstore_to_trace[].
>
> ctxm->type is fixed by the current backing-store query type and matches
> the array index of ctx->ctx_arr. Set ctxm->type from the current loop
> variable instead of depending on resp->type.
>
> Also update the loop to advance type from next_valid_type in the for
> statement, which keeps the control flow simpler for non-valid and
> unchanged entries.
>
> Fixes: 6a4d0774f02d ("bnxt_en: Add support for new backing store query firmware API")
> Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
> ---
> v7:
> - rename the patch to match the new fix
> - advance type from next_valid_type in the for loop statement
Next time, please wait the full 24 hours before posting a new version. Thanks.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Tested-by: Michael Chan <michael.chan@broadcom.com>
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5469 bytes --]
^ permalink raw reply
* [PATCH] igc: fix Tx timestamp timeout caused by unlocked TIMINCA write in adj fine
From: Bob Van Valzah @ 2026-03-29 3:25 UTC (permalink / raw)
To: intel-wired-lan; +Cc: anthony.l.nguyen, netdev, julianstj, jeff
[-- Attachment #1: Type: text/plain, Size: 3897 bytes --]
Hi,
We found a race in igc_ptp_adjfine_i225() that causes "Tx timestamp
timeout" errors and eventually wedges EXTTS when a PTP grandmaster
(ptp4l with hardware timestamping) runs concurrently with PHC
frequency discipline (any GPSDO calling clock_adjtime ADJ_FREQUENCY).
Root cause: igc_ptp_adjfine_i225() writes IGC_TIMINCA without holding
any lock. Every other PTP clock operation in igc_ptp.c (adjtime,
gettime, settime) holds tmreg_lock, but adjfine does not. When the
increment rate changes while the hardware is capturing a TX timestamp,
the captured value is corrupt. The driver retries for
IGC_PTP_TX_TIMEOUT (15s), then logs the timeout and frees the skb.
Repeated occurrences eventually prevent EXTTS from delivering events.
The attached reproducer (triggers in ~17 seconds on i226):
One thread calling clock_adjtime(ADJ_FREQUENCY) at ~200k/s on the
PHC, another sending UDP packets with SO_TIMESTAMPING requesting
hardware TX timestamps at ~100k/s. A Python reproducer is at:
https://github.com/bobvan/PePPAR-Fix/blob/main/tools/igc_tx_timeout_repro.py
At realistic rates (1 Hz adjfine from a GPSDO + ptp4l at 128 Hz
sync), the race triggers in ~30 minutes.
The attached patch holds ptp_tx_lock around the TIMINCA write and
skips the write if any TX timestamps are pending (tx_tstamp[i].skb
!= NULL), returning -EBUSY. This doesn't fully close the hardware
race (a new TX capture can start between the check and the write),
but at realistic rates the residual probability gives ~25 year MTBF
vs ~30 minutes without the patch.
A complete fix would likely require either disabling TX timestamping
around TIMINCA writes (via TSYNCTXCTL), or making the timeout recovery
path more robust so a single corrupt timestamp doesn't wedge the
subsystem. We'd welcome guidance from the igc maintainers on the
preferred approach.
Tested on:
- Intel i226 (TimeHAT v5 board on Raspberry Pi 5)
- Kernel 6.12.62+rpt-rpi-2712 (Raspberry Pi OS)
- Intel out-of-tree igc driver 5.4.0-7642.46
- Stock upstream igc_ptp.c (same code, same bug)
Bob
---
drivers/net/ethernet/intel/igc/igc_ptp.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index XXXXXXX..XXXXXXX 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -47,8 +47,10 @@ static int igc_ptp_adjfine_i225(struct ptp_clock_info *ptp, long scaled_ppm)
{
struct igc_adapter *igc = container_of(ptp, struct igc_adapter,
ptp_caps);
struct igc_hw *hw = &igc->hw;
+ unsigned long flags;
int neg_adj = 0;
u64 rate;
u32 inca;
+ int i;
if (scaled_ppm < 0) {
neg_adj = 1;
@@ -63,7 +65,21 @@ static int igc_ptp_adjfine_i225(struct ptp_clock_info *ptp, long scaled_ppm)
if (neg_adj)
inca |= ISGN;
- wr32(IGC_TIMINCA, inca);
+ /* Changing the clock increment rate while a TX timestamp is being
+ * captured by the hardware can corrupt the timestamp, causing the
+ * driver to report "Tx timestamp timeout" and eventually wedging
+ * the EXTTS subsystem. Serialize with pending TX timestamps:
+ * skip the rate change if any are in flight.
+ */
+ spin_lock_irqsave(&igc->ptp_tx_lock, flags);
+ for (i = 0; i < IGC_MAX_TX_TSTAMP_REGS; i++) {
+ if (igc->tx_tstamp[i].skb) {
+ spin_unlock_irqrestore(&igc->ptp_tx_lock, flags);
+ return -EBUSY;
+ }
+ }
+ wr32(IGC_TIMINCA, inca);
+ spin_unlock_irqrestore(&igc->ptp_tx_lock, flags);
return 0;
}
--
2.39.2
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: igc_tx_timeout_repro.py --]
[-- Type: text/x-python-script; x-unix-mode=0644; name="igc_tx_timeout_repro.py", Size: 6600 bytes --]
#!/usr/bin/env python3
"""Reproducer for igc driver Tx timestamp timeout bug.
The igc driver (Intel i225/i226) has a race between clock_adjtime
(ADJ_FREQUENCY) and hardware TX timestamping. When adjfine() is
called while a TX timestamp is pending, the timestamp register can
wedge, producing "Tx timestamp timeout" errors in dmesg and eventually
breaking EXTTS (PPS capture).
This reproducer runs two threads:
1. Hammers clock_adjtime(ADJ_FREQUENCY) on the PHC
2. Sends UDP packets with SO_TIMESTAMPING (hardware TX timestamps)
On a vulnerable igc driver, "Tx timestamp timeout" appears in dmesg
within seconds.
Usage:
sudo python3 tools/igc_tx_timeout_repro.py eth1 /dev/ptp0
# Watch with: dmesg -w | grep "Tx timestamp"
Requires root for SO_TIMESTAMPING and clock_adjtime on PHC.
"""
import ctypes
import ctypes.util
import os
import socket
import struct
import sys
import threading
import time
def get_phc_clockid(fd):
return (~fd << 3) | 3
def adjfine_loop(phc_fd, stop_event, stats):
"""Hammer adjfine() on the PHC."""
librt = ctypes.CDLL(ctypes.util.find_library("rt"), use_errno=True)
class Timeval(ctypes.Structure):
_fields_ = [("tv_sec", ctypes.c_long), ("tv_usec", ctypes.c_long)]
class Timex(ctypes.Structure):
_fields_ = [
("modes", ctypes.c_uint),
("offset", ctypes.c_long),
("freq", ctypes.c_long),
("maxerror", ctypes.c_long),
("esterror", ctypes.c_long),
("status", ctypes.c_int),
("constant", ctypes.c_long),
("precision", ctypes.c_long),
("tolerance", ctypes.c_long),
("time", Timeval),
("tick", ctypes.c_long),
("ppsfreq", ctypes.c_long),
("jitter", ctypes.c_long),
("shift", ctypes.c_int),
("stabil", ctypes.c_long),
("jitcnt", ctypes.c_long),
("calcnt", ctypes.c_long),
("errcnt", ctypes.c_long),
("stbcnt", ctypes.c_long),
("tai", ctypes.c_int),
]
ADJ_FREQUENCY = 0x0002
clockid = get_phc_clockid(phc_fd)
n = 0
toggle = False
while not stop_event.is_set():
tx = Timex()
tx.modes = ADJ_FREQUENCY
# Alternate between two tiny frequency offsets
tx.freq = 100 if toggle else -100 # ~0.0015 ppb
toggle = not toggle
ret = librt.clock_adjtime(ctypes.c_int32(clockid), ctypes.byref(tx))
if ret < 0:
print(f"adjfine error: {ctypes.get_errno()}")
break
n += 1
stats["adjfine_count"] = n
def tx_timestamp_loop(iface, stop_event, stats):
"""Send UDP packets requesting hardware TX timestamps."""
# SOL_SOCKET options for timestamping
SO_TIMESTAMPING = 37
SOF_TIMESTAMPING_TX_HARDWARE = (1 << 0)
SOF_TIMESTAMPING_RAW_HARDWARE = (1 << 6)
SOF_TIMESTAMPING_OPT_TSONLY = (1 << 11)
flags = (SOF_TIMESTAMPING_TX_HARDWARE |
SOF_TIMESTAMPING_RAW_HARDWARE |
SOF_TIMESTAMPING_OPT_TSONLY)
sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
sock.setsockopt(socket.SOL_SOCKET, SO_TIMESTAMPING, flags)
# Bind to the specific interface
sock.setsockopt(socket.SOL_SOCKET, socket.SO_BINDTODEVICE,
iface.encode() + b'\0')
sock.settimeout(0.01)
# Send to a harmless destination (localhost or broadcast)
dest = ("224.0.0.1", 9999) # multicast, won't route
payload = b"igc_repro" + b"\x00" * 32
n = 0
errors = 0
while not stop_event.is_set():
try:
sock.sendto(payload, dest)
n += 1
except OSError:
errors += 1
# Don't sleep — maximize collision probability
# But yield to avoid starving the adjfine thread
if n % 100 == 0:
time.sleep(0.0001)
sock.close()
stats["tx_count"] = n
stats["tx_errors"] = errors
def check_dmesg_for_timeout():
"""Check if Tx timestamp timeout appeared in dmesg."""
try:
import subprocess
result = subprocess.run(
["dmesg", "--since", "60 seconds ago"],
capture_output=True, text=True, timeout=2
)
return "Tx timestamp timeout" in result.stdout
except Exception:
return False
def main():
if len(sys.argv) < 3:
print(f"Usage: sudo {sys.argv[0]} <interface> <ptp_device>")
print(f"Example: sudo {sys.argv[0]} eth1 /dev/ptp0")
sys.exit(1)
iface = sys.argv[1]
ptp_dev = sys.argv[2]
duration = float(sys.argv[3]) if len(sys.argv) > 3 else 30.0
if os.geteuid() != 0:
print("ERROR: must run as root (sudo)")
sys.exit(1)
phc_fd = os.open(ptp_dev, os.O_RDWR)
print(f"Opened {ptp_dev} (fd={phc_fd})")
print(f"Interface: {iface}")
print(f"Duration: {duration}s")
print(f"Watch for: dmesg -w | grep 'Tx timestamp timeout'")
print()
stop = threading.Event()
stats = {}
t_adj = threading.Thread(target=adjfine_loop, args=(phc_fd, stop, stats))
t_tx = threading.Thread(target=tx_timestamp_loop, args=(iface, stop, stats))
t_adj.start()
t_tx.start()
start = time.monotonic()
triggered = False
while time.monotonic() - start < duration:
time.sleep(1)
elapsed = time.monotonic() - start
if check_dmesg_for_timeout():
print(f"\n*** Tx timestamp timeout detected after {elapsed:.1f}s ***")
triggered = True
break
print(f" {elapsed:.0f}s: running...", end="\r")
stop.set()
t_adj.join(timeout=2)
t_tx.join(timeout=2)
os.close(phc_fd)
adj_n = stats.get("adjfine_count", 0)
tx_n = stats.get("tx_count", 0)
tx_err = stats.get("tx_errors", 0)
elapsed = time.monotonic() - start
print(f"\nResults ({elapsed:.1f}s):")
print(f" adjfine calls: {adj_n} ({adj_n/elapsed:.0f}/s)")
print(f" TX packets: {tx_n} ({tx_n/elapsed:.0f}/s), {tx_err} errors")
if triggered:
print(f"\n BUG TRIGGERED: igc Tx timestamp timeout")
print(f" The igc driver's PTP timestamp register wedged due to")
print(f" concurrent adjfine() and hardware TX timestamping.")
sys.exit(1)
else:
print(f"\n No timeout detected in {duration}s")
sys.exit(0)
if __name__ == "__main__":
main()
^ permalink raw reply
* [PATCH net-next v2 3/3] selftests: net: add bridge STP mode selection test
From: Andy Roulin @ 2026-03-29 2:58 UTC (permalink / raw)
To: netdev
Cc: bridge, Nikolay Aleksandrov, Ido Schimmel, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Petr Machata,
Donald Hunter, Jonas Gorski, linux-doc, linux-kselftest,
linux-kernel, Andy Roulin
In-Reply-To: <20260329025858.330620-1-aroulin@nvidia.com>
Add a selftest for the IFLA_BR_STP_MODE bridge attribute that verifies:
1. stp_mode defaults to auto on new bridges
2. stp_mode can be toggled between user, kernel, and auto
3. Changing stp_mode while STP is active is rejected with -EBUSY
4. Re-setting the same stp_mode while STP is active succeeds
5. stp_mode user in a network namespace yields userspace STP (stp_state=2)
6. stp_mode kernel forces kernel STP (stp_state=1)
7. stp_mode auto in a netns preserves traditional fallback to kernel STP
8. stp_mode and stp_state can be set atomically in a single message
9. stp_mode persists across STP disable/enable cycles
Test 5 is the key use case: it demonstrates that userspace STP can now
be enabled in non-init network namespaces by setting stp_mode to user
before enabling STP.
Test 8 verifies the atomic usage pattern where both attributes are set
in a single netlink message, which is supported because br_changelink()
processes IFLA_BR_STP_MODE before IFLA_BR_STP_STATE.
The test gracefully skips if the installed iproute2 does not support
the stp_mode attribute.
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Andy Roulin <aroulin@nvidia.com>
---
Notes:
v2:
* Fix shellcheck CI: add SC2329 suppression.
* Add idempotent stp_mode test.
tools/testing/selftests/net/Makefile | 1 +
.../testing/selftests/net/bridge_stp_mode.sh | 281 ++++++++++++++++++
2 files changed, 282 insertions(+)
create mode 100755 tools/testing/selftests/net/bridge_stp_mode.sh
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 6bced3ed798b0..053c7b83c76dd 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 0000000000000..5737a10f002f0
--- /dev/null
+++ b/tools/testing/selftests/net/bridge_stp_mode.sh
@@ -0,0 +1,281 @@
+#!/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"
+
+ log_test "idempotent stp_mode set while STP is 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"
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v2 2/3] docs: net: bridge: document stp_mode attribute
From: Andy Roulin @ 2026-03-29 2:58 UTC (permalink / raw)
To: netdev
Cc: bridge, Nikolay Aleksandrov, Ido Schimmel, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Petr Machata,
Donald Hunter, Jonas Gorski, linux-doc, linux-kselftest,
linux-kernel, Andy Roulin
In-Reply-To: <20260329025858.330620-1-aroulin@nvidia.com>
Add documentation for the IFLA_BR_STP_MODE bridge attribute in the
"User space STP helper" section of the bridge documentation. Reference
the BR_STP_MODE_* values via kernel-doc and describe the use case for
network namespace environments.
Signed-off-by: Andy Roulin <aroulin@nvidia.com>
---
Documentation/networking/bridge.rst | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/Documentation/networking/bridge.rst b/Documentation/networking/bridge.rst
index ef8b73e157b26..c1e6ea52c9e59 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
====
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v2 1/3] net: bridge: add stp_mode attribute for STP mode selection
From: Andy Roulin @ 2026-03-29 2:58 UTC (permalink / raw)
To: netdev
Cc: bridge, Nikolay Aleksandrov, Ido Schimmel, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Petr Machata,
Donald Hunter, Jonas Gorski, linux-doc, linux-kselftest,
linux-kernel, Andy Roulin
In-Reply-To: <20260329025858.330620-1-aroulin@nvidia.com>
The bridge-stp usermode helper is currently restricted to the initial
network namespace, preventing userspace STP daemons (e.g. mstpd) from
operating on bridges in other network 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 use userspace STP.
Add a new bridge attribute IFLA_BR_STP_MODE that allows explicit
per-bridge control over STP mode selection:
BR_STP_MODE_AUTO (default) - Existing behavior: invoke the
/sbin/bridge-stp helper in init_net only; fall back to kernel STP
if it fails or in non-init namespaces.
BR_STP_MODE_USER - Directly enable userspace STP (BR_USER_STP)
without invoking the 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 can only be changed while STP is disabled, or set to the
same value (-EBUSY otherwise). IFLA_BR_STP_MODE is processed before
IFLA_BR_STP_STATE in br_changelink(), so both can be set atomically
in a single netlink message.
This eliminates the need for call_usermodehelper() in user/kernel
modes, addressing the security concerns discussed in the thread at
https://lore.kernel.org/netdev/565B7F7D.80208@nod.at/ and providing
a cleaner alternative to extending the helper into namespaces.
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Assisted-by: Claude:claude-opus-4-6
Signed-off-by: Andy Roulin <aroulin@nvidia.com>
---
Notes:
v2:
* Add rt-link.yaml netlink spec update.
* Allow idempotent stp_mode set while STP is active.
* Move stp_mode next to root_port to fill a struct hole.
* Rephrase BR_STP_MODE_USER doc.
Documentation/netlink/specs/rt-link.yaml | 11 +++++++
include/uapi/linux/if_link.h | 39 ++++++++++++++++++++++++
net/bridge/br_device.c | 1 +
net/bridge/br_netlink.c | 18 ++++++++++-
net/bridge/br_private.h | 1 +
net/bridge/br_stp_if.c | 17 ++++++-----
6 files changed, 79 insertions(+), 8 deletions(-)
diff --git a/Documentation/netlink/specs/rt-link.yaml b/Documentation/netlink/specs/rt-link.yaml
index df4b56beb8187..15fb3b1cd51da 100644
--- a/Documentation/netlink/specs/rt-link.yaml
+++ b/Documentation/netlink/specs/rt-link.yaml
@@ -833,6 +833,13 @@ definitions:
entries:
- p2p
- mp
+ -
+ name: br-stp-mode
+ type: enum
+ entries:
+ - auto
+ - user
+ - kernel
attribute-sets:
-
@@ -1543,6 +1550,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/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 83a96c56b8cad..58727fbf81e56 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_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 f7502e62dd357..a35ceae0a6f2c 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 0264730938f4b..f5b462a040b92 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,17 @@ 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 (br->stp_enabled != BR_NO_STP && mode != br->stp_mode) {
+ 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 +1648,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 +1701,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 6dbca845e625d..03e9f20181175 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -523,6 +523,7 @@ struct net_bridge {
unsigned char topology_change;
unsigned char topology_change_detected;
u16 root_port;
+ u32 stp_mode;
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 cc4b27ff1b088..fa2271c5d84fe 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,7 +164,7 @@ 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_debug(br, "userspace STP started\n");
} else {
@@ -180,12 +182,13 @@ 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_mode == BR_STP_MODE_AUTO) {
+ int err = br_stp_call_user(br, "stop");
+
+ if (err)
+ br_err(br, "failed to stop userspace STP (%d)\n", err);
+ }
/* To start timers on any ports left in blocking */
spin_lock_bh(&br->lock);
--
2.43.0
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: bridge: add stp_mode attribute for STP mode selection
From: Andy Roulin @ 2026-03-29 2:58 UTC (permalink / raw)
To: netdev
Cc: bridge, Nikolay Aleksandrov, Ido Schimmel, Andrew Lunn,
David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Simon Horman, Jonathan Corbet, Shuah Khan, Petr Machata,
Donald Hunter, Jonas Gorski, linux-doc, linux-kselftest,
linux-kernel, Andy Roulin
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
v2:
Patch #1:
* Add rt-link.yaml netlink spec update.
* Allow idempotent stp_mode set while STP is active.
* Move stp_mode next to root_port to fill a struct
hole.
* Rephrase BR_STP_MODE_USER doc.
Patch #3:
* Fix shellcheck CI: add SC2329 suppression.
* Add idempotent stp_mode test.
Suggested-by: Ido Schimmel <idosch@nvidia.com>
Signed-off-by: Andy Roulin <aroulin@nvidia.com>
Andy Roulin (3):
net: bridge: add stp_mode attribute for STP mode selection
docs: net: bridge: document stp_mode attribute
selftests: net: add bridge STP mode selection test
Documentation/netlink/specs/rt-link.yaml | 11 +
Documentation/networking/bridge.rst | 22 ++
include/uapi/linux/if_link.h | 39 +++
net/bridge/br_device.c | 1 +
net/bridge/br_netlink.c | 18 +-
net/bridge/br_private.h | 1 +
net/bridge/br_stp_if.c | 17 +-
tools/testing/selftests/net/Makefile | 1 +
.../testing/selftests/net/bridge_stp_mode.sh | 281 ++++++++++++++++++
9 files changed, 383 insertions(+), 8 deletions(-)
create mode 100755 tools/testing/selftests/net/bridge_stp_mode.sh
--
2.43.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox