netdev - Re: [PATCH net-next] bridge: vlan: allow to suppress local mac install for all vlans

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Wed, 26 Aug 2015 05:56:27 -0700
From:	Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
To:	vyasevic@...hat.com
Cc:	netdev@...r.kernel.org, toshiaki.makita1@...il.com,
	stephen@...workplumber.org, bridge@...ts.linux-foundation.org,
	wkok@...ulusnetworks.com, davem@...emloft.net
Subject: Re: [PATCH net-next] bridge: vlan: allow to suppress local mac install for all vlans


> On Aug 26, 2015, at 5:42 AM, Vlad Yasevich <vyasevic@...hat.com> wrote:
> 
> On 08/24/2015 08:55 PM, Nikolay Aleksandrov wrote:
>> From: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>> 
>> This patch adds a new knob that, when enabled, allows to suppress the
>> installation of local fdb entries in newly created vlans. This could
>> pose a big scalability issue if we have a large number of ports and a
>> large number of vlans, e.g. in a 48 port device with 2000 vlans these
>> entries easily go up to 96000.
>> Note that packets for these macs are still received properly because they
>> are added in vlan 0 as "own" macs and referenced when fdb lookup by vlan
>> results in a miss.
>> Also note that vlan membership of ingress port and the bridge device
>> as egress are still being correctly enforced.
>> 
>> The default (0/off) is keeping the current behaviour.
>> 
>> Based on a patch by Wilson Kok (wkok@...ulusnetworks.com).
>> 
>> Signed-off-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
>> ---
>> As usual I'll post iproute2 patch if this one gets accepted.
>> 
> 
> ... snip...
> 
>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>> index 3cef6892c0bb..f9efa1b07994 100644
>> --- a/net/bridge/br_vlan.c
>> +++ b/net/bridge/br_vlan.c
>> @@ -98,11 +98,12 @@ static int __vlan_add(struct net_port_vlans *v, u16 vid, u16 flags)
>> 			return err;
>> 	}
>> 
>> -	err = br_fdb_insert(br, p, dev->dev_addr, vid);
>> -	if (err) {
>> -		br_err(br, "failed insert local address into bridge "
>> -		       "forwarding table\n");
>> -		goto out_filt;
>> +	if (!br_vlan_ignore_local_fdb(br) || !v->port_idx) {
>> +		err = br_fdb_insert(br, p, dev->dev_addr, vid);
>> +		if (err) {
>> +			br_err(br, "failed insert local address into bridge forwarding table\n");
>> +			goto out_filt;
>> +		}
>> 	}
>> 
> 
> One question.  Does it make sense to push this down into br_fdb_insert?
> This patch prevents automatic entries from being added.  But what about
> manual entries for a local fdb?  The code in br_fdb_add() will still a
> vid 0 entry as well as entries for all vlans currently configured on the port.
> 
> -vlad
> 

Good point, it would make sense if we go this way, but as Dave suggested it’d be better
to fix the root cause of the scalability issue rather than trying to work around it, so I’m
dropping this patch for now and will try to come up with a different solution, need to look
into this more.

>> 	set_bit(vid, v->vlan_bitmap);
>> @@ -492,6 +493,13 @@ int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val)
>> 	return 0;
>> }
>> 
>> +int br_vlan_ignore_local_fdb_toggle(struct net_bridge *br, unsigned long val)
>> +{
>> +	br->vlan_ignore_local_fdb = val ? true : false;
>> +
>> +	return 0;
>> +}
>> +
>> int br_vlan_set_proto(struct net_bridge *br, unsigned long val)
>> {
>> 	int err = 0;

--
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