[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <8c099860e0c0dc70bf4936cb672c6579c2553495.camel@microchip.com>
Date: Thu, 24 Mar 2022 15:31:32 +0100
From: Steen Hegelund <steen.hegelund@...rochip.com>
To: Casper Andersson <casper.casan@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Paolo Abeni <pabeni@...hat.com>,
Lars Povlsen <lars.povlsen@...rochip.com>,
<UNGLinuxDriver@...rochip.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/2] net: sparx5: Refactor mdb handling
according to feedback
On Thu, 2022-03-24 at 12:38 +0100, Casper Andersson wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> - Remove mact_lookup and use new mact_find instead.
> - Make pgid_read_mask function.
> - Set PGID arbiter to start searching at PGID_BASE + 8.
>
> This is according to feedback on previous patch.
> https://lore.kernel.org/netdev/20220322081823.wqbx7vud4q7qtjuq@wse-c0155/T/#t
>
> Signed-off-by: Casper Andersson <casper.casan@...il.com>
> ---
> .../microchip/sparx5/sparx5_mactable.c | 19 ++++---------------
> .../ethernet/microchip/sparx5/sparx5_main.h | 3 ++-
> .../ethernet/microchip/sparx5/sparx5_pgid.c | 3 +++
> .../microchip/sparx5/sparx5_switchdev.c | 18 ++++++++----------
> .../ethernet/microchip/sparx5/sparx5_vlan.c | 7 +++++++
> 5 files changed, 24 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> index 35abb3d0ce19..a5837dbe0c7e 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_mactable.c
> @@ -212,19 +212,7 @@ bool sparx5_mact_find(struct sparx5 *sparx5,
>
> mutex_unlock(&sparx5->lock);
>
> - return ret == 0;
> -}
> -
> -static int sparx5_mact_lookup(struct sparx5 *sparx5,
> - const unsigned char mac[ETH_ALEN],
> - u16 vid)
> -{
> - u32 pcfg2;
> -
> - if (sparx5_mact_find(sparx5, mac, vid, &pcfg2))
> - return 1;
> -
> - return 0;
> + return ret;
> }
>
> int sparx5_mact_forget(struct sparx5 *sparx5,
> @@ -305,9 +293,10 @@ int sparx5_add_mact_entry(struct sparx5 *sparx5,
> {
> struct sparx5_mact_entry *mact_entry;
> int ret;
> + u32 cfg2;
>
> - ret = sparx5_mact_lookup(sparx5, addr, vid);
> - if (ret)
> + ret = sparx5_mact_find(sparx5, addr, vid, &cfg2);
> + if (!ret)
> return 0;
>
> /* In case the entry already exists, don't add it again to SW,
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> index 8e77d7ee8e68..b197129044b5 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_main.h
> @@ -65,10 +65,10 @@ enum sparx5_vlan_port_type {
> #define PGID_IPV6_MC_CTRL (PGID_BASE + 5)
> #define PGID_BCAST (PGID_BASE + 6)
> #define PGID_CPU (PGID_BASE + 7)
> +#define PGID_MCAST_START (PGID_BASE + 8)
>
> #define PGID_TABLE_SIZE 3290
>
> -#define PGID_MCAST_START 65
> #define IFH_LEN 9 /* 36 bytes */
> #define NULL_VID 0
> #define SPX5_MACT_PULL_DELAY (2 * HZ)
> @@ -325,6 +325,7 @@ void sparx5_mact_init(struct sparx5 *sparx5);
>
> /* sparx5_vlan.c */
> void sparx5_pgid_update_mask(struct sparx5_port *port, int pgid, bool enable);
> +void sparx5_pgid_read_mask(struct sparx5 *sparx5, int pgid, u32 portmask[3]);
> void sparx5_update_fwd(struct sparx5 *sparx5);
> void sparx5_vlan_init(struct sparx5 *sparx5);
> void sparx5_vlan_port_setup(struct sparx5 *sparx5, int portno);
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> index 851a559269e1..af8b435009f4 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_pgid.c
> @@ -19,6 +19,9 @@ int sparx5_pgid_alloc_mcast(struct sparx5 *spx5, u16 *idx)
> {
> int i;
>
> + /* The multicast area starts at index 65, but the first 7
> + * are reserved for flood masks and CPU. Start alloc after that.
> + */
> for (i = PGID_MCAST_START; i < PGID_TABLE_SIZE; i++) {
> if (spx5->pgid_map[i] == SPX5_PGID_FREE) {
> spx5->pgid_map[i] = SPX5_PGID_MULTICAST;
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> index 2d8e0b81c839..5389fffc694a 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_switchdev.c
> @@ -406,11 +406,11 @@ static int sparx5_handle_port_mdb_add(struct net_device *dev,
>
> res = sparx5_mact_find(spx5, v->addr, vid, &mact_entry);
>
> - if (res) {
> + if (res == 0) {
> pgid_idx = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_GET(mact_entry);
>
> - /* MC_IDX has an offset of 65 in the PGID table. */
> - pgid_idx += PGID_MCAST_START;
> + /* MC_IDX starts after the port masks in the PGID table */
> + pgid_idx += SPX5_PORTS;
> sparx5_pgid_update_mask(port, pgid_idx, true);
> } else {
> err = sparx5_pgid_alloc_mcast(spx5, &pgid_idx);
> @@ -468,17 +468,15 @@ static int sparx5_handle_port_mdb_del(struct net_device *dev,
>
> res = sparx5_mact_find(spx5, v->addr, vid, &mact_entry);
>
> - if (res) {
> + if (res == 0) {
> pgid_idx = LRN_MAC_ACCESS_CFG_2_MAC_ENTRY_ADDR_GET(mact_entry);
>
> - /* MC_IDX has an offset of 65 in the PGID table. */
> - pgid_idx += PGID_MCAST_START;
> + /* MC_IDX starts after the port masks in the PGID table */
> + pgid_idx += SPX5_PORTS;
> sparx5_pgid_update_mask(port, pgid_idx, false);
>
> - pgid_entry[0] = spx5_rd(spx5, ANA_AC_PGID_CFG(pgid_idx));
> - pgid_entry[1] = spx5_rd(spx5, ANA_AC_PGID_CFG1(pgid_idx));
> - pgid_entry[2] = spx5_rd(spx5, ANA_AC_PGID_CFG2(pgid_idx));
> - if (pgid_entry[0] == 0 && pgid_entry[1] == 0 && pgid_entry[2] == 0) {
> + sparx5_pgid_read_mask(spx5, pgid_idx, pgid_entry);
> + if (bitmap_empty((unsigned long *)pgid_entry, SPX5_PORTS)) {
> /* No ports are in MC group. Remove entry */
> err = sparx5_mdb_del_entry(dev, spx5, v->addr, vid, pgid_idx);
> if (err)
> diff --git a/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
> b/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
> index 8e56ffa1c4f7..37e4ac965849 100644
> --- a/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
> +++ b/drivers/net/ethernet/microchip/sparx5/sparx5_vlan.c
> @@ -138,6 +138,13 @@ void sparx5_pgid_update_mask(struct sparx5_port *port, int pgid, bool enable)
> }
> }
>
> +void sparx5_pgid_read_mask(struct sparx5 *spx5, int pgid, u32 portmask[3])
> +{
> + portmask[0] = spx5_rd(spx5, ANA_AC_PGID_CFG(pgid));
> + portmask[1] = spx5_rd(spx5, ANA_AC_PGID_CFG1(pgid));
> + portmask[2] = spx5_rd(spx5, ANA_AC_PGID_CFG2(pgid));
> +}
> +
> void sparx5_update_fwd(struct sparx5 *sparx5)
> {
> DECLARE_BITMAP(workmask, SPX5_PORTS);
> --
> 2.30.2
>
Reviewed-by: Steen Hegelund <Steen.Hegelund@...rochip.com>
--
Best Regards
Steen
-=-=-=-=-=-=-=-=-=-=-=-=-=-=
steen.hegelund@...rochip.com
Powered by blists - more mailing lists