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