[PATCH 0/3] tun: flag cleanup

[PATCH 0/3] tun: flag cleanup

[Michael S. Tsirkin]

this patchset cleans up flag handling in tun.
It logically belongs to net tree, but
I'm basing support for new virtio header on
top of it.  Similar situation will take place
with vhost patches.
Rusty, Dave, what's your take?

Merge this patch through virtio tree?
Merge both this patch and virtio 1.0 patches through vhost tree?

I'm fine with any option.

Please review/ack patches in any case.

Michael S. Tsirkin (3):
  tun: move internal flag defines out of uapi
  tun: reuse IFF_ flags internally
  tun: drop most type defines

 include/uapi/linux/if_tun.h |  14 -----
 drivers/net/tun.c           | 125 ++++++++++++++++----------------------------
 2 files changed, 46 insertions(+), 93 deletions(-)

-- 
MST

[PATCH 1/3] tun: move internal flag defines out of uapi

[Michael S. Tsirkin]

TUN_ flags are internal and never exposed
to userspace. Any application using it is almost
certainly buggy.

Move them out to tun.c, we'll remove them in follow-up patches.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/uapi/linux/if_tun.h | 14 --------------
 drivers/net/tun.c           | 14 ++++++++++++++
 2 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index e9502dd..b82c276 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -23,20 +23,6 @@
 /* Read queue size */
 #define TUN_READQ_SIZE	500
 
-/* TUN device flags */
-#define TUN_TUN_DEV 	0x0001	
-#define TUN_TAP_DEV	0x0002
-#define TUN_TYPE_MASK   0x000f
-
-#define TUN_FASYNC	0x0010
-#define TUN_NOCHECKSUM	0x0020
-#define TUN_NO_PI	0x0040
-/* This flag has no real effect */
-#define TUN_ONE_QUEUE	0x0080
-#define TUN_PERSIST 	0x0100	
-#define TUN_VNET_HDR 	0x0200
-#define TUN_TAP_MQ      0x0400
-
 /* Ioctl defines */
 #define TUNSETNOCSUM  _IOW('T', 200, int) 
 #define TUNSETDEBUG   _IOW('T', 201, int) 
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 2e18ddd..81735f5 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -103,6 +103,20 @@ do {								\
 } while (0)
 #endif
 
+/* TUN device flags */
+#define TUN_TUN_DEV 	0x0001
+#define TUN_TAP_DEV	0x0002
+#define TUN_TYPE_MASK   0x000f
+
+#define TUN_FASYNC	0x0010
+#define TUN_NOCHECKSUM	0x0020
+#define TUN_NO_PI	0x0040
+/* This flag has no real effect */
+#define TUN_ONE_QUEUE	0x0080
+#define TUN_PERSIST 	0x0100
+#define TUN_VNET_HDR 	0x0200
+#define TUN_TAP_MQ      0x0400
+
 #define GOODCOPY_LEN 128
 
 #define FLT_EXACT_COUNT 8
-- 
MST

[PATCH 2/3] tun: reuse IFF_ flags internally

[Michael S. Tsirkin]

By reusing IFF_ flags for internal tun device flags,
we can get rid of a bunch of code translating back
and forth.

This cleanup exposes a bug: TUNGETFEATURES reports IFF_TUN and IFF_TAP
which aren't legal for TUNSETFEATURES (but, correctly, doesn't report
IFF_PERSIST which also isn't legal there).

I'm not fixing this bug at the moment, just in case some weird
userspace depends on it, using TUNGETFEATURES to check device type.

