Re: [PATCH net-next v4 06/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_change_mtu lacks channel_changing protection, causing a race condition with RDMA vport configuration on failure.
--

commit 1868db24408cc9d4a118e97d242371346d9b0c67
Author: Long Li <longli@microsoft.com>

net: mana: swap queue sets in mana_change_mtu

This commit updates mana_change_mtu to carry the MTU in the queue set,
replacing RX buffers and rebuilding all queues using a temporary queue set
swap instead of just detaching and attaching.

> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index bb9ef4e634a6e..7f7833f3e1aad 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -902,32 +902,37 @@ int mana_pre_alloc_rxbufs(struct mana_port_context *mpc, int new_mtu, int num_qu
>  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;
> +	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;
> -	}
> +	scratch = mana_qset_scratch_alloc(mpc);
> +	if (!scratch)
> +		return -ENOMEM;
>  
> -	WRITE_ONCE(ndev->mtu, new_mtu);
> +	err = mana_alloc_qset(mpc, scratch, mpc->num_queues,
> +			      mpc->rx_queue_size, mpc->tx_queue_size,
> +			      mpc->priv_flags, new_mtu, &newq);
> +	if (err)
> +		goto free_scratch;

[Severity: High]
This is a pre-existing issue, but does mana_change_mtu() need to set the
channel_changing flag to protect against concurrent RDMA operations? Unlike
the ethtool queue configuration paths, this function initiates a queue set
swap without taking vport_mutex to set apc->channel_changing to true.

>  
> -	err = mana_attach(ndev);
> +	err = mana_publish_qset(mpc, &newq, &oldq);
>  	if (err) {
> -		netdev_err(ndev, "mana_attach failed: %d\n", err);
> -		WRITE_ONCE(ndev->mtu, old_mtu);
> +		mana_free_qset(scratch, &newq);
> +		goto free_scratch;
>  	}
>  
> -out:
> -	mana_pre_dealloc_rxbufs(mpc);
> +	mana_free_qset(scratch, &oldq);
> +
> +free_scratch:
> +	mana_publish_close_if_needed(mpc);
> +	mana_qset_scratch_free(scratch);
>  	return err;
>  }

[Severity: High]
This isn't a bug introduced by this patch, but can this failure path race
with RDMA vport configuration?

On failure, mana_publish_close_if_needed() triggers a vport teardown via
mana_dealloc_queues() and mana_destroy_vport(). Because the
channel_changing flag is not set, a concurrent RDMA operation (such as
creating an RDMA RAW QP from userspace) could grab the vport mid-teardown,
specifically between mana_uncfg_vport() and mana_pf_deregister_hw_vport().

If this race occurs, the Ethernet driver might send a hardware deregister
command for a vport that RDMA just configured, potentially causing hardware
state corruption or PCIe errors.

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