Bug 142927 - [vlan] [patch] handle parent interface link layer address changes in if_vlan(4)
Summary: [vlan] [patch] handle parent interface link layer address changes in if_vlan(4)
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: 8.0-STABLE
Hardware: Any Any
: Normal Affects Only Me
Assignee: Andrew Thompson
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-01-18 06:00 UTC by Nikolay Denev
Modified: 2018-10-02 19:21 UTC (History)
1 user (show)

See Also:
bugmeister: mfc-stable10?
bugmeister: mfc-stable9?
bugmeister: mfc-stable8?


Attachments
file.diff (4.56 KB, patch)
2010-01-18 06:00 UTC, Nikolay Denev
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikolay Denev 2010-01-18 06:00:16 UTC
When if_vlan(4) interfaces are being configured on another network interface the parent's MAC address is being setup as the if_vlan(4)'s interface link layer address.
The problem is that if the parent interface link layer address is changed later, the if_vlan(4)'s link layer address remains the same and it stops functioning.

Fix: * Declare a new EVENTHANDLER called "iflladdr_event" similar to the "ifaddr_event" in sys/net/if_var.h
* Teach sys/net/if.c:ifhwioctl() to invoke the new handler on SIOCSIFLLADDR commands.
* Teach if_bridge(4) to invoke the handler when it's MAC address is changed due to adding or removing interfaces. (not all adds/removes, only those that change if_bridge's lladdr)
* Teach if_lagg(4) to invoke the handler in its lagg_lladdr() function.
* Teach if_vlan(4) to register/deregister the handler on module load/unload and add vlan_iflladdr() function to change the llladdrs on the attached vlans of the parent interface that fired the event.



Patch attached with submission follows:
How-To-Repeat: This can happen if one for example uses if_vlan(4) on top of if_lagg(4) and configures them in rc.conf using the ${interface}.${vlan} method.
The rc scipts create the "empty" if_lagg(4) interface with MAC of 00:00:00:00:00:00 then attach the vlans on it copying the same all zeroes MAC address, and then it adds the physical interfaces to the if_lagg(4) interface giving it's MAC address, but this leaves the vlans with all zeroes MACs and not working.
Comment 1 Mark Linimon freebsd_committer freebsd_triage 2010-01-18 06:46:02 UTC
Responsible Changed
From-To: freebsd-bugs->freebsd-net

Over to maintainer(s).
Comment 2 dfilter service freebsd_committer 2010-01-18 20:34:16 UTC
Author: thompsa
Date: Mon Jan 18 20:34:00 2010
New Revision: 202588
URL: http://svn.freebsd.org/changeset/base/202588

Log:
  Declare a new EVENTHANDLER called iflladdr_event which signals that the L2
  address on an interface has changed. This lets stacked interfaces such as
  vlan(4) detect that their lower interface has changed and adjust things in
  order to keep working. Previously this situation broke at least vlan(4) and
  lagg(4) configurations.
  
  The EVENTHANDLER_INVOKE call was not placed within if_setlladdr() due to the
  risk of a loop.
  
  PR:		kern/142927
  Submitted by:	Nikolay Denev

Modified:
  head/sys/net/if.c
  head/sys/net/if_bridge.c
  head/sys/net/if_lagg.c
  head/sys/net/if_var.h
  head/sys/net/if_vlan.c
  head/sys/netgraph/ng_eiface.c
  head/sys/netgraph/ng_ether.c
  head/sys/netgraph/ng_fec.c

Modified: head/sys/net/if.c
==============================================================================
--- head/sys/net/if.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/net/if.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -2268,6 +2268,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp,
 			return (error);
 		error = if_setlladdr(ifp,
 		    ifr->ifr_addr.sa_data, ifr->ifr_addr.sa_len);
+		EVENTHANDLER_INVOKE(iflladdr_event, ifp);
 		break;
 
 	case SIOCAIFGROUP:

