On Thu, 10 Nov 2016 22:16:55 +0100 Vincent Gross <[email protected]> wrote:
> On Sat, 5 Nov 2016 12:41:39 +0100 > Vincent Gross <[email protected]> wrote: > > > Updated diff, I reworked the logic to handle the if_get/if_put dance > > in vxlan_multicast_join(), and fixed an uninitialized variable. > > > > Ok ? > > Anyone to comment or ok ? this blocks the submission of > other changes on the network stack. Come on ! Don't be shy ! http://quigon.bsws.de/papers/2015/asiabsdcon/mgp00042.html http://quigon.bsws.de/papers/2015/asiabsdcon/mgp00043.html > > > > > Index: net/if_vxlan.c > > =================================================================== > > RCS file: /cvs/src/sys/net/if_vxlan.c,v > > retrieving revision 1.51 > > diff -u -p -r1.51 if_vxlan.c > > --- net/if_vxlan.c 25 Oct 2016 16:31:08 -0000 1.51 > > +++ net/if_vxlan.c 5 Nov 2016 11:36:02 -0000 > > @@ -47,6 +47,8 @@ > > #include <netinet/udp_var.h> > > #include <netinet/in_pcb.h> > > > > +#include <netinet6/in6_var.h> > > + > > #if NPF > 0 > > #include <net/pfvar.h> > > #endif > > @@ -61,7 +63,12 @@ struct vxlan_softc { > > struct arpcom sc_ac; > > struct ifmedia sc_media; > > > > - struct ip_moptions sc_imo; > > + union { > > + struct ip_moptions u_imo; > > + struct ip6_moptions u_imo6; > > + } sc_imu; > > +#define sc_imo sc_imu.u_imo > > +#define sc_im6o sc_imu.u_imo6 > > void *sc_ahcookie; > > void *sc_lhcookie; > > void *sc_dhcookie; > > @@ -129,10 +136,6 @@ vxlan_clone_create(struct if_clone *ifc, > > M_DEVBUF, M_NOWAIT|M_ZERO)) == NULL) > > return (ENOMEM); > > > > - sc->sc_imo.imo_membership = malloc( > > - (sizeof(struct in_multi *) * IP_MIN_MEMBERSHIPS), > > M_IPMOPTS, > > - M_WAITOK|M_ZERO); > > - sc->sc_imo.imo_max_memberships = IP_MIN_MEMBERSHIPS; > > sc->sc_dstport = htons(VXLAN_PORT); > > sc->sc_vnetid = VXLAN_VNI_UNSET; > > > > @@ -190,7 +193,6 @@ vxlan_clone_destroy(struct ifnet *ifp) > > ifmedia_delete_instance(&sc->sc_media, IFM_INST_ANY); > > ether_ifdetach(ifp); > > if_detach(ifp); > > - free(sc->sc_imo.imo_membership, M_IPMOPTS, 0); > > free(sc, M_DEVBUF, sizeof(*sc)); > > > > return (0); > > @@ -199,11 +201,33 @@ vxlan_clone_destroy(struct ifnet *ifp) > > void > > vxlan_multicast_cleanup(struct ifnet *ifp) > > { > > - struct vxlan_softc *sc = (struct vxlan_softc > > *)ifp->if_softc; > > - struct ip_moptions *imo = &sc->sc_imo; > > - struct ifnet *mifp; > > + struct vxlan_softc *sc = (struct vxlan_softc > > *)ifp->if_softc; > > + struct ip_moptions *imo; > > + struct in_multi **imm; > > + struct ip6_moptions *im6o; > > + struct in6_multi_mship *im6m, *im6m_next; > > + struct ifnet *mifp = NULL; > > + > > + switch (sc->sc_dst.ss_family) { > > + case AF_INET: > > + imo = &sc->sc_imo; > > + mifp = if_get(imo->imo_ifidx); > > + imm = imo->imo_membership; > > + while (imo->imo_num_memberships > 0) > > + > > in_delmulti(imm[--imo->imo_num_memberships]); > > + free(imm, M_IPMOPTS, > > + sizeof(struct in_multi *) * > > imo->imo_num_memberships); > > + break; > > + case AF_INET6: > > + im6o = &sc->sc_im6o; > > + mifp = if_get(im6o->im6o_ifidx); > > + LIST_FOREACH_SAFE(im6m, &im6o->im6o_memberships, > > i6mm_chain, > > + im6m_next) > > + in6_leavegroup(im6m); > > + break; > > + } > > + bzero(&sc->sc_imu, sizeof(sc->sc_imu)); > > > > - mifp = if_get(imo->imo_ifidx); > > if (mifp != NULL) { > > if (sc->sc_ahcookie != NULL) { > > hook_disestablish(mifp->if_addrhooks, > > sc->sc_ahcookie); @@ -219,14 +243,9 @@ > > vxlan_multicast_cleanup(struct ifnet *if sc->sc_dhcookie); > > sc->sc_dhcookie = NULL; > > } > > - > > - if_put(mifp); > > } > > > > - if (imo->imo_num_memberships > 0) { > > - > > in_delmulti(imo->imo_membership[--imo->imo_num_memberships]); > > - imo->imo_ifidx = 0; > > - } > > + if_put(mifp); > > } > > > > int > > @@ -234,55 +253,141 @@ vxlan_multicast_join(struct ifnet *ifp, > > struct sockaddr *dst) > > { > > struct vxlan_softc *sc = ifp->if_softc; > > - struct ip_moptions *imo = &sc->sc_imo; > > + struct ip_moptions *imo; > > + struct ip6_moptions *im6o; > > + struct in6_multi_mship *im6m; > > struct sockaddr_in *src4, *dst4; > > #ifdef INET6 > > - struct sockaddr_in6 *dst6; > > + struct sockaddr_in6 *src6, *dst6; > > #endif /* INET6 */ > > struct ifaddr *ifa; > > - struct ifnet *mifp; > > + struct ifnet *mifp = NULL, *m6ifp = NULL; > > + struct rtentry *rt; > > + int error; > > > > switch (dst->sa_family) { > > case AF_INET: > > dst4 = satosin(dst); > > + src4 = satosin(src); > > if (!IN_MULTICAST(dst4->sin_addr.s_addr)) > > return (0); > > + if (src4->sin_addr.s_addr == INADDR_ANY || > > + IN_MULTICAST(src4->sin_addr.s_addr)) > > + return (EINVAL); > > + if ((ifa = ifa_ifwithaddr(src, sc->sc_rdomain)) == > > NULL || > > + (mifp = ifa->ifa_ifp) == NULL || > > + (mifp->if_flags & IFF_MULTICAST) == 0) > > + return (EADDRNOTAVAIL); > > + > > + imo = &sc->sc_imo; > > + imo->imo_membership = mallocarray(sizeof(struct > > in_multi *), > > + IP_MIN_MEMBERSHIPS, M_IPMOPTS, > > M_WAITOK|M_ZERO); > > + if (imo->imo_membership == NULL || > > + (imo->imo_membership[0] = > > + in_addmulti(&dst4->sin_addr, mifp)) == NULL) { > > + free(imo->imo_membership, M_IPMOPTS, > > + sizeof(struct in_multi *) * > > IP_MIN_MEMBERSHIPS); > > + return (ENOBUFS); > > + } > > + imo->imo_max_memberships = IP_MIN_MEMBERSHIPS; > > + imo->imo_num_memberships++; > > + imo->imo_ifidx = mifp->if_index; > > + if (sc->sc_ttl > 0) > > + imo->imo_ttl = sc->sc_ttl; > > + else > > + imo->imo_ttl = IP_DEFAULT_MULTICAST_TTL; > > + imo->imo_loop = 0; > > break; > > #ifdef INET6 > > case AF_INET6: > > dst6 = satosin6(dst); > > + src6 = satosin6(src); > > if (!IN6_IS_ADDR_MULTICAST(&dst6->sin6_addr)) > > return (0); > > + if (IN6_IS_ADDR_UNSPECIFIED(&src6->sin6_addr) || > > + IN6_IS_ADDR_MULTICAST(&src6->sin6_addr)) > > + return (EINVAL); > > + if (IN6_IS_ADDR_MC_LINKLOCAL(&dst6->sin6_addr) || > > + IN6_IS_ADDR_MC_INTFACELOCAL(&dst6->sin6_addr)) > > { > > + /* Scoped mcast destination : get index > > from dst6 */ > > + if (dst6->sin6_scope_id) > > + m6ifp = > > if_get(dst6->sin6_scope_id); > > + } else { > > + /* > > + * unscoped multicast destination : get > > ifindex > > + * from src6, or routing table as fallack > > + */ > > + if (src6->sin6_scope_id) > > + m6ifp = > > if_get(src6->sin6_scope_id); > > + if (m6ifp == NULL) { > > + rt = rtalloc(dst, RT_RESOLVE, > > sc->sc_rdomain); > > + if (rt) > > + m6ifp = > > if_get(rt->rt_ifidx); > > + rtfree(rt); > > + } > > + } > > + if (!m6ifp || !ISSET(m6ifp->if_flags, > > IFF_MULTICAST)) { > > + if_put(m6ifp); > > + return (EADDRNOTAVAIL); > > + } > > + > > + if ((error = in6_embedscope(&src6->sin6_addr, src6, > > NULL)) || > > + (error = in6_embedscope(&dst6->sin6_addr, dst6, > > NULL))) { > > + if_put(m6ifp); > > + return (error); > > + } > > + /* do we own this address ? */ > > + rt = rtalloc(src, 0, sc->sc_rdomain); > > + if (!rtisvalid(rt) || !ISSET(rt->rt_flags, > > RTF_LOCAL)) { > > + if_put(m6ifp); > > + rtfree(rt); > > + return (EADDRNOTAVAIL); > > + } > > + rtfree(rt); > > > > - /* Multicast mode is currently not supported for > > IPv6 */ > > - return (EAFNOSUPPORT); > > + /* > > + * If src6 has a global scope, then we do no > > further checks > > + * because whichever interface we reach on the > > remote > > + * endpoint, the EP will have all it needs to reach > > us back on > > + * unicast should it decide to do so (think dynamic > > and > > + * multipoint). > > + * > > + * However, if the source address has a scope > > smaller than > > + * global, we must ensure that the source address > > is inside > > + * the destination address' zone. > > + */ > > + if (IN6_IS_ADDR_LINKLOCAL(&src6->sin6_addr) || > > + IN6_IS_ADDR_SITELOCAL(&src6->sin6_addr)) { > > + /* > > + * As a first shot, let's say that > > sin6_scope_id > > + * must be equal. > > + */ > > + if (src6->sin6_scope_id != > > dst6->sin6_scope_id) { > > + if_put(m6ifp); > > + return (EADDRNOTAVAIL); > > + } > > + } > > + > > + im6o = &sc->sc_im6o; > > + im6o->im6o_ifidx = m6ifp->if_index; > > + im6m = in6_joingroup(m6ifp, &dst6->sin6_addr, > > &error); > > + if (!im6m) { > > + if_put(m6ifp); > > + return (error); > > + } > > + LIST_INSERT_HEAD(&im6o->im6o_memberships, im6m, > > i6mm_chain); > > + if (sc->sc_ttl > 0) > > + im6o->im6o_hlim = sc->sc_ttl; > > + else > > + im6o->im6o_hlim = ip6_defmcasthlim; > > + im6o->im6o_loop = 0; > > + mifp = m6ifp; > > + break; > > #endif /* INET6 */ > > default: > > return (EAFNOSUPPORT); > > } > > > > - src4 = satosin(src); > > - dst4 = satosin(dst); > > - > > - if (src4->sin_addr.s_addr == INADDR_ANY || > > - IN_MULTICAST(src4->sin_addr.s_addr)) > > - return (EINVAL); > > - if ((ifa = ifa_ifwithaddr(src, sc->sc_rdomain)) == NULL || > > - (mifp = ifa->ifa_ifp) == NULL || > > - (mifp->if_flags & IFF_MULTICAST) == 0) > > - return (EADDRNOTAVAIL); > > - > > - if ((imo->imo_membership[0] = > > - in_addmulti(&dst4->sin_addr, mifp)) == NULL) > > - return (ENOBUFS); > > - > > - imo->imo_num_memberships++; > > - imo->imo_ifidx = mifp->if_index; > > - if (sc->sc_ttl > 0) > > - imo->imo_ttl = sc->sc_ttl; > > - else > > - imo->imo_ttl = IP_DEFAULT_MULTICAST_TTL; > > - imo->imo_loop = 0; > > > > /* > > * Use interface hooks to track any changes on the > > interface @@ -297,6 +402,8 @@ vxlan_multicast_join(struct ifnet > > *ifp, panic("%s: cannot allocate interface hook", > > mifp->if_xname); > > > > + if_put(m6ifp); > > + > > return (0); > > } > > > > @@ -902,7 +1009,8 @@ vxlan_output(struct ifnet *ifp, struct m > > break; > > #ifdef INET6 > > case AF_INET6: > > - error = ip6_output(m, 0, NULL, IPV6_MINMTU, 0, > > NULL); > > + error = ip6_output(m, 0, NULL, IPV6_MINMTU, > > + &sc->sc_im6o, NULL); > > break; > > #endif /* INET6 */ > > default: > > >
