Hi, As said in Subject:.
I would like to get comments on the m_adj/m_pullup dance at the end of vxlan_lookup() ; I do this because ether_input() access the ethernet header with mtod(), and under some conditions the mbuf handled would have its first data chunk empty (mh_len == 0). What is the rule of thumb regarding m_pullup/mtod use versus m_copydata ? Aside from the mbuf issue, is this Ok ? Index: net/if_vxlan.c =================================================================== RCS file: /cvs/src/sys/net/if_vxlan.c,v retrieving revision 1.44 diff -u -p -r1.44 if_vxlan.c --- net/if_vxlan.c 4 Sep 2016 11:14:44 -0000 1.44 +++ net/if_vxlan.c 24 Sep 2016 08:37:22 -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,47 +253,136 @@ 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 *imm; struct sockaddr_in *src4, *dst4; - struct sockaddr_in6 *dst6; + struct sockaddr_in6 *src6, *dst6; struct ifaddr *ifa; - struct ifnet *mifp; + struct ifnet *mifp = NULL; + struct rtentry *rt; + int error; - if (dst->sa_family == AF_INET) { + switch (dst->sa_family) { + case AF_INET: dst4 = satosin(dst); + src4 = satosin(src); if (!IN_MULTICAST(dst4->sin_addr.s_addr)) return (0); - } else if (dst->sa_family == AF_INET6) { + 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; + 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) + mifp = if_get(dst6->sin6_scope_id); + } else { + /* + * unscoped multicast destination : get ifindex + * from src6, or routing table as fallack + */ + if (src6->sin6_scope_id) + mifp = if_get(src6->sin6_scope_id); + if (mifp == NULL) { + rt = rtalloc(dst, RT_RESOLVE, sc->sc_rdomain); + if (rt) + mifp = if_get(rt->rt_ifidx); + rtfree(rt); + } + } + if (!mifp || !ISSET(mifp->if_flags, IFF_MULTICAST)) { + if_put(mifp); + return (EADDRNOTAVAIL); + } + + if ((error = in6_embedscope(&src6->sin6_addr, src6, NULL)) || + (error = in6_embedscope(&dst6->sin6_addr, dst6, NULL))) { + if_put(mifp); + 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(mifp); + rtfree(rt); + return (EADDRNOTAVAIL); + } + rtfree(rt); - /* Multicast mode is currently not supported for IPv6 */ + /* + * 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(mifp); + return (EADDRNOTAVAIL); + } + } + + im6o = &sc->sc_im6o; + im6o->im6o_ifidx = mifp->if_index; + imm = in6_joingroup(mifp, &dst6->sin6_addr, &error); + if (!imm) { + if_put(mifp); + return (error); + } + LIST_INSERT_HEAD(&im6o->im6o_memberships, imm, i6mm_chain); + if (sc->sc_ttl > 0) + im6o->im6o_hlim = sc->sc_ttl; + else + im6o->im6o_hlim = ip6_defmcasthlim; + im6o->im6o_loop = 0; + break; + 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 @@ -289,6 +397,9 @@ vxlan_multicast_join(struct ifnet *ifp, panic("%s: cannot allocate interface hook", mifp->if_xname); + if (imm) + if_put(mifp); + return (0); } @@ -636,7 +747,9 @@ vxlan_lookup(struct mbuf *m, struct udph return (0); found: - m_adj(m, skip); + m_adj(m, skip - ETHER_ALIGN); + m = m_pullup(m, ETHER_HDR_LEN + ETHER_ALIGN); + m_adj(m, ETHER_ALIGN); ifp = &sc->sc_ac.ac_if; if ((eh = mtod(m, struct ether_header *)) == NULL) @@ -843,7 +956,8 @@ vxlan_output(struct ifnet *ifp, struct m error = ip_output(m, NULL, NULL, IP_RAWOUTPUT, &sc->sc_imo, NULL, 0); else - error = ip6_output(m, 0, NULL, IPV6_MINMTU, 0, NULL); + error = ip6_output(m, 0, NULL, IPV6_MINMTU, + &sc->sc_im6o, NULL); if (error) ifp->if_oerrors++;