[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4acfdc6f-3a68-d228-b371-7d1ee98604ed@lab.ntt.co.jp>
Date: Tue, 24 Jul 2018 13:02:21 +0900
From: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To: Jakub Kicinski <jakub.kicinski@...ronome.com>
Cc: Toshiaki Makita <toshiaki.makita1@...il.com>,
netdev@...r.kernel.org, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Jesper Dangaard Brouer <brouer@...hat.com>
Subject: Re: [PATCH v3 bpf-next 6/8] xdp: Add a flag for disabling napi_direct
of xdp_return_frame in xdp_mem_info
On 2018/07/24 12:38, Jakub Kicinski wrote:
> On Tue, 24 Jul 2018 11:43:11 +0900, Toshiaki Makita wrote:
>> On 2018/07/24 10:22, Jakub Kicinski wrote:
>>> On Mon, 23 Jul 2018 00:13:06 +0900, Toshiaki Makita wrote:
>>>> From: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
>>>>
>>>> We need some mechanism to disable napi_direct on calling
>>>> xdp_return_frame_rx_napi() from some context.
>>>> When veth gets support of XDP_REDIRECT, it will redirects packets which
>>>> are redirected from other devices. On redirection veth will reuse
>>>> xdp_mem_info of the redirection source device to make return_frame work.
>>>> But in this case .ndo_xdp_xmit() called from veth redirection uses
>>>> xdp_mem_info which is not guarded by NAPI, because the .ndo_xdp_xmit is
>>>> not called directly from the rxq which owns the xdp_mem_info.
>>>>
>>>> This approach introduces a flag in xdp_mem_info to indicate that
>>>> napi_direct should be disabled even when _rx_napi variant is used.
>>>>
>>>> Signed-off-by: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
>>>
>>> To be clear - you will modify flags of the original source device if it
>>> ever redirected a frame to a software device like veth? Seems a bit
>>> heavy handed. The xdp_return_frame_rx_napi() is only really used on
>>> error paths, but still.. Also as you note the original NAPI can run
>>> concurrently with your veth dest one, but also with NAPIs of other veth
>>> devices, so the non-atomic xdp.rxq->mem.flags |= XDP_MEM_RF_NO_DIRECT;
>>> makes me worried.
>>
>> xdp_mem_info is copied in xdp_frame in convert_to_xdp_frame() so the
>> field is local to the frame. Changing flags affects only the frame.
>> xdp.rxq is local to NAPI thread, so no worries about atomicity.
>
> Ah, right! mem_info used to be just 8B, now it would be 12B.
> Alternatively we could perhaps add this info to struct redirect_info,
> through xdp_do_redirect() to avoid the per-frame cost. I'm not sure
> that's better.
OK, let me check if this works.
--
Toshiaki Makita
Powered by blists - more mailing lists