Skip to contentSkip to conversation

tun: Interface to query tun/tap features.

PATCH In review Rusty Russell 5 participants 34 messages Jun 25, 2008 – Jul 24, 2008

Subsystems (6)
Networking Network drivers Documentation virtio core +2 more subsystems
lore
Share & tools
lore

Rusty Russell

author
#0 lore raw

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 <[email protected]>
---
 drivers/net/tun.c      |    8 ++++++++
 include/linux/if_tun.h |    1 +
 2 files changed, 9 insertions(+)
drivers/net/tun.c +8 -0
diff -r 8414a579e106 drivers/net/tun.c
@@ -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;
include/linux/if_tun.h +1 -0
diff -r 8414a579e106 include/linux/if_tun.h
@@ -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
author
#1 lore raw

PATCH 2/4 tun: TUNSETFEATURES to set gso features.

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 <[email protected]>'s fix to hold rtnl sem).

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   43 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/if_tun.h |    7 +++++++
 2 files changed, 50 insertions(+)
drivers/net/tun.c +49 -0
diff -r e08e6c5130ff drivers/net/tun.c
@@ -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;
include/linux/if_tun.h +7 -0
diff -r e08e6c5130ff include/linux/if_tun.h
@@ -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;
author
#2 lore raw

PATCH 2/4 tun: TUNSETFEATURES to set gso features.

In reply to Rusty Russell

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 <[email protected]>'s fix to hold rtnl sem).

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   43 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/if_tun.h |    7 +++++++
 2 files changed, 50 insertions(+)
drivers/net/tun.c +49 -0
diff -r e08e6c5130ff drivers/net/tun.c
@@ -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;
include/linux/if_tun.h +7 -0
diff -r e08e6c5130ff include/linux/if_tun.h
@@ -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;
author
#3 lore raw

PATCH 3/4 tun: Allow GSO using virtio_net_hdr

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/if_tun.h |    2 +
 2 files changed, 91 insertions(+), 1 deletion(-)
drivers/net/tun.c +91 -2
diff -r d94590c1550a drivers/net/tun.c
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <linux/nsproxy.h>
+#include <linux/virtio_net.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 

@@ -283,12 +284,24 @@ static __inline__ ssize_t tun_get_user(s
 	struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) };
 	struct sk_buff *skb;
 	size_t len = count, align = 0;
+	struct virtio_net_hdr gso = { 0 };
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) > count)
 			return -EINVAL;
 
 		if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
+			return -EFAULT;
+	}
+
+	if (tun->flags & TUN_VNET_HDR) {
+		if ((len -= sizeof(gso)) > count)
+			return -EINVAL;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
+
+		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 	}
 

@@ -322,8 +335,45 @@ static __inline__ ssize_t tun_get_user(s
 		break;
 	};
 
-	if (tun->flags & TUN_NOCHECKSUM)
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
+		pr_debug("GSO!\n");
+		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+		case VIRTIO_NET_HDR_GSO_TCPV4:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+			break;
+		case VIRTIO_NET_HDR_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+
+		if (gso.gso_type & VIRTIO_NET_HDR_GSO_ECN)
+			skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
+
+		skb_shinfo(skb)->gso_size = gso.gso_size;
+		if (skb_shinfo(skb)->gso_size == 0) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+
+		/* Header must be checked, and gso_segs computed. */
+		skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
+		skb_shinfo(skb)->gso_segs = 0;
+	}
 
 	netif_rx_ni(skb);
 	tun->dev->last_rx = jiffies;

@@ -367,6 +417,39 @@ static __inline__ ssize_t tun_put_user(s
 		if (memcpy_toiovec(iv, (void *) &pi, sizeof(pi)))
 			return -EFAULT;
 		total += sizeof(pi);
+	}
+
+	if (tun->flags & TUN_VNET_HDR) {
+		struct virtio_net_hdr gso = { 0 }; /* no info leak */
+		if ((len -= sizeof(gso)) < 0)
+			return -EINVAL;
+
+		if (skb_is_gso(skb)) {
+			struct skb_shared_info *sinfo = skb_shinfo(skb);
+
+			/* This is a hint as to how much should be linear. */
+			gso.hdr_len = skb_headlen(skb);
+			gso.gso_size = sinfo->gso_size;
+			if (sinfo->gso_type & SKB_GSO_TCPV4)
+				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
+			else if (sinfo->gso_type & SKB_GSO_TCPV6)
+				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+			else
+				BUG();
+			if (sinfo->gso_type & SKB_GSO_TCP_ECN)
+				gso.gso_type |= VIRTIO_NET_HDR_GSO_ECN;
+		} else
+			gso.gso_type = VIRTIO_NET_HDR_GSO_NONE;
+
+		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			gso.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
+			gso.csum_start = skb->csum_start - skb_headroom(skb);
+			gso.csum_offset = skb->csum_offset;
+		} /* else everything is zero */
+
+		if (unlikely(memcpy_toiovec(iv, (void *)&gso, sizeof(gso))))
+			return -EFAULT;
+		total += sizeof(gso);
 	}
 
 	len = min_t(int, skb->len, len);

@@ -583,6 +666,11 @@ static int tun_set_iff(struct net *net, 
 	else
 		tun->flags &= ~TUN_ONE_QUEUE;
 
+	if (ifr->ifr_flags & IFF_VNET_HDR)
+		tun->flags |= TUN_VNET_HDR;
+	else
+		tun->flags &= ~TUN_VNET_HDR;
+
 	file->private_data = tun;
 	tun->attached = 1;
 	get_net(dev_net(tun->dev));

@@ -669,7 +757,8 @@ static int tun_chr_ioctl(struct inode *i
 		/* 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,
+		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
+				IFF_VNET_HDR,
 				(unsigned int __user*)argp);
 	}
include/linux/if_tun.h +2 -0
diff -r d94590c1550a include/linux/if_tun.h
@@ -33,6 +33,7 @@
 #define TUN_NO_PI	0x0040
 #define TUN_ONE_QUEUE	0x0080
 #define TUN_PERSIST 	0x0100	
+#define TUN_VNET_HDR 	0x0200
 
 /* Ioctl defines */
 #define TUNSETNOCSUM  _IOW('T', 200, int) 

@@ -50,6 +51,7 @@
 #define IFF_TAP		0x0002
 #define IFF_NO_PI	0x1000
 #define IFF_ONE_QUEUE	0x2000
+#define IFF_VNET_HDR	0x4000
 
 /* Features for GSO (TUNSETOFFLOAD). */
 #define TUN_F_CSUM	0x01	/* You can hand me unchecksummed packets. */
author
#4 lore raw

PATCH 3/4 tun: Allow GSO using virtio_net_hdr

In reply to Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/if_tun.h |    2 +
 2 files changed, 91 insertions(+), 1 deletion(-)
drivers/net/tun.c +91 -2
diff -r d94590c1550a drivers/net/tun.c
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <linux/nsproxy.h>
+#include <linux/virtio_net.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 

@@ -283,12 +284,24 @@ static __inline__ ssize_t tun_get_user(s
 	struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) };
 	struct sk_buff *skb;
 	size_t len = count, align = 0;
+	struct virtio_net_hdr gso = { 0 };
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) > count)
 			return -EINVAL;
 
 		if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
