On Fri, Dec 30, 2016 at 18:57 +0100, Mike Belopuhov wrote: > On Thu, Dec 29, 2016 at 09:30 +0100, Martin Pieuchot wrote: > > On 29/12/16(Thu) 01:15, Alexander Bluhm wrote: > > > On Fri, Dec 23, 2016 at 12:09:32AM +0100, Martin Pieuchot wrote: > > > > On 22/12/16(Thu) 20:45, Mike Belopuhov wrote: > > > > > I think this is what is required here. Works here, but YMMV. > > > > > > > > splnet() in a pseudo-driver seems completely wrong, you could get rid of > > > > it. > > > > > > Yes, but that is another issue. Can we get the netlock splasserts > > > fixed first? This diff looks good to me. > > > > Sure I'm ok with the diff. > > > > I agree with Martin and have cooked a diff but couldn't test it yet. > This is it for the reference. >
I got to test the diff and I had to make another adjustment: vxlan_if_change is setup as a detach hook, however dohooks is called very early in if_detach before we remove IP addresses from the interface. It makes vxlan_config find these IP addresses just fine and re-add its own detach hook again. This repeats ad infinitum hogging the machine. I couldn't think of anything better than deferring an operation via a task. Seems to do the trick. OK? diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c index e9bc1cb8305..082e4ab5814 100644 --- sys/net/if_vxlan.c +++ sys/net/if_vxlan.c @@ -71,10 +71,12 @@ struct vxlan_softc { in_port_t sc_dstport; u_int sc_rdomain; int64_t sc_vnetid; u_int8_t sc_ttl; + struct task sc_task; + LIST_ENTRY(vxlan_softc) sc_entry; }; void vxlanattach(int); int vxlanioctl(struct ifnet *, u_long, caddr_t); @@ -89,10 +91,11 @@ void vxlan_media_status(struct ifnet *, struct ifmediareq *); int vxlan_config(struct ifnet *, struct sockaddr *, struct sockaddr *); int vxlan_output(struct ifnet *, struct mbuf *); void vxlan_addr_change(void *); void vxlan_if_change(void *); void vxlan_link_change(void *); +void vxlan_config_defer(void *); int vxlan_sockaddr_cmp(struct sockaddr *, struct sockaddr *); uint16_t vxlan_sockaddr_port(struct sockaddr *); struct if_clone vxlan_cloner = @@ -166,10 +169,12 @@ vxlan_clone_create(struct if_clone *ifc, int unit) */ ifp->if_mtu = ETHERMTU - sizeof(struct ether_header); ifp->if_mtu -= sizeof(struct vxlanudphdr) + sizeof(struct ipovly); #endif + task_set(&sc->sc_task, vxlan_config_defer, sc); + LIST_INSERT_HEAD(&vxlan_tagh[VXLAN_TAGHASH(0)], sc, sc_entry); vxlan_enable++; return (0); } @@ -178,13 +183,13 @@ int vxlan_clone_destroy(struct ifnet *ifp) { struct vxlan_softc *sc = ifp->if_softc; int s; - s = splnet(); + NET_LOCK(s); vxlan_multicast_cleanup(ifp); - splx(s); + NET_UNLOCK(s); vxlan_enable--; LIST_REMOVE(sc, sc_entry); ifmedia_delete_instance(&sc->sc_media, IFM_INST_ANY); @@ -392,11 +397,11 @@ int vxlanioctl(struct ifnet *ifp, u_long cmd, caddr_t data) { struct vxlan_softc *sc = (struct vxlan_softc *)ifp->if_softc; struct ifreq *ifr = (struct ifreq *)data; struct if_laddrreq *lifr = (struct if_laddrreq *)data; - int error = 0, s; + int error = 0; switch (cmd) { case SIOCSIFADDR: ifp->if_flags |= IFF_UP; /* FALLTHROUGH */ @@ -417,24 +422,20 @@ vxlanioctl(struct ifnet *ifp, u_long cmd, caddr_t data) case SIOCSIFMEDIA: error = ifmedia_ioctl(ifp, ifr, &sc->sc_media, cmd); break; case SIOCSLIFPHYADDR: - s = splnet(); error = vxlan_config(ifp, (struct sockaddr *)&lifr->addr, (struct sockaddr *)&lifr->dstaddr); - splx(s); break; case SIOCDIFPHYADDR: - s = splnet(); vxlan_multicast_cleanup(ifp); bzero(&sc->sc_src, sizeof(sc->sc_src)); bzero(&sc->sc_dst, sizeof(sc->sc_dst)); sc->sc_dstport = htons(VXLAN_PORT); - splx(s); break; case SIOCGLIFPHYADDR: if (sc->sc_dst.ss_family == AF_UNSPEC) { error = EADDRNOTAVAIL; @@ -451,14 +452,12 @@ vxlanioctl(struct ifnet *ifp, u_long cmd, caddr_t data) ifr->ifr_rdomainid > RT_TABLEID_MAX || !rtable_exists(ifr->ifr_rdomainid)) { error = EINVAL; break; } - s = splnet(); sc->sc_rdomain = ifr->ifr_rdomainid; (void)vxlan_config(ifp, NULL, NULL); - splx(s); break; case SIOCGLIFPHYRTABLE: ifr->ifr_rdomainid = sc->sc_rdomain; break; @@ -468,14 +467,12 @@ vxlanioctl(struct ifnet *ifp, u_long cmd, caddr_t data) error = EINVAL; break; } if (sc->sc_ttl == (u_int8_t)ifr->ifr_ttl) break; - s = splnet(); sc->sc_ttl = (u_int8_t)(ifr->ifr_ttl); (void)vxlan_config(ifp, NULL, NULL); - splx(s); break; case SIOCGLIFPHYTTL: ifr->ifr_ttl = (int)sc->sc_ttl; break; @@ -489,14 +486,12 @@ vxlanioctl(struct ifnet *ifp, u_long cmd, caddr_t data) ifr->ifr_vnetid < VXLAN_VNI_MIN)) { error = EINVAL; break; } - s = splnet(); sc->sc_vnetid = (int)ifr->ifr_vnetid; (void)vxlan_config(ifp, NULL, NULL); - splx(s); break; case SIOCGVNETID: if ((sc->sc_vnetid != VXLAN_VNI_ANY) && (sc->sc_vnetid > VXLAN_VNI_MAX || @@ -507,14 +502,12 @@ vxlanioctl(struct ifnet *ifp, u_long cmd, caddr_t data) ifr->ifr_vnetid = sc->sc_vnetid; break; case SIOCDVNETID: - s = splnet(); sc->sc_vnetid = VXLAN_VNI_UNSET; (void)vxlan_config(ifp, NULL, NULL); - splx(s); break; default: error = ether_ioctl(ifp, &sc->sc_ac, cmd, data); break; @@ -923,57 +916,61 @@ vxlan_output(struct ifnet *ifp, struct mbuf *m) void vxlan_addr_change(void *arg) { struct vxlan_softc *sc = arg; struct ifnet *ifp = &sc->sc_ac.ac_if; - int s, error; + int error; /* * Reset the configuration after resume or any possible address * configuration changes. */ - s = splnet(); if ((error = vxlan_config(ifp, NULL, NULL))) { /* * The source address of the tunnel can temporarily disappear, * after a link state change when running the DHCP client, * so keep it configured. */ } - splx(s); } void vxlan_if_change(void *arg) { struct vxlan_softc *sc = arg; - struct ifnet *ifp = &sc->sc_ac.ac_if; - int s, error; /* - * Reset the configuration after the parent interface disappeared. + * Defer resetting the configuration after the parent interface + * disappeared to give system a chance to update configuration. */ - s = splnet(); - if ((error = vxlan_config(ifp, NULL, NULL)) != 0) { - /* The configured tunnel addresses are invalid, remove them */ - bzero(&sc->sc_src, sizeof(sc->sc_src)); - bzero(&sc->sc_dst, sizeof(sc->sc_dst)); - } - splx(s); + task_add(systq, &sc->sc_task); } void vxlan_link_change(void *arg) { struct vxlan_softc *sc = arg; struct ifnet *ifp = &sc->sc_ac.ac_if; - int s; /* * The machine might have lost its multicast associations after * link state changes. This fixes a problem with VMware after * suspend/resume of the host or guest. */ - s = splnet(); (void)vxlan_config(ifp, NULL, NULL); - splx(s); +} + +void +vxlan_config_defer(void *arg) +{ + struct vxlan_softc *sc = arg; + struct ifnet *ifp = &sc->sc_ac.ac_if; + int s; + + NET_LOCK(s); + if (vxlan_config(ifp, NULL, NULL) != 0) { + /* The configured tunnel addresses are invalid, remove them */ + bzero(&sc->sc_src, sizeof(sc->sc_src)); + bzero(&sc->sc_dst, sizeof(sc->sc_dst)); + } + NET_UNLOCK(s); }