Modified: head/sys/net/if_bridge.c
==============================================================================
--- head/sys/net/if_bridge.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/net/if_bridge.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -917,6 +917,7 @@ bridge_delete_member(struct bridge_softc
 			    IF_LLADDR(sc->sc_ifp), ETHER_ADDR_LEN);
 			sc->sc_ifaddr = fif;
 		}
+		EVENTHANDLER_INVOKE(iflladdr_event, sc->sc_ifp);
 	}
 
 	bridge_mutecaps(sc);	/* recalcuate now this interface is removed */
@@ -1033,6 +1034,7 @@ bridge_ioctl_add(struct bridge_softc *sc
 	    !memcmp(IF_LLADDR(sc->sc_ifp), sc->sc_defaddr, ETHER_ADDR_LEN)) {
 		bcopy(IF_LLADDR(ifs), IF_LLADDR(sc->sc_ifp), ETHER_ADDR_LEN);
 		sc->sc_ifaddr = ifs;
+		EVENTHANDLER_INVOKE(iflladdr_event, sc->sc_ifp);
 	}
 
 	ifs->if_bridge = sc;

Modified: head/sys/net/if_lagg.c
==============================================================================
--- head/sys/net/if_lagg.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/net/if_lagg.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -305,6 +305,7 @@ lagg_lladdr(struct lagg_softc *sc, uint8
 	/* Let the protocol know the MAC has changed */
 	if (sc->sc_lladdr != NULL)
 		(*sc->sc_lladdr)(sc);
+	EVENTHANDLER_INVOKE(iflladdr_event, ifp);
 }
 
 static void

Modified: head/sys/net/if_var.h
==============================================================================
--- head/sys/net/if_var.h	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/net/if_var.h	Mon Jan 18 20:34:00 2010	(r202588)
@@ -339,6 +339,9 @@ void	if_maddr_runlock(struct ifnet *ifp)
 } while(0)
 
 #ifdef _KERNEL
+/* interface link layer address change event */
+typedef void (*iflladdr_event_handler_t)(void *, struct ifnet *);
+EVENTHANDLER_DECLARE(iflladdr_event, iflladdr_event_handler_t);
 /* interface address change event */
 typedef void (*ifaddr_event_handler_t)(void *, struct ifnet *);
 EVENTHANDLER_DECLARE(ifaddr_event, ifaddr_event_handler_t);