+			return -EFAULT;
+	}
+
+	if (tun->flags & TUN_VNET_HDR) {
+		if ((len -= sizeof(gso)) > count)
+			return -EINVAL;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
+
+		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 	}
 

@@ -322,8 +335,45 @@ static __inline__ ssize_t tun_get_user(s
 		break;
 	};
 
-	if (tun->flags & TUN_NOCHECKSUM)
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
+		pr_debug("GSO!\n");
+		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+		case VIRTIO_NET_HDR_GSO_TCPV4:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+			break;
+		case VIRTIO_NET_HDR_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+
+		if (gso.gso_type & VIRTIO_NET_HDR_GSO_ECN)
+			skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
+
+		skb_shinfo(skb)->gso_size = gso.gso_size;
+		if (skb_shinfo(skb)->gso_size == 0) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+
+		/* Header must be checked, and gso_segs computed. */
+		skb_shinfo(skb)->gso_type |= SKB_GSO_DODGY;
+		skb_shinfo(skb)->gso_segs = 0;
+	}
 
 	netif_rx_ni(skb);
 	tun->dev->last_rx = jiffies;

@@ -367,6 +417,39 @@ static __inline__ ssize_t tun_put_user(s
 		if (memcpy_toiovec(iv, (void *) &pi, sizeof(pi)))
 			return -EFAULT;
 		total += sizeof(pi);
+	}
+
+	if (tun->flags & TUN_VNET_HDR) {
+		struct virtio_net_hdr gso = { 0 }; /* no info leak */
+		if ((len -= sizeof(gso)) < 0)
+			return -EINVAL;
+
+		if (skb_is_gso(skb)) {
+			struct skb_shared_info *sinfo = skb_shinfo(skb);
+
+			/* This is a hint as to how much should be linear. */
+			gso.hdr_len = skb_headlen(skb);
+			gso.gso_size = sinfo->gso_size;
+			if (sinfo->gso_type & SKB_GSO_TCPV4)
+				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV4;
+			else if (sinfo->gso_type & SKB_GSO_TCPV6)
+				gso.gso_type = VIRTIO_NET_HDR_GSO_TCPV6;
+			else
+				BUG();
+			if (sinfo->gso_type & SKB_GSO_TCP_ECN)
+				gso.gso_type |= VIRTIO_NET_HDR_GSO_ECN;
+		} else
+			gso.gso_type = VIRTIO_NET_HDR_GSO_NONE;
+
+		if (skb->ip_summed == CHECKSUM_PARTIAL) {
+			gso.flags = VIRTIO_NET_HDR_F_NEEDS_CSUM;
+			gso.csum_start = skb->csum_start - skb_headroom(skb);
+			gso.csum_offset = skb->csum_offset;
+		} /* else everything is zero */
+
+		if (unlikely(memcpy_toiovec(iv, (void *)&gso, sizeof(gso))))
+			return -EFAULT;
+		total += sizeof(gso);
 	}
 
 	len = min_t(int, skb->len, len);

