tun: only queue packets on device
authorMichael S. Tsirkin <mst@redhat.com>
Mon, 3 Dec 2012 10:07:14 +0000 (10:07 +0000)
committerDavid S. Miller <davem@davemloft.net>
Mon, 3 Dec 2012 20:07:36 +0000 (15:07 -0500)
Historically tun supported two modes of operation:
- in default mode, a small number of packets would get queued
  at the device, the rest would be queued in qdisc
- in one queue mode, all packets would get queued at the device

This might have made sense up to a point where we made the
queue depth for both modes the same and set it to
a huge value (500) so unless the consumer
is stuck the chance of losing packets is small.

Thus in practice both modes behave the same, but the
default mode has some problems:
- if packets are never consumed, fragments are never orphaned
  which cases a DOS for sender using zero copy transmit
- overrun errors are hard to diagnose: fifo error is incremented
  only once so you can not distinguish between
  userspace that is stuck and a transient failure,
  tcpdump on the device does not show any traffic

Userspace solves this simply by enabling IFF_ONE_QUEUE
but there seems to be little point in not doing the
right thing for everyone, by default.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
drivers/net/tun.c
include/uapi/linux/if_tun.h

index 71f6874d804825ac922c613b327c4d8e1d4181de..a1b2389e6d7fdacce6abb7b3b1ed7d24a2029d05 100644 (file)
@@ -690,21 +690,8 @@ static netdev_tx_t tun_net_xmit(struct sk_buff *skb, struct net_device *dev)
         * number of queues.
         */
        if (skb_queue_len(&tfile->socket.sk->sk_receive_queue)
-                         >= dev->tx_queue_len / tun->numqueues){
-               if (!(tun->flags & TUN_ONE_QUEUE)) {
-                       /* Normal queueing mode. */
-                       /* Packet scheduler handles dropping of further packets. */
-                       netif_stop_subqueue(dev, txq);
-
-                       /* We won't see all dropped packets individually, so overrun
-                        * error is more appropriate. */
-                       dev->stats.tx_fifo_errors++;
-               } else {
-                       /* Single queue mode.
-                        * Driver handles dropping of all packets itself. */
-                       goto drop;
-               }
-       }
+                         >= dev->tx_queue_len / tun->numqueues)
+               goto drop;
 
        /* Orphan the skb - required as we might hang on to it
         * for indefinite time. */
@@ -1319,7 +1306,6 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
                        schedule();
                        continue;
                }
-               netif_wake_subqueue(tun->dev, tfile->queue_index);
 
                ret = tun_put_user(tun, tfile, skb, iv, len);
                kfree_skb(skb);
@@ -1482,6 +1468,9 @@ static int tun_flags(struct tun_struct *tun)
        if (tun->flags & TUN_NO_PI)
                flags |= IFF_NO_PI;
 
+       /* This flag has no real effect.  We track the value for backwards
+        * compatibility.
+        */
        if (tun->flags & TUN_ONE_QUEUE)
                flags |= IFF_ONE_QUEUE;
 
@@ -1632,6 +1621,9 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
        else
                tun->flags &= ~TUN_NO_PI;
 
+       /* This flag has no real effect.  We track the value for backwards
+        * compatibility.
+        */
        if (ifr->ifr_flags & IFF_ONE_QUEUE)
                tun->flags |= TUN_ONE_QUEUE;
        else
index 958497ad5bb563f40c6a46cf7e8aac951ef98d36..2835b85fd46d85ae47ec857caa93db1ee5ba79b9 100644 (file)
@@ -31,6 +31,7 @@
 #define TUN_FASYNC     0x0010
 #define TUN_NOCHECKSUM 0x0020
 #define TUN_NO_PI      0x0040
+/* This flag has no real effect */
 #define TUN_ONE_QUEUE  0x0080
 #define TUN_PERSIST    0x0100  
 #define TUN_VNET_HDR   0x0200
@@ -60,6 +61,7 @@
 #define IFF_TUN                0x0001
 #define IFF_TAP                0x0002
 #define IFF_NO_PI      0x1000
+/* This flag has no real effect */
 #define IFF_ONE_QUEUE  0x2000
 #define IFF_VNET_HDR   0x4000
 #define IFF_TUN_EXCL   0x8000