Modified: head/sys/net/if_vlan.c
==============================================================================
--- head/sys/net/if_vlan.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/net/if_vlan.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -138,6 +138,7 @@ SYSCTL_INT(_net_link_vlan, OID_AUTO, sof
 static MALLOC_DEFINE(M_VLAN, VLANNAME, "802.1Q Virtual LAN Interface");
 
 static eventhandler_tag ifdetach_tag;
+static eventhandler_tag iflladdr_tag;
 
 /*
  * We have a global mutex, that is used to serialize configuration
@@ -199,6 +200,7 @@ static	int vlan_clone_create(struct if_c
 static	int vlan_clone_destroy(struct if_clone *, struct ifnet *);
 
 static	void vlan_ifdetach(void *arg, struct ifnet *ifp);
+static  void vlan_iflladdr(void *arg, struct ifnet *ifp);
 
 static	struct if_clone vlan_cloner = IFC_CLONE_INITIALIZER(VLANNAME, NULL,
     IF_MAXUNIT, NULL, vlan_clone_match, vlan_clone_create, vlan_clone_destroy);
@@ -463,6 +465,41 @@ vlan_setmulti(struct ifnet *ifp)
 }
 
 /*
+ * A handler for parent interface link layer address changes.
+ * If the parent interface link layer address is changed we
+ * should also change it on all children vlans.
+ */
+static void
+vlan_iflladdr(void *arg __unused, struct ifnet *ifp)
+{
+	struct ifvlan *ifv;
+	int i;
+
+	/*
+	 * Check if it's a trunk interface first of all
+	 * to avoid needless locking.
+	 */
+	if (ifp->if_vlantrunk == NULL)
+		return;
+
+	VLAN_LOCK();
+	/*
+	 * OK, it's a trunk.  Loop over and change all vlan's lladdrs on it.
+	 */
+#ifdef VLAN_ARRAY
+	for (i = 0; i < VLAN_ARRAY_SIZE; i++)
+		if ((ifv = ifp->if_vlantrunk->vlans[i]))
+			if_setlladdr(ifv->ifv_ifp, IF_LLADDR(ifp), ETHER_ADDR_LEN);
+#else /* VLAN_ARRAY */
+	for (i = 0; i < (1 << ifp->if_vlantrunk->hwidth); i++)
+		LIST_FOREACH(ifv, &ifp->if_vlantrunk->hash[i], ifv_list)
+			if_setlladdr(ifv->ifv_ifp, IF_LLADDR(ifp), ETHER_ADDR_LEN);
+#endif /* VLAN_ARRAY */
+	VLAN_UNLOCK();
+
+}
+
+/*
  * A handler for network interface departure events.
  * Track departure of trunks here so that we don't access invalid
  * pointers or whatever if a trunk is ripped from under us, e.g.,
@@ -537,6 +574,10 @@ vlan_modevent(module_t mod, int type, vo
 		    vlan_ifdetach, NULL, EVENTHANDLER_PRI_ANY);
 		if (ifdetach_tag == NULL)
 			return (ENOMEM);
+		iflladdr_tag = EVENTHANDLER_REGISTER(iflladdr_event,
+		    vlan_iflladdr, NULL, EVENTHANDLER_PRI_ANY);
+		if (iflladdr_tag == NULL)
+			return (ENOMEM);
 		VLAN_LOCK_INIT();
 		vlan_input_p = vlan_input;
 		vlan_link_state_p = vlan_link_state;
@@ -555,6 +596,7 @@ vlan_modevent(module_t mod, int type, vo
 	case MOD_UNLOAD:
 		if_clone_detach(&vlan_cloner);
 		EVENTHANDLER_DEREGISTER(ifnet_departure_event, ifdetach_tag);
+		EVENTHANDLER_DEREGISTER(iflladdr_event, iflladdr_tag);
 		vlan_input_p = NULL;
 		vlan_link_state_p = NULL;
 		vlan_trunk_cap_p = NULL;

Modified: head/sys/netgraph/ng_eiface.c
==============================================================================
--- head/sys/netgraph/ng_eiface.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/netgraph/ng_eiface.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -431,6 +431,7 @@ ng_eiface_rcvmsg(node_p node, item_p ite
 			}
 			error = if_setlladdr(priv->ifp,
 			    (u_char *)msg->data, ETHER_ADDR_LEN);
+			EVENTHANDLER_INVOKE(iflladdr_event, priv->ifp);
 			break;
 		    }
 

Modified: head/sys/netgraph/ng_ether.c
==============================================================================
--- head/sys/netgraph/ng_ether.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/netgraph/ng_ether.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -481,6 +481,7 @@ ng_ether_rcvmsg(node_p node, item_p item
 			}
 			error = if_setlladdr(priv->ifp,
 			    (u_char *)msg->data, ETHER_ADDR_LEN);
+			EVENTHANDLER_INVOKE(iflladdr_event, priv->ifp);
 			break;
 		    }
 		case NGM_ETHER_GET_PROMISC:

Modified: head/sys/netgraph/ng_fec.c
==============================================================================
--- head/sys/netgraph/ng_fec.c	Mon Jan 18 20:25:29 2010	(r202587)
+++ head/sys/netgraph/ng_fec.c	Mon Jan 18 20:34:00 2010	(r202588)
@@ -433,6 +433,7 @@ ng_fec_addport(struct ng_fec_private *pr
 
 	/* Set up phony MAC address. */
 	if_setlladdr(bifp, IF_LLADDR(ifp), ETHER_ADDR_LEN);
+	EVENTHANDLER_INVOKE(iflladdr_event, bifp);
 
 	/* Save original input vector */
 	new->fec_if_input = bifp->if_input;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 3 Andrew Thompson freebsd_committer 2010-01-18 20:35:53 UTC
State Changed
From-To: open->patched

Committed, thanks! 


Comment 4 Andrew Thompson freebsd_committer 2010-01-18 20:35:53 UTC
Responsible Changed
From-To: freebsd-net->thompsa

Grab.
Comment 5 dfilter service freebsd_committer 2010-05-25 03:36:17 UTC
Author: thompsa
Date: Tue May 25 02:36:06 2010
New Revision: 208526
URL: http://svn.freebsd.org/changeset/base/208526

Log:
  MFC r202588
  
   Declare a new EVENTHANDLER called iflladdr_event which signals that the L2
   address on an interface has changed. This lets stacked interfaces such as
   vlan(4) detect that their lower interface has changed and adjust things in
   order to keep working. Previously this situation broke at least vlan(4) and
   lagg(4) configurations.
  
   The EVENTHANDLER_INVOKE call was not placed within if_setlladdr() due to the
   risk of a loop.
  
   PR:		kern/142927
   Submitted by:	Nikolay Denev
  
  MFC r202611
  
   Do not hold the lock over if_setlladdr() as it calls into the interface driver
   init routine.

Modified:
  stable/8/sys/net/if.c
  stable/8/sys/net/if_bridge.c
  stable/8/sys/net/if_lagg.c
  stable/8/sys/net/if_var.h
  stable/8/sys/net/if_vlan.c
  stable/8/sys/netgraph/ng_eiface.c
  stable/8/sys/netgraph/ng_ether.c
  stable/8/sys/netgraph/ng_fec.c
Directory Properties:
  stable/8/sys/   (props changed)
  stable/8/sys/amd64/include/xen/   (props changed)
  stable/8/sys/cddl/contrib/opensolaris/   (props changed)
  stable/8/sys/contrib/dev/acpica/   (props changed)
  stable/8/sys/contrib/pf/   (props changed)
  stable/8/sys/dev/xen/xenpci/   (props changed)
  stable/8/sys/geom/sched/   (props changed)

Modified: stable/8/sys/net/if.c
==============================================================================
--- stable/8/sys/net/if.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/net/if.c	Tue May 25 02:36:06 2010	(r208526)
@@ -2416,6 +2416,7 @@ ifhwioctl(u_long cmd, struct ifnet *ifp,
 			return (error);
 		error = if_setlladdr(ifp,
 		    ifr->ifr_addr.sa_data, ifr->ifr_addr.sa_len);
+		EVENTHANDLER_INVOKE(iflladdr_event, ifp);
 		break;
 
 	case SIOCAIFGROUP:

Modified: stable/8/sys/net/if_bridge.c
==============================================================================
--- stable/8/sys/net/if_bridge.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/net/if_bridge.c	Tue May 25 02:36:06 2010	(r208526)
@@ -937,6 +937,7 @@ bridge_delete_member(struct bridge_softc
 			    IF_LLADDR(sc->sc_ifp), ETHER_ADDR_LEN);
 			sc->sc_ifaddr = fif;
 		}
+		EVENTHANDLER_INVOKE(iflladdr_event, sc->sc_ifp);
 	}
 
 	bridge_mutecaps(sc);	/* recalcuate now this interface is removed */
