| Message ID | 1514541604-12728-3-git-send-email-jasowang@redhat.com |
|---|---|
| State | New, archived |
| Headers | show |
| Series |
|
| Related | show |
On Fri, 29 Dec 2017 18:00:04 +0800 Jason Wang <jasowang@redhat.com> wrote: > This patch implements XDP transmission for TAP. Since we can't create > new queues for TAP during XDP set, exist ptr_ring was reused for > queuing XDP buffers. To differ xdp_buff from sk_buff, TUN_XDP_FLAG > (0x1ULL) was encoded into lowest bit of xpd_buff pointer during > ptr_ring_produce, and was decoded during consuming. XDP metadata was > stored in the headroom of the packet which should work in most of > cases since driver usually reserve enough headroom. Very minor changes > were done for vhost_net: it just need to peek the length depends on > the type of pointer. > > Tests was done on two Intel E5-2630 2.40GHz machines connected back to > back through two 82599ES. Traffic were generated through MoonGen and > testpmd(rxonly) in guest reports 2.97Mpps when xdp_redirect_map is > doing redirection from ixgbe to TAP. IMHO a performance measurement without something to compare against is useless. What was the performance before? > Cc: Jesper Dangaard Brouer <brouer@redhat.com> > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > drivers/net/tun.c | 205 ++++++++++++++++++++++++++++++++++++++++--------- > drivers/vhost/net.c | 13 +++- > include/linux/if_tun.h | 17 ++++ > 3 files changed, 197 insertions(+), 38 deletions(-) > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c > index 2c89efe..be6d993 100644 > --- a/drivers/net/tun.c > +++ b/drivers/net/tun.c > @@ -240,6 +240,24 @@ struct tun_struct { > struct tun_steering_prog __rcu *steering_prog; > }; > > +bool tun_is_xdp_buff(void *ptr) > +{ > + return (unsigned long)ptr & TUN_XDP_FLAG; > +} > +EXPORT_SYMBOL(tun_is_xdp_buff); > + > +void *tun_xdp_to_ptr(void *ptr) > +{ > + return (void *)((unsigned long)ptr | TUN_XDP_FLAG); > +} > +EXPORT_SYMBOL(tun_xdp_to_ptr); > + > +void *tun_ptr_to_xdp(void *ptr) > +{ > + return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG); > +} > +EXPORT_SYMBOL(tun_ptr_to_xdp); > + > static int tun_napi_receive(struct napi_struct *napi, int budget) > { > struct tun_file *tfile = container_of(napi, struct tun_file, napi); > @@ -630,12 +648,25 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile) > return tun; > } > > +static void tun_ptr_free(void *ptr) > +{ > + if (!ptr) > + return; > + if (tun_is_xdp_buff(ptr)) { > + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); > + > + put_page(virt_to_head_page(xdp->data)); (Yet another XDP-free call point, I need to convert to use an accessor later to transition driver into an xdp_buff return API) > + } else { > + __skb_array_destroy_skb(ptr); > + } > +} > + > static void tun_queue_purge(struct tun_file *tfile) > { > - struct sk_buff *skb; > + void *ptr; > > - while ((skb = ptr_ring_consume(&tfile->tx_ring)) != NULL) > - kfree_skb(skb); > + while ((ptr = ptr_ring_consume(&tfile->tx_ring)) != NULL) > + tun_ptr_free(ptr); > > skb_queue_purge(&tfile->sk.sk_write_queue); > skb_queue_purge(&tfile->sk.sk_error_queue); > @@ -688,8 +719,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean) > unregister_netdevice(tun->dev); > } > if (tun) > - ptr_ring_cleanup(&tfile->tx_ring, > - __skb_array_destroy_skb); > + ptr_ring_cleanup(&tfile->tx_ring, tun_ptr_free); > sock_put(&tfile->sk); > } > } > @@ -1201,6 +1231,54 @@ static const struct net_device_ops tun_netdev_ops = { > .ndo_get_stats64 = tun_net_get_stats64, > }; > > +static int tun_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) > +{ > + struct tun_struct *tun = netdev_priv(dev); > + struct xdp_buff *buff = xdp->data_hard_start; > + int headroom = xdp->data - xdp->data_hard_start; > + struct tun_file *tfile; > + u32 numqueues; > + int ret = 0; > + > + /* Assure headroom is available and buff is properly aligned */ > + if (unlikely(headroom < sizeof(*xdp) || tun_is_xdp_buff(xdp))) > + return -ENOSPC; > + > + *buff = *xdp; > + > + rcu_read_lock(); > + > + numqueues = READ_ONCE(tun->numqueues); > + if (!numqueues) { > + ret = -ENOSPC; > + goto out; > + } > + tfile = rcu_dereference(tun->tfiles[smp_processor_id() % > + numqueues]); Several concurrent CPUs can get the same 'tfile'. > + /* Encode the XDP flag into lowest bit for consumer to differ > + * XDP buffer from sk_buff. > + */ > + if (ptr_ring_produce(&tfile->tx_ring, tun_xdp_to_ptr(buff))) { > + this_cpu_inc(tun->pcpu_stats->tx_dropped); > + ret = -ENOSPC; > + } The ptr_ring_produce() will take a lock per packet, limiting the performance. (Again a case where I would have liked a bulk API for ndo_xdp_xmit()). > + > +out: > + rcu_read_unlock(); > + return ret; > +} > + > +static void tun_xdp_flush(struct net_device *dev) > +{ > + struct tun_struct *tun = netdev_priv(dev); > + struct tun_file *tfile = tun->tfiles[0]; > + > + /* Notify and wake up reader process */ > + if (tfile->flags & TUN_FASYNC) > + kill_fasync(&tfile->fasync, SIGIO, POLL_IN); > + tfile->socket.sk->sk_data_ready(tfile->socket.sk); > +} > + > static const struct net_device_ops tap_netdev_ops = { > .ndo_uninit = tun_net_uninit, > .ndo_open = tun_net_open, > @@ -1218,6 +1296,8 @@ static const struct net_device_ops tap_netdev_ops = { > .ndo_set_rx_headroom = tun_set_headroom, > .ndo_get_stats64 = tun_net_get_stats64, > .ndo_bpf = tun_xdp, > + .ndo_xdp_xmit = tun_xdp_xmit, > + .ndo_xdp_flush = tun_xdp_flush, > }; > > static void tun_flow_init(struct tun_struct *tun) > @@ -1841,6 +1921,40 @@ static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from) > return result; > } > > +static ssize_t tun_put_user_xdp(struct tun_struct *tun, > + struct tun_file *tfile, > + struct xdp_buff *xdp, > + struct iov_iter *iter) > +{ > + int vnet_hdr_sz = 0; > + size_t size = xdp->data_end - xdp->data; > + struct tun_pcpu_stats *stats; > + size_t ret; > + > + if (tun->flags & IFF_VNET_HDR) { > + struct virtio_net_hdr gso = { 0 }; > + > + vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz); > + if (unlikely(iov_iter_count(iter) < vnet_hdr_sz)) > + return -EINVAL; > + if (unlikely(copy_to_iter(&gso, sizeof(gso), iter) != > + sizeof(gso))) > + return -EFAULT; > + iov_iter_advance(iter, vnet_hdr_sz - sizeof(gso)); > + } > + > + ret = copy_to_iter(xdp->data, size, iter) + vnet_hdr_sz; > + > + stats = get_cpu_ptr(tun->pcpu_stats); > + u64_stats_update_begin(&stats->syncp); > + stats->tx_packets++; > + stats->tx_bytes += ret; > + u64_stats_update_end(&stats->syncp); > + put_cpu_ptr(tun->pcpu_stats); > + > + return ret; > +} > + > /* Put packet to the user space buffer */ > static ssize_t tun_put_user(struct tun_struct *tun, > struct tun_file *tfile, [...] > error = -ERESTARTSYS; > @@ -1977,36 +2090,42 @@ static struct sk_buff *tun_ring_recv(struct tun_file *tfile, int noblock, > > out: > *err = error; > - return skb; > + return ptr; > } > > static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, > struct iov_iter *to, > - int noblock, struct sk_buff *skb) > + int noblock, void *ptr) > { > ssize_t ret; > int err; > > tun_debug(KERN_INFO, tun, "tun_do_read\n"); > > - if (!iov_iter_count(to)) { > - if (skb) > - kfree_skb(skb); > - return 0; > - } > + if (!iov_iter_count(to)) > + tun_ptr_free(ptr); > > - if (!skb) { > + if (!ptr) { > /* Read frames from ring */ > - skb = tun_ring_recv(tfile, noblock, &err); > - if (!skb) > + ptr = tun_ring_recv(tfile, noblock, &err); > + if (!ptr) > return err; > } > > - ret = tun_put_user(tun, tfile, skb, to); > - if (unlikely(ret < 0)) > - kfree_skb(skb); > - else > - consume_skb(skb); > + if (tun_is_xdp_buff(ptr)) { > + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); > + > + ret = tun_put_user_xdp(tun, tfile, xdp, to); > + put_page(virt_to_head_page(xdp->data)); This again tie us info a page-refcnt based XDP scheme. > + } else { > + struct sk_buff *skb = ptr; > + > + ret = tun_put_user(tun, tfile, skb, to); > + if (unlikely(ret < 0)) > + kfree_skb(skb); > + else > + consume_skb(skb); > + } > > return ret; > } [...] > @@ -3191,8 +3323,7 @@ struct socket *tun_get_socket(struct file *file) > struct tun_file *tfile; > if (file->f_op != &tun_fops) > return ERR_PTR(-EINVAL); > - tfile = file->private_data; > - if (!tfile) > + tfile = file->private_data; if (!tfile) This change looks like a typo... > return ERR_PTR(-EBADFD); > return &tfile->socket; > }
Hi Jason,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/XDP-transmission-for-tuntap/20180101-105946
config: i386-randconfig-x072-201800 (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers//net/tun.c: In function 'tun_xdp_to_ptr':
>> drivers//net/tun.c:251:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (void *)((unsigned long)ptr | TUN_XDP_FLAG);
^
drivers//net/tun.c: In function 'tun_ptr_to_xdp':
drivers//net/tun.c:257:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG);
^
vim +251 drivers//net/tun.c
248
249 void *tun_xdp_to_ptr(void *ptr)
250 {
> 251 return (void *)((unsigned long)ptr | TUN_XDP_FLAG);
252 }
253 EXPORT_SYMBOL(tun_xdp_to_ptr);
254
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Mon, 2018-01-01 at 11:48 +0800, kbuild test robot wrote: > Hi Jason, > > I love your patch! Perhaps something to improve: > > [auto build test WARNING on net-next/master] > > url: https://github.com/0day-ci/linux/commits/Jason-Wang/XDP-transmission-for-tuntap/20180101-105946 > config: i386-randconfig-x072-201800 (attached as .config) > compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All warnings (new ones prefixed by >>): > > drivers//net/tun.c: In function 'tun_xdp_to_ptr': > > > drivers//net/tun.c:251:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > > return (void *)((unsigned long)ptr | TUN_XDP_FLAG); > ^ > drivers//net/tun.c: In function 'tun_ptr_to_xdp': > drivers//net/tun.c:257:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] > return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG); > ^ > > vim +251 drivers//net/tun.c > > 248 > 249 void *tun_xdp_to_ptr(void *ptr) > 250 { > > 251 return (void *)((unsigned long)ptr | TUN_XDP_FLAG); Does TUN_XDP_FLAG really need to be 0x1ULL? Wouldn't 0x1UL suffice?
Hi Jason,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/XDP-transmission-for-tuntap/20180101-105946
config: i386-randconfig-s1-201800 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
net/socket.o: In function `tun_xdp_to_ptr':
>> net/socket.c:3363: multiple definition of `tun_xdp_to_ptr'
drivers/vhost/net.o:drivers/vhost/net.c:1026: first defined here
net/socket.o: In function `tun_ptr_to_xdp':
>> include/linux/if_tun.h:50: multiple definition of `tun_ptr_to_xdp'
drivers/vhost/net.o:include/linux/if_tun.h:50: first defined here
vim +3363 net/socket.c
ac5a488e Sridhar Samudrala 2006-08-07 3352
306b13eb Tom Herbert 2017-07-28 3353 int kernel_sendpage_locked(struct sock *sk, struct page *page, int offset,
306b13eb Tom Herbert 2017-07-28 3354 size_t size, int flags)
306b13eb Tom Herbert 2017-07-28 3355 {
306b13eb Tom Herbert 2017-07-28 3356 struct socket *sock = sk->sk_socket;
306b13eb Tom Herbert 2017-07-28 3357
306b13eb Tom Herbert 2017-07-28 3358 if (sock->ops->sendpage_locked)
306b13eb Tom Herbert 2017-07-28 3359 return sock->ops->sendpage_locked(sk, page, offset, size,
306b13eb Tom Herbert 2017-07-28 3360 flags);
306b13eb Tom Herbert 2017-07-28 3361
306b13eb Tom Herbert 2017-07-28 3362 return sock_no_sendpage_locked(sk, page, offset, size, flags);
306b13eb Tom Herbert 2017-07-28 @3363 }
306b13eb Tom Herbert 2017-07-28 3364 EXPORT_SYMBOL(kernel_sendpage_locked);
306b13eb Tom Herbert 2017-07-28 3365
:::::: The code at line 3363 was first introduced by commit
:::::: 306b13eb3cf9515a8214bbf5d69d811371d05792 proto_ops: Add locked held versions of sendmsg and sendpage
:::::: TO: Tom Herbert <tom@quantonium.net>
:::::: CC: David S. Miller <davem@davemloft.net>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Jason,
I love your patch! Perhaps something to improve:
[auto build test WARNING on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jason-Wang/XDP-transmission-for-tuntap/20180101-105946
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__
sparse warnings: (new ones prefixed by >>)
vim +1274 drivers/net/tun.c
1270
1271 static void tun_xdp_flush(struct net_device *dev)
1272 {
1273 struct tun_struct *tun = netdev_priv(dev);
> 1274 struct tun_file *tfile = tun->tfiles[0];
1275
1276 /* Notify and wake up reader process */
1277 if (tfile->flags & TUN_FASYNC)
1278 kill_fasync(&tfile->fasync, SIGIO, POLL_IN);
1279 tfile->socket.sk->sk_data_ready(tfile->socket.sk);
1280 }
1281
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 2017年12月29日 20:32, Jesper Dangaard Brouer wrote: > On Fri, 29 Dec 2017 18:00:04 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> This patch implements XDP transmission for TAP. Since we can't create >> new queues for TAP during XDP set, exist ptr_ring was reused for >> queuing XDP buffers. To differ xdp_buff from sk_buff, TUN_XDP_FLAG >> (0x1ULL) was encoded into lowest bit of xpd_buff pointer during >> ptr_ring_produce, and was decoded during consuming. XDP metadata was >> stored in the headroom of the packet which should work in most of >> cases since driver usually reserve enough headroom. Very minor changes >> were done for vhost_net: it just need to peek the length depends on >> the type of pointer. >> >> Tests was done on two Intel E5-2630 2.40GHz machines connected back to >> back through two 82599ES. Traffic were generated through MoonGen and >> testpmd(rxonly) in guest reports 2.97Mpps when xdp_redirect_map is >> doing redirection from ixgbe to TAP. > IMHO a performance measurement without something to compare against is > useless. What was the performance before? Forgot to mention, skb mode of xdp_redirect_map gives us 2.5Mpps, so we get about 20% improvements. > > >> Cc: Jesper Dangaard Brouer <brouer@redhat.com> >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> drivers/net/tun.c | 205 ++++++++++++++++++++++++++++++++++++++++--------- >> drivers/vhost/net.c | 13 +++- >> include/linux/if_tun.h | 17 ++++ >> 3 files changed, 197 insertions(+), 38 deletions(-) >> >> diff --git a/drivers/net/tun.c b/drivers/net/tun.c >> index 2c89efe..be6d993 100644 >> --- a/drivers/net/tun.c >> +++ b/drivers/net/tun.c >> @@ -240,6 +240,24 @@ struct tun_struct { >> struct tun_steering_prog __rcu *steering_prog; >> }; >> >> +bool tun_is_xdp_buff(void *ptr) >> +{ >> + return (unsigned long)ptr & TUN_XDP_FLAG; >> +} >> +EXPORT_SYMBOL(tun_is_xdp_buff); >> + >> +void *tun_xdp_to_ptr(void *ptr) >> +{ >> + return (void *)((unsigned long)ptr | TUN_XDP_FLAG); >> +} >> +EXPORT_SYMBOL(tun_xdp_to_ptr); >> + >> +void *tun_ptr_to_xdp(void *ptr) >> +{ >> + return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG); >> +} >> +EXPORT_SYMBOL(tun_ptr_to_xdp); >> + >> static int tun_napi_receive(struct napi_struct *napi, int budget) >> { >> struct tun_file *tfile = container_of(napi, struct tun_file, napi); >> @@ -630,12 +648,25 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile) >> return tun; >> } >> >> +static void tun_ptr_free(void *ptr) >> +{ >> + if (!ptr) >> + return; >> + if (tun_is_xdp_buff(ptr)) { >> + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); >> + >> + put_page(virt_to_head_page(xdp->data)); > (Yet another XDP-free call point, I need to convert to use an accessor > later to transition driver into an xdp_buff return API) That would be very useful if we can reuse driver specific recycling method. (We probably still need some synchronization since tun_ptr_free is called in different contexts). > >> + } else { >> + __skb_array_destroy_skb(ptr); >> + } >> +} >> + >> static void tun_queue_purge(struct tun_file *tfile) >> { >> - struct sk_buff *skb; >> + void *ptr; >> >> - while ((skb = ptr_ring_consume(&tfile->tx_ring)) != NULL) >> - kfree_skb(skb); >> + while ((ptr = ptr_ring_consume(&tfile->tx_ring)) != NULL) >> + tun_ptr_free(ptr); >> >> skb_queue_purge(&tfile->sk.sk_write_queue); >> skb_queue_purge(&tfile->sk.sk_error_queue); >> @@ -688,8 +719,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean) >> unregister_netdevice(tun->dev); >> } >> if (tun) >> - ptr_ring_cleanup(&tfile->tx_ring, >> - __skb_array_destroy_skb); >> + ptr_ring_cleanup(&tfile->tx_ring, tun_ptr_free); >> sock_put(&tfile->sk); >> } >> } >> @@ -1201,6 +1231,54 @@ static const struct net_device_ops tun_netdev_ops = { >> .ndo_get_stats64 = tun_net_get_stats64, >> }; >> >> +static int tun_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) >> +{ >> + struct tun_struct *tun = netdev_priv(dev); >> + struct xdp_buff *buff = xdp->data_hard_start; >> + int headroom = xdp->data - xdp->data_hard_start; >> + struct tun_file *tfile; >> + u32 numqueues; >> + int ret = 0; >> + >> + /* Assure headroom is available and buff is properly aligned */ >> + if (unlikely(headroom < sizeof(*xdp) || tun_is_xdp_buff(xdp))) >> + return -ENOSPC; >> + >> + *buff = *xdp; >> + >> + rcu_read_lock(); >> + >> + numqueues = READ_ONCE(tun->numqueues); >> + if (!numqueues) { >> + ret = -ENOSPC; >> + goto out; >> + } >> + tfile = rcu_dereference(tun->tfiles[smp_processor_id() % >> + numqueues]); > Several concurrent CPUs can get the same 'tfile'. Yes, but I believe we don't have better solution here since we can do per-cpu queues without notifying guest (userspace). > >> + /* Encode the XDP flag into lowest bit for consumer to differ >> + * XDP buffer from sk_buff. >> + */ >> + if (ptr_ring_produce(&tfile->tx_ring, tun_xdp_to_ptr(buff))) { >> + this_cpu_inc(tun->pcpu_stats->tx_dropped); >> + ret = -ENOSPC; >> + } > The ptr_ring_produce() will take a lock per packet, limiting the > performance. (Again a case where I would have liked a bulk API for > ndo_xdp_xmit()). Yes and I'm considering to use your bulk method in cpu map on top in the future. The main bottleneck now is not ndo_xdp_xmit() (8Mpps) but vhost (3Mpps), with some improvements, vhost an have about 7Mpps but still slower than ndo_xdp_xmit(). > > >> + >> +out: >> + rcu_read_unlock(); >> + return ret; >> +} >> + >> +static void tun_xdp_flush(struct net_device *dev) >> +{ >> + struct tun_struct *tun = netdev_priv(dev); >> + struct tun_file *tfile = tun->tfiles[0]; >> + >> + /* Notify and wake up reader process */ >> + if (tfile->flags & TUN_FASYNC) >> + kill_fasync(&tfile->fasync, SIGIO, POLL_IN); >> + tfile->socket.sk->sk_data_ready(tfile->socket.sk); >> +} >> + >> static const struct net_device_ops tap_netdev_ops = { >> .ndo_uninit = tun_net_uninit, >> .ndo_open = tun_net_open, >> @@ -1218,6 +1296,8 @@ static const struct net_device_ops tap_netdev_ops = { >> .ndo_set_rx_headroom = tun_set_headroom, >> .ndo_get_stats64 = tun_net_get_stats64, >> .ndo_bpf = tun_xdp, >> + .ndo_xdp_xmit = tun_xdp_xmit, >> + .ndo_xdp_flush = tun_xdp_flush, >> }; >> >> static void tun_flow_init(struct tun_struct *tun) >> @@ -1841,6 +1921,40 @@ static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from) >> return result; >> } >> >> +static ssize_t tun_put_user_xdp(struct tun_struct *tun, >> + struct tun_file *tfile, >> + struct xdp_buff *xdp, >> + struct iov_iter *iter) >> +{ >> + int vnet_hdr_sz = 0; >> + size_t size = xdp->data_end - xdp->data; >> + struct tun_pcpu_stats *stats; >> + size_t ret; >> + >> + if (tun->flags & IFF_VNET_HDR) { >> + struct virtio_net_hdr gso = { 0 }; >> + >> + vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz); >> + if (unlikely(iov_iter_count(iter) < vnet_hdr_sz)) >> + return -EINVAL; >> + if (unlikely(copy_to_iter(&gso, sizeof(gso), iter) != >> + sizeof(gso))) >> + return -EFAULT; >> + iov_iter_advance(iter, vnet_hdr_sz - sizeof(gso)); >> + } >> + >> + ret = copy_to_iter(xdp->data, size, iter) + vnet_hdr_sz; >> + >> + stats = get_cpu_ptr(tun->pcpu_stats); >> + u64_stats_update_begin(&stats->syncp); >> + stats->tx_packets++; >> + stats->tx_bytes += ret; >> + u64_stats_update_end(&stats->syncp); >> + put_cpu_ptr(tun->pcpu_stats); >> + >> + return ret; >> +} >> + >> /* Put packet to the user space buffer */ >> static ssize_t tun_put_user(struct tun_struct *tun, >> struct tun_file *tfile, > [...] >> error = -ERESTARTSYS; >> @@ -1977,36 +2090,42 @@ static struct sk_buff *tun_ring_recv(struct tun_file *tfile, int noblock, >> >> out: >> *err = error; >> - return skb; >> + return ptr; >> } >> >> static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, >> struct iov_iter *to, >> - int noblock, struct sk_buff *skb) >> + int noblock, void *ptr) >> { >> ssize_t ret; >> int err; >> >> tun_debug(KERN_INFO, tun, "tun_do_read\n"); >> >> - if (!iov_iter_count(to)) { >> - if (skb) >> - kfree_skb(skb); >> - return 0; >> - } >> + if (!iov_iter_count(to)) >> + tun_ptr_free(ptr); >> >> - if (!skb) { >> + if (!ptr) { >> /* Read frames from ring */ >> - skb = tun_ring_recv(tfile, noblock, &err); >> - if (!skb) >> + ptr = tun_ring_recv(tfile, noblock, &err); >> + if (!ptr) >> return err; >> } >> >> - ret = tun_put_user(tun, tfile, skb, to); >> - if (unlikely(ret < 0)) >> - kfree_skb(skb); >> - else >> - consume_skb(skb); >> + if (tun_is_xdp_buff(ptr)) { >> + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); >> + >> + ret = tun_put_user_xdp(tun, tfile, xdp, to); >> + put_page(virt_to_head_page(xdp->data)); > This again tie us info a page-refcnt based XDP scheme. > > >> + } else { >> + struct sk_buff *skb = ptr; >> + >> + ret = tun_put_user(tun, tfile, skb, to); >> + if (unlikely(ret < 0)) >> + kfree_skb(skb); >> + else >> + consume_skb(skb); >> + } >> >> return ret; >> } > [...] > >> @@ -3191,8 +3323,7 @@ struct socket *tun_get_socket(struct file *file) >> struct tun_file *tfile; >> if (file->f_op != &tun_fops) >> return ERR_PTR(-EINVAL); >> - tfile = file->private_data; >> - if (!tfile) >> + tfile = file->private_data; if (!tfile) > This change looks like a typo... Oops, will fix this. Thanks! > >> return ERR_PTR(-EBADFD); >> return &tfile->socket; >> } >
On 2018年01月01日 11:55, Joe Perches wrote: > On Mon, 2018-01-01 at 11:48 +0800, kbuild test robot wrote: >> Hi Jason, >> >> I love your patch! Perhaps something to improve: >> >> [auto build test WARNING on net-next/master] >> >> url: https://github.com/0day-ci/linux/commits/Jason-Wang/XDP-transmission-for-tuntap/20180101-105946 >> config: i386-randconfig-x072-201800 (attached as .config) >> compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025 >> reproduce: >> # save the attached .config to linux build tree >> make ARCH=i386 >> >> All warnings (new ones prefixed by >>): >> >> drivers//net/tun.c: In function 'tun_xdp_to_ptr': >>>> drivers//net/tun.c:251:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] >> return (void *)((unsigned long)ptr | TUN_XDP_FLAG); >> ^ >> drivers//net/tun.c: In function 'tun_ptr_to_xdp': >> drivers//net/tun.c:257:9: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast] >> return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG); >> ^ >> >> vim +251 drivers//net/tun.c >> >> 248 >> 249 void *tun_xdp_to_ptr(void *ptr) >> 250 { >> > 251 return (void *)((unsigned long)ptr | TUN_XDP_FLAG); > Does TUN_XDP_FLAG really need to be 0x1ULL? > Wouldn't 0x1UL suffice? > 0x1UL should be fine. Thanks
diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 2c89efe..be6d993 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -240,6 +240,24 @@ struct tun_struct { struct tun_steering_prog __rcu *steering_prog; }; +bool tun_is_xdp_buff(void *ptr) +{ + return (unsigned long)ptr & TUN_XDP_FLAG; +} +EXPORT_SYMBOL(tun_is_xdp_buff); + +void *tun_xdp_to_ptr(void *ptr) +{ + return (void *)((unsigned long)ptr | TUN_XDP_FLAG); +} +EXPORT_SYMBOL(tun_xdp_to_ptr); + +void *tun_ptr_to_xdp(void *ptr) +{ + return (void *)((unsigned long)ptr & ~TUN_XDP_FLAG); +} +EXPORT_SYMBOL(tun_ptr_to_xdp); + static int tun_napi_receive(struct napi_struct *napi, int budget) { struct tun_file *tfile = container_of(napi, struct tun_file, napi); @@ -630,12 +648,25 @@ static struct tun_struct *tun_enable_queue(struct tun_file *tfile) return tun; } +static void tun_ptr_free(void *ptr) +{ + if (!ptr) + return; + if (tun_is_xdp_buff(ptr)) { + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); + + put_page(virt_to_head_page(xdp->data)); + } else { + __skb_array_destroy_skb(ptr); + } +} + static void tun_queue_purge(struct tun_file *tfile) { - struct sk_buff *skb; + void *ptr; - while ((skb = ptr_ring_consume(&tfile->tx_ring)) != NULL) - kfree_skb(skb); + while ((ptr = ptr_ring_consume(&tfile->tx_ring)) != NULL) + tun_ptr_free(ptr); skb_queue_purge(&tfile->sk.sk_write_queue); skb_queue_purge(&tfile->sk.sk_error_queue); @@ -688,8 +719,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean) unregister_netdevice(tun->dev); } if (tun) - ptr_ring_cleanup(&tfile->tx_ring, - __skb_array_destroy_skb); + ptr_ring_cleanup(&tfile->tx_ring, tun_ptr_free); sock_put(&tfile->sk); } } @@ -1201,6 +1231,54 @@ static const struct net_device_ops tun_netdev_ops = { .ndo_get_stats64 = tun_net_get_stats64, }; +static int tun_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp) +{ + struct tun_struct *tun = netdev_priv(dev); + struct xdp_buff *buff = xdp->data_hard_start; + int headroom = xdp->data - xdp->data_hard_start; + struct tun_file *tfile; + u32 numqueues; + int ret = 0; + + /* Assure headroom is available and buff is properly aligned */ + if (unlikely(headroom < sizeof(*xdp) || tun_is_xdp_buff(xdp))) + return -ENOSPC; + + *buff = *xdp; + + rcu_read_lock(); + + numqueues = READ_ONCE(tun->numqueues); + if (!numqueues) { + ret = -ENOSPC; + goto out; + } + tfile = rcu_dereference(tun->tfiles[smp_processor_id() % + numqueues]); + /* Encode the XDP flag into lowest bit for consumer to differ + * XDP buffer from sk_buff. + */ + if (ptr_ring_produce(&tfile->tx_ring, tun_xdp_to_ptr(buff))) { + this_cpu_inc(tun->pcpu_stats->tx_dropped); + ret = -ENOSPC; + } + +out: + rcu_read_unlock(); + return ret; +} + +static void tun_xdp_flush(struct net_device *dev) +{ + struct tun_struct *tun = netdev_priv(dev); + struct tun_file *tfile = tun->tfiles[0]; + + /* Notify and wake up reader process */ + if (tfile->flags & TUN_FASYNC) + kill_fasync(&tfile->fasync, SIGIO, POLL_IN); + tfile->socket.sk->sk_data_ready(tfile->socket.sk); +} + static const struct net_device_ops tap_netdev_ops = { .ndo_uninit = tun_net_uninit, .ndo_open = tun_net_open, @@ -1218,6 +1296,8 @@ static const struct net_device_ops tap_netdev_ops = { .ndo_set_rx_headroom = tun_set_headroom, .ndo_get_stats64 = tun_net_get_stats64, .ndo_bpf = tun_xdp, + .ndo_xdp_xmit = tun_xdp_xmit, + .ndo_xdp_flush = tun_xdp_flush, }; static void tun_flow_init(struct tun_struct *tun) @@ -1841,6 +1921,40 @@ static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from) return result; } +static ssize_t tun_put_user_xdp(struct tun_struct *tun, + struct tun_file *tfile, + struct xdp_buff *xdp, + struct iov_iter *iter) +{ + int vnet_hdr_sz = 0; + size_t size = xdp->data_end - xdp->data; + struct tun_pcpu_stats *stats; + size_t ret; + + if (tun->flags & IFF_VNET_HDR) { + struct virtio_net_hdr gso = { 0 }; + + vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz); + if (unlikely(iov_iter_count(iter) < vnet_hdr_sz)) + return -EINVAL; + if (unlikely(copy_to_iter(&gso, sizeof(gso), iter) != + sizeof(gso))) + return -EFAULT; + iov_iter_advance(iter, vnet_hdr_sz - sizeof(gso)); + } + + ret = copy_to_iter(xdp->data, size, iter) + vnet_hdr_sz; + + stats = get_cpu_ptr(tun->pcpu_stats); + u64_stats_update_begin(&stats->syncp); + stats->tx_packets++; + stats->tx_bytes += ret; + u64_stats_update_end(&stats->syncp); + put_cpu_ptr(tun->pcpu_stats); + + return ret; +} + /* Put packet to the user space buffer */ static ssize_t tun_put_user(struct tun_struct *tun, struct tun_file *tfile, @@ -1938,15 +2052,14 @@ static ssize_t tun_put_user(struct tun_struct *tun, return total; } -static struct sk_buff *tun_ring_recv(struct tun_file *tfile, int noblock, - int *err) +static void *tun_ring_recv(struct tun_file *tfile, int noblock, int *err) { DECLARE_WAITQUEUE(wait, current); - struct sk_buff *skb = NULL; + void *ptr = NULL; int error = 0; - skb = ptr_ring_consume(&tfile->tx_ring); - if (skb) + ptr = ptr_ring_consume(&tfile->tx_ring); + if (ptr) goto out; if (noblock) { error = -EAGAIN; @@ -1957,8 +2070,8 @@ static struct sk_buff *tun_ring_recv(struct tun_file *tfile, int noblock, current->state = TASK_INTERRUPTIBLE; while (1) { - skb = ptr_ring_consume(&tfile->tx_ring); - if (skb) + ptr = ptr_ring_consume(&tfile->tx_ring); + if (ptr) break; if (signal_pending(current)) { error = -ERESTARTSYS; @@ -1977,36 +2090,42 @@ static struct sk_buff *tun_ring_recv(struct tun_file *tfile, int noblock, out: *err = error; - return skb; + return ptr; } static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile, struct iov_iter *to, - int noblock, struct sk_buff *skb) + int noblock, void *ptr) { ssize_t ret; int err; tun_debug(KERN_INFO, tun, "tun_do_read\n"); - if (!iov_iter_count(to)) { - if (skb) - kfree_skb(skb); - return 0; - } + if (!iov_iter_count(to)) + tun_ptr_free(ptr); - if (!skb) { + if (!ptr) { /* Read frames from ring */ - skb = tun_ring_recv(tfile, noblock, &err); - if (!skb) + ptr = tun_ring_recv(tfile, noblock, &err); + if (!ptr) return err; } - ret = tun_put_user(tun, tfile, skb, to); - if (unlikely(ret < 0)) - kfree_skb(skb); - else - consume_skb(skb); + if (tun_is_xdp_buff(ptr)) { + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); + + ret = tun_put_user_xdp(tun, tfile, xdp, to); + put_page(virt_to_head_page(xdp->data)); + } else { + struct sk_buff *skb = ptr; + + ret = tun_put_user(tun, tfile, skb, to); + if (unlikely(ret < 0)) + kfree_skb(skb); + else + consume_skb(skb); + } return ret; } @@ -2143,12 +2262,12 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len, { struct tun_file *tfile = container_of(sock, struct tun_file, socket); struct tun_struct *tun = tun_get(tfile); - struct sk_buff *skb = m->msg_control; + void *ptr = m->msg_control; int ret; if (!tun) { ret = -EBADFD; - goto out_free_skb; + goto out_free; } if (flags & ~(MSG_DONTWAIT|MSG_TRUNC|MSG_ERRQUEUE)) { @@ -2160,7 +2279,7 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len, SOL_PACKET, TUN_TX_TIMESTAMP); goto out; } - ret = tun_do_read(tun, tfile, &m->msg_iter, flags & MSG_DONTWAIT, skb); + ret = tun_do_read(tun, tfile, &m->msg_iter, flags & MSG_DONTWAIT, ptr); if (ret > (ssize_t)total_len) { m->msg_flags |= MSG_TRUNC; ret = flags & MSG_TRUNC ? ret : total_len; @@ -2171,12 +2290,25 @@ static int tun_recvmsg(struct socket *sock, struct msghdr *m, size_t total_len, out_put_tun: tun_put(tun); -out_free_skb: - if (skb) - kfree_skb(skb); +out_free: + tun_ptr_free(ptr); return ret; } +static int tun_ptr_peek_len(void *ptr) +{ + if (likely(ptr)) { + if (tun_is_xdp_buff(ptr)) { + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); + + return xdp->data_end - xdp->data; + } + return __skb_array_len_with_tag(ptr); + } else { + return 0; + } +} + static int tun_peek_len(struct socket *sock) { struct tun_file *tfile = container_of(sock, struct tun_file, socket); @@ -2187,7 +2319,7 @@ static int tun_peek_len(struct socket *sock) if (!tun) return 0; - ret = PTR_RING_PEEK_CALL(&tfile->tx_ring, __skb_array_len_with_tag); + ret = PTR_RING_PEEK_CALL(&tfile->tx_ring, tun_ptr_peek_len); tun_put(tun); return ret; @@ -3110,7 +3242,7 @@ static int tun_queue_resize(struct tun_struct *tun) ret = ptr_ring_resize_multiple(rings, n, dev->tx_queue_len, GFP_KERNEL, - __skb_array_destroy_skb); + tun_ptr_free); kfree(rings); return ret; @@ -3191,8 +3323,7 @@ struct socket *tun_get_socket(struct file *file) struct tun_file *tfile; if (file->f_op != &tun_fops) return ERR_PTR(-EINVAL); - tfile = file->private_data; - if (!tfile) + tfile = file->private_data; if (!tfile) return ERR_PTR(-EBADFD); return &tfile->socket; } diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c index c316555..a5a1db6 100644 --- a/drivers/vhost/net.c +++ b/drivers/vhost/net.c @@ -175,6 +175,17 @@ static void vhost_net_buf_unproduce(struct vhost_net_virtqueue *nvq) } } +static int vhost_net_buf_peek_len(void *ptr) +{ + if (tun_is_xdp_buff(ptr)) { + struct xdp_buff *xdp = tun_ptr_to_xdp(ptr); + + return xdp->data_end - xdp->data; + } + + return __skb_array_len_with_tag(ptr); +} + static int vhost_net_buf_peek(struct vhost_net_virtqueue *nvq) { struct vhost_net_buf *rxq = &nvq->rxq; @@ -186,7 +197,7 @@ static int vhost_net_buf_peek(struct vhost_net_virtqueue *nvq) return 0; out: - return __skb_array_len_with_tag(vhost_net_buf_get_ptr(rxq)); + return vhost_net_buf_peek_len(vhost_net_buf_get_ptr(rxq)); } static void vhost_net_buf_init(struct vhost_net_buf *rxq) diff --git a/include/linux/if_tun.h b/include/linux/if_tun.h index bdee9b8..1cafdc2 100644 --- a/include/linux/if_tun.h +++ b/include/linux/if_tun.h @@ -17,9 +17,14 @@ #include <uapi/linux/if_tun.h> +#define TUN_XDP_FLAG 0x1ULL + #if defined(CONFIG_TUN) || defined(CONFIG_TUN_MODULE) struct socket *tun_get_socket(struct file *); struct ptr_ring *tun_get_tx_ring(struct file *file); +bool tun_is_xdp_buff(void *ptr); +void *tun_xdp_to_ptr(void *ptr); +void *tun_ptr_to_xdp(void *ptr); #else #include <linux/err.h> #include <linux/errno.h> @@ -33,5 +38,17 @@ static inline struct ptr_ring *tun_get_tx_ring(struct file *f) { return ERR_PTR(-EINVAL); } +static inline bool tun_is_xdp_buff(void *ptr) +{ + return false; +} +void *tun_xdp_to_ptr(void *ptr) +{ + return NULL; +} +void *tun_ptr_to_xdp(void *ptr) +{ + return NULL; +} #endif /* CONFIG_TUN */ #endif /* __IF_TUN_H */
This patch implements XDP transmission for TAP. Since we can't create new queues for TAP during XDP set, exist ptr_ring was reused for queuing XDP buffers. To differ xdp_buff from sk_buff, TUN_XDP_FLAG (0x1ULL) was encoded into lowest bit of xpd_buff pointer during ptr_ring_produce, and was decoded during consuming. XDP metadata was stored in the headroom of the packet which should work in most of cases since driver usually reserve enough headroom. Very minor changes were done for vhost_net: it just need to peek the length depends on the type of pointer. Tests was done on two Intel E5-2630 2.40GHz machines connected back to back through two 82599ES. Traffic were generated through MoonGen and testpmd(rxonly) in guest reports 2.97Mpps when xdp_redirect_map is doing redirection from ixgbe to TAP. Cc: Jesper Dangaard Brouer <brouer@redhat.com> Signed-off-by: Jason Wang <jasowang@redhat.com> --- drivers/net/tun.c | 205 ++++++++++++++++++++++++++++++++++++++++--------- drivers/vhost/net.c | 13 +++- include/linux/if_tun.h | 17 ++++ 3 files changed, 197 insertions(+), 38 deletions(-)