Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 28 Jun 2013 04:40:46 +0900 (JST)
From:      Hiroki Sato <hrs@FreeBSD.org>
To:        net@FreeBSD.org
Cc:        bz@FreeBSD.org, rpaulo@FreeBSD.org, ume@FreeBSD.org
Subject:   RFC: if_bridge(4) ND6_IFF_AUTO_LINKLOCAL
Message-ID:  <20130628.044046.1779486668165395187.hrs@allbsd.org>

next in thread | raw e-mail | index | archive | help
----Security_Multipart0(Fri_Jun_28_04_40_46_2013_856)--
Content-Type: Multipart/Mixed;
 boundary="--Next_Part(Fri_Jun_28_04_40_46_2013_678)--"
Content-Transfer-Encoding: 7bit

----Next_Part(Fri_Jun_28_04_40_46_2013_678)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

 I would like your comments about the attached patch.  This allows
 IFT_BRIDGE interfaces to accept ND6_IFF_AUTO_LINKLOCAL and
 autoconfiguration of a link-local address with EUI-64 interface id.

 One thing I am concerned about is the case when the parent interface
 and the member interfaces have own link-local scope zones at the same
 time since it leads to scope violation.  More specifically, It does
 not occur only in the following cases:

 1. bridge0 has an IPv6 address and the members do not.

 2. bridge0 does not have an IPv6 address and only one of the members
    does.

 I think we can allow only them and forbid the other cases like both
 bridge0 and the members have IPv6 addresses at the same time.  The
 attached patch implements this restriction by removing overlapped
 link-local scope zones.  I believe the restriction does not matter in
 practical configurations, but please correct me if I am missing
 something.

-- Hiroki

----Next_Part(Fri_Jun_28_04_40_46_2013_678)--
Content-Type: Text/X-Patch; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="iflla_20130627-1.diff"

- Allow ND6_IFF_AUTO_LINKLOCAL for IFT_BRIDGE.  An interface with IFT_BRIDGE
  is initialized with !ND6_IFF_AUTO_LINKLOCAL && !ND6_IFF_ACCEPT_RTADV
  regardless of net.inet6.ip6.accept_rtadv and net.inet6.ip6.auto_linklocal.
  To configure an autoconfigured link-local address (RFC 4862), the
  following rc.conf(5) configuration can be used:

  ifconfig_bridge0_ipv6="inet6 auto_linklocal"

- if_bridge(4) now removes IPv6 addresses on a member interface to be
  added when the parent interface or one of the existing member
  interfaces has an IPv6 address.  if_bridge(4) merges each link-local
  scope zone which the member interfaces form respectively, so it causes
  address scope violation.  Removal of the IPv6 addresses prevents it.

- if_lagg(4) now removes IPv6 addresses on a member interfaces
  unconditionally.

MFC after:	1 week

====
Index: sys/netinet6/in6.c
===================================================================
--- sys/netinet6/in6.c	(revision 252096)
+++ sys/netinet6/in6.c	(working copy)
@@ -1987,6 +1987,32 @@ in6ifa_ifpwithaddr(struct ifnet *ifp, struct in6_a
 }

 /*
+ * Find a link-local scoped address on ifp and return it if any.
+ */
+struct in6_ifaddr *
+in6ifa_llaonifp(struct ifnet *ifp)
+{
+	struct sockaddr_in6 *sin6;
+	struct ifaddr *ifa;
+
+	if (ND_IFINFO(ifp)->flags & ND6_IFF_IFDISABLED)
+		return (NULL);
+	if_addr_rlock(ifp);
+	TAILQ_FOREACH(ifa, &ifp->if_addrhead, ifa_link) {
+		if (ifa->ifa_addr->sa_family != AF_INET6)
+			continue;
+		sin6 = (struct sockaddr_in6 *)ifa->ifa_addr;
+		if (IN6_IS_SCOPE_LINKLOCAL(&sin6->sin6_addr) ||
+		    IN6_IS_ADDR_MC_INTFACELOCAL(&sin6->sin6_addr) ||
+		    IN6_IS_ADDR_MC_NODELOCAL(&sin6->sin6_addr))
+			break;
+	}
+	if_addr_runlock(ifp);
+
+	return ((struct in6_ifaddr *)ifa);
+}
+
+/*
  * Convert IP6 address to printable (loggable) representation. Caller
  * has to make sure that ip6buf is at least INET6_ADDRSTRLEN long.
  */