@@ -583,6 +666,11 @@ static int tun_set_iff(struct net *net, 
 	else
 		tun->flags &= ~TUN_ONE_QUEUE;
 
+	if (ifr->ifr_flags & IFF_VNET_HDR)
+		tun->flags |= TUN_VNET_HDR;
+	else
+		tun->flags &= ~TUN_VNET_HDR;
+
 	file->private_data = tun;
 	tun->attached = 1;
 	get_net(dev_net(tun->dev));

@@ -669,7 +757,8 @@ static int tun_chr_ioctl(struct inode *i
 		/* 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,
+		return put_user(IFF_TUN | IFF_TAP | IFF_NO_PI | IFF_ONE_QUEUE |
+				IFF_VNET_HDR,
 				(unsigned int __user*)argp);
 	}
include/linux/if_tun.h +2 -0
diff -r d94590c1550a include/linux/if_tun.h
@@ -33,6 +33,7 @@
 #define TUN_NO_PI	0x0040
 #define TUN_ONE_QUEUE	0x0080
 #define TUN_PERSIST 	0x0100	
+#define TUN_VNET_HDR 	0x0200
 
 /* Ioctl defines */
 #define TUNSETNOCSUM  _IOW('T', 200, int) 

@@ -50,6 +51,7 @@
 #define IFF_TAP		0x0002
 #define IFF_NO_PI	0x1000
 #define IFF_ONE_QUEUE	0x2000
+#define IFF_VNET_HDR	0x4000
 
 /* Features for GSO (TUNSETOFFLOAD). */
 #define TUN_F_CSUM	0x01	/* You can hand me unchecksummed packets. */
author
#5 lore raw

PATCH 4/4 lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

(Might not apply cleanly to current Linus, there are other lguest things
going on, but this gives you the idea at least).

Guest -> Host 1GB TCP:
Before: Seconds 16.6282 xmit 250498 recv 3 timeout 248355
After: Seconds 9.86102 xmit 241989 recv 192014 timeout 231224

Host -> Guest 1GB TCP:
Before: Seconds 11.0831 xmit 324742 recv 1910 timeout 323429
After: Seconds 10.6626 xmit 342489 recv 24 timeout 341173

Signed-off-by Rusty Russell <[email protected]>
---
 Documentation/lguest/lguest.c |   34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)
Documentation/lguest/lguest.c +20 -14
diff -r 7358caf10bd8 Documentation/lguest/lguest.c
@@ -928,11 +928,9 @@ static void handle_net_output(int fd, st
 	while ((head = get_vq_desc(vq, iov, &out, &in)) != vq->vring.num) {
 		if (in)
 			errx(1, "Input buffers in output queue?");
-		/* Check header, but otherwise ignore it (we told the Guest we
-		 * supported no features, so it shouldn't have anything
-		 * interesting). */
-		(void)convert(&iov[0], struct virtio_net_hdr);
-		len = writev(vq->dev->fd, iov+1, out-1);
+		len = writev(vq->dev->fd, iov, out);
+		if (len < 0)
+			err(1, "Writing network packet to tun");
 		add_used_and_trigger(fd, vq, head, len);
 		num++;
 	}

@@ -949,7 +947,6 @@ static bool handle_tun_input(int fd, str
 	unsigned int head, in_num, out_num;
 	int len;
 	struct iovec iov[dev->vq->vring.num];
-	struct virtio_net_hdr *hdr;
 
 	/* First we need a network buffer from the Guests's recv virtqueue. */
 	head = get_vq_desc(dev->vq, iov, &out_num, &in_num);

@@ -970,18 +969,13 @@ static bool handle_tun_input(int fd, str
 	} else if (out_num)
 		errx(1, "Output buffers in network recv queue?");
 
-	/* First element is the header: we set it to 0 (no features). */
-	hdr = convert(&iov[0], struct virtio_net_hdr);
-	hdr->flags = 0;
-	hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
-
 	/* Read the packet from the device directly into the Guest's buffer. */
-	len = readv(dev->fd, iov+1, in_num-1);
+	len = readv(dev->fd, iov, in_num);
 	if (len <= 0)
 		err(1, "reading network");
 
 	/* Tell the Guest about the new packet. */
-	add_used_and_trigger(fd, dev->vq, head, sizeof(*hdr) + len);
+	add_used_and_trigger(fd, dev->vq, head, len);
 
 	verbose("tun input packet len %i [%02x %02x] (%s)\n", len,
 		((u8 *)iov[1].iov_base)[0], ((u8 *)iov[1].iov_base)[1],

@@ -1492,10 +1486,14 @@ static int get_tun_device(char tapif[IFN
 	 * the truth, I completely blundered my way through this code, but it
 	 * works now! */
 	netfd = open_or_die("/dev/net/tun", O_RDWR);
-	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
+	ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
 	strcpy(ifr.ifr_name, "tap%d");
 	if (ioctl(netfd, TUNSETIFF, &ifr) != 0)
 		err(1, "configuring /dev/net/tun");
+
+	if (ioctl(netfd, TUNSETFEATURES,
+		  TUN_F_CSUM|TUN_F_TSO4|TUN_F_TSO6|TUN_F_TSO_ECN) != 0)
+		err(1, "Could not set features for tun device");
 
 	/* We don't need checksums calculated for packets coming in this
 	 * device: trust us! */

@@ -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);
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);
+	add_feature(dev, VIRTIO_NET_F_MAC);
+	add_feature(dev, VIRTIO_NET_F_GUEST_TSO4);
+	add_feature(dev, VIRTIO_NET_F_GUEST_TSO6);
+	add_feature(dev, VIRTIO_NET_F_GUEST_ECN);
+	add_feature(dev, VIRTIO_NET_F_HOST_TSO4);
+	add_feature(dev, VIRTIO_NET_F_HOST_TSO6);
+	add_feature(dev, VIRTIO_NET_F_HOST_ECN);
 	set_config(dev, sizeof(conf), &conf);
 
 	/* We don't need the socket any more; setup is done. */
#6 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

Documentation/lguest/lguest.c static void setup_tun_net(char *arg) · line 1565

Quoted 6 lines · Rusty Russell
Documentation/lguest/lguest.c +2 -0
@@ -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.

Documentation/lguest/lguest.c static void setup_tun_net(char *arg) · line 1566

Quoted 1 line
Documentation/lguest/lguest.c +1 -0
+	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

#7 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell

Documentation/lguest/lguest.c static void setup_tun_net(char *arg) · line 1565

Quoted 6 lines · Rusty Russell
Documentation/lguest/lguest.c +2 -0
@@ -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.

Documentation/lguest/lguest.c static void setup_tun_net(char *arg) · line 1566

Quoted 1 line
Documentation/lguest/lguest.c +1 -0
+	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

author
#8 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

Quoted 9 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review static void setup_tun_net(char *arg) · line 1565

Quoted 6 lines · Rusty Russell
(quoted diff) +2 -0
@@ -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.

Hmm, not in the version here?

Quoted 8 lines

Review static void setup_tun_net(char *arg) · line 1566

Quoted 1 line
(quoted diff) +1 -0
+	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.

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

Here's two fixes on top of previous patch:

drivers/net/tun.c +13 -13
diff -u b/drivers/net/tun.c b/drivers/net/tun.c
@@ -298,11 +298,11 @@
 		if ((len -= sizeof(gso)) > count)
 			return -EINVAL;
 
-		if (gso.hdr_len > len)
-			return -EINVAL;
-
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
 	}
 
 	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {

@@ -324,6 +324,16 @@
 		return -EFAULT;
 	}
 
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		skb_reset_mac_header(skb);

@@ -335,16 +345,6 @@
 		break;
 	};
 
-	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (!skb_partial_csum_set(skb, gso.csum_start,
-					  gso.csum_offset)) {
-			tun->dev->stats.rx_dropped++;
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-	} else if (tun->flags & TUN_NOCHECKSUM)
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-
 	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
author
#9 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Anthony Liguori
Quoted 9 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review static void setup_tun_net(char *arg) · line 1565

Quoted 6 lines · Rusty Russell
(quoted diff) +2 -0
@@ -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.

Hmm, not in the version here?

Quoted 8 lines

Review static void setup_tun_net(char *arg) · line 1566

Quoted 1 line
(quoted diff) +1 -0
+	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.

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

Here's two fixes on top of previous patch:

drivers/net/tun.c +13 -13
diff -u b/drivers/net/tun.c b/drivers/net/tun.c
@@ -298,11 +298,11 @@
 		if ((len -= sizeof(gso)) > count)
 			return -EINVAL;
 
-		if (gso.hdr_len > len)
-			return -EINVAL;
-
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
 	}
 
 	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {

@@ -324,6 +324,16 @@
 		return -EFAULT;
 	}
 
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		skb_reset_mac_header(skb);

@@ -335,16 +345,6 @@
 		break;
 	};
 
-	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (!skb_partial_csum_set(skb, gso.csum_start,
-					  gso.csum_offset)) {
-			tun->dev->stats.rx_dropped++;
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-	} else if (tun->flags & TUN_NOCHECKSUM)
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-
 	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
#10 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

Quoted 14 lines · Rusty Russell
Quoted 11 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review static void setup_tun_net(char *arg) · line 1566

Quoted 7 lines · Rusty Russell
(quoted diff) +2 -0
@@ -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.

Quoted 2 lines

Hmm, not in the version here?

Sorry, misread apparently.

Quoted 4 lines
Quoted 4 lines

Review static void setup_tun_net(char *arg) · line 1568

Quoted 2 lines
(quoted diff) +1 -0
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);

You set this feature, but I never see the virtio-net driver acknowledge the feature.

I still don't see GUEST_CSUM ever get referenced in virtio_net.c. What's the intention of this feature bit? Could I be missing a virtio_net patch? I'm using the latest bits in Linus' tree.

Quoted 7 lines
Quoted 6 lines

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.

Quoted 5 lines

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

This still doesn't fix the problem. I can manually assign an IP address and even do netperf runs but I cannot get a dhcp address (dhclient is picky about the udp csum). Also, the RX performance is so low that I'm sure a ton of packets are getting dropped. However, this patchset is extremely promising, here are the results with KVM for TX:

w/o gso
[ 3] 0.0-10.0 sec 593 MBytes 498 Mbits/sec

w/gso
[ 5] 0.0-10.0 sec 1.86 GBytes 1.60 Gbits/sec

So that's a huge increase. Unfortunately, RX drops from 1.04 Gbits/sec to only a few hundred Kbit/sec. I'm pretty sure this is the checksumming issue.

Also, when I exit KVM, QEMU zombies and I notice:

Message from syslogd@squirrel at Jun 26 13:02:07 ...
kernel: unregister_netdevice: waiting for tap0 to become free. Usage
count = 3

Message from syslogd@squirrel at Jun 26 13:02:17 ...
kernel: unregister_netdevice: waiting for tap0 to become free. Usage
count = 0

Once the refcount drops to 0, the process exits. It looks fishy to me though.

Regards,

Anthony Liguori

Quoted 55 lines

Here's two fixes on top of previous patch:

