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

[Andrew Lunn <andrew@lunn.ch>]

On Wed, Mar 17, 2021 at 12:14:18PM +0100, Tobias Waldekranz wrote:
> On Tue, Mar 16, 2021 at 11:39, Vladimir Oltean <olteanv@gmail.com> wrote:
> > On Mon, Mar 15, 2021 at 10:14:00PM +0100, Tobias Waldekranz wrote:
> >> 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