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>