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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250219181427.3d7aa28f@kernel.org>
Date: Wed, 19 Feb 2025 18:14:27 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: Daniel Xu <dxu@...uu.xyz>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
 daniel@...earbox.net, hawk@...nel.org, john.fastabend@...il.com,
 michael.chan@...adcom.com, pavan.chebbi@...adcom.com, ap420073@...il.com
Subject: Re: [PATCH net 1/2] bnxt: don't reject XDP installation when HDS
 isn't forced on

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