netdev - Re: [PATCH 2/2] bonding: restore NETIF_F_VLAN_CHALLENGED properly in bond_del

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <28701.1307147211@death>
Date:	Fri, 03 Jun 2011 17:26:51 -0700
From:	Jay Vosburgh <fubar@...ibm.com>
To:	Jiri Bohac <jbohac@...e.cz>
cc:	Andy Gospodarek <andy@...yhouse.net>,
	"David S. Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	Pedro Garcia <pedro.netdev@...devamos.com>,
	Patrick McHardy <kaber@...sh.net>
Subject: Re: [PATCH 2/2] bonding: restore NETIF_F_VLAN_CHALLENGED properly in bond_del_vlan()

Jiri Bohac <jbohac@...e.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@...e.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@...ibm.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html