All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steven Galgano <sgalgano@adjacentlink.com>
To: David Miller <davem@davemloft.net>,
	mst@redhat.com, jasowang@redhat.com, xemul@parallels.com,
	wuzhy@linux.vnet.ibm.com, therbert@google.com, yamato@redhat.com,
	richardcochran@gmail.com
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Brian.Adamson@nrl.navy.mil, jgiovatto@adjacentlink.com
Subject: [PATCH v3] tuntap: add flow control to support back pressure
Date: Mon, 14 Apr 2014 09:21:47 -0400	[thread overview]
Message-ID: <534BE0EB.2080409@adjacentlink.com> (raw)
In-Reply-To: <20140414.003449.294037863611224523.davem@davemloft.net>

Added optional per queue flow control support using IFF_FLOW_CONTROL. When the 
IFF_FLOW_CONTROL TUNSETIFF flag is specified it will set a per queue flag to 
indicate that the queue should be stopped using netif_tx_stop_queue(), rather 
than discarding frames once full. After reading a frame from the respective 
stopped queue, a netif_tx_wake_queue() is issued to signal resource 
availability.

The per queue TUN_FLOW_CONTROL flag is stored in struct tun_file. This provides 
the flexibility to enable flow control on all, none or some queues when using 
IFF_MULTI_QUEUE. When not using IFF_MULTI_QUEUE, IFF_FLOW_CONTROL will apply to 
the single queue. No changes were made to the default drop frame policy.

This change adds support for back pressure use cases.

Reported-by: Brian Adamson <brian.adamson@nrl.navy.mil>
Tested-by: Joseph Giovatto <jgiovatto@adjacentlink.com>
Signed-off-by: Steven Galgano <sgalgano@adjacentlink.com>
---
Version 3 patch reformatted commit message to be 80 columns max width. 
 Corrected Tested-By email address.
Version 2 patch added support for individual queues when applying flow
control instead of using netif_tx_stop_all_queues()/netif_tx_wake_all_queues().

 drivers/net/tun.c           | 32 ++++++++++++++++++++++++++++----
 include/uapi/linux/if_tun.h |  2 ++
 2 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index ee328ba..3d09f5a 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -137,7 +137,7 @@ struct tun_file {
 	struct tun_struct __rcu *tun;
 	struct net *net;
 	struct fasync_struct *fasync;
-	/* only used for fasnyc */
+	/* used for fasnyc and flow control */
 	unsigned int flags;
 	union {
 		u16 queue_index;
@@ -783,8 +783,19 @@ 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) * numqueues
-			  >= dev->tx_queue_len)
-		goto drop;
+			>= dev->tx_queue_len) {
+		if (tfile->flags & TUN_FLOW_CONTROL) {
+			/* Resources unavailable stop queue */
+			netif_tx_stop_queue(netdev_get_tx_queue(dev, txq));
+
+			/* We won't see all dropped packets individually, so
+			 * over run error is more appropriate.
+			 */
+			dev->stats.tx_fifo_errors++;
+		} else {
+			goto drop;
+		}
+	}
 
 	if (unlikely(skb_orphan_frags(skb, GFP_ATOMIC)))
 		goto drop;
@@ -1333,6 +1344,7 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 	DECLARE_WAITQUEUE(wait, current);
 	struct sk_buff *skb;
 	ssize_t ret = 0;
+	struct netdev_queue *ntxq;
 
 	tun_debug(KERN_INFO, tun, "tun_do_read\n");
 
@@ -1362,6 +1374,12 @@ static ssize_t tun_do_read(struct tun_struct *tun, struct tun_file *tfile,
 			continue;
 		}
 
+		ntxq = netdev_get_tx_queue(tun->dev, tfile->queue_index);
+
+		if (tfile->flags & TUN_FLOW_CONTROL &&
+		    netif_tx_queue_stopped(ntxq))
+			netif_tx_wake_queue(ntxq);
+
 		ret = tun_put_user(tun, tfile, skb, iv, len);
 		kfree_skb(skb);
 		break;
@@ -1732,6 +1750,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 	else
 		tun->flags &= ~TUN_TAP_MQ;
 
+	if (ifr->ifr_flags & IFF_FLOW_CONTROL)
+		tfile->flags |= TUN_FLOW_CONTROL;
+	else
+		tfile->flags &= ~TUN_FLOW_CONTROL;
+
 	/* Make sure persistent devices do not get stuck in
 	 * xoff state.
 	 */
@@ -1900,7 +1923,8 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		 * This is needed because we never checked for invalid flags on
 		 * TUNSETIFF. */
 		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
-				IFF_VNET_HDR | IFF_MULTI_QUEUE,
+				IFF_VNET_HDR | IFF_MULTI_QUEUE |
+				IFF_FLOW_CONTROL,
 				(unsigned int __user*)argp);
 	} else if (cmd == TUNSETQUEUE)
 		return tun_set_queue(file, &ifr);
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index e9502dd..bcf2790 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -36,6 +36,7 @@
 #define TUN_PERSIST 	0x0100	
 #define TUN_VNET_HDR 	0x0200
 #define TUN_TAP_MQ      0x0400
+#define TUN_FLOW_CONTROL 0x0800
 
 /* Ioctl defines */
 #define TUNSETNOCSUM  _IOW('T', 200, int) 
@@ -70,6 +71,7 @@
 #define IFF_MULTI_QUEUE 0x0100
 #define IFF_ATTACH_QUEUE 0x0200
 #define IFF_DETACH_QUEUE 0x0400
+#define IFF_FLOW_CONTROL 0x0010
 /* read-only flag */
 #define IFF_PERSIST	0x0800
 #define IFF_NOFILTER	0x1000

  reply	other threads:[~2014-04-14 13:21 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-10  2:19 [PATCH] tuntap: add flow control to support back pressure Steven Galgano
     [not found] ` <20140410102931.GA12077@redhat.com>
2014-04-11  1:42   ` Steven Galgano
2014-04-11  3:02     ` Jason Wang
2014-04-11 16:41     ` Brian Adamson
2014-04-13 14:14       ` Michael S. Tsirkin
2014-04-14  1:28         ` Steven Galgano
2014-04-14  5:40           ` Michael S. Tsirkin
2014-04-14 18:45             ` Brian Adamson
2014-04-13 14:17     ` Michael S. Tsirkin
2014-04-14  1:30       ` [PATCH v2] " Steven Galgano
2014-04-14  1:40         ` David Miller
2014-04-14  4:19           ` Steven Galgano
2014-04-14  4:34             ` David Miller
2014-04-14 13:21               ` Steven Galgano [this message]
2014-04-14 13:31                 ` [PATCH v3] " Michael S. Tsirkin
2014-04-14 13:43                   ` Steven Galgano
2014-04-14  7:02             ` [PATCH v2] " Michael S. Tsirkin
2014-04-11  2:57   ` [PATCH] " Jason Wang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=534BE0EB.2080409@adjacentlink.com \
    --to=sgalgano@adjacentlink.com \
    --cc=Brian.Adamson@nrl.navy.mil \
    --cc=davem@davemloft.net \
    --cc=jasowang@redhat.com \
    --cc=jgiovatto@adjacentlink.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=netdev@vger.kernel.org \
    --cc=richardcochran@gmail.com \
    --cc=therbert@google.com \
    --cc=wuzhy@linux.vnet.ibm.com \
    --cc=xemul@parallels.com \
    --cc=yamato@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.