diff -u b/drivers/net/tun.c b/drivers/net/tun.c

drivers/net/tun.c +13 -13
@@ -298,11 +298,11 @@
 		if ((len -= sizeof(gso)) > count)
 			return -EINVAL;
 
-		if (gso.hdr_len > len)
-			return -EINVAL;
-
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
 	}
 
 	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {

@@ -324,6 +324,16 @@
 		return -EFAULT;
 	}
 
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		skb_reset_mac_header(skb);

@@ -335,16 +345,6 @@
 		break;
 	};
 
-	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (!skb_partial_csum_set(skb, gso.csum_start,
-					  gso.csum_offset)) {
-			tun->dev->stats.rx_dropped++;
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-	} else if (tun->flags & TUN_NOCHECKSUM)
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-
 	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
#11 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell
Quoted 14 lines · Rusty Russell
Quoted 11 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review static void setup_tun_net(char *arg) · line 1566

Quoted 7 lines · Rusty Russell
(quoted diff) +2 -0
@@ -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.

Quoted 2 lines

Hmm, not in the version here?

Sorry, misread apparently.

Quoted 4 lines
Quoted 4 lines

Review static void setup_tun_net(char *arg) · line 1568

Quoted 2 lines
(quoted diff) +1 -0
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);

You set this feature, but I never see the virtio-net driver acknowledge the feature.

I still don't see GUEST_CSUM ever get referenced in virtio_net.c. What's the intention of this feature bit? Could I be missing a virtio_net patch? I'm using the latest bits in Linus' tree.

Quoted 9 lines
Quoted 6 lines

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.

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

This still doesn't fix the problem. I can manually assign an IP address and even do netperf runs but I cannot get a dhcp address (dhclient is picky about the udp csum). Also, the RX performance is so low that I'm sure a ton of packets are getting dropped. However, this patchset is extremely promising, here are the results with KVM for TX:

w/o gso
[ 3] 0.0-10.0 sec 593 MBytes 498 Mbits/sec

w/gso
[ 5] 0.0-10.0 sec 1.86 GBytes 1.60 Gbits/sec

So that's a huge increase. Unfortunately, RX drops from 1.04 Gbits/sec to only a few hundred Kbit/sec. I'm pretty sure this is the checksumming issue.

Also, when I exit KVM, QEMU zombies and I notice:

Message from syslogd@squirrel at Jun 26 13:02:07 ...
kernel: unregister_netdevice: waiting for tap0 to become free. Usage count = 3

Message from syslogd@squirrel at Jun 26 13:02:17 ...
kernel: unregister_netdevice: waiting for tap0 to become free. Usage count = 0

Once the refcount drops to 0, the process exits. It looks fishy to me though.

Regards,

Anthony Liguori

Quoted 55 lines

Here's two fixes on top of previous patch:

diff -u b/drivers/net/tun.c b/drivers/net/tun.c

drivers/net/tun.c +13 -13
@@ -298,11 +298,11 @@
 		if ((len -= sizeof(gso)) > count)
 			return -EINVAL;
 
-		if (gso.hdr_len > len)
-			return -EINVAL;
-
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
 	}
 
 	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {

@@ -324,6 +324,16 @@
 		return -EFAULT;
 	}
 
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		skb_reset_mac_header(skb);

@@ -335,16 +345,6 @@
 		break;
 	};
 
-	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (!skb_partial_csum_set(skb, gso.csum_start,
-					  gso.csum_offset)) {
-			tun->dev->stats.rx_dropped++;
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-	} else if (tun->flags & TUN_NOCHECKSUM)
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-
 	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
author
#12 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

Quoted 11 lines · On Friday 27 June 2008 04:16:25 Anthony Liguori wrote:
Quoted 6 lines · Rusty Russell
Quoted 5 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review

Quoted 1 line · Rusty Russell
(quoted diff) +1 -0
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);

You set this feature, but I never see the virtio-net driver acknowledge the feature.

I still don't see GUEST_CSUM ever get referenced in virtio_net.c. What's the intention of this feature bit? Could I be missing a virtio_net patch? I'm using the latest bits in Linus' tree.

It says that the guest can does csum offload. It's not advertised in the current Linus tree (it got snuck in by Mark after the large-packets patches still sitting in my tree). It should be tho, since the driver can handle it. See patch at end.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 4 lines

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

This still doesn't fix the problem. I can manually assign an IP address and even do netperf runs but I cannot get a dhcp address (dhclient is picky about the udp csum).

I'll retest this when I'm back home with my machines. Perhaps it's something to do with the csum issue.

Quoted 5 lines

Also, when I exit KVM, QEMU zombies and I notice:

Message from syslogd@squirrel at Jun 26 13:02:07 ...
kernel: unregister_netdevice: waiting for tap0 to become free. Usage
count = 3

That seems odd; there is a leak in the vringfd interface which I know about, but this looks like a GSO packet from the tun device is sticking around somehow.

Cheers,
Rusty.

Subject: virtio_net: Set VIRTIO_NET_F_GUEST_CSUM feature
Date: Fri, 13 Jun 2008 14:27:34 +0100
From: Mark McLoughlin <[email protected]>

We can handle receiving partial csums, so set the
appropriate feature bit.

Signed-off-by Mark McLoughlin <[email protected]>
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/virtio_net.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/virtio_net.c +2 -1
@@ -550,7 +550,8 @@ static struct virtio_device_id id_table[
 };
 
 static unsigned int features[] = {
-	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
+	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
+	VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
 	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
 	VIRTIO_NET_F_HOST_ECN, VIRTIO_F_NOTIFY_ON_EMPTY,
 };
author
#13 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Anthony Liguori
Quoted 11 lines · On Friday 27 June 2008 04:16:25 Anthony Liguori wrote:
Quoted 6 lines · Rusty Russell
Quoted 5 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review

Quoted 1 line · Rusty Russell
(quoted diff) +1 -0
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);

You set this feature, but I never see the virtio-net driver acknowledge the feature.

I still don't see GUEST_CSUM ever get referenced in virtio_net.c. What's the intention of this feature bit? Could I be missing a virtio_net patch? I'm using the latest bits in Linus' tree.

It says that the guest can does csum offload. It's not advertised in the current Linus tree (it got snuck in by Mark after the large-packets patches still sitting in my tree). It should be tho, since the driver can handle it. See patch at end.

Quoted 6 lines
Quoted 6 lines
Quoted 5 lines

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.

Quoted 8 lines
Quoted 4 lines

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

This still doesn't fix the problem. I can manually assign an IP address and even do netperf runs but I cannot get a dhcp address (dhclient is picky about the udp csum).

I'll retest this when I'm back home with my machines. Perhaps it's something to do with the csum issue.

Quoted 5 lines

Also, when I exit KVM, QEMU zombies and I notice:

Message from syslogd@squirrel at Jun 26 13:02:07 ...
kernel: unregister_netdevice: waiting for tap0 to become free. Usage
count = 3

That seems odd; there is a leak in the vringfd interface which I know about, but this looks like a GSO packet from the tun device is sticking around somehow.

Cheers,
Rusty.

