Message ID | 20110603201424.GB24804@midget.suse.cz |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
Jiri Bohac <jbohac@suse.cz> wrote: >Since commit ad1afb00, bond_del_vlan() never restores >NETIF_F_VLAN_CHALLENGED as intended. bond->vlan_list is never >empty once the 8021q module is loaded, because the special VLAN 0 >is always kept registered on the bond interface. Change the >condition to check if bond->vlan_list contains exactly one item >instead of checking for an empty list. > >Signed-off-by: Jiri Bohac <jbohac@suse.cz> > >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 17b4dd9..4d317cd 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -329,9 +329,10 @@ static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id) > > kfree(vlan); > >- if (list_empty(&bond->vlan_list) && >+ if (bond->vlan_list.next->next == &bond->vlan_list && > (bond->slave_cnt == 0)) { >- /* Last VLAN removed and no slaves, so >+ /* Last VLAN removed (the only member of vlan_list >+ * is the special vid == 0 vlan) and no slaves, so > * restore block on adding VLANs. This will > * be removed once new slaves that are not > * VLAN challenged will be added. Could we do this instead in bond_release, when the last slave is removed? The CHALLENGED flag just prevents adding new VLANs; existing ones would persist until a new slave was added. Since CHALLENGED slaves are in the minority these days (looks like just IPoIB, one wimax and one obscure ethernet chipset) we could even invert the logic: only assert CHALLENGED for the master when such a slave is added. We'd have to issue a NETDEV_CHANGEADDR when the master picks up or releases its MAC address so the VLANs would pick it up, but that's not a really big deal, and we probably ought to do that anyway. -J --- -Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 17b4dd9..4d317cd 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -329,9 +329,10 @@ static int bond_del_vlan(struct bonding *bond, unsigned short vlan_id) kfree(vlan); - if (list_empty(&bond->vlan_list) && + if (bond->vlan_list.next->next == &bond->vlan_list && (bond->slave_cnt == 0)) { - /* Last VLAN removed and no slaves, so + /* Last VLAN removed (the only member of vlan_list + * is the special vid == 0 vlan) and no slaves, so * restore block on adding VLANs. This will * be removed once new slaves that are not * VLAN challenged will be added.
Since commit ad1afb00, bond_del_vlan() never restores NETIF_F_VLAN_CHALLENGED as intended. bond->vlan_list is never empty once the 8021q module is loaded, because the special VLAN 0 is always kept registered on the bond interface. Change the condition to check if bond->vlan_list contains exactly one item instead of checking for an empty list. Signed-off-by: Jiri Bohac <jbohac@suse.cz>