netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] veth: Update XDP feature set when bringing up device
@ 2023-09-11 13:58 Toke Høiland-Jørgensen
  2023-09-12 10:07 ` Paolo Abeni
  2023-09-12 14:40 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2023-09-11 13:58 UTC (permalink / raw)
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Lorenzo Bianconi, Kumar Kartikeya Dwivedi,
	Stanislav Fomichev, Gerhard Engleder, Simon Horman
  Cc: Toke Høiland-Jørgensen, Marek Majtyka, netdev, bpf

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@redhat.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;
 }
 
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH net] veth: Update XDP feature set when bringing up device
  2023-09-11 13:58 [PATCH net] veth: Update XDP feature set when bringing up device Toke Høiland-Jørgensen
@ 2023-09-12 10:07 ` Paolo Abeni
  2023-09-12 12:54   ` Toke Høiland-Jørgensen
  2023-09-12 14:40 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2023-09-12 10:07 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, Lorenzo Bianconi,
	Kumar Kartikeya Dwivedi, Stanislav Fomichev, Gerhard Engleder,
	Simon Horman
  Cc: Marek Majtyka, netdev, bpf

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@redhat.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?

Thanks,

Paolo


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net] veth: Update XDP feature set when bringing up device
  2023-09-12 10:07 ` Paolo Abeni
@ 2023-09-12 12:54   ` Toke Høiland-Jørgensen
  2023-09-12 13:10     ` Paolo Abeni
  0 siblings, 1 reply; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2023-09-12 12:54 UTC (permalink / raw)
  To: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Lorenzo Bianconi, Kumar Kartikeya Dwivedi,
	Stanislav Fomichev, Gerhard Engleder, Simon Horman
  Cc: Marek Majtyka, netdev, bpf

Paolo Abeni <pabeni@redhat.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@redhat.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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net] veth: Update XDP feature set when bringing up device
  2023-09-12 12:54   ` Toke Høiland-Jørgensen
@ 2023-09-12 13:10     ` Paolo Abeni
  2023-09-12 13:15       ` Toke Høiland-Jørgensen
  0 siblings, 1 reply; 6+ messages in thread
From: Paolo Abeni @ 2023-09-12 13:10 UTC (permalink / raw)
  To: Toke Høiland-Jørgensen, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Alexei Starovoitov, Daniel Borkmann,
	Jesper Dangaard Brouer, John Fastabend, Lorenzo Bianconi,
	Kumar Kartikeya Dwivedi, Stanislav Fomichev, Gerhard Engleder,
	Simon Horman
  Cc: Marek Majtyka, netdev, bpf

On Tue, 2023-09-12 at 14:54 +0200, Toke Høiland-Jørgensen wrote:
> Paolo Abeni <pabeni@redhat.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@redhat.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?

I think the latter is simpler and works for me. The self-test could
target net-next, the fix is going to land there shortly after -net.

Thanks!

Paolo


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net] veth: Update XDP feature set when bringing up device
  2023-09-12 13:10     ` Paolo Abeni
@ 2023-09-12 13:15       ` Toke Høiland-Jørgensen
  0 siblings, 0 replies; 6+ messages in thread
From: Toke Høiland-Jørgensen @ 2023-09-12 13:15 UTC (permalink / raw)
  To: Paolo Abeni, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Alexei Starovoitov, Daniel Borkmann, Jesper Dangaard Brouer,
	John Fastabend, Lorenzo Bianconi, Kumar Kartikeya Dwivedi,
	Stanislav Fomichev, Gerhard Engleder, Simon Horman
  Cc: Marek Majtyka, netdev, bpf

Paolo Abeni <pabeni@redhat.com> writes:

> On Tue, 2023-09-12 at 14:54 +0200, Toke Høiland-Jørgensen wrote:
>> Paolo Abeni <pabeni@redhat.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@redhat.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?
>
> I think the latter is simpler and works for me. The self-test could
> target net-next, the fix is going to land there shortly after -net.

ACK, SGTM!

-Toke


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH net] veth: Update XDP feature set when bringing up device
  2023-09-11 13:58 [PATCH net] veth: Update XDP feature set when bringing up device Toke Høiland-Jørgensen
  2023-09-12 10:07 ` Paolo Abeni
@ 2023-09-12 14:40 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-09-12 14:40 UTC (permalink / raw)
  To: =?utf-8?b?VG9rZSBIw7hpbGFuZC1Kw7hyZ2Vuc2VuIDx0b2tlQHJlZGhhdC5jb20+?=
  Cc: davem, edumazet, kuba, pabeni, ast, daniel, hawk, john.fastabend,
	lorenzo, memxor, sdf, gerhard, horms, alardam, netdev, bpf

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Mon, 11 Sep 2023 15:58:25 +0200 you 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.
> 
> [...]

Here is the summary with links:
  - [net] veth: Update XDP feature set when bringing up device
    https://git.kernel.org/netdev/net/c/7a6102aa6df0

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-12 14:40 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-09-11 13:58 [PATCH net] veth: Update XDP feature set when bringing up device Toke Høiland-Jørgensen
2023-09-12 10:07 ` Paolo Abeni
2023-09-12 12:54   ` Toke Høiland-Jørgensen
2023-09-12 13:10     ` Paolo Abeni
2023-09-12 13:15       ` Toke Høiland-Jørgensen
2023-09-12 14:40 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).