Index: sys/netinet6/in6_var.h
===================================================================
--- sys/netinet6/in6_var.h	(revision 252096)
+++ sys/netinet6/in6_var.h	(working copy)
@@ -800,6 +800,7 @@ void	in6_setmaxmtu(void);
 int	in6_if2idlen(struct ifnet *);
 struct in6_ifaddr *in6ifa_ifpforlinklocal(struct ifnet *, int);
 struct in6_ifaddr *in6ifa_ifpwithaddr(struct ifnet *, struct in6_addr *);
+struct in6_ifaddr *in6ifa_llaonifp(struct ifnet *);
 char	*ip6_sprintf(char *, const struct in6_addr *);
 int	in6_addr2zoneid(struct ifnet *, struct in6_addr *, u_int32_t *);
 int	in6_matchlen(struct in6_addr *, struct in6_addr *);
Index: sys/netinet6/in6_ifattach.c
===================================================================
--- sys/netinet6/in6_ifattach.c	(revision 252096)
+++ sys/netinet6/in6_ifattach.c	(working copy)
@@ -266,6 +266,7 @@ found:

 	/* get EUI64 */
 	switch (ifp->if_type) {
+	case IFT_BRIDGE:
 	case IFT_ETHER:
 	case IFT_L2VLAN:
 	case IFT_FDDI:
@@ -777,8 +778,7 @@ in6_ifattach(struct ifnet *ifp, struct ifnet *alti
 	/*
 	 * assign a link-local address, if there's none.
 	 */
-	if (ifp->if_type != IFT_BRIDGE &&
-	    !(ND_IFINFO(ifp)->flags & ND6_IFF_IFDISABLED) &&
+	if (!(ND_IFINFO(ifp)->flags & ND6_IFF_IFDISABLED) &&
 	    ND_IFINFO(ifp)->flags & ND6_IFF_AUTO_LINKLOCAL) {
 		int error;

Index: sys/netinet6/nd6.c
===================================================================
--- sys/netinet6/nd6.c	(revision 252096)
+++ sys/netinet6/nd6.c	(working copy)
@@ -176,13 +176,25 @@ nd6_ifattach(struct ifnet *ifp)

 	nd->flags = ND6_IFF_PERFORMNUD;

-	/* A loopback interface always has ND6_IFF_AUTO_LINKLOCAL. */
-	if (V_ip6_auto_linklocal || (ifp->if_flags & IFF_LOOPBACK))
+	/* A loopback interface always has ND6_IFF_AUTO_LINKLOCAL.
+	 * XXXHRS: Clear ND6_IFF_AUTO_LINKLOCAL on an IFT_BRIDGE interface by
+	 * default regardless of the V_ip6_auto_linklocal configuration to
+	 * give a reasonable default behavior.
+	 */
+	if ((V_ip6_auto_linklocal && ifp->if_type != IFT_BRIDGE) ||
+	    (ifp->if_flags & IFF_LOOPBACK))
 		nd->flags |= ND6_IFF_AUTO_LINKLOCAL;
-
-	/* A loopback interface does not need to accept RTADV. */
-	if (V_ip6_accept_rtadv && !(ifp->if_flags & IFF_LOOPBACK))
-		nd->flags |= ND6_IFF_ACCEPT_RTADV;
+	/*
+	 * A loopback interface does not need to accept RTADV.
+	 * XXXHRS: Clear ND6_IFF_ACCEPT_RTADV on an IFT_BRIDGE interface by
+	 * default regardless of the V_ip6_accept_rtadv configuration to
+	 * prevent the interface from accepting RA messages arrived
+	 * on one of the member interfaces with ND6_IFF_ACCEPT_RTADV.
+	 */
+	if (V_ip6_accept_rtadv &&
+	    !(ifp->if_flags & IFF_LOOPBACK) &&
+	    (ifp->if_type != IFT_BRIDGE))
+			nd->flags |= ND6_IFF_ACCEPT_RTADV;
 	if (V_ip6_no_radr && !(ifp->if_flags & IFF_LOOPBACK))
 		nd->flags |= ND6_IFF_NO_RADR;

Index: sys/net/if_bridge.c
===================================================================
--- sys/net/if_bridge.c	(revision 252096)
+++ sys/net/if_bridge.c	(working copy)
@@ -118,6 +118,7 @@ __FBSDID("$FreeBSD$");
 #ifdef INET6
 #include <netinet/ip6.h>
 #include <netinet6/ip6_var.h>
+#include <netinet6/in6_ifattach.h>
 #endif
 #if defined(INET) || defined(INET6)
 #include <netinet/ip_carp.h>
@@ -1041,14 +1042,6 @@ bridge_ioctl_add(struct bridge_softc *sc, void *ar
 	if (ifs->if_bridge != NULL)
 		return (EBUSY);

-	bif = malloc(sizeof(*bif), M_DEVBUF, M_NOWAIT|M_ZERO);
-	if (bif == NULL)
-		return (ENOMEM);
-
-	bif->bif_ifp = ifs;
-	bif->bif_flags = IFBIF_LEARNING | IFBIF_DISCOVER;
-	bif->bif_savedcaps = ifs->if_capenable;
-
 	switch (ifs->if_type) {
 	case IFT_ETHER:
 	case IFT_L2VLAN:
@@ -1056,10 +1049,71 @@ bridge_ioctl_add(struct bridge_softc *sc, void *ar
 		/* permitted interface types */
 		break;
 	default:
-		error = EINVAL;
-		goto out;
+		return (EINVAL);
 	}

+#ifdef INET6
+	/*
+	 * Two valid inet6 addresses with link-local scope must not be
+	 * on the parent interface and the member interfaces at the
+	 * same time.  This restriction is needed to prevent violation
+	 * of link-local scope zone.  Attempts to add a member
+	 * interface which has inet6 addresses when the parent has
+	 * inet6 triggers removal of all inet6 addresses on the member
+	 * interface.
+	 */
+
+	/* Check if the parent interface has a link-local scope addr. */
+	if (in6ifa_llaonifp(sc->sc_ifp) != NULL) {
+		/*
+		 * If any, remove all inet6 addresses from the member
+		 * interfaces.
+		 */
+		BRIDGE_XLOCK(sc);
+		LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
+ 			if (in6ifa_llaonifp(bif->bif_ifp)) {
+				in6_ifdetach(bif->bif_ifp);
+				if_printf(sc->sc_ifp,
+				    "IPv6 addresses on %s have been removed "
+				    "before adding it as a member to prevent "
+				    "IPv6 address scope violation.\n",
+				    bif->bif_ifp->if_xname);
+			}
+		}
+		BRIDGE_XDROP(sc);
+		if (in6ifa_llaonifp(ifs)) {
+			in6_ifdetach(ifs);
+			if_printf(sc->sc_ifp,
+			    "IPv6 addresses on %s have been removed "
+			    "before adding it as a member to prevent "
+			    "IPv6 address scope violation.\n",
+			    ifs->if_xname);
+		}
+	} else {
+		struct in6_ifaddr *ia6_m, *ia6_s;
+		/*
+		 * If not, check whether one of the existing member
+		 * interfaces have inet6 address.  If any, remove
+		 * inet6 addresses on the interface to be added.
+		 */
+		BRIDGE_XLOCK(sc);
+		LIST_FOREACH(bif, &sc->sc_iflist, bif_next) {
+			ia6_m = in6ifa_llaonifp(bif->bif_ifp);
+			if (ia6_m != NULL)
+				break;
+		}
+		BRIDGE_XDROP(sc);
+		ia6_s = in6ifa_llaonifp(ifs);
+
+		if (ia6_m != NULL && ia6_s != NULL) {
+			in6_ifdetach(ifs);
+			if_printf(sc->sc_ifp, "IPv6 addresses on %s have "
+				  "been removed before adding it as a member "
+				  "to prevent IPv6 address scope violation.\n",
+				  ifs->if_xname);
+		}
+	}
+#endif
 	/* Allow the first Ethernet member to define the MTU */
 	if (LIST_EMPTY(&sc->sc_iflist))
 		sc->sc_ifp->if_mtu = ifs->if_mtu;
@@ -1066,10 +1120,17 @@ bridge_ioctl_add(struct bridge_softc *sc, void *ar
 	else if (sc->sc_ifp->if_mtu != ifs->if_mtu) {
 		if_printf(sc->sc_ifp, "invalid MTU: %lu(%s) != %lu\n",
 		    ifs->if_mtu, ifs->if_xname, sc->sc_ifp->if_mtu);
-		error = EINVAL;
-		goto out;
+		return (EINVAL);
 	}

+	bif = malloc(sizeof(*bif), M_DEVBUF, M_NOWAIT|M_ZERO);
+	if (bif == NULL)
+		return (ENOMEM);
+
+	bif->bif_ifp = ifs;
+	bif->bif_flags = IFBIF_LEARNING | IFBIF_DISCOVER;
+	bif->bif_savedcaps = ifs->if_capenable;
+
 	/*
 	 * Assign the interface's MAC address to the bridge if it's the first
 	 * member and the MAC address of the bridge has not been changed from
@@ -1104,12 +1165,10 @@ bridge_ioctl_add(struct bridge_softc *sc, void *ar
 			BRIDGE_LOCK(sc);
 			break;
 	}
-	if (error)
+
+	if (error) {
 		bridge_delete_member(sc, bif, 0);
-out:
-	if (error) {
-		if (bif != NULL)
-			free(bif, M_DEVBUF);
+		free(bif, M_DEVBUF);
 	}
 	return (error);
 }
@@ -3408,7 +3467,7 @@ bridge_fragment(struct ifnet *ifp, struct mbuf *m,
 				continue;
 			}
 			bcopy(eh, mtod(m0, caddr_t), ETHER_HDR_LEN);
-		} else
+		} else
 			m_freem(m);
 	}

Index: sys/net/if_lagg.c
===================================================================
--- sys/net/if_lagg.c	(revision 252096)
+++ sys/net/if_lagg.c	(working copy)
@@ -63,6 +63,8 @@ __FBSDID("$FreeBSD$");

 #ifdef INET6
 #include <netinet/ip6.h>
+#include <netinet6/in6_var.h>
+#include <netinet6/in6_ifattach.h>
 #endif

 #include <net/if_vlan_var.h>
@@ -543,6 +545,34 @@ lagg_port_create(struct lagg_softc *sc, struct ifn
 	if (ifp->if_type != IFT_ETHER)
 		return (EPROTONOSUPPORT);

+#ifdef INET6
+	/*
+	 * The member interface should not have inet6 address because
+	 * two interfaces with a valid link-local scope zone must not be
+	 * merged in any form.  This restriction is needed to
+	 * prevent violation of link-local scope zone.  Attempts to
+	 * add a member interface which has inet6 addresses triggers
+	 * removal of all inet6 addresses on the member interface.
+	 */
+	SLIST_FOREACH(lp, &sc->sc_ports, lp_entries) {
+		if (in6ifa_llaonifp(lp->lp_ifp)) {
+			in6_ifdetach(lp->lp_ifp);
+			if_printf(sc->sc_ifp,
+			    "IPv6 addresses on %s have been removed "
+			    "before adding it as a member to prevent "
+			    "IPv6 address scope violation.\n",
+			    lp->lp_ifp->if_xname);
+		}
+	}
+	if (in6ifa_llaonifp(ifp)) {
+		in6_ifdetach(ifp);
+		if_printf(sc->sc_ifp,
+		    "IPv6 addresses on %s have been removed "
+		    "before adding it as a member to prevent "
+		    "IPv6 address scope violation.\n",
+		    ifp->if_xname);
+	}
+#endif
 	/* Allow the first Ethernet member to define the MTU */
 	if (SLIST_EMPTY(&sc->sc_ports))
 		sc->sc_ifp->if_mtu = ifp->if_mtu;

----Next_Part(Fri_Jun_28_04_40_46_2013_678)----

----Security_Multipart0(Fri_Jun_28_04_40_46_2013_856)--
Content-Type: application/pgp-signature
Content-Transfer-Encoding: 7bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (FreeBSD)

iEYEABECAAYFAlHMlT4ACgkQTyzT2CeTzy1KLACgwICrLP8qZFdCbp/Ov5+DAWpD
w9gAn1F/Wv2Je3VtpGS6lVKqvK2MssyC
=9FOy
-----END PGP SIGNATURE-----

----Security_Multipart0(Fri_Jun_28_04_40_46_2013_856)----



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20130628.044046.1779486668165395187.hrs>