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);
}
