[RFC] net: dsa: mv88e6xxx disable IGMP snooping on cpu port

[RFC] net: dsa: mv88e6xxx disable IGMP snooping on cpu port

[Fabio Estevam]

From: Steffen Bätz <steffen@innosonix.de>

Don't enable IGMP snooping on CPU ports because the IGMP JOIN
packet would never forward to the next bridge, but loop back to
the actual cpu port.

The mv88e6320 manual describes the MV88E6XXX_PORT_CTL0_IGMP_MLD_SNOOP
bit as follows:

"IGMP and MLD Snooping. When this bit is set to a one and this port
receives an IPv4 IGMP frame or an IPv6MLD frame, the frame is switched
to the CPU port overriding the destination ports determined by the DA
mapping.
When this bit is cleared to a zero IGMP/MLD frames are not treated
specially.
IGMP/MLD Snooping is intended to be used on Normal Network or Provider
ports only (see Frame Mode bits
below) and only if Cut Through (88E6632 only) is disabled on the port
(Port offset 0x1F) as the IPv6 Snoop point may be after byte 64."

If this bit is set (it was set at ALL ports), the mv88e6320 will snoop
for any IGMP messages, and route them to the configured CPU port. This
will hinder any outgoing IGMP messages from the CPU from leaving the
switch, since they are immediately looped back to the CPU itself.

Fixes: 54d792f257c6 ("net: dsa: Centralise global and port setup code into mv88e6xxx.")
Signed-off-by: Steffen Bätz <steffen@innosonix.de>
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index b73d1d6747b7..af098d65ed71 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3354,9 +3354,14 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 	 * If this is the upstream port for this switch, enable
 	 * forwarding of unknown unicasts and multicasts.
 	 */
-	reg = MV88E6XXX_PORT_CTL0_IGMP_MLD_SNOOP |
-		MV88E6185_PORT_CTL0_USE_TAG | MV88E6185_PORT_CTL0_USE_IP |
+	reg = MV88E6185_PORT_CTL0_USE_TAG | MV88E6185_PORT_CTL0_USE_IP |
 		MV88E6XXX_PORT_CTL0_STATE_FORWARDING;
+	/* Don't enable IGMP snooping on CPU ports because the IGMP JOIN
+	 * packet would never forward to the next bridge, but loop back to
+	 * the actual cpu port.
+	 */
+	if (!dsa_is_cpu_port(ds, port))
+		reg |= MV88E6XXX_PORT_CTL0_IGMP_MLD_SNOOP;
 	err = mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL0, reg);
 	if (err)
 		return err;
-- 
2.34.1

Re: [RFC] net: dsa: mv88e6xxx disable IGMP snooping on cpu port

[Andrew Lunn]

On Mon, Mar 27, 2023 at 10:48:32AM -0300, Fabio Estevam wrote:
> From: Steffen Bätz <steffen@innosonix.de>
> 
> Don't enable IGMP snooping on CPU ports because the IGMP JOIN
> packet would never forward to the next bridge, but loop back to
> the actual cpu port.
> 
> The mv88e6320 manual describes the MV88E6XXX_PORT_CTL0_IGMP_MLD_SNOOP
> bit as follows:
> 
> "IGMP and MLD Snooping. When this bit is set to a one and this port
> receives an IPv4 IGMP frame or an IPv6MLD frame, the frame is switched
> to the CPU port overriding the destination ports determined by the DA
> mapping.
> When this bit is cleared to a zero IGMP/MLD frames are not treated
> specially.
> IGMP/MLD Snooping is intended to be used on Normal Network or Provider
> ports only (see Frame Mode bits
> below) and only if Cut Through (88E6632 only) is disabled on the port
> (Port offset 0x1F) as the IPv6 Snoop point may be after byte 64."
> 
> If this bit is set (it was set at ALL ports), the mv88e6320 will snoop
> for any IGMP messages, and route them to the configured CPU port. This
> will hinder any outgoing IGMP messages from the CPU from leaving the
> switch, since they are immediately looped back to the CPU itself.

Hi Fabio, Steffen

It seems like you need the same change for DSA ports as well?

I did test IGMP snooping many years ago and it seemed to work. Has
there been any recent change in this code? Or is any of this behaviour
specific to the 6320? I probably tested using 6352, or 6390.

	 Andrew

Re: [RFC] net: dsa: mv88e6xxx disable IGMP snooping on cpu port

[Andrew Lunn]

On Mon, Mar 27, 2023 at 04:51:20PM +0200, Steffen Bätz - Innosonix GmbH wrote:
> Hi Andrew,
> Yes, strangely, this piece of code has stayed the same for years.
> But we only face this behaviour if you add a bridge interface on top of the DSA
> ports.
> The setup looks like: eth0 (physical port of the imx8mn) <-> lan3/lan4 (DSA
> ports of the mv88e6320)
> 
> ip link add br0 type bridge
> ip link set br0 up
> ip link set lan3 master br0
> ip link set lan4 master br0
> ip link set lan3 up
> ip link set lan4 up
> dhcpcd -b br0
> 
> If now you try to receive an audio multicast stream, like from 239.255.84.1,
> the neighbour bridge will not forward this stream to our board. And there is no
> entry in the MDB of the external switch.

Ah, O.K. That is not actually IGMP snooping. It is normal usage of
IGMP. I probably did not test that. I think all my multicast source
and sinks where on switch ports, and i tested that the switch did not
flood multicast to all ports, just ports with listeners.

So your change looks O.K, but as i said, it probably should apply to
DSA ports as well as CPU ports. And i suggest you reword the commit
message to differentiate between IGMP snooping and an IGMP listener on
the bridge.

    Andrew