Re: [PATCH net-next 1/2] net: sparx5: Add arbiter for managing PGID table

[Steen Hegelund <steen.hegelund@microchip.com>]

Hi Casper,

On Mon, 2022-03-21 at 11:14 +0100, Casper Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> The PGID (Port Group ID) table holds port masks
> for different purposes. The first 72 are reserved
> for port destination masks, flood masks, and CPU
> forwarding. The rest are shared between multicast,
> link aggregation, and virtualization profiles. The
> GLAG area is reserved to not be used by anything
> else, since it is a subset of the MCAST area.
> 
> The arbiter keeps track of which entries are in
> use. You can ask for a free ID or give back one
> you are done using.
> 
> Signed-off-by: Casper Andersson <casper.casan@gmail.com>
> ---
>  .../net/ethernet/microchip/sparx5/Makefile    |  2 +-
>  .../ethernet/microchip/sparx5/sparx5_main.c   |  3 +
>  .../ethernet/microchip/sparx5/sparx5_main.h   | 21 +++++++
>  .../ethernet/microchip/sparx5/sparx5_pgid.c   | 60 +++++++++++++++++++
>  4 files changed, 85 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/Makefile
> b/drivers/net/ethernet/microchip/sparx5/Makefile
> index e9dd348a6ebb..4402c3ed1dc5 100644
> --- a/drivers/net/ethernet/microchip/sparx5/Makefile
> +++ b/drivers/net/ethernet/microchip/sparx5/Makefile
> @@ -8,4 +8,4 @@ obj-$(CONFIG_SPARX5_SWITCH) += sparx5-switch.o
>  sparx5-switch-objs  := sparx5_main.o sparx5_packet.o \
>   sparx5_netdev.o sparx5_phylink.o sparx5_port.o sparx5_mactable.o sparx5_vlan.o \
>   sparx5_switchdev.o sparx5_calendar.o sparx5_ethtool.o sparx5_fdma.o \
> - sparx5_ptp.o
> + sparx5_ptp.o sparx5_pgid.o
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> index 5f7c7030ce03..01be7bd84181 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.c
> @@ -626,6 +626,9 @@ static int sparx5_start(struct sparx5 *sparx5)
>         /* Init MAC table, ageing */
>         sparx5_mact_init(sparx5);
> 
> +       /* Init PGID table arbitrator */
> +       sparx5_pgid_init(sparx5);
> +
>         /* Setup VLANs */
>         sparx5_vlan_init(sparx5);
> 
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index df68a0891029..e97fa091c740 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -66,6 +66,12 @@ enum sparx5_vlan_port_type {
>  #define PGID_BCAST            (PGID_BASE + 6)
>  #define PGID_CPU              (PGID_BASE + 7)
> 
> +#define PGID_TABLE_SIZE               3290
> +
> +#define PGID_MCAST_START 65

This overlaps with PGID_UC_FLOOD above.  You should drop this.
Please see this description:

https://microchip-ung.github.io/sparx-5_reginfo/reginfo_sparx-5.html?select=ana_ac,pgid

> 
> +#define PGID_GLAG_START 833
> +#define PGID_GLAG_END 1088

You do not appear to put the GLAG feature into use so you should remove these for now.

> +
>  #define IFH_LEN                9 /* 36 bytes */
>  #define NULL_VID               0
>  #define SPX5_MACT_PULL_DELAY   (2 * HZ)
> @@ -271,6 +277,8 @@ struct sparx5 {
>         struct mutex ptp_lock; /* lock for ptp interface state */
>         u16 ptp_skbs;
>         int ptp_irq;
> +       /* PGID allocation map */
> +       u8 pgid_map[PGID_TABLE_SIZE];
>  };
> 
>  /* sparx5_switchdev.c */
> @@ -359,6 +367,19 @@ void sparx5_ptp_txtstamp_release(struct sparx5_port *port,
>                                  struct sk_buff *skb);
>  irqreturn_t sparx5_ptp_irq_handler(int irq, void *args);
> 
> +/* sparx5_pgid.c */
> +enum sparx5_pgid_type {
> +       SPX5_PGID_FREE,
> +       SPX5_PGID_RESERVED,
> +       SPX5_PGID_MULTICAST,
> +       SPX5_PGID_GLAG
> +};
> +
> +void sparx5_pgid_init(struct sparx5 *spx5);
> +int sparx5_pgid_alloc_glag(struct sparx5 *spx5, u16 *idx);
> +int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx);
> +int sparx5_pgid_free(struct sparx5 *spx5, u16 idx);
> +
>  /* Clock period in picoseconds */
>  static inline u32 sparx5_clk_period(enum sparx5_core_clockfreq cclock)
>  {
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> new file mode 100644
> index 000000000000..90366fcb9958
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#include "sparx5_main.h"
> +
> +void sparx5_pgid_init(struct sparx5 *spx5)
> +{
> +       int i;
> +
> +       for (i = 0; i < PGID_TABLE_SIZE; i++)
> +               spx5->pgid_map[i] = SPX5_PGID_FREE;
> +
> +       /* Reserved for unicast, flood control, broadcast, and CPU.
> +        * These cannot be freed.
> +        */
> +       for (i = 0; i <= PGID_CPU; i++)
> +               spx5->pgid_map[i] = SPX5_PGID_RESERVED;
> +}
> +
> +int sparx5_pgid_alloc_glag(struct sparx5 *spx5, u16 *idx)
> +{
> +       int i;
> +
> +       for (i = PGID_GLAG_START; i <= PGID_GLAG_END; i++)
> +               if (spx5->pgid_map[i] == SPX5_PGID_FREE) {
> +                       spx5->pgid_map[i] = SPX5_PGID_GLAG;
> +                       *idx = i;
> +                       return 0;
> +               }
> +
> +       return -EBUSY;
> +}

You do not appear to put the GLAG feature into use so you should remove this function for now.

> +
> +int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx)
> +{
> +       int i;
> +
> +       for (i = PGID_MCAST_START; i < PGID_TABLE_SIZE; i++) {
> +               if (i == PGID_GLAG_START)
> +                       i = PGID_GLAG_END + 1;
> +
> +               if (spx5->pgid_map[i] == SPX5_PGID_FREE) {
> +                       spx5->pgid_map[i] = SPX5_PGID_MULTICAST;
> +                       *idx = i;
> +                       return 0;
> +               }
> +       }
> +
> +       return -EBUSY;
> +}
> +
> +int sparx5_pgid_free(struct sparx5 *spx5, u16 idx)
> +{
> +       if (idx <= PGID_CPU || idx >= PGID_TABLE_SIZE)
> +               return -EINVAL;
> +
> +       if (spx5->pgid_map[idx] == SPX5_PGID_FREE)
> +               return -EINVAL;
> +
> +       spx5->pgid_map[idx] = SPX5_PGID_FREE;
> +       return 0;
> +}
> --
> 2.30.2
>