Linux Virtualization - Jun 2008 - [PATCH 1/4] tun: Interface to query tun/tap features.

The problem with introducing checksum offload and gso to tun is they
need to set dev->features to enable GSO and/or checksumming, which is
supposed to be done before register_netdevice(), ie. as part of
TUNSETIFF.

Unfortunately, TUNSETIFF has always just ignored flags it doesn't
understand, so there's no good way of detecting whether the kernel
supports new IFF_ flags.

This patch implements a TUNGETFEATURES ioctl which returns all the valid IFF
flags.  It could be extended later to include other features.

Here's an example program which uses it:

#include <linux/if_tun.h>
#include <sys/types.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <err.h>
#include <stdio.h>

static struct {
	unsigned int flag;
	const char *name;
} known_flags[] = {
	{ IFF_TUN, "TUN" },
	{ IFF_TAP, "TAP" },
	{ IFF_NO_PI, "NO_PI" },
	{ IFF_ONE_QUEUE, "ONE_QUEUE" },
};

int main()
{
	unsigned int features, i;

	int netfd = open("/dev/net/tun", O_RDWR);
	if (netfd < 0)
		err(1, "Opening /dev/net/tun");

	if (ioctl(netfd, TUNGETFEATURES, &features) != 0) {
		printf("Kernel does not support TUNGETFEATURES, guessing\n");
		features = (IFF_TUN|IFF_TAP|IFF_NO_PI|IFF_ONE_QUEUE);
	}
	printf("Available features are: ");
	for (i = 0; i < sizeof(known_flags)/sizeof(known_flags[0]); i++) {
		if (features & known_flags[i].flag) {
			features &= ~known_flags[i].flag;
			printf("%s ", known_flags[i].name);
		}
	}
	if (features)
		printf("(UNKNOWN %#x)", features);
	printf("\n");
	return 0;
}

Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
 drivers/net/tun.c      |    8 ++++++++
 include/linux/if_tun.h |    1 +
 2 files changed, 9 insertions(+)

diff -r 8414a579e106 drivers/net/tun.c
--- a/drivers/net/tun.c	Tue Apr 22 07:36:45 2008 +1000
+++ b/drivers/net/tun.c	Tue Apr 22 07:37:33 2008 +1000
@@ -625,6 +625,14 @@ static int tun_chr_ioctl(struct inode *i
 		return 0;
 	}
 
