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
| ||
|
Date: Fri, 12 Apr 2019 12:23:08 +0800 From: Jason Wang <jasowang@...hat.com> To: David Woodhouse <dwmw2@...radead.org>, Toke Høiland-Jørgensen <toke@...hat.com>, netdev@...r.kernel.org Cc: "Michael S. Tsirkin" <mst@...hat.com> Subject: Re: Tun congestion/BQL On 2019/4/11 下午5:16, David Woodhouse wrote: > On Thu, 2019-04-11 at 17:04 +0800, Jason Wang wrote: >> Btw, forget to mention, I modify your patch to use >> netif_stop/wake_subqueue() instead. > Hm... > > > --- /usr/src/debug/kernel-5.0.fc29/linux-5.0.5- > 200.fc29.x86_64/drivers/net/tun.c2019-03-03 23:21:29.000000000 +0000 > +++ /home/fedora/tun/tun.c 2019-04-11 09:11:20.781683956 +0000 > @@ -1118,8 +1118,14 @@ static netdev_tx_t tun_net_xmit(struct s > > nf_reset(skb); > > - if (ptr_ring_produce(&tfile->tx_ring, skb)) > + if (ptr_ring_produce(&tfile->tx_ring, skb)) { > + netif_stop_subqueue(tun->dev, txq); > goto drop; > + } > + > + if (ptr_ring_full(&tfile->tx_ring)) { > + netif_stop_subqueue(tun->dev, txq); > + } > > /* Notify and wake up reader process */ > if (tfile->flags & TUN_FASYNC) > @@ -2229,6 +2235,8 @@ static ssize_t tun_do_read(struct tun_st > consume_skb(skb); > } > > + netif_wake_subqueue(tun->dev, tfile->queue_index); > + > return ret; > } > > > That gives me > > Socket Message Elapsed Messages > Size Size Time Okay Errors Throughput > bytes bytes secs # # 10^6bits/sec > > 212992 1400 10.90 2277474 0 2340.95 > 212992 10.90 1174728 1207.47 > > Without the first netif_stop_subqueue() call in the case where we > actually *do* drop the packet (which AFAICT should never happen?) it > looks like this: Looks like subqueue wakeup could be done after subqueue stop, so you probably need to check !ptr_ring_full() before waking up the queue. But it may still have issues: - still racy between tun_net_xmit() and tun_do_read() to solve this need to protect the ring full check and subqueue status changing with producer lock - still racy when there're multiple tun_net_xmit() - but probing producer index may lead cacheline bounce which is kinda conflict with the design of ptr_ring which don't want consumer to peek producer index If we really want to go this way, need more careful thought e.g need bring back the IFF_ONE_QUEUE behaviour and clear SOCK_ZEROCOPY when IFF_ONE_QUEUE is set. > > Socket Message Elapsed Messages > Size Size Time Okay Errors Throughput > bytes bytes secs # # 10^6bits/sec > > 212992 1400 10.00 7675563 0 8596.61 > 212992 10.00 1441186 1614.13 > > And without the patch at all, it was still faster when I just let it > drop packets: > > Socket Message Elapsed Messages > Size Size Time Okay Errors Throughput > bytes bytes secs # # 10^6bits/sec > > 212992 1400 10.00 8143709 0 9120.93 > 212992 10.00 1545035 1730.44 > > I think this is expected, the patch brings some overheads. Thanks
Powered by blists - more mailing lists