On Tuesday, August 14, 2012 4:44:26 am Andrey V. Elsukov wrote:
> On 07.08.2012 16:05, John Baldwin wrote:
> > I think the problem is the assertion is wrong.  We could add a new DETACHING
> > flag, but I think the simplest fix is to just remove it.  I'm not sure if a 
> > similar assertion in if_delmulti_ifma() should also be removed.
> 
> Hi, John.
> 
> This fixes the problem, thanks.

Can you actually try this patch instead?  I think I'd rather fix it this way
(this reworks how I originally tried to fix this):

Index: if_vlan.c
===================================================================
--- if_vlan.c   (revision 239294)
+++ if_vlan.c   (working copy)
@@ -192,7 +192,7 @@ static      int vlan_setflags(struct ifnet *ifp, int st
 static int vlan_setmulti(struct ifnet *ifp);
 static int vlan_transmit(struct ifnet *ifp, struct mbuf *m);
 static void vlan_unconfig(struct ifnet *ifp);
-static void vlan_unconfig_locked(struct ifnet *ifp);
+static void vlan_unconfig_locked(struct ifnet *ifp, int departing);
 static int vlan_config(struct ifvlan *ifv, struct ifnet *p, uint16_t tag);
 static void vlan_link_state(struct ifnet *ifp);
 static void vlan_capabilities(struct ifvlan *ifv);
@@ -577,7 +577,7 @@ vlan_ifdetach(void *arg __unused, struct ifnet *if
 #ifdef VLAN_ARRAY
        for (i = 0; i < VLAN_ARRAY_SIZE; i++)
                if ((ifv = ifp->if_vlantrunk->vlans[i])) {
-                       vlan_unconfig_locked(ifv->ifv_ifp);
+                       vlan_unconfig_locked(ifv->ifv_ifp, 1);
                        if (ifp->if_vlantrunk == NULL)
                                break;
                }
@@ -585,7 +585,7 @@ vlan_ifdetach(void *arg __unused, struct ifnet *if
 restart:
        for (i = 0; i < (1 << ifp->if_vlantrunk->hwidth); i++)
                if ((ifv = LIST_FIRST(&ifp->if_vlantrunk->hash[i]))) {
-                       vlan_unconfig_locked(ifv->ifv_ifp);
+                       vlan_unconfig_locked(ifv->ifv_ifp, 1);
                        if (ifp->if_vlantrunk)
                                goto restart;   /* trunk->hwidth can change */
                        else
@@ -968,7 +968,7 @@ vlan_clone_create(struct if_clone *ifc, char *name
                error = vlan_config(ifv, p, vid);
                if (error != 0) {
                        /*
-                        * Since we've partialy failed, we need to back
+                        * Since we've partially failed, we need to back
                         * out all the way, otherwise userland could get
                         * confused.  Thus, we destroy the interface.
                         */
@@ -1307,17 +1307,18 @@ vlan_unconfig(struct ifnet *ifp)
 {
 
        VLAN_LOCK();
-       vlan_unconfig_locked(ifp);
+       vlan_unconfig_locked(ifp, 0);
        VLAN_UNLOCK();
 }
 
 static void
-vlan_unconfig_locked(struct ifnet *ifp)
+vlan_unconfig_locked(struct ifnet *ifp, int departing)
 {
        struct ifvlantrunk *trunk;
        struct vlan_mc_entry *mc;
        struct ifvlan *ifv;
        struct ifnet  *parent;
+       int error;
 
        VLAN_LOCK_ASSERT();
 
@@ -1337,14 +1338,21 @@ static void
                 */
                while ((mc = SLIST_FIRST(&ifv->vlan_mc_listhead)) != NULL) {
                        /*
-                        * This may fail if the parent interface is
-                        * being detached.  Regardless, we should do a
-                        * best effort to free this interface as much
-                        * as possible as all callers expect vlan
-                        * destruction to succeed.
+                        * If the parent interface is being detached,
+                        * all it's multicast addresses have already
+                        * been removed.  Warn about errors if
+                        * if_delmulti() does fail, but don't abort as
+                        * all callers expect vlan destruction to
+                        * succeed.
                         */
-                       (void)if_delmulti(parent,
-                           (struct sockaddr *)&mc->mc_addr);
+                       if (!departing) {
+                               error = if_delmulti(parent,
+                                   (struct sockaddr *)&mc->mc_addr);
+                               if (error)
+                                       if_printf(ifp,
+                   "Failed to delete multicast address from parent: %d\n",
+                                           error);
+                       }
                        SLIST_REMOVE_HEAD(&ifv->vlan_mc_listhead, mc_entries);
                        free(mc, M_VLAN);
                }

-- 
John Baldwin
_______________________________________________
freebsd-net@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-net
To unsubscribe, send any mail to "freebsd-net-unsubscr...@freebsd.org"

Reply via email to