Re: [PATCH net] veth: Avoid drop packets when xdp_redirect performs

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Heng Qi <hengqi@linux.alibaba.com> writes:

> 在 2022/10/21 下午2:31, Heng Qi 写道:
>>
>>
>> 在 2022/10/21 上午12:34, Toke Høiland-Jørgensen 写道:
>>> Heng Qi <hengqi@linux.alibaba.com> writes:
>>>
>>>> maybe we should consider a simpler method: when loading xdp in veth,
>>>> we can automatically enable the napi ring of peer veth, which seems to
>>>> have no performance impact and functional impact on the veth pair, and
>>>> no longer requires users to do more things for peer veth (after all,
>>>> they may be unaware of more requirements for peer veth). Do you think
>>>> this is feasible?
>>> It could be, perhaps? One issue is what to do once the XDP program is
>>> then unloaded? We should probably disable NAPI on the peer in this case,
>>> but then we'd need to track whether it was enabled by loading an XDP
>>> program; we don't want to disable GRO/NAPI if the user requested it
>>> explicitly. This kind of state tracking gets icky fast, so I guess it'll
>>> depend on the patch...
>>
>> Regarding tracking whether we disable the napi of peer veth when 
>> unloading
>> the veth's xdp program, this can actually be handled cleanly.
>>
>> We need to note that when peer veth enable GRO, the peer veth device will
>> update the `dev->wanted_features` with NETIF_F_GRO of peer veth (refer to
>> __netdev_update_features() and veth_set_features() ).
>>
>> When veth loads the xdp program and the napi of peer veth is not ready
>> (that is, peer veth does not load the xdp program or has no enable gro),
>> at this time, we can choose `ethtool -K veth0 gro on` to enable the 
>> napi of
>> peer veth, this command also makes the peer veth device update their
>> wanted_features, or choose we automatically enable napi for peer veth.
>>
>> If we want to unload the xdp program for veth, peer veth cannot directly
>> disable its napi, because we need to judge whether peer veth is 
>> gro_requested
>> ( ref to veth_gro_requested() ) or has its priv->_xdp_prog, if so, just
>> clean veth's xdp environment and disable the napi of veth instead of
>> directly disable the napi of peer veth, because of the existence of the
>> gro_requested and the xdp program loading on peer veth.
>>
>> But, if peer veth does not have gro_requested or xdp_program loading 
>> on itself,
>> then we can directly disable the napi of peer veth.
>
> Hi, Toke. Do you think the above solution is effective for the problem 
> of veth
> xdp_rediect dropping packets? Or is there more to add? If the above solution
> seems to be no problem for the time being, I'm ready to start with this idea
> and try to make the corresponding patch.

I think it might? Please write a patch and we can have a look :)

-Toke