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.toshi...@lab.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.toshi...@lab.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. > > Would you mind elaborating why not handle the RX completely in the NAPI > > context of the original device? > > Originally it was difficult to implement .ndo_xdp_xmit() and > .ndo_xdp_flush() model without creating NAPI in veth. Now it is changed > so I'm not sure how difficult it is at this point. > But in any case I want to avoid stack inflation by veth NAPI. (Imagine > some misconfiguration like calling XDP_TX on both side of veth...) True :/
