netdev - Re: [PATCH RFC v4 bpf-next 08/11] tun: Support xdp in the Tx path for skb

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: <0dc6768e-b8fa-f0b3-3c58-5135640f114a@redhat.com>
Date:   Mon, 2 Mar 2020 11:28:13 +0800
From:   Jason Wang <jasowang@...hat.com>
To:     David Ahern <dsahern@...nel.org>, netdev@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org,
        prashantbhole.linux@...il.com, brouer@...hat.com, toke@...hat.com,
        mst@...hat.com, toshiaki.makita1@...il.com, daniel@...earbox.net,
        john.fastabend@...il.com, ast@...nel.org, kafai@...com,
        songliubraving@...com, yhs@...com, andriin@...com,
        dsahern@...il.com, David Ahern <dahern@...italocean.com>
Subject: Re: [PATCH RFC v4 bpf-next 08/11] tun: Support xdp in the Tx path for
 skb


On 2020/2/27 上午11:20, David Ahern wrote:
> From: David Ahern <dahern@...italocean.com>
>
> Add support to run Tx path program on packets arriving at a tun
> device as an skb.
>
> XDP_TX return code means move the packet to the Tx path of the device.
> For a program run in the Tx / egress path, XDP_TX is essentially the
> same as "continue on" which is XDP_PASS.
>
> Conceptually, XDP_REDIRECT for this path can work the same as it
> does for the Rx path, but that return code is left for a follow
> on series.
>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@...il.com>
> Signed-off-by: David Ahern <dahern@...italocean.com>
> ---
>   drivers/net/tun.c | 69 ++++++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 66 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 6aae398b904b..dcae6521a39d 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1059,6 +1059,63 @@ static unsigned int run_ebpf_filter(struct tun_struct *tun,
>   	return len;
>   }
>   
> +static struct sk_buff *tun_prepare_xdp_skb(struct sk_buff *skb)
> +{
> +	if (skb_shared(skb) || skb_cloned(skb)) {
> +		struct sk_buff *nskb;
> +
> +		nskb = skb_copy(skb, GFP_ATOMIC);
> +		consume_skb(skb);
> +		return nskb;
> +	}
> +
> +	return skb;
> +}
> +
> +static u32 tun_do_xdp_tx_generic(struct tun_struct *tun,
> +				 struct net_device *dev,
> +				 struct sk_buff *skb)
> +{
> +	struct bpf_prog *xdp_prog;
> +	u32 act = XDP_PASS;
> +
> +	xdp_prog = rcu_dereference(tun->xdp_egress_prog);
> +	if (xdp_prog) {
> +		struct xdp_txq_info txq = { .dev = dev };
> +		struct xdp_buff xdp;
> +
> +		skb = tun_prepare_xdp_skb(skb);
> +		if (!skb) {
> +			act = XDP_DROP;
> +			goto out;
> +		}
> +
> +		xdp.txq = &txq;
> +
> +		act = do_xdp_generic_core(skb, &xdp, xdp_prog);
> +		switch (act) {
> +		case XDP_TX:    /* for Tx path, XDP_TX == XDP_PASS */
> +			act = XDP_PASS;
> +			break;


Jute a note here, I agree for TX XDP it may be better to do this.

But for offloaded program we need different semantic. Or we can deal 
this with attach types?

Thanks


> +		case XDP_PASS:
> +			break;
> +		case XDP_REDIRECT:
> +			/* fall through */
> +		default:
> +			bpf_warn_invalid_xdp_action(act);
> +			/* fall through */
> +		case XDP_ABORTED:
> +			trace_xdp_exception(tun->dev, xdp_prog, act);
> +			/* fall through */
> +		case XDP_DROP:
> +			break;
> +		}
> +	}
> +
> +out:
> +	return act;
> +}
> +
>   /* Net device start xmit */
>   static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>   {
> @@ -1066,6 +1123,7 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>   	int txq = skb->queue_mapping;
>   	struct tun_file *tfile;
>   	int len = skb->len;
> +	u32 act;
>   
>   	rcu_read_lock();
>   	tfile = rcu_dereference(tun->tfiles[txq]);
> @@ -1107,9 +1165,13 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>   
>   	nf_reset_ct(skb);
>   
> -	if (ptr_ring_produce(&tfile->tx_ring, skb))
> +	act = tun_do_xdp_tx_generic(tun, dev, skb);
> +	if (act != XDP_PASS)
>   		goto drop;
>   
> +	if (ptr_ring_produce(&tfile->tx_ring, skb))
> +		goto err_out;
> +
>   	/* Notify and wake up reader process */
>   	if (tfile->flags & TUN_FASYNC)
>   		kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
> @@ -1118,10 +1180,11 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
>   	rcu_read_unlock();
>   	return NETDEV_TX_OK;
>   
> -drop:
> -	this_cpu_inc(tun->pcpu_stats->tx_dropped);
> +err_out:
>   	skb_tx_error(skb);
>   	kfree_skb(skb);
> +drop:
> +	this_cpu_inc(tun->pcpu_stats->tx_dropped);
>   	rcu_read_unlock();
>   	return NET_XMIT_DROP;
>   }