Subject: virtio_net: Set VIRTIO_NET_F_GUEST_CSUM feature
Date: Fri, 13 Jun 2008 14:27:34 +0100
From: Mark McLoughlin <[email protected]>

We can handle receiving partial csums, so set the
appropriate feature bit.

Signed-off-by Mark McLoughlin <[email protected]>
Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/virtio_net.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/virtio_net.c +2 -1
@@ -550,7 +550,8 @@ static struct virtio_device_id id_table[
 };
 
 static unsigned int features[] = {
-	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
+	VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
+	VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
 	VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
 	VIRTIO_NET_F_HOST_ECN, VIRTIO_F_NOTIFY_ON_EMPTY,
 };
#14 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell
Quoted 33 lines · Rusty Russell
Quoted 8 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review static void setup_tun_net(char *arg) · line 1565

Quoted 6 lines · Rusty Russell
(quoted diff) +2 -0
@@ -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.

Hmm, not in the version here?

Quoted 7 lines

Review static void setup_tun_net(char *arg) · line 1566

Quoted 1 line
(quoted diff) +1 -0
+	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.

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

Here's two fixes on top of previous patch:

diff -u b/drivers/net/tun.c b/drivers/net/tun.c

drivers/net/tun.c +0 -0
@@ -298,11 +298,11 @@
 		if ((len -= sizeof(gso)) > count)
 			return -EINVAL;

drivers/net/tun.c line 306

Quoted 10 lines
drivers/net/tun.c +3 -3
 
-		if (gso.hdr_len > len)
-			return -EINVAL;
-
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
 	}

Yep, looks better now.

Quoted 26 lines
drivers/net/tun.c +10 -3
 	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {

@@ -324,6 +324,16 @@
 		return -EFAULT;
 	}
 
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		skb_reset_mac_header(skb);

@@ -335,16 +345,6 @@
 		break;
 	};
 
-	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (!skb_partial_csum_set(skb, gso.csum_start,
-					  gso.csum_offset)) {

drivers/net/tun.c line 350

Quoted 10 lines
drivers/net/tun.c +0 -7
-			tun->dev->stats.rx_dropped++;
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-	} else if (tun->flags & TUN_NOCHECKSUM)
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-
 	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {

Do you want to resent the GSO patch with all the latest fixes ? ie other things (stat counters) I pointed out in the prev email.
I'll ack it.

Thanx
Max

#15 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell
Quoted 33 lines · Rusty Russell
Quoted 8 lines · On Thursday 26 June 2008 05:07:18 Anthony Liguori wrote:

Review static void setup_tun_net(char *arg) · line 1565

Quoted 6 lines · Rusty Russell
(quoted diff) +2 -0
@@ -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.

Hmm, not in the version here?

Quoted 7 lines

Review static void setup_tun_net(char *arg) · line 1566

Quoted 1 line
(quoted diff) +1 -0
+	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.

OK, found this: wrong args to skb_partial_csum_set. It was found by Mark McLoughlin before, I just lost the fix when I extracted this into a separate patch. I chose to move the call to skb_partial_csum_set(), rather than use his fix (which assumed a tap not tun device).

Here's two fixes on top of previous patch:

diff -u b/drivers/net/tun.c b/drivers/net/tun.c

drivers/net/tun.c +0 -0
@@ -298,11 +298,11 @@
 		if ((len -= sizeof(gso)) > count)
 			return -EINVAL;

drivers/net/tun.c line 306

Quoted 10 lines
drivers/net/tun.c +3 -3
 
-		if (gso.hdr_len > len)
-			return -EINVAL;
-
 		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
 	}

Yep, looks better now.

Quoted 26 lines
drivers/net/tun.c +10 -3
 	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {

@@ -324,6 +324,16 @@
 		return -EFAULT;
 	}
 
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
+		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
 	switch (tun->flags & TUN_TYPE_MASK) {
 	case TUN_TUN_DEV:
 		skb_reset_mac_header(skb);

@@ -335,16 +345,6 @@
 		break;
 	};
 
-	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
-		if (!skb_partial_csum_set(skb, gso.csum_start,
-					  gso.csum_offset)) {

drivers/net/tun.c line 350

Quoted 10 lines
drivers/net/tun.c +0 -7
-			tun->dev->stats.rx_dropped++;
-			kfree_skb(skb);
-			return -EINVAL;
-		}
-	} else if (tun->flags & TUN_NOCHECKSUM)
-		skb->ip_summed = CHECKSUM_UNNECESSARY;
-
 	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
 		pr_debug("GSO!\n");
 		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {

Do you want to resent the GSO patch with all the latest fixes ? ie other things (stat counters) I pointed out in the prev email.
I'll ack it.

Thanx
Max

author
#16 lore raw

PATCH 4/4 lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell

(Might not apply cleanly to current Linus, there are other lguest things
going on, but this gives you the idea at least).

Guest -> Host 1GB TCP:
Before: Seconds 16.6282 xmit 250498 recv 3 timeout 248355
After: Seconds 9.86102 xmit 241989 recv 192014 timeout 231224

Host -> Guest 1GB TCP:
Before: Seconds 11.0831 xmit 324742 recv 1910 timeout 323429
After: Seconds 10.6626 xmit 342489 recv 24 timeout 341173

Signed-off-by Rusty Russell <[email protected]>
---
 Documentation/lguest/lguest.c |   34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)
