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

[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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
 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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
>  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-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > > ---
> > >  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