netdev - Re: [PATCH net] veth: Avoid drop packets when xdp

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: <87r0yxgxba.fsf@toke.dk>
Date:   Mon, 24 Oct 2022 15:34:17 +0200
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Heng Qi <hengqi@...ux.alibaba.com>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, netdev@...r.kernel.org,
        Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH net] veth: Avoid drop packets when xdp_redirect performs

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

> 在 2022/10/21 下午2:31, Heng Qi 写道:
>>
>>
>> 在 2022/10/21 上午12:34, Toke Høiland-Jørgensen 写道:
>>> Heng Qi <hengqi@...ux.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