netdev - Re: [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops

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: <878rxkjgtp.fsf@waldekranz.com>
Date:   Fri, 19 Nov 2021 16:38:58 +0100
From:   Tobias Waldekranz <tobias@...dekranz.com>
To:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org
Cc:     Florian Fainelli <f.fainelli@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        DENG Qingfang <dqfext@...il.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>
Subject: Re: [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops ::
 port_bridge_tx_fwd_{,un}offload

On Tue, Oct 26, 2021 at 19:26, Vladimir Oltean <vladimir.oltean@....com> wrote:
> We don't really need new switch API for these, and with new switches
> which intend to add support for this feature, it will become cumbersome
> to maintain.
>
> Simply pass a boolean argument to ->port_bridge_join which the driver
> must set to true if it implements the TX forwarding offload feature.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
> ---
>  drivers/net/dsa/b53/b53_common.c       |  3 +-
>  drivers/net/dsa/b53/b53_priv.h         |  3 +-
>  drivers/net/dsa/dsa_loop.c             |  3 +-
>  drivers/net/dsa/hirschmann/hellcreek.c |  3 +-
>  drivers/net/dsa/lan9303-core.c         |  3 +-
>  drivers/net/dsa/lantiq_gswip.c         |  3 +-
>  drivers/net/dsa/microchip/ksz_common.c |  3 +-
>  drivers/net/dsa/microchip/ksz_common.h |  2 +-
>  drivers/net/dsa/mt7530.c               |  2 +-
>  drivers/net/dsa/mv88e6xxx/chip.c       | 71 ++++++++++++--------------
>  drivers/net/dsa/ocelot/felix.c         |  2 +-
>  drivers/net/dsa/qca8k.c                |  3 +-
>  drivers/net/dsa/rtl8366rb.c            |  3 +-
>  drivers/net/dsa/sja1105/sja1105_main.c | 22 ++++++--
>  drivers/net/dsa/xrs700x/xrs700x.c      |  2 +-
>  include/net/dsa.h                      | 10 ++--
>  net/dsa/dsa_priv.h                     |  1 +
>  net/dsa/port.c                         | 39 ++------------
>  net/dsa/switch.c                       |  3 +-
>  19 files changed, 81 insertions(+), 100 deletions(-)
>
...
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 73613a667f6c..0aac71dece29 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2448,8 +2448,27 @@ static int mv88e6xxx_bridge_map(struct mv88e6xxx_chip *chip,
>  	return 0;
>  }
>  
> +/* Treat the software bridge as a virtual single-port switch behind the
> + * CPU and map in the PVT. First dst->last_switch elements are taken by
> + * physical switches, so start from beyond that range.
> + */
> +static int mv88e6xxx_map_virtual_bridge_to_pvt(struct dsa_switch *ds,
> +					       unsigned int bridge_num)
> +{
> +	u8 dev = bridge_num + ds->dst->last_switch;
> +	struct mv88e6xxx_chip *chip = ds->priv;
> +	int err;
> +
> +	mv88e6xxx_reg_lock(chip);

This deadlocks every time. The caller already holds this lock from both
call sites.

> +	err = mv88e6xxx_pvt_map(chip, dev, 0);
> +	mv88e6xxx_reg_unlock(chip);
> +
> +	return err;
> +}
> +
...