Re: [RFC PATCH 0/2] enable broadcom tagging for bcm531x5 switches

[Florian Fainelli <f.fainelli@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1836 bytes --]



On 6/19/2019 2:18 AM, Benedikt Spranger wrote:
> On Tue, 18 Jun 2019 11:16:23 -0700
> Florian Fainelli <f.fainelli@gmail.com> wrote:
> 
>> How is that a problem for other machines? Does that lead to some kind
>> of broadcast storm because there are machines that keep trying to
>> respond to ARP solicitations?
> Mirroring broadcast packages on the interface they are coming in, is
> IMHO a poor idea. As a result any switch connected to wan update the
> MAC table and send packages on a port where they do not belong to.
> Just imagine to send a DHCP request. The BPi R1 acts as nearly perfect
> black hole in such a situation.

Fair enough.

> 
>> The few aspects that bother me, not in any particular order, are that:
>>
>> - you might be able to not change anything and just get away with the
>> one line patch below that sets skb->offload_fwd_mark to 1 to indicate
>> to the bridge, not to bother with sending a copy of the packet, since
>> the HW took care of that already
> 
> I can test it, but i like to note that the changed function is not
> executed in case of bcm53125.

Indeed, that won't work, how about implementing port_egress_flood() for
b53? That would make sure that multicast is flooded (as well as unicast)
before the switch port is enslaved into the bridge and this should take
care of both your problem and the lack of multicast flooding once
Broadcom tags are turned on. You might also need to set
skb->fwd_offload_mark accordingly in case you still see duplicate
packets, though that should not happen anymore AFAICT.

Something like this should take care of that (untested). You might have
to explicitly set the IMP port (port 8) in B53_UC_FWD_EN and
B53_MC_FWD_EN, though since you turn on management mode, this may not be
required. I might have some time tomorrow to test that on a Lamobo R1.
-- 
Florian

[-- Attachment #2: b53-egress-floods.patch --]
[-- Type: text/plain, Size: 2684 bytes --]

diff --git a/drivers/net/dsa/b53/b53_common.c b/drivers/net/dsa/b53/b53_common.c
index c8040ecf4425..a47f5bc667bd 100644
--- a/drivers/net/dsa/b53/b53_common.c
+++ b/drivers/net/dsa/b53/b53_common.c
@@ -342,6 +342,13 @@ static void b53_set_forwarding(struct b53_device *dev, int enable)
 	b53_read8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, &mgmt);
 	mgmt |= B53_MII_DUMB_FWDG_EN;
 	b53_write8(dev, B53_CTRL_PAGE, B53_SWITCH_CTRL, mgmt);
+
+	/* Look at B53_UC_FWD_EN and B53_MC_FWD_EN to decide whether
+	 * frames should be flooed or not.
+	 */
+	b53_read8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, &mgmt);
+	mgmt |= B53_UC_FWD_EN | B53_MC_FWD_EN;
+	b53_write8(dev, B53_CTRL_PAGE, B53_IP_MULTICAST_CTRL, mgmt);
 }
 
 static void b53_enable_vlan(struct b53_device *dev, bool enable,
@@ -1748,6 +1755,31 @@ void b53_br_fast_age(struct dsa_switch *ds, int port)
 }
 EXPORT_SYMBOL(b53_br_fast_age);
 
+int b53_br_egress_floods(struct dsa_switch *ds, int port,
+			 bool unicast, bool multicast)
+{
+	struct b53_device *dev = ds->priv;
+	u16 uc, mc;
+
+	b53_read16(dev, B53_CTRL_PAGE, B53_UC_FWD_EN, &uc);
+	if (unicast)
+		uc |= BIT(port);
+	else
+		uc &= ~BIT(port);
+	b53_write16(dev, B53_CTRL_PAGE, B53_UC_FWD_EN, uc);
+
+	b53_read16(dev, B53_CTRL_PAGE, B53_MC_FWD_EN, &mc);
+	if (multicast)
+		mc |= BIT(port);
+	else
+		mc &= ~BIT(port);
+	b53_write16(dev, B53_CTRL_PAGE, B53_MC_FWD_EN, mc);
+
+	return 0;
+
+}
+EXPORT_SYMBOL(b53_br_egress_floods);
+
 static bool b53_possible_cpu_port(struct dsa_switch *ds, int port)
 {
 	/* Broadcom switches will accept enabling Broadcom tags on the
@@ -1948,6 +1980,7 @@ static const struct dsa_switch_ops b53_switch_ops = {
 	.port_bridge_leave	= b53_br_leave,
 	.port_stp_state_set	= b53_br_set_stp_state,
 	.port_fast_age		= b53_br_fast_age,
+	.port_egress_floods	= b53_br_egress_floods,
 	.port_vlan_filtering	= b53_vlan_filtering,
 	.port_vlan_prepare	= b53_vlan_prepare,
 	.port_vlan_add		= b53_vlan_add,
diff --git a/drivers/net/dsa/b53/b53_priv.h b/drivers/net/dsa/b53/b53_priv.h
index f25bc80c4ffc..a7dd8acc281b 100644
--- a/drivers/net/dsa/b53/b53_priv.h
+++ b/drivers/net/dsa/b53/b53_priv.h
@@ -319,6 +319,8 @@ int b53_br_join(struct dsa_switch *ds, int port, struct net_device *bridge);
 void b53_br_leave(struct dsa_switch *ds, int port, struct net_device *bridge);
 void b53_br_set_stp_state(struct dsa_switch *ds, int port, u8 state);
 void b53_br_fast_age(struct dsa_switch *ds, int port);
+int b53_br_egress_floods(struct dsa_switch *ds, int port,
+			 bool unicast, bool multicast);
 void b53_port_event(struct dsa_switch *ds, int port);
 void b53_phylink_validate(struct dsa_switch *ds, int port,
 			  unsigned long *supported,