netdev - Re: [PATCH RFC v2 2/9] veth: Add driver 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] [day] [month] [year] [list]

Date:   Mon, 11 Jun 2018 19:55:04 +0900
From:   Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:     netdev@...r.kernel.org
Cc:     Toshiaki Makita <toshiaki.makita1@...il.com>,
        Jesper Dangaard Brouer <brouer@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH RFC v2 2/9] veth: Add driver XDP

On 2018/06/11 1:02, Toshiaki Makita wrote:
> From: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
> 
> This is basic implementation of veth driver XDP.
> 
> Incoming packets are sent from the peer veth device in the form of skb,
> so this is generally doing the same thing as generic XDP.
> 
> This itself is not so useful, but a starting point to implement other
> useful veth XDP features like TX and REDIRECT.
> 
> This introduces NAPI when XDP is enabled, because XDP is now heavily
> relies on NAPI context. Use ptr_ring to emulate NIC ring. Tx function
> enqueues packets to the ring and peer NAPI handler drains the ring.
> 
> Currently only one ring is allocated for each veth device, so it does
> not scale on multiqueue env. This can be resolved by allocating rings
> on the per-queue basis later.
> 
> Note that NAPI is not used but netif_rx is used when XDP is not loaded,
> so this does not change the default behaviour.
> 
> v2:
> - Squashed with the patch adding NAPI.
> - Implement adjust_tail.
> - Don't acquire consumer lock because it is guarded by NAPI.
> - Make poll_controller noop since it is unnecessary.
> - Register rxq_info on enabling XDP rather than on opening the device.
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
> ---
...
> @@ -203,6 +480,9 @@ static int veth_close(struct net_device *dev)
>  	if (peer)
>  		netif_carrier_off(peer);
>  
> +	if (rtnl_dereference(priv->xdp_prog))
> +		veth_disable_xdp(dev);

Found that it is not safe to disable napi at this point, because packets
still can come from the peer and the ring may be filled after cleanup.
I'll fix the problem in v3, but wait for feedback some more.

-- 
Toshiaki Makita