[RFC VLAN 00/10]: VLAN netlink support try 2

[RFC VLAN 00/10]: VLAN netlink support try 2

[Patrick McHardy]

The last VLAN patchset was outdated, sorry about the mixup. These are the
correct patches. The iproute patch I posted was correct, so no repost of
that one.


 include/linux/if_link.h  |   25 ++
 include/linux/if_vlan.h  |    7 
 net/8021q/Makefile       |    2 
 net/8021q/vlan.c         |  537 +++++++++++++++++++++++------------------------
 net/8021q/vlan.h         |   21 +
 net/8021q/vlan_dev.c     |  170 +++++---------
 net/8021q/vlan_netlink.c |  209 ++++++++++++++++++
 net/8021q/vlanproc.c     |    4 
 8 files changed, 586 insertions(+), 389 deletions(-)

Patrick McHardy (10):
      [VLAN]: Fix off-by-ones in VLAN ID checks
      [VLAN]: Convert name-based configuration functions to struct netdevice *
      [VLAN]: Move some device intialization code to dev->init callback
      [VLAN]: Move vlan_group allocation to seperate function
      [VLAN]: Split up device checks
      [VLAN]: Move device registation to seperate function
      [VLAN]: Return proper error codes in register_vlan_device
      [VLAN]: Use 32 bit value for skb->priority mapping
      [VLAN]: Keep track of number of QoS mappings
      [VLAN]: Use rtnl_link API

[RFC VLAN 01/10]: Fix off-by-ones in VLAN ID checks

[Patrick McHardy]

[VLAN]: Fix off-by-ones in VLAN ID checks

The valid range of VLAN IDs is 0-4095, fix using ID 4095.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 576d73325b7ce3d454c544dcdfea0a2e3ae6486e
tree 2039651421302cf1863cd4b70f502d18a04bc7ac
parent 6a0492fd68c8c5f528e4277bec20bef8047aec2e
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200

 net/8021q/vlan.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index ceef57c..f3b1328 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -173,7 +173,7 @@ static struct vlan_group *__vlan_find_group(int real_dev_ifindex)
 	return NULL;
 }
 
-/*  Find the protocol handler.  Assumes VID < VLAN_VID_MASK.
+/*  Find the protocol handler.  Assumes VID <= VLAN_VID_MASK.
  *
  * Must be invoked with RCU read lock (no preempt)
  */
@@ -222,7 +222,7 @@ static int unregister_vlan_dev(struct net_device *real_dev,
 #endif
 
 	/* sanity check */
-	if (vlan_id >= VLAN_VID_MASK)
+	if (vlan_id > VLAN_VID_MASK)
 		return -EINVAL;
 
 	ASSERT_RTNL();
@@ -256,11 +256,11 @@ static int unregister_vlan_dev(struct net_device *real_dev,
 			/* If the group is now empty, kill off the
 			 * group.
 			 */
-			for (i = 0; i < VLAN_VID_MASK; i++)
+			for (i = 0; i <= VLAN_VID_MASK; i++)
 				if (vlan_group_get_device(grp, i))
 					break;
 
-			if (i == VLAN_VID_MASK) {
+			if (i > VLAN_VID_MASK) {
 				if (real_dev->features & NETIF_F_HW_VLAN_RX)
 					real_dev->vlan_rx_register(real_dev, NULL);
 
@@ -392,7 +392,7 @@ static struct net_device *register_vlan_device(const char *eth_IF_name,
 		__FUNCTION__, eth_IF_name, VLAN_ID);
 #endif
 
-	if (VLAN_ID >= VLAN_VID_MASK)
+	if (VLAN_ID > VLAN_VID_MASK)
 		goto out_ret_null;
 
 	/* find the device relating to eth_IF_name. */

[RFC VLAN 02/10]: Convert name-based configuration functions to struct netdevice *

[Patrick McHardy]

[VLAN]: Convert name-based configuration functions to struct netdevice *

Move the device lookup and checks to the ioctl handler under the RTNL and
change all name-based interfaces to take a struct net_device * instead.

This allows to use them from a netlink interface, which identifies devices
based on ifindex not name. It also avoids races between the ioctl interface
and the (upcoming) netlink interface since now all changes happen under the
RTNL.

As a nice side effect this greatly simplifies error handling in the helper
functions and fixes a number of incorrect error codes like -EINVAL for
device not found.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 9f146bc68a86cb588f3150894b3af5646cfb4fdc
tree f28cb67843e6c566b6b3923aaefc5097c1c82d58
parent 576d73325b7ce3d454c544dcdfea0a2e3ae6486e
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200

 net/8021q/vlan.c     |  152 +++++++++++++++++++++-----------------------------
 net/8021q/vlan.h     |   13 +++-
 net/8021q/vlan_dev.c |  143 +++++++++++++----------------------------------
 3 files changed, 109 insertions(+), 199 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index f3b1328..788324a 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -278,43 +278,16 @@ static int unregister_vlan_dev(struct net_device *real_dev,
 	return ret;
 }
 
-static int unregister_vlan_device(const char *vlan_IF_name)
+static int unregister_vlan_device(struct net_device *dev)
 {
-	struct net_device *dev = NULL;
 	int ret;
 
+	ret = unregister_vlan_dev(VLAN_DEV_INFO(dev)->real_dev,
+				  VLAN_DEV_INFO(dev)->vlan_id);
+	unregister_netdevice(dev);
 
-	dev = dev_get_by_name(vlan_IF_name);
-	ret = -EINVAL;
-	if (dev) {
-		if (dev->priv_flags & IFF_802_1Q_VLAN) {
-			rtnl_lock();
-
-			ret = unregister_vlan_dev(VLAN_DEV_INFO(dev)->real_dev,
-						  VLAN_DEV_INFO(dev)->vlan_id);
-
-			dev_put(dev);
-			unregister_netdevice(dev);
-
-			rtnl_unlock();
-
-			if (ret == 1)
-				ret = 0;
-		} else {
-			printk(VLAN_ERR
-			       "%s: ERROR:	Tried to remove a non-vlan device "
-			       "with VLAN code, name: %s  priv_flags: %hX\n",
-			       __FUNCTION__, dev->name, dev->priv_flags);
-			dev_put(dev);
-			ret = -EPERM;
-		}
-	} else {
-#ifdef VLAN_DEBUG
-		printk(VLAN_DBG "%s: WARNING: Could not find dev.\n", __FUNCTION__);
-#endif
-		ret = -EINVAL;
-	}
-
+	if (ret == 1)
+		ret = 0;
 	return ret;
 }
 
@@ -378,12 +351,11 @@ static struct lock_class_key vlan_netdev_xmit_lock_key;
  *  Returns the device that was created, or NULL if there was
  *  an error of some kind.
  */
-static struct net_device *register_vlan_device(const char *eth_IF_name,
+static struct net_device *register_vlan_device(struct net_device *real_dev,
 					       unsigned short VLAN_ID)
 {
 	struct vlan_group *grp;
 	struct net_device *new_dev;
-	struct net_device *real_dev; /* the ethernet device */
 	char name[IFNAMSIZ];
 	int i;
 
@@ -395,46 +367,36 @@ static struct net_device *register_vlan_device(const char *eth_IF_name,
 	if (VLAN_ID > VLAN_VID_MASK)
 		goto out_ret_null;
 
-	/* find the device relating to eth_IF_name. */
-	real_dev = dev_get_by_name(eth_IF_name);
-	if (!real_dev)
-		goto out_ret_null;
-
 	if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
 		printk(VLAN_DBG "%s: VLANs not supported on %s.\n",
 			__FUNCTION__, real_dev->name);
-		goto out_put_dev;
+		goto out_ret_null;
 	}
 
 	if ((real_dev->features & NETIF_F_HW_VLAN_RX) &&
 	    !real_dev->vlan_rx_register) {
 		printk(VLAN_DBG "%s: Device %s has buggy VLAN hw accel.\n",
 			__FUNCTION__, real_dev->name);
-		goto out_put_dev;
+		goto out_ret_null;
 	}
 
 	if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
 	    (!real_dev->vlan_rx_add_vid || !real_dev->vlan_rx_kill_vid)) {
 		printk(VLAN_DBG "%s: Device %s has buggy VLAN hw accel.\n",
 			__FUNCTION__, real_dev->name);
-		goto out_put_dev;
+		goto out_ret_null;
 	}
 
-	/* From this point on, all the data structures must remain
-	 * consistent.
-	 */
-	rtnl_lock();
-
 	/* The real device must be up and operating in order to
 	 * assosciate a VLAN device with it.
 	 */
 	if (!(real_dev->flags & IFF_UP))
-		goto out_unlock;
+		goto out_ret_null;
 
 	if (__find_vlan_dev(real_dev, VLAN_ID) != NULL) {
 		/* was already registered. */
 		printk(VLAN_DBG "%s: ALREADY had VLAN registered\n", __FUNCTION__);
-		goto out_unlock;
+		goto out_ret_null;
 	}
 
 	/* Gotta set up the fields for the device. */
@@ -471,7 +433,7 @@ static struct net_device *register_vlan_device(const char *eth_IF_name,
 			       vlan_setup);
 
 	if (new_dev == NULL)
-		goto out_unlock;
+		goto out_ret_null;
 
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "Allocated new name -:%s:-\n", new_dev->name);
@@ -577,9 +539,8 @@ static struct net_device *register_vlan_device(const char *eth_IF_name,
 	if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
 		real_dev->vlan_rx_add_vid(real_dev, VLAN_ID);
 
-	rtnl_unlock();
-
-
+	/* Account for reference in struct vlan_dev_info */
+	dev_hold(real_dev);
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "Allocated new device successfully, returning.\n");
 #endif
@@ -590,17 +551,11 @@ out_free_arrays:
 
 out_free_unregister:
 	unregister_netdev(new_dev);
-	goto out_unlock;
+	goto out_ret_null;
 
 out_free_newdev:
 	free_netdev(new_dev);
 
-out_unlock:
-	rtnl_unlock();
-
-out_put_dev:
-	dev_put(real_dev);
-
 out_ret_null:
 	return NULL;
 }
@@ -693,9 +648,10 @@ out:
  */
 static int vlan_ioctl_handler(void __user *arg)
 {
-	int err = 0;
+	int err;
 	unsigned short vid = 0;
 	struct vlan_ioctl_args args;
+	struct net_device *dev = NULL;
 
 	if (copy_from_user(&args, arg, sizeof(struct vlan_ioctl_args)))
 		return -EFAULT;
@@ -708,34 +664,59 @@ static int vlan_ioctl_handler(void __user *arg)
 	printk(VLAN_DBG "%s: args.cmd: %x\n", __FUNCTION__, args.cmd);
 #endif
 
+	rtnl_lock();
+
 	switch (args.cmd) {
 	case SET_VLAN_INGRESS_PRIORITY_CMD:
+	case SET_VLAN_EGRESS_PRIORITY_CMD:
+	case SET_VLAN_FLAG_CMD:
+	case ADD_VLAN_CMD:
+	case DEL_VLAN_CMD:
+	case GET_VLAN_REALDEV_NAME_CMD:
+	case GET_VLAN_VID_CMD:
+		err = -ENODEV;
+		dev = __dev_get_by_name(args.device1);
+		if (!dev)
+			goto out;
+
+		err = -EINVAL;
+		if (args.cmd != ADD_VLAN_CMD &&
+		    !(dev->priv_flags & IFF_802_1Q_VLAN))
+			goto out;
+	}
+
+	switch (args.cmd) {
+	case SET_VLAN_INGRESS_PRIORITY_CMD:
+		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
-		err = vlan_dev_set_ingress_priority(args.device1,
-						    args.u.skb_priority,
-						    args.vlan_qos);
+			break;
+		vlan_dev_set_ingress_priority(dev,
+					      args.u.skb_priority,
+					      args.vlan_qos);
 		break;
 
 	case SET_VLAN_EGRESS_PRIORITY_CMD:
+		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
-		err = vlan_dev_set_egress_priority(args.device1,
+			break;
+		err = vlan_dev_set_egress_priority(dev,
 						   args.u.skb_priority,
 						   args.vlan_qos);
 		break;
 
 	case SET_VLAN_FLAG_CMD:
+		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
-		err = vlan_dev_set_vlan_flag(args.device1,
+			break;
+		err = vlan_dev_set_vlan_flag(dev,
 					     args.u.flag,
 					     args.vlan_qos);
 		break;
 
 	case SET_VLAN_NAME_TYPE_CMD:
+		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
+			break;
 		if ((args.u.name_type >= 0) &&
 		    (args.u.name_type < VLAN_NAME_TYPE_HIGHEST)) {
 			vlan_name_type = args.u.name_type;
@@ -746,13 +727,10 @@ static int vlan_ioctl_handler(void __user *arg)
 		break;
 
 	case ADD_VLAN_CMD:
+		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
-		/* we have been given the name of the Ethernet Device we want to
-		 * talk to:  args.dev1	 We also have the
-		 * VLAN ID:  args.u.VID
-		 */
-		if (register_vlan_device(args.device1, args.u.VID)) {
+			break;
+		if (register_vlan_device(dev, args.u.VID)) {
 			err = 0;
 		} else {
 			err = -EINVAL;
@@ -760,12 +738,10 @@ static int vlan_ioctl_handler(void __user *arg)
 		break;
 
 	case DEL_VLAN_CMD:
+		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
-			return -EPERM;
-		/* Here, the args.dev1 is the actual VLAN we want
-		 * to get rid of.
-		 */
-		err = unregister_vlan_device(args.device1);
+			break;
+		err = unregister_vlan_device(dev);
 		break;
 
 	case GET_VLAN_INGRESS_PRIORITY_CMD:
@@ -789,9 +765,7 @@ static int vlan_ioctl_handler(void __user *arg)
 		err = -EINVAL;
 		break;
 	case GET_VLAN_REALDEV_NAME_CMD:
-		err = vlan_dev_get_realdev_name(args.device1, args.u.device2);
-		if (err)
-			goto out;
+		vlan_dev_get_realdev_name(dev, args.u.device2);
 		if (copy_to_user(arg, &args,
 				 sizeof(struct vlan_ioctl_args))) {
 			err = -EFAULT;
@@ -799,9 +773,7 @@ static int vlan_ioctl_handler(void __user *arg)
 		break;
 
 	case GET_VLAN_VID_CMD:
-		err = vlan_dev_get_vid(args.device1, &vid);
-		if (err)
-			goto out;
+		vlan_dev_get_vid(dev, &vid);
 		args.u.VID = vid;
 		if (copy_to_user(arg, &args,
 				 sizeof(struct vlan_ioctl_args))) {
@@ -813,9 +785,11 @@ static int vlan_ioctl_handler(void __user *arg)
 		/* pass on to underlying device instead?? */
 		printk(VLAN_DBG "%s: Unknown VLAN CMD: %x \n",
 			__FUNCTION__, args.cmd);
-		return -EINVAL;
+		err = -EINVAL;
+		break;
 	}
 out:
+	rtnl_unlock();
 	return err;
 }
 
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index 1976cdb..b837390 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -62,11 +62,14 @@ int vlan_dev_set_mac_address(struct net_device *dev, void* addr);
 int vlan_dev_open(struct net_device* dev);
 int vlan_dev_stop(struct net_device* dev);
 int vlan_dev_ioctl(struct net_device* dev, struct ifreq *ifr, int cmd);
-int vlan_dev_set_ingress_priority(char* dev_name, __u32 skb_prio, short vlan_prio);
-int vlan_dev_set_egress_priority(char* dev_name, __u32 skb_prio, short vlan_prio);
-int vlan_dev_set_vlan_flag(char* dev_name, __u32 flag, short flag_val);
-int vlan_dev_get_realdev_name(const char* dev_name, char* result);
-int vlan_dev_get_vid(const char* dev_name, unsigned short* result);
+void vlan_dev_set_ingress_priority(const struct net_device *dev,
+				   u32 skb_prio, short vlan_prio);
+int vlan_dev_set_egress_priority(const struct net_device *dev,
+				 u32 skb_prio, short vlan_prio);
+int vlan_dev_set_vlan_flag(const struct net_device *dev,
+			   u32 flag, short flag_val);
+void vlan_dev_get_realdev_name(const struct net_device *dev, char *result);
+void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result);
 void vlan_dev_set_multicast_list(struct net_device *vlan_dev);
 
 #endif /* !(__BEN_VLAN_802_1Q_INC__) */
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index ec46084..0b7e03e 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -534,136 +534,69 @@ int vlan_dev_change_mtu(struct net_device *dev, int new_mtu)
 	return 0;
 }
 
-int vlan_dev_set_ingress_priority(char *dev_name, __u32 skb_prio, short vlan_prio)
+void vlan_dev_set_ingress_priority(const struct net_device *dev,
+				   u32 skb_prio, short vlan_prio)
 {
-	struct net_device *dev = dev_get_by_name(dev_name);
-
-	if (dev) {
-		if (dev->priv_flags & IFF_802_1Q_VLAN) {
-			/* see if a priority mapping exists.. */
-			VLAN_DEV_INFO(dev)->ingress_priority_map[vlan_prio & 0x7] = skb_prio;
-			dev_put(dev);
-			return 0;
-		}
-
-		dev_put(dev);
-	}
-	return -EINVAL;
+	VLAN_DEV_INFO(dev)->ingress_priority_map[vlan_prio & 0x7] = skb_prio;
 }
 
-int vlan_dev_set_egress_priority(char *dev_name, __u32 skb_prio, short vlan_prio)
+int vlan_dev_set_egress_priority(const struct net_device *dev,
+				 u32 skb_prio, short vlan_prio)
 {
-	struct net_device *dev = dev_get_by_name(dev_name);
 	struct vlan_priority_tci_mapping *mp = NULL;
 	struct vlan_priority_tci_mapping *np;
 
-	if (dev) {
-		if (dev->priv_flags & IFF_802_1Q_VLAN) {
-			/* See if a priority mapping exists.. */
-			mp = VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF];
-			while (mp) {
-				if (mp->priority == skb_prio) {
-					mp->vlan_qos = ((vlan_prio << 13) & 0xE000);
-					dev_put(dev);
-					return 0;
-				}
-				mp = mp->next;
-			}
-
-			/* Create a new mapping then. */
-			mp = VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF];
-			np = kmalloc(sizeof(struct vlan_priority_tci_mapping), GFP_KERNEL);
-			if (np) {
-				np->next = mp;
-				np->priority = skb_prio;
-				np->vlan_qos = ((vlan_prio << 13) & 0xE000);
-				VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF] = np;
-				dev_put(dev);
-				return 0;
-			} else {
-				dev_put(dev);
-				return -ENOBUFS;
-			}
+	/* See if a priority mapping exists.. */
+	mp = VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF];
+	while (mp) {
+		if (mp->priority == skb_prio) {
+			mp->vlan_qos = ((vlan_prio << 13) & 0xE000);
+			return 0;
 		}
-		dev_put(dev);
+		mp = mp->next;
 	}
-	return -EINVAL;
+
+	/* Create a new mapping then. */
+	mp = VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF];
+	np = kmalloc(sizeof(struct vlan_priority_tci_mapping), GFP_KERNEL);
+	if (!np)
+		return -ENOBUFS;
+
+	np->next = mp;
+	np->priority = skb_prio;
+	np->vlan_qos = ((vlan_prio << 13) & 0xE000);
+	VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF] = np;
+	return 0;
 }
 
 /* Flags are defined in the vlan_dev_info class in include/linux/if_vlan.h file. */
-int vlan_dev_set_vlan_flag(char *dev_name, __u32 flag, short flag_val)
+int vlan_dev_set_vlan_flag(const struct net_device *dev,
+			   u32 flag, short flag_val)
 {
-	struct net_device *dev = dev_get_by_name(dev_name);
-
-	if (dev) {
-		if (dev->priv_flags & IFF_802_1Q_VLAN) {
-			/* verify flag is supported */
-			if (flag == 1) {
-				if (flag_val) {
-					VLAN_DEV_INFO(dev)->flags |= 1;
-				} else {
-					VLAN_DEV_INFO(dev)->flags &= ~1;
-				}
-				dev_put(dev);
-				return 0;
-			} else {
-				printk(KERN_ERR  "%s: flag %i is not valid.\n",
-					__FUNCTION__, (int)(flag));
-				dev_put(dev);
-				return -EINVAL;
-			}
+	/* verify flag is supported */
+	if (flag == 1) {
+		if (flag_val) {
+			VLAN_DEV_INFO(dev)->flags |= 1;
 		} else {
-			printk(KERN_ERR
-			       "%s: %s is not a vlan device, priv_flags: %hX.\n",
-			       __FUNCTION__, dev->name, dev->priv_flags);
-			dev_put(dev);
+			VLAN_DEV_INFO(dev)->flags &= ~1;
 		}
-	} else {
-		printk(KERN_ERR  "%s: Could not find device: %s\n",
-			__FUNCTION__, dev_name);
+		return 0;
 	}
-
+	printk(KERN_ERR  "%s: flag %i is not valid.\n",
+			__FUNCTION__, (int)(flag));
 	return -EINVAL;
 }
 
-
-int vlan_dev_get_realdev_name(const char *dev_name, char* result)
+void vlan_dev_get_realdev_name(const struct net_device *dev, char *result)
 {
-	struct net_device *dev = dev_get_by_name(dev_name);
-	int rv = 0;
-	if (dev) {
-		if (dev->priv_flags & IFF_802_1Q_VLAN) {
-			strncpy(result, VLAN_DEV_INFO(dev)->real_dev->name, 23);
-			rv = 0;
-		} else {
-			rv = -EINVAL;
-		}
-		dev_put(dev);
-	} else {
-		rv = -ENODEV;
-	}
-	return rv;
+	strncpy(result, VLAN_DEV_INFO(dev)->real_dev->name, 23);
 }
 
-int vlan_dev_get_vid(const char *dev_name, unsigned short* result)
+void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result)
 {
-	struct net_device *dev = dev_get_by_name(dev_name);
-	int rv = 0;
-	if (dev) {
-		if (dev->priv_flags & IFF_802_1Q_VLAN) {
-			*result = VLAN_DEV_INFO(dev)->vlan_id;
-			rv = 0;
-		} else {
-			rv = -EINVAL;
-		}
-		dev_put(dev);
-	} else {
-		rv = -ENODEV;
-	}
-	return rv;
+	*result = VLAN_DEV_INFO(dev)->vlan_id;
 }
 
-
 int vlan_dev_set_mac_address(struct net_device *dev, void *addr_struct_p)
 {
 	struct sockaddr *addr = (struct sockaddr *)(addr_struct_p);

[RFC VLAN 03/10]: Move some device intialization code to dev->init callback

[Patrick McHardy]

[VLAN]: Move some device intialization code to dev->init callback

Move some device initialization code to new dev->init callback to make
it shareable with netlink. Additionally this fixes a minor bug, dev->iflink
is set after registration, which causes an incorrect value in the initial
netlink message.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit ffed416aac143a9431c7d9d6b3214288809eb89a
tree 5c4be92ecd9fddaac6899d5a704398ba95c9bdcb
parent 9f146bc68a86cb588f3150894b3af5646cfb4fdc
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200

 net/8021q/vlan.c |   98 +++++++++++++++++++++++++++---------------------------
 1 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 788324a..7da8210 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -291,6 +291,53 @@ static int unregister_vlan_device(struct net_device *dev)
 	return ret;
 }
 
+/*
+ * vlan network devices have devices nesting below it, and are a special
+ * "super class" of normal network devices; split their locks off into a
+ * separate class since they always nest.
+ */
+static struct lock_class_key vlan_netdev_xmit_lock_key;
+
+static int vlan_dev_init(struct net_device *dev)
+{
+	struct net_device *real_dev = VLAN_DEV_INFO(dev)->real_dev;
+
+	/* IFF_BROADCAST|IFF_MULTICAST; ??? */
+	dev->flags  = real_dev->flags & ~IFF_UP;
+	dev->iflink = real_dev->ifindex;
+	dev->state  = (real_dev->state & ((1<<__LINK_STATE_NOCARRIER) |
+					  (1<<__LINK_STATE_DORMANT))) |
+		      (1<<__LINK_STATE_PRESENT);
+
+	/* need 4 bytes for extra VLAN header info,
+	 * hope the underlying device can handle it.
+	 */
+	dev->mtu = real_dev->mtu;
+
+	/* TODO: maybe just assign it to be ETHERNET? */
+	dev->type = real_dev->type;
+
+	memcpy(dev->broadcast, real_dev->broadcast, real_dev->addr_len);
+	memcpy(dev->dev_addr, real_dev->dev_addr, real_dev->addr_len);
+	dev->addr_len = real_dev->addr_len;
+
+	if (real_dev->features & NETIF_F_HW_VLAN_TX) {
+		dev->hard_header     = real_dev->hard_header;
+		dev->hard_header_len = real_dev->hard_header_len;
+		dev->hard_start_xmit = vlan_dev_hwaccel_hard_start_xmit;
+		dev->rebuild_header  = real_dev->rebuild_header;
+	} else {
+		dev->hard_header     = vlan_dev_hard_header;
+		dev->hard_header_len = real_dev->hard_header_len + VLAN_HLEN;
+		dev->hard_start_xmit = vlan_dev_hard_start_xmit;
+		dev->rebuild_header  = vlan_dev_rebuild_header;
+	}
+	dev->hard_header_parse = real_dev->hard_header_parse;
+
+	lockdep_set_class(&dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
+	return 0;
+}
+
 static void vlan_setup(struct net_device *new_dev)
 {
 	SET_MODULE_OWNER(new_dev);
@@ -311,6 +358,7 @@ static void vlan_setup(struct net_device *new_dev)
 
 	/* set up method calls */
 	new_dev->change_mtu = vlan_dev_change_mtu;
+	new_dev->init = vlan_dev_init;
 	new_dev->open = vlan_dev_open;
 	new_dev->stop = vlan_dev_stop;
 	new_dev->set_mac_address = vlan_dev_set_mac_address;
@@ -339,14 +387,6 @@ static void vlan_transfer_operstate(const struct net_device *dev, struct net_dev
 	}
 }
 
-/*
- * vlan network devices have devices nesting below it, and are a special
- * "super class" of normal network devices; split their locks off into a
- * separate class since they always nest.
- */
-static struct lock_class_key vlan_netdev_xmit_lock_key;
-
-
 /*  Attach a VLAN device to a mac address (ie Ethernet Card).
  *  Returns the device that was created, or NULL if there was
  *  an error of some kind.
@@ -437,47 +477,10 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "Allocated new name -:%s:-\n", new_dev->name);
-#endif
-	/* IFF_BROADCAST|IFF_MULTICAST; ??? */
-	new_dev->flags = real_dev->flags;
-	new_dev->flags &= ~IFF_UP;
-
-	new_dev->state = (real_dev->state & ((1<<__LINK_STATE_NOCARRIER) |
-					     (1<<__LINK_STATE_DORMANT))) |
-			 (1<<__LINK_STATE_PRESENT);
-
-	/* need 4 bytes for extra VLAN header info,
-	 * hope the underlying device can handle it.
-	 */
-	new_dev->mtu = real_dev->mtu;
-
-	/* TODO: maybe just assign it to be ETHERNET? */
-	new_dev->type = real_dev->type;
-
-	new_dev->hard_header_len = real_dev->hard_header_len;
-	if (!(real_dev->features & NETIF_F_HW_VLAN_TX)) {
-		/* Regular ethernet + 4 bytes (18 total). */
-		new_dev->hard_header_len += VLAN_HLEN;
-	}
-
 	VLAN_MEM_DBG("new_dev->priv malloc, addr: %p  size: %i\n",
 		     new_dev->priv,
 		     sizeof(struct vlan_dev_info));
-
-	memcpy(new_dev->broadcast, real_dev->broadcast, real_dev->addr_len);
-	memcpy(new_dev->dev_addr, real_dev->dev_addr, real_dev->addr_len);
-	new_dev->addr_len = real_dev->addr_len;
-
-	if (real_dev->features & NETIF_F_HW_VLAN_TX) {
-		new_dev->hard_header = real_dev->hard_header;
-		new_dev->hard_start_xmit = vlan_dev_hwaccel_hard_start_xmit;
-		new_dev->rebuild_header = real_dev->rebuild_header;
-	} else {
-		new_dev->hard_header = vlan_dev_hard_header;
-		new_dev->hard_start_xmit = vlan_dev_hard_start_xmit;
-		new_dev->rebuild_header = vlan_dev_rebuild_header;
-	}
-	new_dev->hard_header_parse = real_dev->hard_header_parse;
+#endif
 
 	VLAN_DEV_INFO(new_dev)->vlan_id = VLAN_ID; /* 1 through VLAN_VID_MASK */
 	VLAN_DEV_INFO(new_dev)->real_dev = real_dev;
@@ -492,9 +495,6 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 	if (register_netdevice(new_dev))
 		goto out_free_newdev;
 
-	lockdep_set_class(&new_dev->_xmit_lock, &vlan_netdev_xmit_lock_key);
-
-	new_dev->iflink = real_dev->ifindex;
 	vlan_transfer_operstate(real_dev, new_dev);
 	linkwatch_fire_event(new_dev); /* _MUST_ call rfc2863_policy() */
 

[RFC VLAN 04/10]: Move vlan_group allocation to seperate function

[Patrick McHardy]

[VLAN]: Move vlan_group allocation to seperate function

Move group allocation to a seperate function to clean up the code a bit
and allocate groups before registering the device. Device registration
is globally visible and causes netlink events, so we shouldn't fail
afterwards.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 77fa17d4f74fcc3a330b95023fc335713eba995a
tree d1fdc97531109725b31bf1feb27440a5af370d4a
parent ffed416aac143a9431c7d9d6b3214288809eb89a
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:44 +0200

 net/8021q/vlan.c |   78 ++++++++++++++++++++++++++++--------------------------
 1 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 7da8210..f193d78 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -197,6 +197,34 @@ static void vlan_group_free(struct vlan_group *grp)
 	kfree(grp);
 }
 
+static struct vlan_group *vlan_group_alloc(int ifindex)
+{
+	struct vlan_group *grp;
+	unsigned int size;
+	unsigned int i;
+
+	grp = kzalloc(sizeof(struct vlan_group), GFP_KERNEL);
+	if (!grp)
+		return NULL;
+
+	size = sizeof(struct net_device *) * VLAN_GROUP_ARRAY_PART_LEN;
+
+	for (i = 0; i < VLAN_GROUP_ARRAY_SPLIT_PARTS; i++) {
+		grp->vlan_devices_arrays[i] = kzalloc(size, GFP_KERNEL);
+		if (!grp->vlan_devices_arrays[i])
+			goto err;
+	}
+
+	grp->real_dev_ifindex = ifindex;
+	hlist_add_head_rcu(&grp->hlist,
+			   &vlan_group_hash[vlan_grp_hashfn(ifindex)]);
+	return grp;
+
+err:
+	vlan_group_free(grp);
+	return NULL;
+}
+
 static void vlan_rcu_free(struct rcu_head *rcu)
 {
 	vlan_group_free(container_of(rcu, struct vlan_group, rcu));
@@ -394,10 +422,9 @@ static void vlan_transfer_operstate(const struct net_device *dev, struct net_dev
 static struct net_device *register_vlan_device(struct net_device *real_dev,
 					       unsigned short VLAN_ID)
 {
-	struct vlan_group *grp;
+	struct vlan_group *grp, *ngrp = NULL;
 	struct net_device *new_dev;
 	char name[IFNAMSIZ];
-	int i;
 
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "%s: if_name -:%s:-	vid: %i\n",
@@ -491,9 +518,15 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 	printk(VLAN_DBG "About to go find the group for idx: %i\n",
 	       real_dev->ifindex);
 #endif
+	grp = __vlan_find_group(real_dev->ifindex);
+	if (!grp) {
+		ngrp = grp = vlan_group_alloc(real_dev->ifindex);
+		if (!grp)
+			goto out_free_newdev;
+	}
 
 	if (register_netdevice(new_dev))
-		goto out_free_newdev;
+		goto out_free_group;
 
 	vlan_transfer_operstate(real_dev, new_dev);
 	linkwatch_fire_event(new_dev); /* _MUST_ call rfc2863_policy() */
@@ -501,34 +534,8 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 	/* So, got the sucker initialized, now lets place
 	 * it into our local structure.
 	 */
-	grp = __vlan_find_group(real_dev->ifindex);
-
-	/* Note, we are running under the RTNL semaphore
-	 * so it cannot "appear" on us.
-	 */
-	if (!grp) { /* need to add a new group */
-		grp = kzalloc(sizeof(struct vlan_group), GFP_KERNEL);
-		if (!grp)
-			goto out_free_unregister;
-
-		for (i=0; i < VLAN_GROUP_ARRAY_SPLIT_PARTS; i++) {
-			grp->vlan_devices_arrays[i] = kzalloc(
-				sizeof(struct net_device *)*VLAN_GROUP_ARRAY_PART_LEN,
-				GFP_KERNEL);
-
-			if (!grp->vlan_devices_arrays[i])
-				goto out_free_arrays;
-		}
-
-		/* printk(KERN_ALERT "VLAN REGISTER:  Allocated new group.\n"); */
-		grp->real_dev_ifindex = real_dev->ifindex;
-
-		hlist_add_head_rcu(&grp->hlist,
-				   &vlan_group_hash[vlan_grp_hashfn(real_dev->ifindex)]);
-
-		if (real_dev->features & NETIF_F_HW_VLAN_RX)
-			real_dev->vlan_rx_register(real_dev, grp);
-	}
+	if (ngrp && real_dev->features & NETIF_F_HW_VLAN_RX)
+		real_dev->vlan_rx_register(real_dev, ngrp);
 
 	vlan_group_set_device(grp, VLAN_ID, new_dev);
 
@@ -546,12 +553,9 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 #endif
 	return new_dev;
 
-out_free_arrays:
-	vlan_group_free(grp);
-
-out_free_unregister:
-	unregister_netdev(new_dev);
-	goto out_ret_null;
+out_free_group:
+	if (ngrp)
+		vlan_group_free(ngrp);
 
 out_free_newdev:
 	free_netdev(new_dev);

[RFC VLAN 05/10]: Split up device checks

[Patrick McHardy]

[VLAN]: Split up device checks

Move the checks of the underlying device to a seperate function.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 2d952c2f38391a43f380028a42106ee8880574be
tree e877b367527691ea5abc0342bf5212fa1e97205d
parent 77fa17d4f74fcc3a330b95023fc335713eba995a
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200

 net/8021q/vlan.c |   56 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index f193d78..6233885 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -415,57 +415,65 @@ static void vlan_transfer_operstate(const struct net_device *dev, struct net_dev
 	}
 }
 
-/*  Attach a VLAN device to a mac address (ie Ethernet Card).
- *  Returns the device that was created, or NULL if there was
- *  an error of some kind.
- */
-static struct net_device *register_vlan_device(struct net_device *real_dev,
-					       unsigned short VLAN_ID)
+static int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id)
 {
-	struct vlan_group *grp, *ngrp = NULL;
-	struct net_device *new_dev;
-	char name[IFNAMSIZ];
-
-#ifdef VLAN_DEBUG
-	printk(VLAN_DBG "%s: if_name -:%s:-	vid: %i\n",
-		__FUNCTION__, eth_IF_name, VLAN_ID);
-#endif
-
-	if (VLAN_ID > VLAN_VID_MASK)
-		goto out_ret_null;
-
 	if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
 		printk(VLAN_DBG "%s: VLANs not supported on %s.\n",
 			__FUNCTION__, real_dev->name);
-		goto out_ret_null;
+		return -EOPNOTSUPP;
 	}
 
 	if ((real_dev->features & NETIF_F_HW_VLAN_RX) &&
 	    !real_dev->vlan_rx_register) {
 		printk(VLAN_DBG "%s: Device %s has buggy VLAN hw accel.\n",
 			__FUNCTION__, real_dev->name);
-		goto out_ret_null;
+		return -EOPNOTSUPP;
 	}
 
 	if ((real_dev->features & NETIF_F_HW_VLAN_FILTER) &&
 	    (!real_dev->vlan_rx_add_vid || !real_dev->vlan_rx_kill_vid)) {
 		printk(VLAN_DBG "%s: Device %s has buggy VLAN hw accel.\n",
 			__FUNCTION__, real_dev->name);
-		goto out_ret_null;
+		return -EOPNOTSUPP;
 	}
 
 	/* The real device must be up and operating in order to
 	 * assosciate a VLAN device with it.
 	 */
 	if (!(real_dev->flags & IFF_UP))
-		goto out_ret_null;
+		return -ENETDOWN;
 
-	if (__find_vlan_dev(real_dev, VLAN_ID) != NULL) {
+	if (__find_vlan_dev(real_dev, vlan_id) != NULL) {
 		/* was already registered. */
 		printk(VLAN_DBG "%s: ALREADY had VLAN registered\n", __FUNCTION__);
-		goto out_ret_null;
+		return -EEXIST;
 	}
 
+	return 0;
+}
+
+/*  Attach a VLAN device to a mac address (ie Ethernet Card).
+ *  Returns the device that was created, or NULL if there was
+ *  an error of some kind.
+ */
+static struct net_device *register_vlan_device(struct net_device *real_dev,
+					       unsigned short VLAN_ID)
+{
+	struct vlan_group *grp, *ngrp = NULL;
+	struct net_device *new_dev;
+	char name[IFNAMSIZ];
+
+#ifdef VLAN_DEBUG
+	printk(VLAN_DBG "%s: if_name -:%s:-	vid: %i\n",
+		__FUNCTION__, eth_IF_name, VLAN_ID);
+#endif
+
+	if (VLAN_ID > VLAN_VID_MASK)
+		goto out_ret_null;
+
+	if (vlan_check_real_dev(real_dev, VLAN_ID) < 0)
+		goto out_ret_null;
+
 	/* Gotta set up the fields for the device. */
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "About to allocate name, vlan_name_type: %i\n",

[RFC VLAN 06/10]: Move device registation to seperate function

[Patrick McHardy]

[VLAN]: Move device registation to seperate function

Move device registration and configuration of the underlying device to a
seperate function.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 312dc0ee04bed170ad8091f777fcc2108105ca2c
tree 34a83935bc1bfbac67cc50dc83bbdf67645dcc0d
parent 2d952c2f38391a43f380028a42106ee8880574be
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200

 net/8021q/vlan.c |   83 +++++++++++++++++++++++++++++++-----------------------
 1 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 6233885..0fa071d 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -452,6 +452,51 @@ static int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_
 	return 0;
 }
 
+static int register_vlan_dev(struct net_device *dev)
+{
+	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+	struct net_device *real_dev = vlan->real_dev;
+	unsigned short vlan_id = vlan->vlan_id;
+	struct vlan_group *grp, *ngrp = NULL;
+	int err;
+
+	grp = __vlan_find_group(real_dev->ifindex);
+	if (!grp) {
+		ngrp = grp = vlan_group_alloc(real_dev->ifindex);
+		if (!grp)
+			return -ENOBUFS;
+	}
+
+	err = register_netdevice(dev);
+	if (err < 0)
+		goto out_free_group;
+
+	/* Account for reference in struct vlan_dev_info */
+	dev_hold(real_dev);
+
+	vlan_transfer_operstate(real_dev, dev);
+	linkwatch_fire_event(dev); /* _MUST_ call rfc2863_policy() */
+
+	/* So, got the sucker initialized, now lets place
+	 * it into our local structure.
+	 */
+	vlan_group_set_device(grp, vlan_id, dev);
+	if (ngrp && real_dev->features & NETIF_F_HW_VLAN_RX)
+		real_dev->vlan_rx_register(real_dev, ngrp);
+	if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
+		real_dev->vlan_rx_add_vid(real_dev, vlan_id);
+
+	if (vlan_proc_add_dev(dev) < 0)
+		printk(KERN_WARNING "VLAN: failed to add proc entry for %s\n",
+		       dev->name);
+	return 0;
+
+out_free_group:
+	if (ngrp)
+		vlan_group_free(ngrp);
+	return err;
+}
+
 /*  Attach a VLAN device to a mac address (ie Ethernet Card).
  *  Returns the device that was created, or NULL if there was
  *  an error of some kind.
@@ -459,7 +504,6 @@ static int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_
 static struct net_device *register_vlan_device(struct net_device *real_dev,
 					       unsigned short VLAN_ID)
 {
-	struct vlan_group *grp, *ngrp = NULL;
 	struct net_device *new_dev;
 	char name[IFNAMSIZ];
 
@@ -522,37 +566,8 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 	VLAN_DEV_INFO(new_dev)->dent = NULL;
 	VLAN_DEV_INFO(new_dev)->flags = 1;
 
-#ifdef VLAN_DEBUG
-	printk(VLAN_DBG "About to go find the group for idx: %i\n",
-	       real_dev->ifindex);
-#endif
-	grp = __vlan_find_group(real_dev->ifindex);
-	if (!grp) {
-		ngrp = grp = vlan_group_alloc(real_dev->ifindex);
-		if (!grp)
-			goto out_free_newdev;
-	}
-
-	if (register_netdevice(new_dev))
-		goto out_free_group;
-
-	vlan_transfer_operstate(real_dev, new_dev);
-	linkwatch_fire_event(new_dev); /* _MUST_ call rfc2863_policy() */
-
-	/* So, got the sucker initialized, now lets place
-	 * it into our local structure.
-	 */
-	if (ngrp && real_dev->features & NETIF_F_HW_VLAN_RX)
-		real_dev->vlan_rx_register(real_dev, ngrp);
-
-	vlan_group_set_device(grp, VLAN_ID, new_dev);
-
-	if (vlan_proc_add_dev(new_dev)<0)/* create it's proc entry */
-		printk(KERN_WARNING "VLAN: failed to add proc entry for %s\n",
-							 new_dev->name);
-
-	if (real_dev->features & NETIF_F_HW_VLAN_FILTER)
-		real_dev->vlan_rx_add_vid(real_dev, VLAN_ID);
+	if (register_vlan_dev(new_dev) < 0)
+		goto out_free_newdev;
 
 	/* Account for reference in struct vlan_dev_info */
 	dev_hold(real_dev);
@@ -561,10 +576,6 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 #endif
 	return new_dev;
 
-out_free_group:
-	if (ngrp)
-		vlan_group_free(ngrp);
-
 out_free_newdev:
 	free_netdev(new_dev);
 

[RFC VLAN 07/10]: Return proper error codes in register_vlan_device

[Patrick McHardy]

[VLAN]: Return proper error codes in register_vlan_device

The returned device is unused, return proper error codes instead and avoid
having the ioctl handler guess the error.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit f2eb0828c4cfaf4d4aed225b55ca65b052109657
tree 967dc78c7ed8931a62bdaaa0e57304e75bfe91b9
parent 312dc0ee04bed170ad8091f777fcc2108105ca2c
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200

 net/8021q/vlan.c |   32 ++++++++++++++------------------
 1 files changed, 14 insertions(+), 18 deletions(-)

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 0fa071d..03b67b9 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -498,14 +498,14 @@ out_free_group:
 }
 
 /*  Attach a VLAN device to a mac address (ie Ethernet Card).
- *  Returns the device that was created, or NULL if there was
- *  an error of some kind.
+ *  Returns 0 if the device was created or a negative error code otherwise.
  */
-static struct net_device *register_vlan_device(struct net_device *real_dev,
-					       unsigned short VLAN_ID)
+static int register_vlan_device(struct net_device *real_dev,
+				unsigned short VLAN_ID)
 {
 	struct net_device *new_dev;
 	char name[IFNAMSIZ];
+	int err;
 
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "%s: if_name -:%s:-	vid: %i\n",
@@ -513,10 +513,11 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 #endif
 
 	if (VLAN_ID > VLAN_VID_MASK)
-		goto out_ret_null;
+		return -ERANGE;
 
-	if (vlan_check_real_dev(real_dev, VLAN_ID) < 0)
-		goto out_ret_null;
+	err = vlan_check_real_dev(real_dev, VLAN_ID);
+	if (err < 0)
+		return err;
 
 	/* Gotta set up the fields for the device. */
 #ifdef VLAN_DEBUG
@@ -552,7 +553,7 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 			       vlan_setup);
 
 	if (new_dev == NULL)
-		goto out_ret_null;
+		return -ENOBUFS;
 
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "Allocated new name -:%s:-\n", new_dev->name);
@@ -566,7 +567,8 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 	VLAN_DEV_INFO(new_dev)->dent = NULL;
 	VLAN_DEV_INFO(new_dev)->flags = 1;
 
-	if (register_vlan_dev(new_dev) < 0)
+	err = register_vlan_dev(new_dev);
+	if (err < 0)
 		goto out_free_newdev;
 
 	/* Account for reference in struct vlan_dev_info */
@@ -574,13 +576,11 @@ static struct net_device *register_vlan_device(struct net_device *real_dev,
 #ifdef VLAN_DEBUG
 	printk(VLAN_DBG "Allocated new device successfully, returning.\n");
 #endif
-	return new_dev;
+	return 0;
 
 out_free_newdev:
 	free_netdev(new_dev);
-
-out_ret_null:
-	return NULL;
+	return err;
 }
 
 static int vlan_device_event(struct notifier_block *unused, unsigned long event, void *ptr)
@@ -753,11 +753,7 @@ static int vlan_ioctl_handler(void __user *arg)
 		err = -EPERM;
 		if (!capable(CAP_NET_ADMIN))
 			break;
-		if (register_vlan_device(dev, args.u.VID)) {
-			err = 0;
-		} else {
-			err = -EINVAL;
-		}
+		err = register_vlan_device(dev, args.u.VID);
 		break;
 
 	case DEL_VLAN_CMD:

[RFC VLAN 08/10]: Use 32 bit value for skb->priority mapping

[Patrick McHardy]

[VLAN]: Use 32 bit value for skb->priority mapping

skb->priority has only 32 bits and even VLAN uses 32 bit values in its API.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 3151f6fdca072a91c2184192dc1d431a4d9248b7
tree 21f76dd1682e385f7d958d5d5ebb66c3585ed2ca
parent f2eb0828c4cfaf4d4aed225b55ca65b052109657
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200

 include/linux/if_vlan.h |    4 ++--
 net/8021q/vlanproc.c    |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index 81e9bc9..aeddb49 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -99,7 +99,7 @@ static inline void vlan_group_set_device(struct vlan_group *vg, int vlan_id,
 }
 
 struct vlan_priority_tci_mapping {
-	unsigned long priority;
+	u32 priority;
 	unsigned short vlan_qos; /* This should be shifted when first set, so we only do it
 				  * at provisioning time.
 				  * ((skb->priority << 13) & 0xE000)
@@ -112,7 +112,7 @@ struct vlan_dev_info {
 	/** This will be the mapping that correlates skb->priority to
 	 * 3 bits of VLAN QOS tags...
 	 */
-	unsigned long ingress_priority_map[8];
+	u32 ingress_priority_map[8];
 	struct vlan_priority_tci_mapping *egress_priority_map[16]; /* hash table */
 
 	unsigned short vlan_id;        /*  The VLAN Identifier for this interface. */
diff --git a/net/8021q/vlanproc.c b/net/8021q/vlanproc.c
index d216a64..8693b21 100644
--- a/net/8021q/vlanproc.c
+++ b/net/8021q/vlanproc.c
@@ -342,7 +342,7 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 	seq_printf(seq, "Device: %s", dev_info->real_dev->name);
 	/* now show all PRIORITY mappings relating to this VLAN */
 	seq_printf(seq,
-		       "\nINGRESS priority mappings: 0:%lu  1:%lu  2:%lu  3:%lu  4:%lu  5:%lu  6:%lu 7:%lu\n",
+		       "\nINGRESS priority mappings: 0:%u  1:%u  2:%u  3:%u  4:%u  5:%u  6:%u 7:%u\n",
 		       dev_info->ingress_priority_map[0],
 		       dev_info->ingress_priority_map[1],
 		       dev_info->ingress_priority_map[2],
@@ -357,7 +357,7 @@ static int vlandev_seq_show(struct seq_file *seq, void *offset)
 		const struct vlan_priority_tci_mapping *mp
 			= dev_info->egress_priority_map[i];
 		while (mp) {
-			seq_printf(seq, "%lu:%hu ",
+			seq_printf(seq, "%u:%hu ",
 				   mp->priority, ((mp->vlan_qos >> 13) & 0x7));
 			mp = mp->next;
 		}

[RFC VLAN 09/10]: Keep track of number of QoS mappings

[Patrick McHardy]

[VLAN]: Keep track of number of QoS mappings

Keep track of the number of configured ingress/egress QoS mappings to
avoid iteration while calculating the netlink attribute size.

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit e38a7f02fcf29d44a81cb1e697f7381a8764c28d
tree 2e24e2b1421f381c2f13bafe9a10e73c97d7ef7f
parent 3151f6fdca072a91c2184192dc1d431a4d9248b7
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200

 include/linux/if_vlan.h |    3 +++
 net/8021q/vlan_dev.c    |   27 +++++++++++++++++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/linux/if_vlan.h b/include/linux/if_vlan.h
index aeddb49..b46d422 100644
--- a/include/linux/if_vlan.h
+++ b/include/linux/if_vlan.h
@@ -112,7 +112,10 @@ struct vlan_dev_info {
 	/** This will be the mapping that correlates skb->priority to
 	 * 3 bits of VLAN QOS tags...
 	 */
+	unsigned int nr_ingress_mappings;
 	u32 ingress_priority_map[8];
+
+	unsigned int nr_egress_mappings;
 	struct vlan_priority_tci_mapping *egress_priority_map[16]; /* hash table */
 
 	unsigned short vlan_id;        /*  The VLAN Identifier for this interface. */
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 0b7e03e..1644626 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -537,35 +537,50 @@ int vlan_dev_change_mtu(struct net_device *dev, int new_mtu)
 void vlan_dev_set_ingress_priority(const struct net_device *dev,
 				   u32 skb_prio, short vlan_prio)
 {
-	VLAN_DEV_INFO(dev)->ingress_priority_map[vlan_prio & 0x7] = skb_prio;
+	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+
+	if (vlan->ingress_priority_map[vlan_prio & 0x7] && !skb_prio)
+		vlan->nr_ingress_mappings--;
+	else if (!vlan->ingress_priority_map[vlan_prio & 0x7] && skb_prio)
+		vlan->nr_ingress_mappings++;
+
+	vlan->ingress_priority_map[vlan_prio & 0x7] = skb_prio;
 }
 
 int vlan_dev_set_egress_priority(const struct net_device *dev,
 				 u32 skb_prio, short vlan_prio)
 {
+	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
 	struct vlan_priority_tci_mapping *mp = NULL;
 	struct vlan_priority_tci_mapping *np;
+	u32 vlan_qos = (vlan_prio << 13) & 0xE000;
 
 	/* See if a priority mapping exists.. */
-	mp = VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF];
+	mp = vlan->egress_priority_map[skb_prio & 0xF];
 	while (mp) {
 		if (mp->priority == skb_prio) {
-			mp->vlan_qos = ((vlan_prio << 13) & 0xE000);
+			if (mp->vlan_qos && !vlan_qos)
+				vlan->nr_egress_mappings--;
+			else if (!mp->vlan_qos && vlan_qos)
+				vlan->nr_egress_mappings++;
+			mp->vlan_qos = vlan_qos;
 			return 0;
 		}
 		mp = mp->next;
 	}
 
 	/* Create a new mapping then. */
-	mp = VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF];
+	mp = vlan->egress_priority_map[skb_prio & 0xF];
 	np = kmalloc(sizeof(struct vlan_priority_tci_mapping), GFP_KERNEL);
 	if (!np)
 		return -ENOBUFS;
 
 	np->next = mp;
 	np->priority = skb_prio;
-	np->vlan_qos = ((vlan_prio << 13) & 0xE000);
-	VLAN_DEV_INFO(dev)->egress_priority_map[skb_prio & 0xF] = np;
+	np->vlan_qos = vlan_qos;
+	vlan->egress_priority_map[skb_prio & 0xF] = np;
+	if (vlan_qos)
+		vlan->nr_egress_mappings++;
 	return 0;
 }
 

[RFC VLAN 10/10]: Use rtnl_link API

[Patrick McHardy]

[VLAN]: Use rtnl_link API

Signed-off-by: Patrick McHardy <kaber@trash.net>

---
commit 0c8500625d51c2a0c38404d941ab925c23c972b3
tree 9fbf7296bb873dc918d6865aa4437a9dfaf53237
parent e38a7f02fcf29d44a81cb1e697f7381a8764c28d
author Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200
committer Patrick McHardy <kaber@trash.net> Tue, 05 Jun 2007 16:33:45 +0200

 include/linux/if_link.h  |   25 ++++++
 net/8021q/Makefile       |    2 
 net/8021q/vlan.c         |   28 ++++--
 net/8021q/vlan.h         |    8 ++
 net/8021q/vlan_netlink.c |  209 ++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 261 insertions(+), 11 deletions(-)

diff --git a/include/linux/if_link.h b/include/linux/if_link.h
index e46ed94..4bb2d5a 100644
--- a/include/linux/if_link.h
+++ b/include/linux/if_link.h
@@ -153,4 +153,29 @@ enum
 
 #define IFLA_INFO_MAX	(__IFLA_INFO_MAX - 1)
 
+enum
+{
+	IFLA_VLAN_UNSPEC,
+	IFLA_VLAN_ID,
+	IFLA_VLAN_EGRESS_QOS,
+	IFLA_VLAN_INGRESS_QOS,
+	__IFLA_VLAN_MAX,
+};
+
+#define IFLA_VLAN_MAX	(__IFLA_VLAN_MAX - 1)
+
+struct ifla_vlan_qos_mapping
+{
+	__u32 from;
+	__u32 to;
+};
+
+enum {
+	IFLA_VLAN_QOS_UNSPEC,
+	IFLA_VLAN_QOS_MAPPING,
+	__IFLA_VLAN_QOS_MAX
+};
+
+#define IFLA_VLAN_QOS_MAX	(__IFLA_VLAN_QOS_MAX - 1)
+
 #endif /* _LINUX_IF_LINK_H */
diff --git a/net/8021q/Makefile b/net/8021q/Makefile
index 97feb44..10ca7f4 100644
--- a/net/8021q/Makefile
+++ b/net/8021q/Makefile
@@ -4,7 +4,7 @@
 
 obj-$(CONFIG_VLAN_8021Q) += 8021q.o
 
-8021q-objs := vlan.o vlan_dev.o
+8021q-objs := vlan.o vlan_dev.o vlan_netlink.o
 
 ifeq ($(CONFIG_PROC_FS),y)
 8021q-objs += vlanproc.o
diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index 03b67b9..8837d4f 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -97,15 +97,22 @@ static int __init vlan_proto_init(void)
 
 	/* Register us to receive netdevice events */
 	err = register_netdevice_notifier(&vlan_notifier_block);
-	if (err < 0) {
-		dev_remove_pack(&vlan_packet_type);
-		vlan_proc_cleanup();
-		return err;
-	}
+	if (err < 0)
+		goto err1;
 
-	vlan_ioctl_set(vlan_ioctl_handler);
+	err = vlan_netlink_init();
+	if (err < 0)
+		goto err2;
 
+	vlan_ioctl_set(vlan_ioctl_handler);
 	return 0;
+
+err2:
+	unregister_netdevice_notifier(&vlan_notifier_block);
+err1:
+	vlan_proc_cleanup();
+	dev_remove_pack(&vlan_packet_type);
+	return err;
 }
 
 /* Cleanup all vlan devices
@@ -136,6 +143,7 @@ static void __exit vlan_cleanup_module(void)
 {
 	int i;
 
+	vlan_netlink_fini();
 	vlan_ioctl_set(NULL);
 
 	/* Un-register us from receiving netdevice events */
@@ -306,7 +314,7 @@ static int unregister_vlan_dev(struct net_device *real_dev,
 	return ret;
 }
 
-static int unregister_vlan_device(struct net_device *dev)
+int unregister_vlan_device(struct net_device *dev)
 {
 	int ret;
 
@@ -366,7 +374,7 @@ static int vlan_dev_init(struct net_device *dev)
 	return 0;
 }
 
-static void vlan_setup(struct net_device *new_dev)
+void vlan_setup(struct net_device *new_dev)
 {
 	SET_MODULE_OWNER(new_dev);
 
@@ -415,7 +423,7 @@ static void vlan_transfer_operstate(const struct net_device *dev, struct net_dev
 	}
 }
 
-static int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id)
+int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id)
 {
 	if (real_dev->features & NETIF_F_VLAN_CHALLENGED) {
 		printk(VLAN_DBG "%s: VLANs not supported on %s.\n",
@@ -452,7 +460,7 @@ static int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_
 	return 0;
 }
 
-static int register_vlan_dev(struct net_device *dev)
+int register_vlan_dev(struct net_device *dev)
 {
 	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
 	struct net_device *real_dev = vlan->real_dev;
diff --git a/net/8021q/vlan.h b/net/8021q/vlan.h
index b837390..6cb2238 100644
--- a/net/8021q/vlan.h
+++ b/net/8021q/vlan.h
@@ -72,4 +72,12 @@ void vlan_dev_get_realdev_name(const struct net_device *dev, char *result);
 void vlan_dev_get_vid(const struct net_device *dev, unsigned short *result);
 void vlan_dev_set_multicast_list(struct net_device *vlan_dev);
 
+int vlan_check_real_dev(struct net_device *real_dev, unsigned short vlan_id);
+void vlan_setup(struct net_device *dev);
+int register_vlan_dev(struct net_device *dev);
+int unregister_vlan_device(struct net_device *dev);
+
+int vlan_netlink_init(void);
+void vlan_netlink_fini(void);
+
 #endif /* !(__BEN_VLAN_802_1Q_INC__) */
diff --git a/net/8021q/vlan_netlink.c b/net/8021q/vlan_netlink.c
new file mode 100644
index 0000000..92d6a46
--- /dev/null
+++ b/net/8021q/vlan_netlink.c
@@ -0,0 +1,209 @@
+/*
+ *	VLAN netlink control interface
+ *
+ * 	Copyright (c) 2007 Patrick McHardy <kaber@trash.net>
+ *
+ *	This program is free software; you can redistribute it and/or
+ *	modify it under the terms of the GNU General Public License
+ *	as published by the Free Software Foundation; either version
+ *	2 of the License, or (at your option) any later version.
+ */
+
+#include <linux/kernel.h>
+#include <linux/netdevice.h>
+#include <linux/if_vlan.h>
+#include <net/netlink.h>
+#include <net/rtnetlink.h>
+#include "vlan.h"
+
+
+static const struct nla_policy vlan_policy[IFLA_VLAN_MAX + 1] = {
+	[IFLA_VLAN_ID]		= { .type = NLA_U16 },
+	[IFLA_VLAN_EGRESS_QOS]	= { .type = NLA_NESTED },
+	[IFLA_VLAN_INGRESS_QOS] = { .type = NLA_NESTED },
+};
+
+static const struct nla_policy vlan_map_policy[IFLA_VLAN_QOS_MAX + 1] = {
+	[IFLA_VLAN_QOS_MAPPING] = { .len = sizeof(struct ifla_vlan_qos_mapping) },
+};
+
+
+static inline int vlan_validate_qos_map(struct nlattr *attr)
+{
+	if (!attr)
+		return 0;
+	return nla_validate_nested(attr, IFLA_VLAN_QOS_MAX, vlan_map_policy);
+}
+
+static int vlan_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+	u16 id;
+	int err;
+
+	if (!data)
+		return -EINVAL;
+
+	if (data[IFLA_VLAN_ID]) {
+		id = nla_get_u16(data[IFLA_VLAN_ID]);
+		if (id > VLAN_VID_MASK)
+			return -ERANGE;
+	}
+
+	err = vlan_validate_qos_map(data[IFLA_VLAN_INGRESS_QOS]);
+	if (err < 0)
+		return err;
+	err = vlan_validate_qos_map(data[IFLA_VLAN_EGRESS_QOS]);
+	if (err < 0)
+		return err;
+	return 0;
+}
+
+static int vlan_changelink(struct net_device *dev,
+			   struct nlattr *tb[], struct nlattr *data[])
+{
+	struct ifla_vlan_qos_mapping *m;
+	struct nlattr *attr;
+	int rem;
+
+	if (data[IFLA_VLAN_INGRESS_QOS]) {
+		nla_for_each_nested(attr, data[IFLA_VLAN_INGRESS_QOS], rem) {
+			m = nla_data(attr);
+			vlan_dev_set_ingress_priority(dev, m->to, m->from);
+		}
+	}
+	if (data[IFLA_VLAN_EGRESS_QOS]) {
+		nla_for_each_nested(attr, data[IFLA_VLAN_EGRESS_QOS], rem) {
+			m = nla_data(attr);
+			vlan_dev_set_egress_priority(dev, m->from, m->to);
+		}
+	}
+	return 0;
+}
+
+static int vlan_newlink(struct net_device *dev,
+			struct nlattr *tb[], struct nlattr *data[])
+{
+	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+	int err;
+
+	if (!tb[IFLA_LINK])
+		return -EINVAL;
+	if (!data[IFLA_VLAN_ID])
+		return -EINVAL;
+
+	vlan->vlan_id  = nla_get_u16(data[IFLA_VLAN_ID]);
+	vlan->real_dev = __dev_get_by_index(nla_get_u32(tb[IFLA_LINK]));
+
+	if (!vlan->real_dev)
+		return -ENODEV;
+	err = vlan_check_real_dev(vlan->real_dev, vlan->vlan_id);
+	if (err < 0)
+		return err;
+
+	err = vlan_changelink(dev, tb, data);
+	if (err < 0)
+		return err;
+
+	return register_vlan_dev(dev);
+}
+
+static void vlan_dellink(struct net_device *dev)
+{
+	unregister_vlan_device(dev);
+}
+
+static inline size_t vlan_qos_map_size(unsigned int n)
+{
+	if (n == 0)
+		return 0;
+	/* IFLA_VLAN_{EGRESS,INGRESS}_QOS + n * IFLA_VLAN_QOS_MAPPING */
+	return nla_total_size(sizeof(struct nlattr)) +
+	       nla_total_size(sizeof(struct ifla_vlan_qos_mapping)) * n;
+}
+
+static size_t vlan_get_size(struct net_device *dev)
+{
+	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+
+	return nla_total_size(2) +	/* IFLA_VLAN_ID */
+	       vlan_qos_map_size(vlan->nr_ingress_mappings) +
+	       vlan_qos_map_size(vlan->nr_egress_mappings);
+}
+
+static int vlan_fill_info(struct sk_buff *skb, struct net_device *dev)
+{
+	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
+	struct vlan_priority_tci_mapping *pm;
+	struct ifla_vlan_qos_mapping m;
+	struct nlattr *nest;
+	unsigned int i;
+
+	NLA_PUT_U16(skb, IFLA_VLAN_ID, VLAN_DEV_INFO(dev)->vlan_id);
+
+	if (vlan->nr_ingress_mappings) {
+		nest = nla_nest_start(skb, IFLA_VLAN_INGRESS_QOS);
+		if (nest == NULL)
+			goto nla_put_failure;
+
+		for (i = 0; i < ARRAY_SIZE(vlan->ingress_priority_map); i++) {
+			if (!vlan->ingress_priority_map[i])
+				continue;
+
+			m.from = i;
+			m.to   = vlan->ingress_priority_map[i];
+			NLA_PUT(skb, IFLA_VLAN_QOS_MAPPING,
+				sizeof(m), &m);
+		}
+		nla_nest_end(skb, nest);
+	}
+
+	if (vlan->nr_egress_mappings) {
+		nest = nla_nest_start(skb, IFLA_VLAN_EGRESS_QOS);
+		if (nest == NULL)
+			goto nla_put_failure;
+
+		for (i = 0; i < ARRAY_SIZE(vlan->egress_priority_map); i++) {
+			for (pm = vlan->egress_priority_map[i]; pm;
+			     pm = pm->next) {
+				if (!pm->vlan_qos)
+					continue;
+
+				m.from = pm->priority;
+				m.to   = (pm->vlan_qos >> 13) & 0x7;
+				NLA_PUT(skb, IFLA_VLAN_QOS_MAPPING,
+					sizeof(m), &m);
+			}
+		}
+		nla_nest_end(skb, nest);
+	}
+	return 0;
+
+nla_put_failure:
+	return -EMSGSIZE;
+}
+
+static struct rtnl_link_ops vlan_link_ops = {
+	.name		= "vlan",
+	.maxtype	= IFLA_VLAN_MAX,
+	.policy		= vlan_policy,
+	.priv_size	= sizeof(struct vlan_dev_info),
+	.setup		= vlan_setup,
+	.validate	= vlan_validate,
+	.newlink	= vlan_newlink,
+	.changelink	= vlan_changelink,
+	.dellink	= vlan_dellink,
+	.get_size	= vlan_get_size,
+	.fill_info	= vlan_fill_info,
+};
+
+int __init vlan_netlink_init(void)
+{
+	return rtnl_link_register(&vlan_link_ops);
+}
+
+void __exit vlan_netlink_fini(void)
+{
+	rtnl_link_unregister(&vlan_link_ops);
+}
+
+MODULE_ALIAS_RTNL_LINK("vlan");

Re: [RFC VLAN 01/10]: Fix off-by-ones in VLAN ID checks

[Ben Greear]

Patrick McHardy wrote:
> [VLAN]: Fix off-by-ones in VLAN ID checks
>
> The valid range of VLAN IDs is 0-4095, fix using ID 4095.
>   
Per http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf
page 69

0xFFF is reserved for future use, so we should not be using VLANs with 
that VID.

Thanks,
Ben


-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC VLAN 00/10]: VLAN netlink support try 2

[Ben Greear]

Patrick McHardy wrote:
> The last VLAN patchset was outdated, sorry about the mixup. These are the
> correct patches. The iproute patch I posted was correct, so no repost of
> that one.
>
>   
Is there any significant performance penalty in creating VLANs using 
these patches?

If you have a test station handy, could you let us know what 'time' 
shows for a script
creating 4000 VLANs with the old vconfig/ioctl method v/s this new method?

I'll try to look through the rest of the patches in detail later today.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com> 
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC VLAN 01/10]: Fix off-by-ones in VLAN ID checks

[Patrick McHardy]

Ben Greear wrote:
> Patrick McHardy wrote:
> 
>> [VLAN]: Fix off-by-ones in VLAN ID checks
>>
>> The valid range of VLAN IDs is 0-4095, fix using ID 4095.
>>   
> 
> Per http://standards.ieee.org/getieee802/download/802.1Q-1998.pdf
> page 69
> 
> 0xFFF is reserved for future use, so we should not be using VLANs with
> that VID.


Thanks, I just noticed id 4095 didn't work and assumed it was a bug.
I'm going to drop that patch.

Re: [RFC VLAN 00/10]: VLAN netlink support try 2

[Patrick McHardy]

Ben Greear wrote:
> Patrick McHardy wrote:
> 
>> The last VLAN patchset was outdated, sorry about the mixup. These are the
>> correct patches. The iproute patch I posted was correct, so no repost of
>> that one.
>>
>>   
> 
> Is there any significant performance penalty in creating VLANs using
> these patches?
> 
> If you have a test station handy, could you let us know what 'time'
> shows for a script
> creating 4000 VLANs with the old vconfig/ioctl method v/s this new method?


Not significantly more, for 1000 VLANs I get:

ip link add:

real    0m22.836s
user    0m0.100s
sys     0m5.850s

vconfig add:

real    0m19.739s
user    0m0.090s
sys     0m3.600s


ip -b (batch add):

real    0m5.239s
user    0m0.280s
sys     0m3.480s

> I'll try to look through the rest of the patches in detail later today.

Thanks.

Re: [RFC VLAN 00/10]: VLAN netlink support try 2

[Ben Greear]

Patrick McHardy wrote:
> Ben Greear wrote:
>> Patrick McHardy wrote:
>>
>>> The last VLAN patchset was outdated, sorry about the mixup. These are the
>>> correct patches. The iproute patch I posted was correct, so no repost of
>>> that one.
>>>
>>>   
>> Is there any significant performance penalty in creating VLANs using
>> these patches?
>>
>> If you have a test station handy, could you let us know what 'time'
>> shows for a script
>> creating 4000 VLANs with the old vconfig/ioctl method v/s this new method?
> 
> 
> Not significantly more, for 1000 VLANs I get:
> 
> ip link add:
> 
> real    0m22.836s
> user    0m0.100s
> sys     0m5.850s
> 
> vconfig add:
> 
> real    0m19.739s
> user    0m0.090s
> sys     0m3.600s

Thanks for doing the tests.  That small performance drop seems
fine to me.

> ip -b (batch add):
> 
> real    0m5.239s
> user    0m0.280s
> sys     0m3.480s

And that looks quite interesting!

Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC VLAN 00/10]: VLAN netlink support try 2

[Patrick McHardy]

Ben Greear wrote:
> Patrick McHardy wrote:
> 
>> Ben Greear wrote:
>>
>>> Patrick McHardy wrote:
>>>
>>>> The last VLAN patchset was outdated, sorry about the mixup. These
>>>> are the
>>>> correct patches. The iproute patch I posted was correct, so no
>>>> repost of
>>>> that one.
>>>>
>>>>   
>>>
>>> Is there any significant performance penalty in creating VLANs using
>>> these patches?
>>>
>>> If you have a test station handy, could you let us know what 'time'
>>> shows for a script
>>> creating 4000 VLANs with the old vconfig/ioctl method v/s this new
>>> method?
>>
>>
>>
>> Not significantly more, for 1000 VLANs I get:
>>
>> ip link add:
>>
>> real    0m22.836s
>> user    0m0.100s
>> sys     0m5.850s
>>
>> vconfig add:
>>
>> real    0m19.739s
>> user    0m0.090s
>> sys     0m3.600s
> 
> 
> Thanks for doing the tests.  That small performance drop seems
> fine to me.


I'll try to speed it up a bit more, my initial version needed something
like 10s for 1000 VLANs. I suspect the iproute RTM_NEWLINK probing done
for every (non-batched) operation adds quite significantly to the
overhead.

Re: [RFC VLAN 10/10]: Use rtnl_link API

[Ben Greear]

Patrick McHardy wrote:
> [VLAN]: Use rtnl_link API
> 
> Signed-off-by: Patrick McHardy <kaber@trash.net>

> +static int vlan_newlink(struct net_device *dev,
> +			struct nlattr *tb[], struct nlattr *data[])
> +{
> +	struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
> +	int err;
> +
> +	if (!tb[IFLA_LINK])
> +		return -EINVAL;
> +	if (!data[IFLA_VLAN_ID])
> +		return -EINVAL;
> +
> +	vlan->vlan_id  = nla_get_u16(data[IFLA_VLAN_ID]);
> +	vlan->real_dev = __dev_get_by_index(nla_get_u32(tb[IFLA_LINK]));

It seems you are scribbling on dev->priv before you validate that
it is OK to do so?

Or, maybe I'm just confused.

What is 'dev' here?

> +
> +	if (!vlan->real_dev)
> +		return -ENODEV;
> +	err = vlan_check_real_dev(vlan->real_dev, vlan->vlan_id);
> +	if (err < 0)
> +		return err;
> +
> +	err = vlan_changelink(dev, tb, data);
> +	if (err < 0)
> +		return err;
> +
> +	return register_vlan_dev(dev);
> +}


Other than this, this patch and the others in this series
look fine (except for the 4095 VID issue that we communicated
about earlier).

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com

Re: [RFC VLAN 10/10]: Use rtnl_link API

[Patrick McHardy]

Ben Greear wrote:
> Patrick McHardy wrote:
> 
>> [VLAN]: Use rtnl_link API
>>
>> Signed-off-by: Patrick McHardy <kaber@trash.net>
> 
> 
>> +static int vlan_newlink(struct net_device *dev,
>> +            struct nlattr *tb[], struct nlattr *data[])
>> +{
>> +    struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
>> +    int err;
>> +
>> +    if (!tb[IFLA_LINK])
>> +        return -EINVAL;
>> +    if (!data[IFLA_VLAN_ID])
>> +        return -EINVAL;
>> +
>> +    vlan->vlan_id  = nla_get_u16(data[IFLA_VLAN_ID]);
>> +    vlan->real_dev = __dev_get_by_index(nla_get_u32(tb[IFLA_LINK]));
> 
> 
> It seems you are scribbling on dev->priv before you validate that
> it is OK to do so?
> 
> Or, maybe I'm just confused.
> 
> What is 'dev' here?


Its a newly created device, not registered at this point. The
vlan_dev_info structure is used to communicate the settings
to vlan_dev_init() (also by the ioctl wrapper).

> Other than this, this patch and the others in this series
> look fine (except for the 4095 VID issue that we communicated
> about earlier).


Thanks, I'll fix that, add the remaining bits, and repost.
I think its only REORDER_HDR support that is still missing.

Re: [RFC VLAN 10/10]: Use rtnl_link API

[Ben Greear]

Patrick McHardy wrote:
> Ben Greear wrote:
>> Patrick McHardy wrote:
>>
>>> [VLAN]: Use rtnl_link API
>>>
>>> Signed-off-by: Patrick McHardy <kaber@trash.net>
>>
>>> +static int vlan_newlink(struct net_device *dev,
>>> +            struct nlattr *tb[], struct nlattr *data[])
>>> +{
>>> +    struct vlan_dev_info *vlan = VLAN_DEV_INFO(dev);
>>> +    int err;
>>> +
>>> +    if (!tb[IFLA_LINK])
>>> +        return -EINVAL;
>>> +    if (!data[IFLA_VLAN_ID])
>>> +        return -EINVAL;
>>> +
>>> +    vlan->vlan_id  = nla_get_u16(data[IFLA_VLAN_ID]);
>>> +    vlan->real_dev = __dev_get_by_index(nla_get_u32(tb[IFLA_LINK]));
>>
>> It seems you are scribbling on dev->priv before you validate that
>> it is OK to do so?
>>
>> Or, maybe I'm just confused.
>>
>> What is 'dev' here?
> 
> 
> Its a newly created device, not registered at this point. The
> vlan_dev_info structure is used to communicate the settings
> to vlan_dev_init() (also by the ioctl wrapper).

Ok, that makes sense.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com