netdev - Re: [RFC v2 bpf-next 2/7] drivers: net: turn on XDP features

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]

Date:   Tue, 17 Jan 2023 23:05:21 +0100
From:   Niklas Söderlund 
        <niklas.soderlund@...igine.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Lorenzo Bianconi <lorenzo@...nel.org>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, ast@...nel.org, daniel@...earbox.net,
        andrii@...nel.org, davem@...emloft.net, kuba@...nel.org,
        hawk@...nel.org, pabeni@...hat.com, edumazet@...gle.com,
        memxor@...il.com, alardam@...il.com, saeedm@...dia.com,
        anthony.l.nguyen@...el.com, gospo@...adcom.com,
        vladimir.oltean@....com, nbd@....name, john@...ozen.org,
        leon@...nel.org, simon.horman@...igine.com, aelior@...vell.com,
        christophe.jaillet@...adoo.fr, ecree.xilinx@...il.com,
        mst@...hat.com, bjorn@...nel.org, magnus.karlsson@...el.com,
        maciej.fijalkowski@...el.com, intel-wired-lan@...ts.osuosl.org,
        lorenzo.bianconi@...hat.com
Subject: Re: [RFC v2 bpf-next 2/7] drivers: net: turn on XDP features

Hi Toke,

On 2023-01-17 22:58:57 +0100, Toke Høiland-Jørgensen wrote:
> Niklas Söderlund <niklas.soderlund@...igine.com> writes:
> 
> > Hi Lorenzo and Marek,
> >
> > Thanks for your work.
> >
> > On 2023-01-14 16:54:32 +0100, Lorenzo Bianconi wrote:
> >
> > [...]
> >
> >> 
> >> Turn 'hw-offload' feature flag on for:
> >>  - netronome (nfp)
> >>  - netdevsim.
> >
> > Is there a definition of the 'hw-offload' written down somewhere? From 
> > reading this series I take it is the ability to offload a BPF program?  
> 
> Yeah, basically this means "allows loading and attaching programs in
> XDP_MODE_HW", I suppose :)
> 
> > It would also be interesting to read documentation for the other flags 
> > added in this series.
> 
> Yup, we should definitely document them :)
> 
> > [...]
> >
> >> diff --git a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c 
> >> b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> >> index 18fc9971f1c8..5a8ddeaff74d 100644
> >> --- a/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> >> +++ b/drivers/net/ethernet/netronome/nfp/nfp_net_common.c
> >> @@ -2529,10 +2529,14 @@ static void nfp_net_netdev_init(struct nfp_net *nn)
> >>  	netdev->features &= ~NETIF_F_HW_VLAN_STAG_RX;
> >>  	nn->dp.ctrl &= ~NFP_NET_CFG_CTRL_RXQINQ;
> >>  
> >> +	nn->dp.netdev->xdp_features = NETDEV_XDP_ACT_BASIC |
> >> +				      NETDEV_XDP_ACT_HW_OFFLOAD;
> >
> > If my assumption about the 'hw-offload' flag above is correct I think 
> > NETDEV_XDP_ACT_HW_OFFLOAD should be conditioned on that the BPF firmware 
> > flavor is in use.
> >
> >     nn->dp.netdev->xdp_features = NETDEV_XDP_ACT_BASIC;
> >
> >     if (nn->app->type->id == NFP_APP_BPF_NIC)
> >         nn->dp.netdev->xdp_features |= NETDEV_XDP_ACT_HW_OFFLOAD;
> >
> >> +
> >>  	/* Finalise the netdev setup */
> >>  	switch (nn->dp.ops->version) {
> >>  	case NFP_NFD_VER_NFD3:
> >>  		netdev->netdev_ops = &nfp_nfd3_netdev_ops;
> >> +		nn->dp.netdev->xdp_features |= NETDEV_XDP_ACT_XSK_ZEROCOPY;
> >>  		break;
> >>  	case NFP_NFD_VER_NFDK:
> >>  		netdev->netdev_ops = &nfp_nfdk_netdev_ops;
> >
> > This is also a wrinkle I would like to understand. Currently NFP support 
> > zero-copy on NFD3, but not for offloaded BPF programs. But with the BPF 
> > firmware flavor running the device can still support zero-copy for 
> > non-offloaded programs.
> >
> > Is it a problem that the driver advertises support for both 
> > hardware-offload _and_ zero-copy at the same time, even if they can't be 
> > used together but separately?
> 
> Hmm, so the idea with this is to only expose feature flags that are
> supported "right now" (you'll note that some of the drivers turn the
> REDIRECT_TARGET flag on and off at runtime). Having features that are
> "supported but in a different configuration" is one of the points of
> user confusion we want to clear up with the explicit flags.
> 
> So I guess it depends a little bit what you mean by "can't be used
> together"? I believe it's possible to load two programs at the same
> time, one in HW mode and one in native (driver) mode, right? In this
> case, could the driver mode program use XSK zerocopy while the HW mode
> program is also loaded?

Exactly, this is my concern. Two programs can be loaded at the same 
time, one in HW mode and one in native mode. The program in native mode 
can use zero-copy at the same time as another program runs in HW mode.

But the program running in HW mode can never use zero-copy.

-- 
Kind Regards,
Niklas Söderlund