Documentation/lguest/lguest.c +20 -14
diff -r 7358caf10bd8 Documentation/lguest/lguest.c
@@ -928,11 +928,9 @@ static void handle_net_output(int fd, st
 	while ((head = get_vq_desc(vq, iov, &out, &in)) != vq->vring.num) {
 		if (in)
 			errx(1, "Input buffers in output queue?");
-		/* Check header, but otherwise ignore it (we told the Guest we
-		 * supported no features, so it shouldn't have anything
-		 * interesting). */
-		(void)convert(&iov[0], struct virtio_net_hdr);
-		len = writev(vq->dev->fd, iov+1, out-1);
+		len = writev(vq->dev->fd, iov, out);
+		if (len < 0)
+			err(1, "Writing network packet to tun");
 		add_used_and_trigger(fd, vq, head, len);
 		num++;
 	}

@@ -949,7 +947,6 @@ static bool handle_tun_input(int fd, str
 	unsigned int head, in_num, out_num;
 	int len;
 	struct iovec iov[dev->vq->vring.num];
-	struct virtio_net_hdr *hdr;
 
 	/* First we need a network buffer from the Guests's recv virtqueue. */
 	head = get_vq_desc(dev->vq, iov, &out_num, &in_num);

@@ -970,18 +969,13 @@ static bool handle_tun_input(int fd, str
 	} else if (out_num)
 		errx(1, "Output buffers in network recv queue?");
 
-	/* First element is the header: we set it to 0 (no features). */
-	hdr = convert(&iov[0], struct virtio_net_hdr);
-	hdr->flags = 0;
-	hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
-
 	/* Read the packet from the device directly into the Guest's buffer. */
-	len = readv(dev->fd, iov+1, in_num-1);
+	len = readv(dev->fd, iov, in_num);
 	if (len <= 0)
 		err(1, "reading network");
 
 	/* Tell the Guest about the new packet. */
-	add_used_and_trigger(fd, dev->vq, head, sizeof(*hdr) + len);
+	add_used_and_trigger(fd, dev->vq, head, len);
 
 	verbose("tun input packet len %i [%02x %02x] (%s)\n", len,
 		((u8 *)iov[1].iov_base)[0], ((u8 *)iov[1].iov_base)[1],

@@ -1492,10 +1486,14 @@ static int get_tun_device(char tapif[IFN
 	 * the truth, I completely blundered my way through this code, but it
 	 * works now! */
 	netfd = open_or_die("/dev/net/tun", O_RDWR);
-	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
+	ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
 	strcpy(ifr.ifr_name, "tap%d");
 	if (ioctl(netfd, TUNSETIFF, &ifr) != 0)
 		err(1, "configuring /dev/net/tun");
+
+	if (ioctl(netfd, TUNSETFEATURES,
+		  TUN_F_CSUM|TUN_F_TSO4|TUN_F_TSO6|TUN_F_TSO_ECN) != 0)
+		err(1, "Could not set features for tun device");
 
 	/* We don't need checksums calculated for packets coming in this
 	 * device: trust us! */

@@ -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);
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);
+	add_feature(dev, VIRTIO_NET_F_MAC);
+	add_feature(dev, VIRTIO_NET_F_GUEST_TSO4);
+	add_feature(dev, VIRTIO_NET_F_GUEST_TSO6);
+	add_feature(dev, VIRTIO_NET_F_GUEST_ECN);
+	add_feature(dev, VIRTIO_NET_F_HOST_TSO4);
+	add_feature(dev, VIRTIO_NET_F_HOST_TSO6);
+	add_feature(dev, VIRTIO_NET_F_HOST_ECN);
 	set_config(dev, sizeof(conf), &conf);
 
 	/* We don't need the socket any more; setup is done. */
author
#17 lore raw

PATCH 4/4 lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell

(Might not apply cleanly to current Linus, there are other lguest things
going on, but this gives you the idea at least).

Guest -> Host 1GB TCP:
Before: Seconds 16.6282 xmit 250498 recv 3 timeout 248355
After: Seconds 9.86102 xmit 241989 recv 192014 timeout 231224

Host -> Guest 1GB TCP:
Before: Seconds 11.0831 xmit 324742 recv 1910 timeout 323429
After: Seconds 10.6626 xmit 342489 recv 24 timeout 341173

Signed-off-by Rusty Russell <[email protected]>
---
 Documentation/lguest/lguest.c |   34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)
Documentation/lguest/lguest.c +20 -14
diff -r 7358caf10bd8 Documentation/lguest/lguest.c
@@ -928,11 +928,9 @@ static void handle_net_output(int fd, st
 	while ((head = get_vq_desc(vq, iov, &out, &in)) != vq->vring.num) {
 		if (in)
 			errx(1, "Input buffers in output queue?");
-		/* Check header, but otherwise ignore it (we told the Guest we
-		 * supported no features, so it shouldn't have anything
-		 * interesting). */
-		(void)convert(&iov[0], struct virtio_net_hdr);
-		len = writev(vq->dev->fd, iov+1, out-1);
+		len = writev(vq->dev->fd, iov, out);
+		if (len < 0)
+			err(1, "Writing network packet to tun");
 		add_used_and_trigger(fd, vq, head, len);
 		num++;
 	}

@@ -949,7 +947,6 @@ static bool handle_tun_input(int fd, str
 	unsigned int head, in_num, out_num;
 	int len;
 	struct iovec iov[dev->vq->vring.num];
-	struct virtio_net_hdr *hdr;
 
 	/* First we need a network buffer from the Guests's recv virtqueue. */
 	head = get_vq_desc(dev->vq, iov, &out_num, &in_num);

@@ -970,18 +969,13 @@ static bool handle_tun_input(int fd, str
 	} else if (out_num)
 		errx(1, "Output buffers in network recv queue?");
 
-	/* First element is the header: we set it to 0 (no features). */
-	hdr = convert(&iov[0], struct virtio_net_hdr);
-	hdr->flags = 0;
-	hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
-
 	/* Read the packet from the device directly into the Guest's buffer. */
-	len = readv(dev->fd, iov+1, in_num-1);
+	len = readv(dev->fd, iov, in_num);
 	if (len <= 0)
 		err(1, "reading network");
 
 	/* Tell the Guest about the new packet. */
-	add_used_and_trigger(fd, dev->vq, head, sizeof(*hdr) + len);
+	add_used_and_trigger(fd, dev->vq, head, len);
 
 	verbose("tun input packet len %i [%02x %02x] (%s)\n", len,
 		((u8 *)iov[1].iov_base)[0], ((u8 *)iov[1].iov_base)[1],

@@ -1492,10 +1486,14 @@ static int get_tun_device(char tapif[IFN
 	 * the truth, I completely blundered my way through this code, but it
 	 * works now! */
 	netfd = open_or_die("/dev/net/tun", O_RDWR);
-	ifr.ifr_flags = IFF_TAP | IFF_NO_PI;
+	ifr.ifr_flags = IFF_TAP | IFF_NO_PI | IFF_VNET_HDR;
 	strcpy(ifr.ifr_name, "tap%d");
 	if (ioctl(netfd, TUNSETIFF, &ifr) != 0)
 		err(1, "configuring /dev/net/tun");
+
+	if (ioctl(netfd, TUNSETFEATURES,
+		  TUN_F_CSUM|TUN_F_TSO4|TUN_F_TSO6|TUN_F_TSO_ECN) != 0)
+		err(1, "Could not set features for tun device");
 
 	/* We don't need checksums calculated for packets coming in this
 	 * device: trust us! */

@@ -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);
+	add_feature(dev, VIRTIO_NET_F_GUEST_CSUM);
+	add_feature(dev, VIRTIO_NET_F_MAC);
+	add_feature(dev, VIRTIO_NET_F_GUEST_TSO4);
+	add_feature(dev, VIRTIO_NET_F_GUEST_TSO6);
+	add_feature(dev, VIRTIO_NET_F_GUEST_ECN);
+	add_feature(dev, VIRTIO_NET_F_HOST_TSO4);
+	add_feature(dev, VIRTIO_NET_F_HOST_TSO6);
+	add_feature(dev, VIRTIO_NET_F_HOST_ECN);
 	set_config(dev, sizeof(conf), &conf);
 
 	/* We don't need the socket any more; setup is done. */
author
#18 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

Quoted 2 lines · On Thursday 26 June 2008 00:32:12 Rusty Russell wrote:

(Might not apply cleanly to current Linus, there are other lguest things
going on, but this gives you the idea at least).

...

Documentation/lguest/lguest.c static int get_tun_device(char tapif[IFN · line 1496

Quoted 3 lines
Documentation/lguest/lguest.c +3 -0
+	if (ioctl(netfd, TUNSETFEATURES,
+		  TUN_F_CSUM|TUN_F_TSO4|TUN_F_TSO6|TUN_F_TSO_ECN) != 0)
+		err(1, "Could not set features for tun device");

This should be TUNSETOFFLOAD of course. I renamed it at the last minute, because TUNSETFEATURES implies symmetry with TUNGETFEATURES, which is completely different.

Cheers,
Rusty.

author
#19 lore raw

Re: [PATCH 4/4] lguest: Use GSO/IFF_VNET_HDR extensions on tun/tap

In reply to Rusty Russell
Quoted 2 lines · On Thursday 26 June 2008 00:32:12 Rusty Russell wrote:

(Might not apply cleanly to current Linus, there are other lguest things
going on, but this gives you the idea at least).

...

Documentation/lguest/lguest.c static int get_tun_device(char tapif[IFN · line 1496

Quoted 3 lines
Documentation/lguest/lguest.c +3 -0
+	if (ioctl(netfd, TUNSETFEATURES,
+		  TUN_F_CSUM|TUN_F_TSO4|TUN_F_TSO6|TUN_F_TSO_ECN) != 0)
+		err(1, "Could not set features for tun device");

This should be TUNSETOFFLOAD of course. I renamed it at the last minute, because TUNSETFEATURES implies symmetry with TUNGETFEATURES, which is completely different.

Cheers,
Rusty.

#20 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

In reply to Rusty Russell
Quoted 34 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/if_tun.h |    2 +
 2 files changed, 91 insertions(+), 1 deletion(-)

diff -r d94590c1550a drivers/net/tun.c

drivers/net/tun.c +5 -0
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <linux/nsproxy.h>
+#include <linux/virtio_net.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 

@@ -283,12 +284,24 @@ static __inline__ ssize_t tun_get_user(s
 	struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) };
 	struct sk_buff *skb;
 	size_t len = count, align = 0;
+	struct virtio_net_hdr gso = { 0 };
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) > count)
 			return -EINVAL;
 
 		if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
+			return -EFAULT;
+	}
+

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 306

Quoted 10 lines
drivers/net/tun.c +8 -0
+	if (tun->flags & TUN_VNET_HDR) {
+		if ((len -= sizeof(gso)) > count)
+			return -EINVAL;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
+
+		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 	}

Unless I'm missing something the 'if (gso.hdr_len > len)' must be after memcpy_fromiovec().

Quoted 19 lines
drivers/net/tun.c +13 -1
@@ -322,8 +335,45 @@ static __inline__ ssize_t tun_get_user(s
 		break;
 	};
 
-	if (tun->flags & TUN_NOCHECKSUM)
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
+		pr_debug("GSO!\n");
+		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+		case VIRTIO_NET_HDR_GSO_TCPV4:

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 361

Quoted 10 lines
drivers/net/tun.c +10 -0
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+			break;
+		case VIRTIO_NET_HDR_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}

We should use stats.rx_frame_errors instead of stats.rx_dropped to indicated that we dropped it because something was wrong with the framing (headers, etc). Applies to both of the cases above.

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 371

Quoted 10 lines
drivers/net/tun.c +10 -0
+
+		if (gso.gso_type & VIRTIO_NET_HDR_GSO_ECN)
+			skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
+
+		skb_shinfo(skb)->gso_size = gso.gso_size;
+		if (skb_shinfo(skb)->gso_size == 0) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}

Same here.

Everything else looks good.

Max

author
#21 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

Quoted 42 lines · On Wednesday 02 July 2008 15:13:59 Max Krasnyansky wrote:
Quoted 41 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   90
++++++++++++++++++++++++++++++++++++++++++++++++- include/linux/if_tun.h
|    2 +
 2 files changed, 91 insertions(+), 1 deletion(-)

diff -r d94590c1550a drivers/net/tun.c

drivers/net/tun.c +10 -0
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <linux/nsproxy.h>
+#include <linux/virtio_net.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 

@@ -283,12 +284,24 @@ static __inline__ ssize_t tun_get_user(s
 	struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) };
 	struct sk_buff *skb;
 	size_t len = count, align = 0;
+	struct virtio_net_hdr gso = { 0 };
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) > count)
 			return -EINVAL;
 
 		if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
+			return -EFAULT;
+	}
+
+	if (tun->flags & TUN_VNET_HDR) {
+		if ((len -= sizeof(gso)) > count)
+			return -EINVAL;
+
+		if (gso.hdr_len > len)
Quoted 8 lines

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 306

Quoted 5 lines
drivers/net/tun.c +3 -0
+			return -EINVAL;
+
+		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 	}

