On Tue, 29 Nov 2016 15:13:16 +0100
Alexander Bluhm <[email protected]> wrote:
> On Sat, Nov 05, 2016 at 12:41:39PM +0100, Vincent Gross 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 ?
>
> Some nits inline.
[snip]
About sleeping on malloc : better to err on the safe side with
M_NOWAIT.
About resolving the route : you are right, the cloning route is enough
to get the interface index.
New diff with nits fixed :
Index: sys/net/if_vxlan.c
===================================================================
RCS file: /cvs/src/sys/net/if_vxlan.c,v
retrieving revision 1.52
diff -u -p -r1.52 if_vxlan.c
--- sys/net/if_vxlan.c 29 Nov 2016 10:09:57 -0000 1.52
+++ sys/net/if_vxlan.c 30 Nov 2016 22:23:01 -0000
@@ -47,6 +47,10 @@
#include <netinet/udp_var.h>
#include <netinet/in_pcb.h>
+#ifdef INET6
+#include <netinet6/in6_var.h>
+#endif /* INET6 */
+
#if NPF > 0
#include <net/pfvar.h>
#endif
@@ -61,7 +65,14 @@ struct vxlan_softc {
struct arpcom sc_ac;
struct ifmedia sc_media;
- struct ip_moptions sc_imo;
+ union {
+ struct ip_moptions u_imo;
+#ifdef INET6
+ struct ip6_moptions u_im6o;
+#endif /* INET6 */
+ } sc_imu;
+#define sc_imo sc_imu.u_imo
+#define sc_im6o sc_imu.u_im6o
void *sc_ahcookie;
void *sc_lhcookie;
void *sc_dhcookie;
@@ -129,10 +140,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 +197,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 +205,35 @@ 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 *) * IP_MIN_MEMBERSHIPS);
+ break;
+#ifdef INET6
+ 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;
+#endif /* INET6 */
+ }
+ 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 +249,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 +259,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, *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_NOWAIT|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 (ENOMEM);
+ }
+ 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, 0, 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;
+ im6m = in6_joingroup(m6ifp, &dst6->sin6_addr, &error);
+ if (!im6m) {
+ if_put(m6ifp);
+ return (error);
+ }
+ im6o->im6o_ifidx = m6ifp->if_index;
+ 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 +408,8 @@ vxlan_multicast_join(struct ifnet *ifp,
panic("%s: cannot allocate interface hook",
mifp->if_xname);
+ if_put(m6ifp);
+
return (0);
}
@@ -902,7 +1015,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:
