Re: [PATCH 1/2] vlan: only create special VLAN 0 once

[David Miller <davem@davemloft.net>]

From: Jiri Bohac <jbohac@suse.cz>
Date: Fri, 3 Jun 2011 22:07:38 +0200

> Commit ad1afb00 registers a VLAN with vid == 0 for every device to handle
> 802.1p frames.  This is currently done on every NETDEV_UP event and the special
> vlan is never unregistered.  This may have strange effects on drivers
> implementning ndo_vlan_rx_add_vid(). E.g. bonding will allocate a linked-list
> element each time, causing a memory leak.
> 
> Only register the special VLAN once on NETDEV_REGISTER.
> 
> Signed-off-by: Jiri Bohac <jbohac@suse.cz>

I recognize the problem, but this solution isn't all that good.

I am pretty sure that the hardware device driver methods that
implement ndo_vlan_rx_add_vid() assume that the device is up.
Because most drivers completely reset the chip when the
interface is brought up and this will likely clear out the
VLAN ID tables in the chip.

Second, now even devices which don't ever get brought up will
have the VLAN ID 0 thing allocated.

Probably the thing to do is to remove the VLAN ID 0 entry on
NETDEV_DOWN.

Something like:

diff --git a/net/8021q/vlan.c b/net/8021q/vlan.c
index c7a581a..135019d 100644
--- a/net/8021q/vlan.c
+++ b/net/8021q/vlan.c
@@ -379,6 +379,14 @@ static int vlan_device_event(struct notifier_block *unused, unsigned long event,
 		dev->netdev_ops->ndo_vlan_rx_add_vid(dev, 0);
 	}
 
+	if ((event == NETDEV_DOWN) &&
+	    (dev->features & NETIF_F_HW_VLAN_FILTER) &&
+	    dev->netdev_ops->ndo_vlan_rx_kill_vid) {
+		pr_info("8021q: removing VLAN 0 from HW filter on device %s\n",
+			dev->name);
+		dev->netdev_ops->ndo_vlan_rx_kill_vid(dev, 0);
+	}
+
 	grp = rtnl_dereference(dev->vlgrp);
 	if (!grp)
 		goto out;