Unless I'm missing something the 'if (gso.hdr_len > len)' must be after memcpy_fromiovec().

Yes, this was fixed in a followup... there was another bug picked up by markmc too in this patch.

Quoted 4 lines
Quoted 4 lines
drivers/net/tun.c +4 -0
+		case VIRTIO_NET_HDR_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
Quoted 8 lines

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 344

Quoted 4 lines
drivers/net/tun.c +4 -0
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}

We should use stats.rx_frame_errors instead of stats.rx_dropped to indicated that we dropped it because something was wrong with the framing (headers, etc). Applies to both of the cases above.

OK, done (all three). Will repost.

Thanks,
Rusty.

author
#22 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

In reply to Max Krasnyansky
Quoted 42 lines · On Wednesday 02 July 2008 15:13:59 Max Krasnyansky wrote:
Quoted 41 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   90
++++++++++++++++++++++++++++++++++++++++++++++++- include/linux/if_tun.h
|    2 +
 2 files changed, 91 insertions(+), 1 deletion(-)

diff -r d94590c1550a drivers/net/tun.c

drivers/net/tun.c +10 -0
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <linux/nsproxy.h>
+#include <linux/virtio_net.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 

@@ -283,12 +284,24 @@ static __inline__ ssize_t tun_get_user(s
 	struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) };
 	struct sk_buff *skb;
 	size_t len = count, align = 0;
+	struct virtio_net_hdr gso = { 0 };
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) > count)
 			return -EINVAL;
 
 		if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
+			return -EFAULT;
+	}
+
+	if (tun->flags & TUN_VNET_HDR) {
+		if ((len -= sizeof(gso)) > count)
+			return -EINVAL;
+
+		if (gso.hdr_len > len)
Quoted 8 lines

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 306

Quoted 5 lines
drivers/net/tun.c +3 -0
+			return -EINVAL;
+
+		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 	}

Unless I'm missing something the 'if (gso.hdr_len > len)' must be after memcpy_fromiovec().

Yes, this was fixed in a followup... there was another bug picked up by markmc too in this patch.

Quoted 4 lines
Quoted 4 lines
drivers/net/tun.c +4 -0
+		case VIRTIO_NET_HDR_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
Quoted 8 lines

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 344

Quoted 4 lines
drivers/net/tun.c +4 -0
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}

We should use stats.rx_frame_errors instead of stats.rx_dropped to indicated that we dropped it because something was wrong with the framing (headers, etc). Applies to both of the cases above.

OK, done (all three). Will repost.

Thanks,
Rusty.

#23 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

In reply to Rusty Russell
Quoted 34 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>
---
 drivers/net/tun.c      |   90 ++++++++++++++++++++++++++++++++++++++++++++++++-
 include/linux/if_tun.h |    2 +
 2 files changed, 91 insertions(+), 1 deletion(-)

diff -r d94590c1550a drivers/net/tun.c

