Re: [PATCH net 1/2] bnxt: don't reject XDP installation when HDS isn't forced on

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 19 Feb 2025 18:58:02 -0700 Daniel Xu wrote:
> > @@ -395,7 +397,7 @@ static int bnxt_xdp_set(struct bnxt *bp, struct bpf_prog *prog)
> >  			    bp->dev->mtu, BNXT_MAX_PAGE_MODE_MTU);
> >  		return -EOPNOTSUPP;
> >  	}
> > -	if (prog && bp->flags & BNXT_FLAG_HDS) {
> > +	if (prog && dev->cfg->hds_config == ETHTOOL_TCP_DATA_SPLIT_ENABLED) {
> >  		netdev_warn(dev, "XDP is disallowed when HDS is enabled.\n");
> >  		return -EOPNOTSUPP;
> >  	}
> > -- 
> > 2.48.1
> >   
> 
> Nice, that fixed it.
> 
> Tested-by: Daniel Xu <dxu@dxuuu.xyz>

I looked again after sending because it wasn't sitting 100% well with
me. As the commit message says this will work, because it forces all
flags to off. But the driver is also only setting its internal flag
when user requested. So why does it get set in the first place..

I think the real fix may be:

@@ -2071,6 +2072,8 @@ static int ethtool_set_ringparam(struct net_device *dev, void __user *useraddr)
 
        dev->ethtool_ops->get_ringparam(dev, &max, &kernel_ringparam, NULL);
 
+       kernel_ringparam.tcp_data_split = dev->cfg->hds_config;
+
        /* ensure new ring parameters are within the maximums */
        if (ringparam.rx_pending > max.rx_max_pending ||
            ringparam.rx_mini_pending > max.rx_mini_max_pending ||

This is the legacy / ioctl path. We don't hit it in testing, but you
probably hit it via systemd.

At least that's my current theory, waiting for the test kernel 
to deploy.  Sorry for the flip flop..