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.
diff --git sys/net/if_vxlan.c sys/net/if_vxlan.c
index e9bc1cb8305..74437b8b79c 100644
--- sys/net/if_vxlan.c
+++ sys/net/if_vxlan.c
@@ -178,13 +178,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 +392,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 +417,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 +447,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 +462,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 +481,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 +497,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 +911,56 @@ 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;
+
+ NET_ASSERT_LOCKED();
/*
* 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;
+ int error;
+
+ NET_ASSERT_LOCKED();
/*
* Reset the configuration after the parent interface disappeared.
*/
- 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);
}
void
vxlan_link_change(void *arg)
{
struct vxlan_softc *sc = arg;
struct ifnet *ifp = &sc->sc_ac.ac_if;
- int s;
+
+ NET_ASSERT_LOCKED();
/*
* 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);
}
