netdev - Re: [bpf-next V1-RFC PATCH 10/14] tun: setup xdp_rxq

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: <e3c11f1a-794a-e1dc-5f5c-2296cb0d819a@redhat.com>
Date:   Wed, 20 Dec 2017 15:48:01 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>,
        Daniel Borkmann <borkmann@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:     Willem de Bruijn <willemb@...gle.com>,
        "Michael S. Tsirkin" <mst@...hat.com>, netdev@...r.kernel.org,
        dsahern@...il.com, gospo@...adcom.com, bjorn.topel@...el.com,
        michael.chan@...adcom.com
Subject: Re: [bpf-next V1-RFC PATCH 10/14] tun: setup xdp_rxq_info



On 2017年12月13日 19:20, Jesper Dangaard Brouer wrote:
> Driver hook points for xdp_rxq_info:
>   * init+reg: tun_attach
>   * unreg   : __tun_detach
>
> I've done some manual testing of this tun driver, but I would
> appriciate good review and someone else running their use-case tests,
> as I'm not 100% sure I understand the tfile->detached semantics.
>
> Cc: Jason Wang <jasowang@...hat.com>
> Cc: "Michael S. Tsirkin" <mst@...hat.com>
> Cc: Willem de Bruijn <willemb@...gle.com>
> Signed-off-by: Jesper Dangaard Brouer <brouer@...hat.com>
> ---
>   drivers/net/tun.c |   25 ++++++++++++++++++++++++-
>   1 file changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index e367d6310353..f1df08c2c541 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -180,6 +180,7 @@ struct tun_file {
>   	struct list_head next;
>   	struct tun_struct *detached;
>   	struct skb_array tx_array;
> +	struct xdp_rxq_info xdp_rxq;
>   };
>   
>   struct tun_flow_entry {
> @@ -687,8 +688,10 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
>   			    tun->dev->reg_state == NETREG_REGISTERED)
>   				unregister_netdevice(tun->dev);
>   		}
> -		if (tun)
> +		if (tun) {
>   			skb_array_cleanup(&tfile->tx_array);
> +			xdp_rxq_info_unreg(&tfile->xdp_rxq);
> +		}
>   		sock_put(&tfile->sk);
>   	}
>   }
> @@ -728,11 +731,15 @@ static void tun_detach_all(struct net_device *dev)
>   		tun_napi_del(tun, tfile);
>   		/* Drop read queue */
>   		tun_queue_purge(tfile);
> +		skb_array_cleanup(&tfile->tx_array);

Looks like this is unnecessary, skb array will be cleaned up only when 
fd is closed otherwise there will be a double free.

Other looks good.

Thanks

> +		xdp_rxq_info_unreg(&tfile->xdp_rxq);
>   		sock_put(&tfile->sk);
>   	}
>   	list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) {
>   		tun_enable_queue(tfile);
>   		tun_queue_purge(tfile);
> +		skb_array_cleanup(&tfile->tx_array);
> +		xdp_rxq_info_unreg(&tfile->xdp_rxq);
>   		sock_put(&tfile->sk);
>   	}
>   	BUG_ON(tun->numdisabled != 0);
> @@ -784,6 +791,21 @@ static int tun_attach(struct tun_struct *tun, struct file *file,
>   
>   	tfile->queue_index = tun->numqueues;
>   	tfile->socket.sk->sk_shutdown &= ~RCV_SHUTDOWN;
> +
> +	if (tfile->detached) {
> +		/* Re-attach detached tfile, updating XDP queue_index */
> +		WARN_ON(!xdp_rxq_info_is_reg(&tfile->xdp_rxq));
> +
> +		if (tfile->xdp_rxq.queue_index    != tfile->queue_index)
> +			tfile->xdp_rxq.queue_index = tfile->queue_index;
> +	} else {
> +		/* Setup XDP RX-queue info, for new tfile getting attached */
> +		xdp_rxq_info_init(&tfile->xdp_rxq);
> +		tfile->xdp_rxq.dev = tun->dev;
> +		tfile->xdp_rxq.queue_index = tfile->queue_index;
> +		xdp_rxq_info_reg(&tfile->xdp_rxq);
> +	}
> +
>   	rcu_assign_pointer(tfile->tun, tun);
>   	rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
>   	tun->numqueues++;
> @@ -1508,6 +1530,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun,
>   		xdp.data = buf + pad;
>   		xdp_set_data_meta_invalid(&xdp);
>   		xdp.data_end = xdp.data + len;
> +		xdp.rxq = &tfile->xdp_rxq;
>   		orig_data = xdp.data;
>   		act = bpf_prog_run_xdp(xdp_prog, &xdp);
>   
>