netdev - Re: [PATCH net] veth: Update XDP feature set when bringing up device

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, 12 Sep 2023 14:54:30 +0200
From: Toke Høiland-Jørgensen <toke@...hat.com>
To: Paolo Abeni <pabeni@...hat.com>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
 <kuba@...nel.org>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann
 <daniel@...earbox.net>, Jesper Dangaard Brouer <hawk@...nel.org>, John
 Fastabend <john.fastabend@...il.com>, Lorenzo Bianconi
 <lorenzo@...nel.org>, Kumar Kartikeya Dwivedi <memxor@...il.com>,
 Stanislav Fomichev <sdf@...gle.com>, Gerhard Engleder
 <gerhard@...leder-embedded.com>, Simon Horman <horms@...nel.org>
Cc: Marek Majtyka <alardam@...il.com>, netdev@...r.kernel.org,
 bpf@...r.kernel.org
Subject: Re: [PATCH net] veth: Update XDP feature set when bringing up device

Paolo Abeni <pabeni@...hat.com> writes:

> Hi,
>
> On Mon, 2023-09-11 at 15:58 +0200, Toke Høiland-Jørgensen wrote:
>> There's an early return in veth_set_features() if the device is in a down
>> state, which leads to the XDP feature flags not being updated when enabling
>> GRO while the device is down. Which in turn leads to XDP_REDIRECT not
>> working, because the redirect code now checks the flags.
>> 
>> Fix this by updating the feature flags after bringing the device up.
>> 
>> Before this patch:
>> 
>> NETDEV_XDP_ACT_BASIC:		yes
>> NETDEV_XDP_ACT_REDIRECT:	yes
>> NETDEV_XDP_ACT_NDO_XMIT:	no
>> NETDEV_XDP_ACT_XSK_ZEROCOPY:	no
>> NETDEV_XDP_ACT_HW_OFFLOAD:	no
>> NETDEV_XDP_ACT_RX_SG:		yes
>> NETDEV_XDP_ACT_NDO_XMIT_SG:	no
>> 
>> After this patch:
>> 
>> NETDEV_XDP_ACT_BASIC:		yes
>> NETDEV_XDP_ACT_REDIRECT:	yes
>> NETDEV_XDP_ACT_NDO_XMIT:	yes
>> NETDEV_XDP_ACT_XSK_ZEROCOPY:	no
>> NETDEV_XDP_ACT_HW_OFFLOAD:	no
>> NETDEV_XDP_ACT_RX_SG:		yes
>> NETDEV_XDP_ACT_NDO_XMIT_SG:	yes
>> 
>> Fixes: fccca038f300 ("veth: take into account device reconfiguration for xdp_features flag")
>> Fixes: 66c0e13ad236 ("drivers: net: turn on XDP features")
>> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
>> ---
>>  drivers/net/veth.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
>> index 9c6f4f83f22b..0deefd1573cf 100644
>> --- a/drivers/net/veth.c
>> +++ b/drivers/net/veth.c
>> @@ -1446,6 +1446,8 @@ static int veth_open(struct net_device *dev)
>>  		netif_carrier_on(peer);
>>  	}
>>  
>> +	veth_set_xdp_features(dev);
>> +
>>  	return 0;
>>  }
>
> The patch LGTM, thanks!
>
> I think it would be nice to add some specific self-tests here. Could
> you please consider following-up with them?

Sure! Do you want me to resubmit this as well, or are you just going to
apply it as-is and do the selftest as a follow-up?

-Toke