@@ -1052,6 +1053,7 @@ bridge_ioctl_add(struct bridge_softc *sc
 	    !memcmp(IF_LLADDR(sc->sc_ifp), sc->sc_defaddr, ETHER_ADDR_LEN)) {
 		bcopy(IF_LLADDR(ifs), IF_LLADDR(sc->sc_ifp), ETHER_ADDR_LEN);
 		sc->sc_ifaddr = ifs;
+		EVENTHANDLER_INVOKE(iflladdr_event, sc->sc_ifp);
 	}
 
 	ifs->if_bridge = sc;

Modified: stable/8/sys/net/if_lagg.c
==============================================================================
--- stable/8/sys/net/if_lagg.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/net/if_lagg.c	Tue May 25 02:36:06 2010	(r208526)
@@ -303,6 +303,7 @@ lagg_lladdr(struct lagg_softc *sc, uint8
 	/* Let the protocol know the MAC has changed */
 	if (sc->sc_lladdr != NULL)
 		(*sc->sc_lladdr)(sc);
+	EVENTHANDLER_INVOKE(iflladdr_event, ifp);
 }
 
 static void

Modified: stable/8/sys/net/if_var.h
==============================================================================
--- stable/8/sys/net/if_var.h	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/net/if_var.h	Tue May 25 02:36:06 2010	(r208526)
@@ -342,6 +342,9 @@ void	if_maddr_runlock(struct ifnet *ifp)
 } while(0)
 
 #ifdef _KERNEL
