[RFC,v4,bpf-next,07/11] tun: set egress XDP program
diff mbox series

Message ID 20200227032013.12385-8-dsahern@kernel.org
State RFC
Delegated to: BPF Maintainers
Headers show
Series
  • Add support for XDP in egress path
Related show

Commit Message

David Ahern Feb. 27, 2020, 3:20 a.m. UTC
From: David Ahern <dahern@digitalocean.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@digitalocean.com>
Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.com>
---
 drivers/net/tun.c | 34 ++++++++++++++++++++++++++--------
 1 file changed, 26 insertions(+), 8 deletions(-)

Comments

Jason Wang March 2, 2020, 3:32 a.m. UTC | #1
On 2020/2/27 上午11:20, David Ahern wrote:
> From: David Ahern <dahern@digitalocean.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@digitalocean.com>
> Signed-off-by: Prashant Bhole <prashantbhole.linux@gmail.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)
David Ahern March 2, 2020, 3:52 a.m. UTC | #2
On 3/1/20 8:32 PM, Jason Wang wrote:
>> @@ -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?


sure, that works. Will change.

> 
> 
>> +               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.

Right. We removed the manipulation of that flag a few iterations ago
based on a review comment.
David Ahern March 10, 2020, 2:18 a.m. UTC | #3
On 3/1/20 8:32 PM, Jason Wang wrote:
>> @@ -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?
> 
> 

The egress arg is better. xdp_prog vs xdp_egress_prog is an element of
'struct tun'. tun is currently not needed in tun_xdp and is part of
tun_xdp_set already.

Patch
diff mbox series

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)
+		       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);
 
@@ -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)