drivers/net/tun.c +5 -0
@@ -63,6 +63,7 @@
 #include <linux/if_tun.h>
 #include <linux/crc32.h>
 #include <linux/nsproxy.h>
+#include <linux/virtio_net.h>
 #include <net/net_namespace.h>
 #include <net/netns/generic.h>
 

@@ -283,12 +284,24 @@ static __inline__ ssize_t tun_get_user(s
 	struct tun_pi pi = { 0, __constant_htons(ETH_P_IP) };
 	struct sk_buff *skb;
 	size_t len = count, align = 0;
+	struct virtio_net_hdr gso = { 0 };
 
 	if (!(tun->flags & TUN_NO_PI)) {
 		if ((len -= sizeof(pi)) > count)
 			return -EINVAL;
 
 		if(memcpy_fromiovec((void *)&pi, iv, sizeof(pi)))
+			return -EFAULT;
+	}
+

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 306

Quoted 10 lines
drivers/net/tun.c +8 -0
+	if (tun->flags & TUN_VNET_HDR) {
+		if ((len -= sizeof(gso)) > count)
+			return -EINVAL;
+
+		if (gso.hdr_len > len)
+			return -EINVAL;
+
+		if (memcpy_fromiovec((void *)&gso, iv, sizeof(gso)))
 			return -EFAULT;
 	}

Unless I'm missing something the 'if (gso.hdr_len > len)' must be after memcpy_fromiovec().

Quoted 19 lines
drivers/net/tun.c +13 -1
@@ -322,8 +335,45 @@ static __inline__ ssize_t tun_get_user(s
 		break;
 	};
 
-	if (tun->flags & TUN_NOCHECKSUM)
+	if (gso.flags & VIRTIO_NET_HDR_F_NEEDS_CSUM) {
+		if (!skb_partial_csum_set(skb, gso.csum_start,
+					  gso.csum_offset)) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}
+	} else if (tun->flags & TUN_NOCHECKSUM)
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
+
+	if (gso.gso_type != VIRTIO_NET_HDR_GSO_NONE) {
+		pr_debug("GSO!\n");
+		switch (gso.gso_type & ~VIRTIO_NET_HDR_GSO_ECN) {
+		case VIRTIO_NET_HDR_GSO_TCPV4:

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 361

Quoted 10 lines
drivers/net/tun.c +10 -0
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV4;
+			break;
+		case VIRTIO_NET_HDR_GSO_TCPV6:
+			skb_shinfo(skb)->gso_type = SKB_GSO_TCPV6;
+			break;
+		default:
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}

We should use stats.rx_frame_errors instead of stats.rx_dropped to indicated that we dropped it because something was wrong with the framing (headers, etc). Applies to both of the cases above.

drivers/net/tun.c static __inline__ ssize_t tun_get_user(s · line 371

Quoted 10 lines
drivers/net/tun.c +10 -0
+
+		if (gso.gso_type & VIRTIO_NET_HDR_GSO_ECN)
+			skb_shinfo(skb)->gso_type |= SKB_GSO_TCP_ECN;
+
+		skb_shinfo(skb)->gso_size = gso.gso_size;
+		if (skb_shinfo(skb)->gso_size == 0) {
+			tun->dev->stats.rx_dropped++;
+			kfree_skb(skb);
+			return -EINVAL;
+		}

Same here.

Everything else looks good.

Max

#24 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

In reply to Rusty Russell
Quoted 4 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>

I just noticed that we still allocate a linear skb even when GSO is enabled. Please fix this by allocating page frags where necessary. Otherwise GSO is only going to work before memory fragmentation sets in.

IIRC I'd sent out a patch to the virt mailing list with code that did this.

Thanks,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
#25 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

In reply to Rusty Russell
Quoted 4 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>

I just noticed that we still allocate a linear skb even when GSO is enabled. Please fix this by allocating page frags where necessary. Otherwise GSO is only going to work before memory fragmentation sets in.

IIRC I'd sent out a patch to the virt mailing list with code that did this.

Thanks,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#26 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

Quoted 12 lines · On Friday 25 July 2008 00:20:44 Herbert Xu wrote:
Quoted 5 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>

I just noticed that we still allocate a linear skb even when GSO is enabled. Please fix this by allocating page frags where necessary. Otherwise GSO is only going to work before memory fragmentation sets in.

Quoted 4 lines

IIRC I'd sent out a patch to the virt mailing list with code that did this.

Thanks,

Hi Herbert,

Thanks for the reminder. I have this code in the virtio_net patches which
Linus failed to pull. Will just need to share it with the tun code (maybe a skb_alloc_large() helper).

Cheers,
Rusty.

author
#27 lore raw

Re: [PATCH 3/4] tun: Allow GSO using virtio_net_hdr

In reply to Herbert Xu
Quoted 12 lines · On Friday 25 July 2008 00:20:44 Herbert Xu wrote:
Quoted 5 lines · Rusty Russell

Add a IFF_VNET_HDR flag. This uses the same ABI as virtio_net (ie. prepending struct virtio_net_hdr to packets) to indicate GSO and checksum information.

Signed-off-by Rusty Russell <[email protected]>

I just noticed that we still allocate a linear skb even when GSO is enabled. Please fix this by allocating page frags where necessary. Otherwise GSO is only going to work before memory fragmentation sets in.

Quoted 4 lines

IIRC I'd sent out a patch to the virt mailing list with code that did this.

Thanks,

Hi Herbert,

Thanks for the reminder. I have this code in the virtio_net patches which
Linus failed to pull. Will just need to share it with the tun code (maybe a skb_alloc_large() helper).

Cheers,
Rusty.

Acked-by on v1 2/4
#28 lore raw

Re: [PATCH 2/4] tun: TUNSETFEATURES to set gso features.

In reply to Rusty Russell
Quoted 7 lines · Rusty Russell

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 <[email protected]>'s fix to hold rtnl sem).

Signed-off-by Rusty Russell <[email protected]>

Agree. And it's often way more convenient to muck with the TUN settings directly on the fd instead of using external tool.

Acked-by Max Krasnyansky <[email protected]>
Acked-by on v1 2/4
#29 lore raw

Re: [PATCH 2/4] tun: TUNSETFEATURES to set gso features.

In reply to Rusty Russell
Quoted 7 lines · Rusty Russell

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 <[email protected]>'s fix to hold rtnl sem).

Signed-off-by Rusty Russell <[email protected]>

Agree. And it's often way more convenient to muck with the TUN settings directly on the fd instead of using external tool.

Acked-by Max Krasnyansky <[email protected]>
Acked-by on v1 1/4
#30 lore raw
In reply to Rusty Russell
Quoted 11 lines · Rusty Russell

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 <[email protected]>
maintainer
#31 lore raw

From: Max Krasnyansky <[email protected]>
Date: Tue, 01 Jul 2008 21:59:02 -0700

Quoted 5 lines

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 <[email protected]>

I'll apply Rusty's patches after I give them a review too.

Thanks Max.

Acked-by on v1 1/4
#33 lore raw
In reply to Rusty Russell
Quoted 11 lines · Rusty Russell

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 <[email protected]>