On Mon, Apr 14, 2014 at 09:21:47AM -0400, Steven Galgano wrote:
> 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 <[email protected]>
> Tested-by: Joseph Giovatto <[email protected]>
> Signed-off-by: Steven Galgano <[email protected]>
> ---
> 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().

Any plans to try and address the more material comments on v2?

>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to