netdev - Re: [PATCH RFC 1/2] net: dsa: integrate with SWITCHDEV for HW bridging

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <54E3EE40.9010406@gmail.com>
Date:	Tue, 17 Feb 2015 17:43:28 -0800
From:	Florian Fainelli <f.fainelli@...il.com>
To:	Andrew Lunn <andrew@...n.ch>
CC:	netdev@...r.kernel.org, davem@...emloft.net,
	vivien.didelot@...oirfairelinux.com,
	jerome.oufella@...oirfairelinux.com, linux@...ck-us.net,
	cphealy@...il.com, Scott Feldman <sfeldma@...il.com>,
	Jiri Pirko <jiri@...nulli.us>
Subject: Re: [PATCH RFC 1/2] net: dsa: integrate with SWITCHDEV for HW bridging

On 17/02/15 17:19, Andrew Lunn wrote:
>> +/* Return a bitmask of all ports being currently bridged. Note that on
>> + * leave, the mask will still return the bitmask of ports currently bridged,
>> + * prior to port removal, and this is exactly what we want.
>> + */
>> +static u32 dsa_slave_br_port_mask(struct dsa_switch *ds)
>> +{
>> +	unsigned int port;
>> +	u32 mask = 0;
>> +
>> +	for (port = 0; port < DSA_MAX_PORTS; port++) {
>> +		if (!((1 << port) & ds->phys_port_mask))
>> +			continue;
>> +
>> +		if (ds->ports[port]->priv_flags & IFF_BRIDGE_PORT)
>> +			mask |= 1 << port;
>> +	}
>> +
>> +       return mask;
>> +}
>> +
>> +static int dsa_slave_bridge_port_join(struct net_device *dev,
>> +				      struct net_device *bridge)
>> +{
>> +	struct dsa_slave_priv *p = netdev_priv(dev);
>> +	struct dsa_switch *ds = p->parent;
>> +	int ret = -EOPNOTSUPP;
>> +
>> +	if (ds->drv->port_join_bridge)
>> +		ret = ds->drv->port_join_bridge(ds, p->port,
>> +						dsa_slave_br_port_mask(ds));
> 
> Hi Florian
> 
> Shouldn't this bridge port mask also be dependent on bridge?

Yes, you are very right, thankfully this is a RFC patch because of that ;)

> 
> I'm thinking of cases like
> 
> brctl addbr br0
> brctl addif br0 lan0
> brctl addif br0 lan1
> 
> brctl addbr br1
> brctl addif br1 lan2

mask = 0x4 | 0x2 | 0x1 # FAIL

> brctl addif br1 lan3

mask = 0x8 | 0x4 | 0x2 | 0x1 # FAIL

> 
> We have two software bridges, so need two masks.  It does look like
> your hardware and the Marvell hardware supports this, disjoint sets of
> bridged ports.  But with the current implementation, your going to end
> up with one hardware bridge with four ports, and broken STP.

Yep, I will rework that patch set to address that, and I can actually
test that, which is even better, thanks!
-- 
Florian
--
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