Follow-up patches will get rid of some of TUN_ macros,
using IFF_ directly instead.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/tun.c | 83 +++++++++++++++----------------------------------------
 1 file changed, 22 insertions(+), 61 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 81735f5..e4bd542 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -104,19 +104,23 @@ do {								\
 #endif
 
 /* TUN device flags */
-#define TUN_TUN_DEV 	0x0001
-#define TUN_TAP_DEV	0x0002
-#define TUN_TYPE_MASK   0x000f
+#define TUN_TUN_DEV 	IFF_TUN
+#define TUN_TAP_DEV	IFF_TAP
+#define TUN_TYPE_MASK   (IFF_TUN | IFF_TAP)
 
-#define TUN_FASYNC	0x0010
-#define TUN_NOCHECKSUM	0x0020
-#define TUN_NO_PI	0x0040
+/* IFF_ATTACH_QUEUE is never stored in device flags,
+ * overload it to mean fasync when stored there.
+ */
+#define TUN_FASYNC	IFF_ATTACH_QUEUE
+#define TUN_NO_PI	IFF_NO_PI
 /* This flag has no real effect */
-#define TUN_ONE_QUEUE	0x0080
-#define TUN_PERSIST 	0x0100
-#define TUN_VNET_HDR 	0x0200
-#define TUN_TAP_MQ      0x0400
+#define TUN_ONE_QUEUE	IFF_ONE_QUEUE
+#define TUN_PERSIST 	IFF_PERSIST
+#define TUN_VNET_HDR 	IFF_VNET_HDR
+#define TUN_TAP_MQ      IFF_MULTI_QUEUE
 
+#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
+		      IFF_MULTI_QUEUE)
 #define GOODCOPY_LEN 128
 
 #define FLT_EXACT_COUNT 8
@@ -1529,32 +1533,7 @@ static struct proto tun_proto = {
 
 static int tun_flags(struct tun_struct *tun)
 {
-	int flags = 0;
-
-	if (tun->flags & TUN_TUN_DEV)
-		flags |= IFF_TUN;
-	else
-		flags |= IFF_TAP;
-
-	if (tun->flags & TUN_NO_PI)
-		flags |= IFF_NO_PI;
-
-	/* This flag has no real effect.  We track the value for backwards
-	 * compatibility.
-	 */
-	if (tun->flags & TUN_ONE_QUEUE)
-		flags |= IFF_ONE_QUEUE;
-
-	if (tun->flags & TUN_VNET_HDR)
-		flags |= IFF_VNET_HDR;
-
-	if (tun->flags & TUN_TAP_MQ)
-		flags |= IFF_MULTI_QUEUE;
-
-	if (tun->flags & TUN_PERSIST)
-		flags |= IFF_PERSIST;
-
-	return flags;
+	return tun->flags & (TUN_FEATURES | IFF_PERSIST | IFF_TUN | IFF_TAP);
 }
 
 static ssize_t tun_show_flags(struct device *dev, struct device_attribute *attr,
@@ -1714,28 +1693,8 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 
 	tun_debug(KERN_INFO, tun, "tun_set_iff\n");
 
-	if (ifr->ifr_flags & IFF_NO_PI)
-		tun->flags |= TUN_NO_PI;
-	else
-		tun->flags &= ~TUN_NO_PI;
-
-	/* This flag has no real effect.  We track the value for backwards
-	 * compatibility.
-	 */
-	if (ifr->ifr_flags & IFF_ONE_QUEUE)
-		tun->flags |= TUN_ONE_QUEUE;
-	else
-		tun->flags &= ~TUN_ONE_QUEUE;
-
-	if (ifr->ifr_flags & IFF_VNET_HDR)
-		tun->flags |= TUN_VNET_HDR;
-	else
-		tun->flags &= ~TUN_VNET_HDR;
-
-	if (ifr->ifr_flags & IFF_MULTI_QUEUE)
-		tun->flags |= TUN_TAP_MQ;
-	else
-		tun->flags &= ~TUN_TAP_MQ;
+	tun->flags = (tun->flags & ~TUN_FEATURES) |
+		(ifr->ifr_flags & TUN_FEATURES);
 
 	/* Make sure persistent devices do not get stuck in
 	 * xoff state.
@@ -1898,9 +1857,11 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	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 |
-				IFF_VNET_HDR | IFF_MULTI_QUEUE,
+		 * TUNSETIFF.  Why do we report IFF_TUN and IFF_TAP which are
+		 * not legal for TUNSETIFF here?  It's probably a bug, but it
+		 * doesn't seem to be worth fixing.
+		 */
+		return put_user(IFF_TUN | IFF_TAP | TUN_FEATURES,
 				(unsigned int __user*)argp);
 	} else if (cmd == TUNSETQUEUE)
 		return tun_set_queue(file, &ifr);
-- 
MST

[PATCH 3/3] tun: drop most type defines

[Michael S. Tsirkin]

It's just as easy to use IFF_ flags directly,
there's no point in adding our own defines.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/net/tun.c | 64 ++++++++++++++++++++++++-------------------------------
 1 file changed, 28 insertions(+), 36 deletions(-)

diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index e4bd542..06683af 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -104,20 +104,12 @@ do {								\
 #endif
 
 /* TUN device flags */
-#define TUN_TUN_DEV 	IFF_TUN
-#define TUN_TAP_DEV	IFF_TAP
 #define TUN_TYPE_MASK   (IFF_TUN | IFF_TAP)
 
 /* IFF_ATTACH_QUEUE is never stored in device flags,
  * overload it to mean fasync when stored there.
  */
 #define TUN_FASYNC	IFF_ATTACH_QUEUE
-#define TUN_NO_PI	IFF_NO_PI
-/* This flag has no real effect */
-#define TUN_ONE_QUEUE	IFF_ONE_QUEUE
-#define TUN_PERSIST 	IFF_PERSIST
-#define TUN_VNET_HDR 	IFF_VNET_HDR
-#define TUN_TAP_MQ      IFF_MULTI_QUEUE
 
 #define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
 		      IFF_MULTI_QUEUE)
@@ -490,7 +482,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
 		if (tun && tun->numqueues == 0 && tun->numdisabled == 0) {
 			netif_carrier_off(tun->dev);
 
-			if (!(tun->flags & TUN_PERSIST) &&
+			if (!(tun->flags & IFF_PERSIST) &&
 			    tun->dev->reg_state == NETREG_REGISTERED)
 				unregister_netdevice(tun->dev);
 		}
@@ -541,7 +533,7 @@ static void tun_detach_all(struct net_device *dev)
 	}
 	BUG_ON(tun->numdisabled != 0);
 
-	if (tun->flags & TUN_PERSIST)
+	if (tun->flags & IFF_PERSIST)
 		module_put(THIS_MODULE);
 }
 