+	if (cmd == TUNGETFEATURES) {
+		/* Currently this just means: "what IFF flags are valid?".
+		 * 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,
+				(unsigned int __user*)argp);
+	}
+
 	if (!tun)
 		return -EBADFD;
 
diff -r 8414a579e106 include/linux/if_tun.h
--- a/include/linux/if_tun.h	Tue Apr 22 07:36:45 2008 +1000
+++ b/include/linux/if_tun.h	Tue Apr 22 07:37:33 2008 +1000
@@ -42,6 +42,7 @@
 #define TUNSETOWNER   _IOW('T', 204, int)
 #define TUNSETLINK    _IOW('T', 205, int)
 #define TUNSETGROUP   _IOW('T', 206, int)
+#define TUNGETFEATURES _IOR('T', 207, unsigned int)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001

ethtool is useful for setting (some) device fields, but it's
root-only.  Finer feature control is available through a tun-specific
ioctl.

(Includes Mark McLoughlin <markmc at redhat.com>'s fix to hold rtnl
sem).

Signed-off-by: Rusty Russell <rusty at rustcorp.com.au>
---
 drivers/net/tun.c      |   43 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/if_tun.h |    7 +++++++
 2 files changed, 50 insertions(+)

diff -r e08e6c5130ff drivers/net/tun.c
--- a/drivers/net/tun.c	Thu Jun 26 00:17:07 2008 +1000
+++ b/drivers/net/tun.c	Thu Jun 26 00:21:11 2008 +1000
@@ -596,6 +596,46 @@ static int tun_set_iff(struct net *net, 
 	return err;
 }
 
+/* This is like a cut-down ethtool ops, except done via tun fd so no
+ * privs required. */
+static int set_offload(struct net_device *dev, unsigned long arg)
+{
+	unsigned int old_features, features;
+
+	old_features = dev->features;
+	/* Unset features, set them as we chew on the arg. */
+	features = (old_features & ~(NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST
+				    |NETIF_F_TSO_ECN|NETIF_F_TSO|NETIF_F_TSO6));
+
+	if (arg & TUN_F_CSUM) {
+		features |= NETIF_F_HW_CSUM|NETIF_F_SG|NETIF_F_FRAGLIST;
+		arg &= ~TUN_F_CSUM;
+
+		if (arg & (TUN_F_TSO4|TUN_F_TSO6)) {
+			if (arg & TUN_F_TSO_ECN) {
+				features |= NETIF_F_TSO_ECN;
+				arg &= ~TUN_F_TSO_ECN;
+			}
+			if (arg & TUN_F_TSO4)
+				features |= NETIF_F_TSO;
+			if (arg & TUN_F_TSO6)
+				features |= NETIF_F_TSO6;
+			arg &= ~(TUN_F_TSO4|TUN_F_TSO6);
+		}
+	}
+
+	/* This gives the user a way to test for new features in future by
+	 * trying to set them. */
+	if (arg)
+		return -EINVAL;
+
+	dev->features = features;
+	if (old_features != dev->features)
+		netdev_features_change(dev);
+
+	return 0;
+}
+
 static int tun_chr_ioctl(struct inode *inode, struct file *file,
 			 unsigned int cmd, unsigned long arg)
 {
@@ -699,6 +739,15 @@ static int tun_chr_ioctl(struct inode *i
 		tun->debug = arg;
 		break;
 #endif
+
+	case TUNSETOFFLOAD:
+	{
+		int ret;
+		rtnl_lock();
+		ret = set_offload(tun->dev, arg);
+		rtnl_unlock();
+		return ret;
+	}
 
 	case SIOCGIFFLAGS:
 		ifr.ifr_flags = tun->if_flags;
diff -r e08e6c5130ff include/linux/if_tun.h
--- a/include/linux/if_tun.h	Thu Jun 26 00:17:07 2008 +1000
+++ b/include/linux/if_tun.h	Thu Jun 26 00:21:11 2008 +1000
@@ -43,12 +43,19 @@
 #define TUNSETLINK    _IOW('T', 205, int)
 #define TUNSETGROUP   _IOW('T', 206, int)
 #define TUNGETFEATURES _IOR('T', 207, unsigned int)
+#define TUNSETOFFLOAD _IOW('T', 208, unsigned int)
 
 /* TUNSETIFF ifr flags */
 #define IFF_TUN		0x0001
 #define IFF_TAP		0x0002
 #define IFF_NO_PI	0x1000
 #define IFF_ONE_QUEUE	0x2000
+
+/* Features for GSO (TUNSETOFFLOAD). */
+#define TUN_F_CSUM	0x01	/* You can hand me unchecksummed packets. */
+#define TUN_F_TSO4	0x02	/* I can handle TSO for IPv4 packets */
+#define TUN_F_TSO6	0x04	/* I can handle TSO for IPv6 packets */
+#define TUN_F_TSO_ECN	0x08	/* I can handle TSO with ECN bits. */
 
 struct tun_pi {
 	unsigned short flags;

Rusty Russell wrote:
> @@ -1563,6 +1561,16 @@ static void setup_tun_net(char *arg)
>  	/* Tell Guest what MAC address to use. */
>  	add_feature(dev, VIRTIO_NET_F_MAC);
>  	add_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY);
> +	/* Expect Guest to handle everything except UFO */
> +	add_feature(dev, VIRTIO_NET_F_CSUM);
You're setting this feature twice.
> +	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);
You set this feature, but I never see the virtio-net driver acknowledge 
the feature.  Curiously, my implementation with KVM is struggling 
because UDP packet checksums are not correct so the DHCP client is 
ignoring them.  If I disable CSUM offload, things it works fine (using 
the virtio-net header).  The problem is only host=>guest, guest=>host is 
fine.

Regards,

Anthony Liguori

Rusty Russell wrote:
> The problem with introducing checksum offload and gso to tun is they
> need to set dev->features to enable GSO and/or checksumming, which is
> supposed to be done before register_netdevice(), ie. as part of
> TUNSETIFF.
> 
> Unfortunately, TUNSETIFF has always just ignored flags it doesn't
> understand, so there's no good way of detecting whether the kernel
> supports new IFF_ flags.
> 
> This patch implements a TUNGETFEATURES ioctl which returns all the valid
IFF
> flags.  It could be extended later to include other features.
<snip>

Looks good.

Dave, do you want me to put all outstanding TUN patches into a git tree so
that you can pull them in one shot ?
Otherwise if you're ok with applying them one by one please apply this one.

Acked-by: Max Krasnyansky <maxk at qualcomm.com>