Re: [RFC PATCH net-next 6/6] net: dsa: eliminate dsa_switch_ops :: port_bridge_tx_fwd_{,un}offload

[Tobias Waldekranz <tobias@waldekranz.com>]

On Tue, Oct 26, 2021 at 19:26, Vladimir Oltean <vladimir.oltean@nxp.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@nxp.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;
> +}
> +
...