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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <YFKx8faByAd2cUiH@lunn.ch>
Date:   Thu, 18 Mar 2021 02:50:41 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Tobias Waldekranz <tobias@...dekranz.com>
Cc:     Vladimir Oltean <olteanv@...il.com>, davem@...emloft.net,
        kuba@...nel.org, vivien.didelot@...il.com, f.fainelli@...il.com,
        netdev@...r.kernel.org
Subject: 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 <olteanv@...il.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@...dekranz.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