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