| Message ID | 20200227032013.12385-8-dsahern@kernel.org |
|---|---|
| State | RFC |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | Add support for XDP in egress path | expand |
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)
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.
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.
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)