netdev - Re: [PATCH RFC v4 bpf-next 07/11] tun: set egress XDP program

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: <25d0253e-d1b8-ca6c-4e2f-3e5c35d6d661@redhat.com>
Date:   Mon, 2 Mar 2020 11:32:07 +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 07/11] tun: set egress XDP program


On 2020/2/27 上午11:20, David Ahern wrote:
> From: David Ahern <dahern@...italocean.com>
>
> This patch adds a way to set tx path XDP program in tun driver
> by handling XDP_SETUP_PROG_EGRESS and XDP_QUERY_PROG_EGRESS in
> ndo_bpf handler.
>
> Signed-off-by: David Ahern <dahern@...italocean.com>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@...il.com>
> ---
>   drivers/net/tun.c | 34 ++++++++++++++++++++++++++--------
>   1 file changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 7cc5a1acaef2..6aae398b904b 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -239,6 +239,7 @@ struct tun_struct {
>   	u32 rx_batched;
>   	struct tun_pcpu_stats __percpu *pcpu_stats;
>   	struct bpf_prog __rcu *xdp_prog;
> +	struct bpf_prog __rcu *xdp_egress_prog;
>   	struct tun_prog __rcu *steering_prog;
>   	struct tun_prog __rcu *filter_prog;
>   	struct ethtool_link_ksettings link_ksettings;
> @@ -1189,15 +1190,21 @@ tun_net_get_stats64(struct net_device *dev, struct rtnl_link_stats64 *stats)
>   }
>   
>   static int tun_xdp_set(struct net_device *dev, struct bpf_prog *prog,
> -		       struct netlink_ext_ack *extack)


How about accept a bpf_prog ** here, then there's no need to check 
egress when deferencing the pointer?


> +		       bool egress, struct netlink_ext_ack *extack)
>   {
>   	struct tun_struct *tun = netdev_priv(dev);
>   	struct tun_file *tfile;
>   	struct bpf_prog *old_prog;
>   	int i;
>   
> -	old_prog = rtnl_dereference(tun->xdp_prog);
> -	rcu_assign_pointer(tun->xdp_prog, prog);
> +	if (egress) {
> +		old_prog = rtnl_dereference(tun->xdp_egress_prog);
> +		rcu_assign_pointer(tun->xdp_egress_prog, prog);
> +	} else {
> +		old_prog = rtnl_dereference(tun->xdp_prog);
> +		rcu_assign_pointer(tun->xdp_prog, prog);
> +	}
> +
>   	if (old_prog)
>   		bpf_prog_put(old_prog);


Btw, for egress type of XDP there's no need to toggle SOCK_XDP flag 
which is only needed for RX XDP.

Thanks


>   
> @@ -1218,12 +1225,16 @@ static int tun_xdp_set(struct net_device *dev, struct bpf_prog *prog,
>   	return 0;
>   }
>   
> -static u32 tun_xdp_query(struct net_device *dev)
> +static u32 tun_xdp_query(struct net_device *dev, bool egress)
>   {
>   	struct tun_struct *tun = netdev_priv(dev);
>   	const struct bpf_prog *xdp_prog;
>   
> -	xdp_prog = rtnl_dereference(tun->xdp_prog);
> +	if (egress)
> +		xdp_prog = rtnl_dereference(tun->xdp_egress_prog);
> +	else
> +		xdp_prog = rtnl_dereference(tun->xdp_prog);
> +
>   	if (xdp_prog)
>   		return xdp_prog->aux->id;
>   
> @@ -1234,13 +1245,20 @@ static int tun_xdp(struct net_device *dev, struct netdev_bpf *xdp)
>   {
>   	switch (xdp->command) {
>   	case XDP_SETUP_PROG:
> -		return tun_xdp_set(dev, xdp->prog, xdp->extack);
> +		return tun_xdp_set(dev, xdp->prog, false, xdp->extack);
> +	case XDP_SETUP_PROG_EGRESS:
> +		return tun_xdp_set(dev, xdp->prog, true, xdp->extack);
>   	case XDP_QUERY_PROG:
> -		xdp->prog_id = tun_xdp_query(dev);
> -		return 0;
> +		xdp->prog_id = tun_xdp_query(dev, false);
> +		break;
> +	case XDP_QUERY_PROG_EGRESS:
> +		xdp->prog_id = tun_xdp_query(dev, true);
> +		break;
>   	default:
>   		return -EINVAL;
>   	}
> +
> +	return 0;
>   }
>   
>   static int tun_net_change_carrier(struct net_device *dev, bool new_carrier)