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 <[email protected]> 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/[email protected]?part=6
