[PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[Vivien Didelot]

The DSA ports must flood unknown unicast and multicast, but the switch
must not flood the CPU ports with unknown multicast, as this results
in a lot of undesirable traffic that the network stack needs to filter
in software.

Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d8d1781810e2..e412ccabd104 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2111,15 +2111,13 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
 static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
 {
 	struct dsa_switch *ds = chip->ds;
-	bool flood;
+	bool uc = dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port);
+	bool mc = dsa_is_dsa_port(ds, port);
 
-	/* Upstream ports flood frames with unknown unicast or multicast DA */
-	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
-	if (chip->info->ops->port_set_egress_floods)
-		return chip->info->ops->port_set_egress_floods(chip, port,
-							       flood, flood);
+	if (!chip->info->ops->port_set_egress_floods)
+		return 0;
 
-	return 0;
+	return chip->info->ops->port_set_egress_floods(chip, port, uc, mc);
 }
 
 static int mv88e6xxx_serdes_power(struct mv88e6xxx_chip *chip, int port,
-- 
2.21.0

Re: [PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[Russell King - ARM Linux admin]

On Wed, Jun 12, 2019 at 06:33:44PM -0400, Vivien Didelot wrote:
> The DSA ports must flood unknown unicast and multicast, but the switch
> must not flood the CPU ports with unknown multicast, as this results
> in a lot of undesirable traffic that the network stack needs to filter
> in software.

What if you have configured IPv6 on the bridge device, and are expecting
the multicasted IPv6 frames for neighbour discovery to work?

> 
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index d8d1781810e2..e412ccabd104 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2111,15 +2111,13 @@ static int mv88e6xxx_setup_message_port(struct mv88e6xxx_chip *chip, int port)
>  static int mv88e6xxx_setup_egress_floods(struct mv88e6xxx_chip *chip, int port)
>  {
>  	struct dsa_switch *ds = chip->ds;
> -	bool flood;
> +	bool uc = dsa_is_dsa_port(ds, port) || dsa_is_cpu_port(ds, port);
> +	bool mc = dsa_is_dsa_port(ds, port);
>  
> -	/* Upstream ports flood frames with unknown unicast or multicast DA */
> -	flood = dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port);
> -	if (chip->info->ops->port_set_egress_floods)
> -		return chip->info->ops->port_set_egress_floods(chip, port,
> -							       flood, flood);
> +	if (!chip->info->ops->port_set_egress_floods)
> +		return 0;
>  
> -	return 0;
> +	return chip->info->ops->port_set_egress_floods(chip, port, uc, mc);
>  }
>  
>  static int mv88e6xxx_serdes_power(struct mv88e6xxx_chip *chip, int port,
> -- 
> 2.21.0
> 
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[Vivien Didelot]

Hi Russell,

On Thu, 13 Jun 2019 00:25:52 +0100, Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote:
> On Wed, Jun 12, 2019 at 06:33:44PM -0400, Vivien Didelot wrote:
> > The DSA ports must flood unknown unicast and multicast, but the switch
> > must not flood the CPU ports with unknown multicast, as this results
> > in a lot of undesirable traffic that the network stack needs to filter
> > in software.
> 
> What if you have configured IPv6 on the bridge device, and are expecting
> the multicasted IPv6 frames for neighbour discovery to work?

Multicast management should still be flooded, so I would expect this
to work? Can you describe the test you want me to run for this case?


Thanks,
Vivien

Re: [PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[David Miller]

From: Vivien Didelot <vivien.didelot@gmail.com>
Date: Wed, 12 Jun 2019 18:33:44 -0400

> The DSA ports must flood unknown unicast and multicast, but the switch
> must not flood the CPU ports with unknown multicast, as this results
> in a lot of undesirable traffic that the network stack needs to filter
> in software.
> 
> Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>

Applied.

Re: [PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[Russell King - ARM Linux admin]

On Sat, Jun 15, 2019 at 01:25:55PM -0700, David Miller wrote:
> From: Vivien Didelot <vivien.didelot@gmail.com>
> Date: Wed, 12 Jun 2019 18:33:44 -0400
> 
> > The DSA ports must flood unknown unicast and multicast, but the switch
> > must not flood the CPU ports with unknown multicast, as this results
> > in a lot of undesirable traffic that the network stack needs to filter
> > in software.
> > 
> > Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> 
> Applied.

Hi Dave,

We found this breaks IPv6, so it shouldn't have been applied (which is
the point I raised when I replied to Vivien).  Vivien is now able to
reproduce that.

I guess you need a revert patch now?

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

Re: [PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[David Miller]

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
Date: Sat, 15 Jun 2019 21:28:10 +0100

> On Sat, Jun 15, 2019 at 01:25:55PM -0700, David Miller wrote:
>> From: Vivien Didelot <vivien.didelot@gmail.com>
>> Date: Wed, 12 Jun 2019 18:33:44 -0400
>> 
>> > The DSA ports must flood unknown unicast and multicast, but the switch
>> > must not flood the CPU ports with unknown multicast, as this results
>> > in a lot of undesirable traffic that the network stack needs to filter
>> > in software.
>> > 
>> > Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
>> 
>> Applied.
> 
> Hi Dave,
> 
> We found this breaks IPv6, so it shouldn't have been applied (which is
> the point I raised when I replied to Vivien).  Vivien is now able to
> reproduce that.
> 
> I guess you need a revert patch now?

Yep, I'll revert, thanks for letting me know.

Re: [PATCH net-next] net: dsa: mv88e6xxx: do not flood CPU with unknown multicast

[Vivien Didelot]

Hi David, Russell,

On Sat, 15 Jun 2019 13:35:13 -0700 (PDT), David Miller <davem@davemloft.net> wrote:
> From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
> Date: Sat, 15 Jun 2019 21:28:10 +0100
> 
> > On Sat, Jun 15, 2019 at 01:25:55PM -0700, David Miller wrote:
> >> From: Vivien Didelot <vivien.didelot@gmail.com>
> >> Date: Wed, 12 Jun 2019 18:33:44 -0400
> >> 
> >> > The DSA ports must flood unknown unicast and multicast, but the switch
> >> > must not flood the CPU ports with unknown multicast, as this results
> >> > in a lot of undesirable traffic that the network stack needs to filter
> >> > in software.
> >> > 
> >> > Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
> >> 
> >> Applied.
> > 
> > Hi Dave,
> > 
> > We found this breaks IPv6, so it shouldn't have been applied (which is
> > the point I raised when I replied to Vivien).  Vivien is now able to
> > reproduce that.
> > 
> > I guess you need a revert patch now?
> 
> Yep, I'll revert, thanks for letting me know.

Indeed this patch requires to enable multicast_querier in some scenarios,
while flooding the CPU ports with unknown multicast traffic may still be
necessary in some cases, e.g. when a non-DSA interface is member of the bridge.

To give a bit more details, Russell's shown me that pinging the bridge's
IPv6 address won't work as is with this patch because the bridge won't flood
the IPv6 neighbor solicitation to the CPU. A good reflex here is to enable
multicast_querier on the bridge, to program its address into the switch.

But I will propose another patch making the flooding of unknown multicast
configurable, to work with all scenarios without making the code too complex.

Thanks,
Vivien