Re: [PATCH net-next v2 05/13] net: mana: swap queue sets in mana_change_mtu

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] mana_xdp_set() is vulnerable to a race condition with RDMA vport creation because it continues to use the legacy mana_detach()/mana_attach() sequence without blocking RDMA, leaving the interface permanently down if a race occurs.
--

--- Patch [5]: [PATCH net-next v2 05/13] net: mana: swap queue sets in mana_change_mtu ---
commit 07ab252b810cb193b26b16516fc0a40653d2b458
Author: Long Li <longli@microsoft.com>

net: mana: swap queue sets in mana_change_mtu

The RX buffer layout depends on the MTU, so changing it rebuilds the
queues. Convert mana_change_mtu() to pre-allocate and swap.

The MTU becomes part of the queue-set configuration, so a new set can be
built for the new MTU while the running one still serves traffic at the
old one, and ndev->mtu is updated only once the new set is live.
Previously it was written before mana_attach() and rolled back on
failure, so a failed change was briefly visible to the stack.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index c0f31b386536a..c858a58577dfb 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -884,35 +884,49 @@ int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_qu
>  	return -ENOMEM;
>  }
>  
> +/* ndev->mtu is updated only once the new set is live (mana_publish_qset), so
> + * a failed allocation leaves the queues and the advertised MTU untouched.
> + */
>  static int mana_change_mtu(struct net_device *ndev, int new_mtu)
>  {
>  	struct mana_port_context *mpc = netdev_priv(ndev);
> -	unsigned int old_mtu = ndev->mtu;
> +	struct mana_port_context *scratch;
> +	struct mana_qset newq, oldq;
>  	int err;
>  
> -	/* Pre-allocate buffers to prevent failure in mana_attach later */
> -	err = mana_pre_alloc_rxbufs(mpc, new_mtu, mpc->num_queues);
> -	if (err) {
> -		netdev_err(ndev, "Insufficient memory for new MTU\n");
> -		return err;
> +	/* Port is down: no queues to rebuild, just record the new MTU.
> +	 * mana_open() will size the RX buffers accordingly.
> +	 */
> +	if (!mpc->port_is_up) {
> +		mpc->configured_mtu = new_mtu;
> +		WRITE_ONCE(ndev->mtu, new_mtu);
> +		return 0;
>  	}
>  
> -	err = mana_detach(ndev, false);
> -	if (err) {
> -		netdev_err(ndev, "mana_detach failed: %d\n", err);
> -		goto out;
> -	}

[Severity: High]
This isn't a bug introduced by this patch, but since it converts
mana_change_mtu() to avoid the mana_detach()/mana_attach() sequence, is
mana_xdp_set() still vulnerable to a race condition with RDMA vport creation?

Looking at mana_xdp_set() in drivers/net/ethernet/microsoft/mana/mana_bpf.c,
it still uses the legacy detach/attach pattern without blocking RDMA:

	err = mana_detach(ndev, false);
	if (err) {
		netdev_err(ndev,
			   "mana_detach failed at xdp set: %d\n", err);
		NL_SET_ERR_MSG_MOD(extack,
				   "XDP: Re-config failed at detach");
		goto err_dealloc_rxbuffs;
	}

	err = mana_attach(ndev);

Because mana_xdp_set() drops vport_use_count to 0 without holding vport_mutex
or setting channel_changing = true, couldn't an RDMA application successfully
acquire the vport in the middle of XDP reconfiguration?

If a race occurs, mana_attach() would fail with -EBUSY because RDMA owns the
vport, leaving the Ethernet port permanently down and requiring manual
intervention to recover. 

> -	WRITE_ONCE(ndev->mtu, new_mtu);
> +	scratch = mana_qset_scratch_alloc(mpc);
> +	if (!scratch)
> +		return -ENOMEM;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260813050418.2906468-1-longli@microsoft.com?part=5