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

> 
> 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.

It's fixed by 06/13 patch.

Long