+/* interface link layer address change event */
+typedef void (*iflladdr_event_handler_t)(void *, struct ifnet *);
+EVENTHANDLER_DECLARE(iflladdr_event, iflladdr_event_handler_t);
 /* interface address change event */
 typedef void (*ifaddr_event_handler_t)(void *, struct ifnet *);
 EVENTHANDLER_DECLARE(ifaddr_event, ifaddr_event_handler_t);

Modified: stable/8/sys/net/if_vlan.c
==============================================================================
--- stable/8/sys/net/if_vlan.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/net/if_vlan.c	Tue May 25 02:36:06 2010	(r208526)
@@ -139,6 +139,7 @@ SYSCTL_INT(_net_link_vlan, OID_AUTO, sof
 static MALLOC_DEFINE(M_VLAN, VLANNAME, "802.1Q Virtual LAN Interface");
 
 static eventhandler_tag ifdetach_tag;
+static eventhandler_tag iflladdr_tag;
 
 /*
  * We have a global mutex, that is used to serialize configuration
@@ -200,6 +201,7 @@ static	int vlan_clone_create(struct if_c
 static	int vlan_clone_destroy(struct if_clone *, struct ifnet *);
 
 static	void vlan_ifdetach(void *arg, struct ifnet *ifp);
+static  void vlan_iflladdr(void *arg, struct ifnet *ifp);
 
 static	struct if_clone vlan_cloner = IFC_CLONE_INITIALIZER(VLANNAME, NULL,
     IF_MAXUNIT, NULL, vlan_clone_match, vlan_clone_create, vlan_clone_destroy);
@@ -464,6 +466,46 @@ vlan_setmulti(struct ifnet *ifp)
 }
 
 /*
+ * A handler for parent interface link layer address changes.
+ * If the parent interface link layer address is changed we
+ * should also change it on all children vlans.
+ */
+static void
+vlan_iflladdr(void *arg __unused, struct ifnet *ifp)
+{
+	struct ifvlan *ifv;
+#ifndef VLAN_ARRAY
+	struct ifvlan *next;
+#endif
+	int i;
+
+	/*
+	 * Check if it's a trunk interface first of all
+	 * to avoid needless locking.
+	 */
+	if (ifp->if_vlantrunk == NULL)
+		return;
+
+	VLAN_LOCK();
+	/*
+	 * OK, it's a trunk.  Loop over and change all vlan's lladdrs on it.
+	 */
+#ifdef VLAN_ARRAY
+	for (i = 0; i < VLAN_ARRAY_SIZE; i++)
+		if ((ifv = ifp->if_vlantrunk->vlans[i])) {
+#else /* VLAN_ARRAY */
+	for (i = 0; i < (1 << ifp->if_vlantrunk->hwidth); i++)
+		LIST_FOREACH_SAFE(ifv, &ifp->if_vlantrunk->hash[i], ifv_list, next) {
+#endif /* VLAN_ARRAY */
+			VLAN_UNLOCK();
+			if_setlladdr(ifv->ifv_ifp, IF_LLADDR(ifp), ETHER_ADDR_LEN);
+			VLAN_LOCK();
+		}
+	VLAN_UNLOCK();
+
+}
+
+/*
  * A handler for network interface departure events.
  * Track departure of trunks here so that we don't access invalid
  * pointers or whatever if a trunk is ripped from under us, e.g.,
@@ -538,6 +580,10 @@ vlan_modevent(module_t mod, int type, vo
 		    vlan_ifdetach, NULL, EVENTHANDLER_PRI_ANY);
 		if (ifdetach_tag == NULL)
 			return (ENOMEM);
+		iflladdr_tag = EVENTHANDLER_REGISTER(iflladdr_event,
+		    vlan_iflladdr, NULL, EVENTHANDLER_PRI_ANY);
+		if (iflladdr_tag == NULL)
+			return (ENOMEM);
 		VLAN_LOCK_INIT();
 		vlan_input_p = vlan_input;
 		vlan_link_state_p = vlan_link_state;
@@ -556,6 +602,7 @@ vlan_modevent(module_t mod, int type, vo
 	case MOD_UNLOAD:
 		if_clone_detach(&vlan_cloner);
 		EVENTHANDLER_DEREGISTER(ifnet_departure_event, ifdetach_tag);
+		EVENTHANDLER_DEREGISTER(iflladdr_event, iflladdr_tag);
 		vlan_input_p = NULL;
 		vlan_link_state_p = NULL;
 		vlan_trunk_cap_p = NULL;

Modified: stable/8/sys/netgraph/ng_eiface.c
==============================================================================
--- stable/8/sys/netgraph/ng_eiface.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/netgraph/ng_eiface.c	Tue May 25 02:36:06 2010	(r208526)
@@ -432,6 +432,7 @@ ng_eiface_rcvmsg(node_p node, item_p ite
 			}
 			error = if_setlladdr(priv->ifp,
 			    (u_char *)msg->data, ETHER_ADDR_LEN);
+			EVENTHANDLER_INVOKE(iflladdr_event, priv->ifp);
 			break;
 		    }
 

Modified: stable/8/sys/netgraph/ng_ether.c
==============================================================================
--- stable/8/sys/netgraph/ng_ether.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/netgraph/ng_ether.c	Tue May 25 02:36:06 2010	(r208526)
@@ -481,6 +481,7 @@ ng_ether_rcvmsg(node_p node, item_p item
 			}
 			error = if_setlladdr(priv->ifp,
 			    (u_char *)msg->data, ETHER_ADDR_LEN);
+			EVENTHANDLER_INVOKE(iflladdr_event, priv->ifp);
 			break;
 		    }
 		case NGM_ETHER_GET_PROMISC:

Modified: stable/8/sys/netgraph/ng_fec.c
==============================================================================
--- stable/8/sys/netgraph/ng_fec.c	Tue May 25 02:28:39 2010	(r208525)
+++ stable/8/sys/netgraph/ng_fec.c	Tue May 25 02:36:06 2010	(r208526)
@@ -433,6 +433,7 @@ ng_fec_addport(struct ng_fec_private *pr
 
 	/* Set up phony MAC address. */
 	if_setlladdr(bifp, IF_LLADDR(ifp), ETHER_ADDR_LEN);
+	EVENTHANDLER_INVOKE(iflladdr_event, bifp);
 
 	/* Save original input vector */
 	new->fec_if_input = bifp->if_input;
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org"
Comment 6 Ed Maste freebsd_committer 2018-10-02 19:21:09 UTC
This is now complete?