Re: [PATCH net-next 5/5] net: dsa: mv88e6xxx: Offload bridge broadcast flooding flag

On Wed, Mar 17, 2021 at 12:14:18PM +0100, Tobias Waldekranz wrote:

On Tue, Mar 16, 2021 at 11:39, Vladimir Oltean [off-list ref] wrote:

quoted

On Mon, Mar 15, 2021 at 10:14:00PM +0100, Tobias Waldekranz wrote:

quoted

These switches have two modes of classifying broadcast:

1. Broadcast is multicast.
2. Broadcast is its own unique thing that is always flooded
   everywhere.

This driver uses the first option, making sure to load the broadcast
address into all active databases. Because of this, we can support
per-port broadcast flooding by (1) making sure to only set the subset
of ports that have it enabled whenever joining a new bridge or VLAN,
and (2) by updating all active databases whenever the setting is
changed on a port.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 68 +++++++++++++++++++++++++++++++-
 1 file changed, 67 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 48e65f22641e..e6987c501fb7 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c

@@ -1950,6 +1950,18 @@ static int mv88e6xxx_broadcast_setup(struct mv88e6xxx_chip *chip, u16 vid)
 	int err;
 
 	for (port = 0; port < mv88e6xxx_num_ports(chip); port++) {
+		struct dsa_port *dp = dsa_to_port(chip->ds, port);
+
+		if (dsa_is_unused_port(chip->ds, port))
+			continue;
+
+		if (dsa_is_user_port(chip->ds, port) && dp->bridge_dev &&
+		    !br_port_flag_is_set(dp->slave, BR_BCAST_FLOOD))

What if dp->slave is not the bridge port, but a LAG? br_port_flag_is_set
will return false.

Good point. I see two ways forward:

- My first idea was to cache a vector per switch that would act as the
  template when creating a new entry. This avoids having the driver
  layer knowing about stacked netdevs etc. But I think that Andrew is
  generally opposed to caching?

Hi Tobias

What i'm mostly against is dynamic memory allocation. If you can
allocate the space for this vector during probe, i have no problems
with that.

     Andrew