@@ -559,7 +551,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte
 		goto out;
 
 	err = -EBUSY;
-	if (!(tun->flags & TUN_TAP_MQ) && tun->numqueues == 1)
+	if (!(tun->flags & IFF_MULTI_QUEUE) && tun->numqueues == 1)
 		goto out;
 
 	err = -E2BIG;
@@ -938,7 +930,7 @@ static void tun_net_init(struct net_device *dev)
 	struct tun_struct *tun = netdev_priv(dev);
 
 	switch (tun->flags & TUN_TYPE_MASK) {
-	case TUN_TUN_DEV:
+	case IFF_TUN:
 		dev->netdev_ops = &tun_netdev_ops;
 
 		/* Point-to-Point TUN Device */
@@ -952,7 +944,7 @@ static void tun_net_init(struct net_device *dev)
 		dev->tx_queue_len = TUN_READQ_SIZE;  /* We prefer our own queue length */
 		break;
 
-	case TUN_TAP_DEV:
+	case IFF_TAP:
 		dev->netdev_ops = &tap_netdev_ops;
 		/* Ethernet TAP Device */
 		ether_setup(dev);
@@ -1043,7 +1035,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	int err;
 	u32 rxhash;
 
-	if (!(tun->flags & TUN_NO_PI)) {
+	if (!(tun->flags & IFF_NO_PI)) {
 		if (len < sizeof(pi))
 			return -EINVAL;
 		len -= sizeof(pi);
@@ -1053,7 +1045,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		offset += sizeof(pi);
 	}
 
-	if (tun->flags & TUN_VNET_HDR) {
+	if (tun->flags & IFF_VNET_HDR) {
 		if (len < tun->vnet_hdr_sz)
 			return -EINVAL;
 		len -= tun->vnet_hdr_sz;
@@ -1070,7 +1062,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		offset += tun->vnet_hdr_sz;
 	}
 
-	if ((tun->flags & TUN_TYPE_MASK) == TUN_TAP_DEV) {
+	if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) {
 		align += NET_IP_ALIGN;
 		if (unlikely(len < ETH_HLEN ||
 			     (gso.hdr_len && __virtio16_to_cpu(false, gso.hdr_len) < ETH_HLEN)))
@@ -1133,8 +1125,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 	}
 
 	switch (tun->flags & TUN_TYPE_MASK) {
-	case TUN_TUN_DEV:
-		if (tun->flags & TUN_NO_PI) {
+	case IFF_TUN:
+		if (tun->flags & IFF_NO_PI) {
 			switch (skb->data[0] & 0xf0) {
 			case 0x40:
 				pi.proto = htons(ETH_P_IP);
@@ -1153,7 +1145,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
 		skb->protocol = pi.proto;
 		skb->dev = tun->dev;
 		break;
-	case TUN_TAP_DEV:
+	case IFF_TAP:
 		skb->protocol = eth_type_trans(skb, tun->dev);
 		break;
 	}
@@ -1254,7 +1246,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 	ssize_t total = 0;
 	int vlan_offset = 0, copied;
 
-	if (!(tun->flags & TUN_NO_PI)) {
+	if (!(tun->flags & IFF_NO_PI)) {
 		if ((len -= sizeof(pi)) < 0)
 			return -EINVAL;
 
@@ -1268,7 +1260,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
 		total += sizeof(pi);
 	}
 
-	if (tun->flags & TUN_VNET_HDR) {
+	if (tun->flags & IFF_VNET_HDR) {
 		struct virtio_net_hdr gso = { 0 }; /* no info leak */
 		if ((len -= tun->vnet_hdr_sz) < 0)
 			return -EINVAL;
@@ -1589,7 +1581,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 			return -EINVAL;
 
 		if (!!(ifr->ifr_flags & IFF_MULTI_QUEUE) !=
-		    !!(tun->flags & TUN_TAP_MQ))
+		    !!(tun->flags & IFF_MULTI_QUEUE))
 			return -EINVAL;
 
 		if (tun_not_capable(tun))
@@ -1602,7 +1594,7 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		if (err < 0)
 			return err;
 
-		if (tun->flags & TUN_TAP_MQ &&
+		if (tun->flags & IFF_MULTI_QUEUE &&
 		    (tun->numqueues + tun->numdisabled > 1)) {
 			/* One or more queue has already been attached, no need
 			 * to initialize the device again.
@@ -1625,11 +1617,11 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
 		/* Set dev type */
 		if (ifr->ifr_flags & IFF_TUN) {
 			/* TUN device */
-			flags |= TUN_TUN_DEV;
+			flags |= IFF_TUN;
 			name = "tun%d";
 		} else if (ifr->ifr_flags & IFF_TAP) {
 			/* TAP device */
-			flags |= TUN_TAP_DEV;
+			flags |= IFF_TAP;
 			name = "tap%d";
 		} else
 			return -EINVAL;
@@ -1822,7 +1814,7 @@ static int tun_set_queue(struct file *file, struct ifreq *ifr)
 		ret = tun_attach(tun, file, false);
 	} else if (ifr->ifr_flags & IFF_DETACH_QUEUE) {
 		tun = rtnl_dereference(tfile->tun);
-		if (!tun || !(tun->flags & TUN_TAP_MQ) || tfile->detached)
+		if (!tun || !(tun->flags & IFF_MULTI_QUEUE) || tfile->detached)
 			ret = -EINVAL;
 		else
 			__tun_detach(tfile, false);
@@ -1928,12 +1920,12 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 		/* Disable/Enable persist mode. Keep an extra reference to the
 		 * module to prevent the module being unprobed.
 		 */
-		if (arg && !(tun->flags & TUN_PERSIST)) {
-			tun->flags |= TUN_PERSIST;
+		if (arg && !(tun->flags & IFF_PERSIST)) {
+			tun->flags |= IFF_PERSIST;
 			__module_get(THIS_MODULE);
 		}
-		if (!arg && (tun->flags & TUN_PERSIST)) {
-			tun->flags &= ~TUN_PERSIST;
+		if (!arg && (tun->flags & IFF_PERSIST)) {
+			tun->flags &= ~IFF_PERSIST;
 			module_put(THIS_MODULE);
 		}
 
@@ -1991,7 +1983,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	case TUNSETTXFILTER:
 		/* Can be set only for TAPs */
 		ret = -EINVAL;
-		if ((tun->flags & TUN_TYPE_MASK) != TUN_TAP_DEV)
+		if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
 			break;
 		ret = update_filter(&tun->txflt, (void __user *)arg);
 		break;
@@ -2050,7 +2042,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	case TUNATTACHFILTER:
 		/* Can be set only for TAPs */
 		ret = -EINVAL;
-		if ((tun->flags & TUN_TYPE_MASK) != TUN_TAP_DEV)
+		if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
 			break;
 		ret = -EFAULT;
 		if (copy_from_user(&tun->fprog, argp, sizeof(tun->fprog)))
@@ -2062,7 +2054,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 	case TUNDETACHFILTER:
 		/* Can be set only for TAPs */
 		ret = -EINVAL;
-		if ((tun->flags & TUN_TYPE_MASK) != TUN_TAP_DEV)
+		if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
 			break;
 		ret = 0;
 		tun_detach_filter(tun, tun->numqueues);
@@ -2070,7 +2062,7 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 
 	case TUNGETFILTER:
 		ret = -EINVAL;
-		if ((tun->flags & TUN_TYPE_MASK) != TUN_TAP_DEV)
+		if ((tun->flags & TUN_TYPE_MASK) != IFF_TAP)
 			break;
 		ret = -EFAULT;
 		if (copy_to_user(argp, &tun->fprog, sizeof(tun->fprog)))
@@ -2263,10 +2255,10 @@ static void tun_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *info
 	strlcpy(info->version, DRV_VERSION, sizeof(info->version));
 
 	switch (tun->flags & TUN_TYPE_MASK) {
-	case TUN_TUN_DEV:
+	case IFF_TUN:
 		strlcpy(info->bus_info, "tun", sizeof(info->bus_info));
 		break;
-	case TUN_TAP_DEV:
+	case IFF_TAP:
 		strlcpy(info->bus_info, "tap", sizeof(info->bus_info));
 		break;
 	}
-- 
MST

Re: [PATCH 1/3] tun: move internal flag defines out of uapi

[Dan Williams]

On Wed, 2014-11-19 at 18:18 +0200, Michael S. Tsirkin wrote:
> TUN_ flags are internal and never exposed
> to userspace. Any application using it is almost
> certainly buggy.

Except for TUN_TUN_DEV and TUN_TAP_DEV and TUN_TYPE_MASK...  which we're
using (for some reason) in NetworkManager, though I'll happily convert
those to IFF_* instead.  It might be worth #defining those to their
IFF_* equivalents since their usage is not technically broken.

Dan

> Move them out to tun.c, we'll remove them in follow-up patches.
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  include/uapi/linux/if_tun.h | 14 --------------
>  drivers/net/tun.c           | 14 ++++++++++++++
>  2 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> index e9502dd..b82c276 100644
> --- a/include/uapi/linux/if_tun.h
> +++ b/include/uapi/linux/if_tun.h
> @@ -23,20 +23,6 @@
>  /* Read queue size */
>  #define TUN_READQ_SIZE	500
>  
> -/* TUN device flags */
> -#define TUN_TUN_DEV 	0x0001	
> -#define TUN_TAP_DEV	0x0002
> -#define TUN_TYPE_MASK   0x000f
> -
> -#define TUN_FASYNC	0x0010
> -#define TUN_NOCHECKSUM	0x0020
> -#define TUN_NO_PI	0x0040
> -/* This flag has no real effect */
> -#define TUN_ONE_QUEUE	0x0080
> -#define TUN_PERSIST 	0x0100	
> -#define TUN_VNET_HDR 	0x0200
> -#define TUN_TAP_MQ      0x0400
> -
>  /* Ioctl defines */
>  #define TUNSETNOCSUM  _IOW('T', 200, int) 
>  #define TUNSETDEBUG   _IOW('T', 201, int) 
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 2e18ddd..81735f5 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -103,6 +103,20 @@ do {								\
>  } while (0)
>  #endif
>  
> +/* TUN device flags */
> +#define TUN_TUN_DEV 	0x0001
> +#define TUN_TAP_DEV	0x0002
> +#define TUN_TYPE_MASK   0x000f
> +
> +#define TUN_FASYNC	0x0010
> +#define TUN_NOCHECKSUM	0x0020
> +#define TUN_NO_PI	0x0040
> +/* This flag has no real effect */
> +#define TUN_ONE_QUEUE	0x0080
> +#define TUN_PERSIST 	0x0100
> +#define TUN_VNET_HDR 	0x0200
> +#define TUN_TAP_MQ      0x0400
> +
>  #define GOODCOPY_LEN 128
>  
>  #define FLT_EXACT_COUNT 8

Re: [PATCH 1/3] tun: move internal flag defines out of uapi

[Michael S. Tsirkin]

On Wed, Nov 19, 2014 at 10:47:14AM -0600, Dan Williams wrote:
> On Wed, 2014-11-19 at 18:18 +0200, Michael S. Tsirkin wrote:
> > TUN_ flags are internal and never exposed
> > to userspace. Any application using it is almost
> > certainly buggy.
> 
> Except for TUN_TUN_DEV and TUN_TAP_DEV and TUN_TYPE_MASK...  which we're
> using (for some reason) in NetworkManager, though I'll happily convert
> those to IFF_* instead.  It might be worth #defining those to their
> IFF_* equivalents since their usage is not technically broken.
> 
> Dan

Hmm you are right, they happen to have the same value.
I'll send v2 leaving these in place.


> > Move them out to tun.c, we'll remove them in follow-up patches.
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  include/uapi/linux/if_tun.h | 14 --------------
> >  drivers/net/tun.c           | 14 ++++++++++++++
> >  2 files changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> > index e9502dd..b82c276 100644
> > --- a/include/uapi/linux/if_tun.h
> > +++ b/include/uapi/linux/if_tun.h
> > @@ -23,20 +23,6 @@
> >  /* Read queue size */
> >  #define TUN_READQ_SIZE	500
> >  
> > -/* TUN device flags */
> > -#define TUN_TUN_DEV 	0x0001	
> > -#define TUN_TAP_DEV	0x0002
> > -#define TUN_TYPE_MASK   0x000f
> > -
> > -#define TUN_FASYNC	0x0010
> > -#define TUN_NOCHECKSUM	0x0020
> > -#define TUN_NO_PI	0x0040
> > -/* This flag has no real effect */
> > -#define TUN_ONE_QUEUE	0x0080
> > -#define TUN_PERSIST 	0x0100	
> > -#define TUN_VNET_HDR 	0x0200
> > -#define TUN_TAP_MQ      0x0400
> > -
> >  /* Ioctl defines */
> >  #define TUNSETNOCSUM  _IOW('T', 200, int) 
> >  #define TUNSETDEBUG   _IOW('T', 201, int) 
> > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > index 2e18ddd..81735f5 100644
> > --- a/drivers/net/tun.c
> > +++ b/drivers/net/tun.c
> > @@ -103,6 +103,20 @@ do {								\
> >  } while (0)
> >  #endif
> >  
> > +/* TUN device flags */
> > +#define TUN_TUN_DEV 	0x0001
> > +#define TUN_TAP_DEV	0x0002
> > +#define TUN_TYPE_MASK   0x000f
> > +
> > +#define TUN_FASYNC	0x0010
> > +#define TUN_NOCHECKSUM	0x0020
> > +#define TUN_NO_PI	0x0040
> > +/* This flag has no real effect */
> > +#define TUN_ONE_QUEUE	0x0080
> > +#define TUN_PERSIST 	0x0100
> > +#define TUN_VNET_HDR 	0x0200
> > +#define TUN_TAP_MQ      0x0400
> > +
> >  #define GOODCOPY_LEN 128
> >  
> >  #define FLT_EXACT_COUNT 8
> 

Re: [PATCH 1/3] tun: move internal flag defines out of uapi

[Michael S. Tsirkin]

On Wed, Nov 19, 2014 at 06:50:17PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 19, 2014 at 10:47:14AM -0600, Dan Williams wrote:
> > On Wed, 2014-11-19 at 18:18 +0200, Michael S. Tsirkin wrote:
> > > TUN_ flags are internal and never exposed
> > > to userspace. Any application using it is almost
> > > certainly buggy.
> > 
> > Except for TUN_TUN_DEV and TUN_TAP_DEV and TUN_TYPE_MASK...  which we're
> > using (for some reason) in NetworkManager, though I'll happily convert
> > those to IFF_* instead.  It might be worth #defining those to their
> > IFF_* equivalents since their usage is not technically broken.
> > 
> > Dan
> 
> Hmm you are right, they happen to have the same value.
> I'll send v2 leaving these in place.
> 

Though I do think userspace shouldn't depend on them generally,
so it might be a good idea to stop using them, even though
I'll fix up my patches to avoid breaking this usecase.



> > > Move them out to tun.c, we'll remove them in follow-up patches.
> > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > ---
> > >  include/uapi/linux/if_tun.h | 14 --------------
> > >  drivers/net/tun.c           | 14 ++++++++++++++
> > >  2 files changed, 14 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> > > index e9502dd..b82c276 100644
> > > --- a/include/uapi/linux/if_tun.h
> > > +++ b/include/uapi/linux/if_tun.h
> > > @@ -23,20 +23,6 @@
> > >  /* Read queue size */
> > >  #define TUN_READQ_SIZE	500
> > >  
> > > -/* TUN device flags */
> > > -#define TUN_TUN_DEV 	0x0001	
> > > -#define TUN_TAP_DEV	0x0002
> > > -#define TUN_TYPE_MASK   0x000f
> > > -
> > > -#define TUN_FASYNC	0x0010
> > > -#define TUN_NOCHECKSUM	0x0020
> > > -#define TUN_NO_PI	0x0040
> > > -/* This flag has no real effect */
> > > -#define TUN_ONE_QUEUE	0x0080
> > > -#define TUN_PERSIST 	0x0100	
> > > -#define TUN_VNET_HDR 	0x0200
> > > -#define TUN_TAP_MQ      0x0400
> > > -
> > >  /* Ioctl defines */
> > >  #define TUNSETNOCSUM  _IOW('T', 200, int) 
> > >  #define TUNSETDEBUG   _IOW('T', 201, int) 
> > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > index 2e18ddd..81735f5 100644
> > > --- a/drivers/net/tun.c
> > > +++ b/drivers/net/tun.c
> > > @@ -103,6 +103,20 @@ do {								\
> > >  } while (0)
> > >  #endif
> > >  
> > > +/* TUN device flags */
> > > +#define TUN_TUN_DEV 	0x0001
> > > +#define TUN_TAP_DEV	0x0002
> > > +#define TUN_TYPE_MASK   0x000f
> > > +
> > > +#define TUN_FASYNC	0x0010
> > > +#define TUN_NOCHECKSUM	0x0020
> > > +#define TUN_NO_PI	0x0040
> > > +/* This flag has no real effect */
> > > +#define TUN_ONE_QUEUE	0x0080
> > > +#define TUN_PERSIST 	0x0100
> > > +#define TUN_VNET_HDR 	0x0200
> > > +#define TUN_TAP_MQ      0x0400
> > > +
> > >  #define GOODCOPY_LEN 128
> > >  
> > >  #define FLT_EXACT_COUNT 8
> > 

Re: [PATCH 1/3] tun: move internal flag defines out of uapi

[Dan Williams]

On Wed, 2014-11-19 at 19:08 +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 19, 2014 at 06:50:17PM +0200, Michael S. Tsirkin wrote:
> > On Wed, Nov 19, 2014 at 10:47:14AM -0600, Dan Williams wrote:
> > > On Wed, 2014-11-19 at 18:18 +0200, Michael S. Tsirkin wrote:
> > > > TUN_ flags are internal and never exposed
> > > > to userspace. Any application using it is almost
> > > > certainly buggy.
> > > 
> > > Except for TUN_TUN_DEV and TUN_TAP_DEV and TUN_TYPE_MASK...  which we're
> > > using (for some reason) in NetworkManager, though I'll happily convert
> > > those to IFF_* instead.  It might be worth #defining those to their
> > > IFF_* equivalents since their usage is not technically broken.
> > > 
> > > Dan
> > 
> > Hmm you are right, they happen to have the same value.
> > I'll send v2 leaving these in place.
> > 
> 
> Though I do think userspace shouldn't depend on them generally,
> so it might be a good idea to stop using them, even though
> I'll fix up my patches to avoid breaking this usecase.

Yeah, I'm doing an NM patch right now to use IFF_*.

Dan

> 
> 
> > > > Move them out to tun.c, we'll remove them in follow-up patches.
> > > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > > > ---
> > > >  include/uapi/linux/if_tun.h | 14 --------------
> > > >  drivers/net/tun.c           | 14 ++++++++++++++
> > > >  2 files changed, 14 insertions(+), 14 deletions(-)
> > > > 
> > > > diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
> > > > index e9502dd..b82c276 100644
> > > > --- a/include/uapi/linux/if_tun.h
> > > > +++ b/include/uapi/linux/if_tun.h
> > > > @@ -23,20 +23,6 @@
> > > >  /* Read queue size */
> > > >  #define TUN_READQ_SIZE	500
> > > >  
> > > > -/* TUN device flags */
> > > > -#define TUN_TUN_DEV 	0x0001	
> > > > -#define TUN_TAP_DEV	0x0002
> > > > -#define TUN_TYPE_MASK   0x000f
> > > > -
> > > > -#define TUN_FASYNC	0x0010
> > > > -#define TUN_NOCHECKSUM	0x0020
> > > > -#define TUN_NO_PI	0x0040
> > > > -/* This flag has no real effect */
> > > > -#define TUN_ONE_QUEUE	0x0080
> > > > -#define TUN_PERSIST 	0x0100	
> > > > -#define TUN_VNET_HDR 	0x0200
> > > > -#define TUN_TAP_MQ      0x0400
> > > > -
> > > >  /* Ioctl defines */
> > > >  #define TUNSETNOCSUM  _IOW('T', 200, int) 
> > > >  #define TUNSETDEBUG   _IOW('T', 201, int) 
> > > > diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> > > > index 2e18ddd..81735f5 100644
> > > > --- a/drivers/net/tun.c
> > > > +++ b/drivers/net/tun.c
> > > > @@ -103,6 +103,20 @@ do {								\
> > > >  } while (0)
> > > >  #endif
> > > >  
> > > > +/* TUN device flags */
> > > > +#define TUN_TUN_DEV 	0x0001
> > > > +#define TUN_TAP_DEV	0x0002
> > > > +#define TUN_TYPE_MASK   0x000f
> > > > +
> > > > +#define TUN_FASYNC	0x0010
> > > > +#define TUN_NOCHECKSUM	0x0020
> > > > +#define TUN_NO_PI	0x0040
> > > > +/* This flag has no real effect */
> > > > +#define TUN_ONE_QUEUE	0x0080
> > > > +#define TUN_PERSIST 	0x0100
> > > > +#define TUN_VNET_HDR 	0x0200
> > > > +#define TUN_TAP_MQ      0x0400
> > > > +
> > > >  #define GOODCOPY_LEN 128
> > > >  
> > > >  #define FLT_EXACT_COUNT 8
> > > 
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html