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]

Message-ID: <87y1q0bz6m.fsf@toke.dk>
Date:   Tue, 17 Jan 2023 22:58:57 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Niklas Söderlund <niklas.soderlund@...igine.com>,
        Lorenzo Bianconi <lorenzo@...nel.org>
Cc:     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

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?

-Toke