On Wed, Dec 16, 2020 at 05:00:55PM +0100, Tobias Waldekranz wrote:
> Support offloading of LAGs to hardware. LAGs may be attached to a
> bridge in which case VLANs, multicast groups, etc. are also offloaded
> as usual.
> 
> Signed-off-by: Tobias Waldekranz <tob...@waldekranz.com>
> ---
>  drivers/net/dsa/mv88e6xxx/chip.c    | 298 +++++++++++++++++++++++++++-
>  drivers/net/dsa/mv88e6xxx/global2.c |   8 +-
>  drivers/net/dsa/mv88e6xxx/global2.h |   5 +
>  drivers/net/dsa/mv88e6xxx/port.c    |  21 ++
>  drivers/net/dsa/mv88e6xxx/port.h    |   5 +
>  5 files changed, 329 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c 
> b/drivers/net/dsa/mv88e6xxx/chip.c
> index eafe6bedc692..bd80b3939157 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -1189,7 +1189,8 @@ static int mv88e6xxx_set_mac_eee(struct dsa_switch *ds, 
> int port,
>  }
>  
>  /* Mask of the local ports allowed to receive frames from a given fabric 
> port */
> -static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int 
> port)
> +static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip *chip, int dev, int 
> port,
> +                            struct net_device **lag)
>  {
>       struct dsa_switch *ds = chip->ds;
>       struct dsa_switch_tree *dst = ds->dst;
> @@ -1201,6 +1202,9 @@ static u16 mv88e6xxx_port_vlan(struct mv88e6xxx_chip 
> *chip, int dev, int port)
>       list_for_each_entry(dp, &dst->ports, list) {
>               if (dp->ds->index == dev && dp->index == port) {
>                       found = true;
> +
> +                     if (dp->lag_dev && lag)
> +                             *lag = dp->lag_dev;
>                       break;
>               }
>       }

I'll let Andrew and Vivien have the decisive word, who are vastly more
familiar with mv88e6xxx than I am, but to me it looks like a bit of a
hack to put this logic here, especially since one of the two callers
(i.e. half) doesn't even care about the LAG.

> @@ -1396,14 +1402,21 @@ static int mv88e6xxx_mac_setup(struct mv88e6xxx_chip 
> *chip)
>  
>  static int mv88e6xxx_pvt_map(struct mv88e6xxx_chip *chip, int dev, int port)
>  {
> +     struct net_device *lag = NULL;
>       u16 pvlan = 0;
>  
>       if (!mv88e6xxx_has_pvt(chip))
>               return 0;
>  
>       /* Skip the local source device, which uses in-chip port VLAN */
> -     if (dev != chip->ds->index)
> -             pvlan = mv88e6xxx_port_vlan(chip, dev, port);
> +     if (dev != chip->ds->index) {
> +             pvlan = mv88e6xxx_port_vlan(chip, dev, port, &lag);
> +
> +             if (lag) {
> +                     dev = MV88E6XXX_G2_PVT_ADRR_DEV_TRUNK;
> +                     port = dsa_lag_id(chip->ds->dst, lag);
> +             }
> +     }

What about the following, which should remove the need of modifying 
mv88e6xxx_port_vlan:

static int mv88e6xxx_pvt_map(struct mv88e6xxx_chip *chip, int dev, int port)
{
        struct dsa_switch *ds = chip->ds;
        struct net_device *lag = NULL;
        u16 pvlan = 0;

        if (!mv88e6xxx_has_pvt(chip))
                return 0;

        /* Skip the local source device, which uses in-chip port VLAN */
        if (dev != ds->index) {
                pvlan = mv88e6xxx_port_vlan(chip, dev, port);
                struct dsa_switch *other_ds;
                struct dsa_port *other_dp;

                other_ds = dsa_switch_find(ds->dst->index, dev);
                other_dp = dsa_to_port(other_ds, port);

                /* XXX needs an explanation for the reinterpreted values of
                 * dev and port
                 */
                if (other_dp->lag_dev) {
                        dev = MV88E6XXX_G2_PVT_ADRR_DEV_TRUNK;
                        port = dsa_lag_id(ds->dst, other_dp->lag_dev);
                }
        }

        return mv88e6xxx_g2_pvt_write(chip, dev, port, pvlan);
}

>  
>       return mv88e6xxx_g2_pvt_write(chip, dev, port, pvlan);
>  }
> @@ -5375,6 +5388,271 @@ static int mv88e6xxx_port_egress_floods(struct 
> dsa_switch *ds, int port,
>       return err;
>  }
>  

Reply via email to