Re: [PATCH v2 1/1] net/cnxk: mark invalid MAC address if it doesn't exist

[Nithin Dabilpuram <nithind1988@gmail.com>]

Acked-by: Nithin Dabilpuram<ndabilpuram@marvell.com>

On Wed, May 28, 2025 at 11:51 AM <skori@marvell.com> wrote:
>
> From: Sunil Kumar Kori <skori@marvell.com>
>
> When user requests to configure a device which is already in
> configured state then first device gets resets to default and
> then reconfigured with latest parameters.
>
> While resetting the device, MAC address table is left stale which
> causes entry update in later state.
>
> Hence marking the MAC address entries as invalid to avoid any error
> due to further operation on MAC table.
>
> Fixes: b75e0aca84b0 ("net/cnxk: add device configuration operation")
> Cc: stable@dpdk.org
>
> Signed-off-by: Sunil Kumar Kori <skori@marvell.com>
> Signed-off-by: Nithin Dabilpuram <ndabilpuram@marvell.com>
> ---
> v1..v2:
> - Instead of restoring the MAC addresses, reset to them as invalid
>   during restore process.
>
>  drivers/net/cnxk/cnxk_ethdev.c     | 26 ++++++++++++--
>  drivers/net/cnxk/cnxk_ethdev.h     |  3 ++
>  drivers/net/cnxk/cnxk_ethdev_ops.c | 57 ++++++++++++++++++++++++++++--
>  3 files changed, 81 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/cnxk/cnxk_ethdev.c b/drivers/net/cnxk/cnxk_ethdev.c
> index b9a0b37425..64d7937be6 100644
> --- a/drivers/net/cnxk/cnxk_ethdev.c
> +++ b/drivers/net/cnxk/cnxk_ethdev.c
> @@ -1230,8 +1230,8 @@ cnxk_nix_configure(struct rte_eth_dev *eth_dev)
>         uint16_t nb_rxq, nb_txq, nb_cq;
>         struct rte_ether_addr *ea;
>         uint64_t rx_cfg;
> +       int rc, i;
>         void *qs;
> -       int rc;
>
>         rc = -EINVAL;
>
> @@ -1286,6 +1286,12 @@ cnxk_nix_configure(struct rte_eth_dev *eth_dev)
>                 roc_nix_tm_fini(nix);
>                 nix_rxchan_cfg_disable(dev);
>                 roc_nix_lf_free(nix);
> +
> +               /* Reset to invalid */
> +               for (i = 0; i < dev->max_mac_entries; i++)
> +                       dev->dmac_idx_map[i] = CNXK_NIX_DMAC_IDX_INVALID;
> +
> +               dev->dmac_filter_count = 1;
>         }
>
>         dev->rx_offloads = rxmode->offloads;
> @@ -1891,7 +1897,7 @@ cnxk_eth_dev_init(struct rte_eth_dev *eth_dev)
>         struct rte_security_ctx *sec_ctx;
>         struct roc_nix *nix = &dev->nix;
>         struct rte_pci_device *pci_dev;
> -       int rc, max_entries;
> +       int rc, max_entries, i;
>
>         eth_dev->dev_ops = &cnxk_eth_dev_ops;
>         eth_dev->rx_queue_count = cnxk_nix_rx_queue_count;
> @@ -1993,6 +1999,17 @@ cnxk_eth_dev_init(struct rte_eth_dev *eth_dev)
>                 goto free_mac_addrs;
>         }
>
> +       dev->dmac_addrs = rte_malloc("dmac_addrs", max_entries * RTE_ETHER_ADDR_LEN, 0);
> +       if (dev->dmac_addrs == NULL) {
> +               plt_err("Failed to allocate memory for dmac addresses");
> +               rc = -ENOMEM;
> +               goto free_mac_addrs;
> +       }
> +
> +       /* Reset to invalid */
> +       for (i = 0; i < max_entries; i++)
> +               dev->dmac_idx_map[i] = CNXK_NIX_DMAC_IDX_INVALID;
> +
>         dev->max_mac_entries = max_entries;
>         dev->dmac_filter_count = 1;
>
> @@ -2051,6 +2068,8 @@ cnxk_eth_dev_init(struct rte_eth_dev *eth_dev)
>
>  free_mac_addrs:
>         rte_free(eth_dev->data->mac_addrs);
> +       rte_free(dev->dmac_addrs);
> +       dev->dmac_addrs = NULL;
>         rte_free(dev->dmac_idx_map);
>  dev_fini:
>         roc_nix_dev_fini(nix);
> @@ -2182,6 +2201,9 @@ cnxk_eth_dev_uninit(struct rte_eth_dev *eth_dev, bool reset)
>         rte_free(dev->dmac_idx_map);
>         dev->dmac_idx_map = NULL;
>
> +       rte_free(dev->dmac_addrs);
> +       dev->dmac_addrs = NULL;
> +
>         rte_free(eth_dev->data->mac_addrs);
>         eth_dev->data->mac_addrs = NULL;
>
> diff --git a/drivers/net/cnxk/cnxk_ethdev.h b/drivers/net/cnxk/cnxk_ethdev.h
> index d62cc1ec20..1ced6dd65e 100644
> --- a/drivers/net/cnxk/cnxk_ethdev.h
> +++ b/drivers/net/cnxk/cnxk_ethdev.h
> @@ -106,6 +106,8 @@
>  /* Fastpath lookup */
>  #define CNXK_NIX_FASTPATH_LOOKUP_MEM "cnxk_nix_fastpath_lookup_mem"
>
> +#define CNXK_NIX_DMAC_IDX_INVALID -1
> +
>  struct cnxk_fc_cfg {
>         enum rte_eth_fc_mode mode;
>         uint8_t rx_pause;
> @@ -342,6 +344,7 @@ struct cnxk_eth_dev {
>         uint8_t max_mac_entries;
>         bool dmac_filter_enable;
>         int *dmac_idx_map;
> +       struct rte_ether_addr *dmac_addrs;
>
>         uint16_t flags;
>         uint8_t ptype_disable;
> diff --git a/drivers/net/cnxk/cnxk_ethdev_ops.c b/drivers/net/cnxk/cnxk_ethdev_ops.c
> index 9970c5ff5c..abef1a9eaf 100644
> --- a/drivers/net/cnxk/cnxk_ethdev_ops.c
> +++ b/drivers/net/cnxk/cnxk_ethdev_ops.c
> @@ -473,11 +473,20 @@ cnxk_nix_mac_addr_add(struct rte_eth_dev *eth_dev, struct rte_ether_addr *addr,
>  {
>         struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
>         struct roc_nix *nix = &dev->nix;
> +       struct rte_ether_addr *current;
>         int rc;
>
> -       PLT_SET_USED(index);
>         PLT_SET_USED(pool);
>
> +       if (dev->dmac_idx_map[index] != CNXK_NIX_DMAC_IDX_INVALID) {
> +               current = &dev->dmac_addrs[index];
> +               plt_nix_dbg("Mac address %02x:%02x:%02x:%02x:%02x:%02x already exists at index %u",
> +                           current->addr_bytes[0], current->addr_bytes[1], current->addr_bytes[2],
> +                           current->addr_bytes[3], current->addr_bytes[4], current->addr_bytes[5],
> +                           index);
> +               return 0;
> +       }
> +
>         rc = roc_nix_mac_addr_add(nix, addr->addr_bytes);
>         if (rc < 0) {
>                 plt_err("Failed to add mac address, rc=%d", rc);
> @@ -485,6 +494,11 @@ cnxk_nix_mac_addr_add(struct rte_eth_dev *eth_dev, struct rte_ether_addr *addr,
>         }
>
>         dev->dmac_idx_map[index] = rc;
> +       plt_nix_dbg("Added mac address %02x:%02x:%02x:%02x:%02x:%02x at index %u(%d)",
> +                   addr->addr_bytes[0], addr->addr_bytes[1], addr->addr_bytes[2],
> +                   addr->addr_bytes[3], addr->addr_bytes[4], addr->addr_bytes[5], index, rc);
> +
> +       memcpy(&dev->dmac_addrs[index], addr, RTE_ETHER_ADDR_LEN);
>
>         /* Enable promiscuous mode at NIX level */
>         roc_nix_npc_promisc_ena_dis(nix, true);
> @@ -506,6 +520,8 @@ cnxk_nix_mac_addr_del(struct rte_eth_dev *eth_dev, uint32_t index)
>         if (rc)
>                 plt_err("Failed to delete mac address, rc=%d", rc);
>
> +       plt_nix_dbg("Deleted mac address at index %u(%d)", index, dev->dmac_idx_map[index]);
> +       dev->dmac_idx_map[index] = CNXK_NIX_DMAC_IDX_INVALID;
>         dev->dmac_filter_count--;
>  }
>
> @@ -1117,6 +1133,23 @@ cnxk_nix_rss_hash_conf_get(struct rte_eth_dev *eth_dev,
>         return 0;
>  }
>
> +static int
> +nix_find_mac_addr(struct rte_eth_dev *eth_dev, struct rte_ether_addr *addr)
> +{
> +       struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
> +       struct rte_ether_addr null_mac_addr;
> +       int i;
> +
> +       memset(&null_mac_addr, 0, sizeof(null_mac_addr));
> +       addr = addr ? addr : &null_mac_addr;
> +       for (i = 0; i < dev->max_mac_entries; i++) {
> +               if (!memcmp(&eth_dev->data->mac_addrs[i], addr, sizeof(*addr)))
> +                       return i;
> +       }
> +
> +       return -ENOENT;
> +}
> +
>  static inline int
>  nix_mc_addr_list_flush(struct rte_eth_dev *eth_dev)
>  {
> @@ -1138,6 +1171,9 @@ nix_mc_addr_list_flush(struct rte_eth_dev *eth_dev)
>                                 return rc;
>                         }
>
> +                       plt_nix_dbg("Deleted mac address at index %u(%d)", i, dev->dmac_idx_map[i]);
> +
> +                       dev->dmac_idx_map[i] = CNXK_NIX_DMAC_IDX_INVALID;
>                         dev->dmac_filter_count--;
>                         /* Update address in NIC data structure */
>                         rte_ether_addr_copy(&null_mac_addr,
> @@ -1155,7 +1191,7 @@ cnxk_nix_mc_addr_list_configure(struct rte_eth_dev *eth_dev, struct rte_ether_ad
>         struct cnxk_eth_dev *dev = cnxk_eth_pmd_priv(eth_dev);
>         struct rte_eth_dev_data *data = eth_dev->data;
>         struct roc_nix *nix = &dev->nix;
> -       int index, mc_addr_cnt = 0;
> +       int index, mc_addr_cnt = 0, j;
>         uint32_t i;
>
>         if (!mc_addr_set || !nb_mc_addr)
> @@ -1174,8 +1210,15 @@ cnxk_nix_mc_addr_list_configure(struct rte_eth_dev *eth_dev, struct rte_ether_ad
>         }
>         nix_mc_addr_list_flush(eth_dev);
>
> +       j = 0;
>         /* Multicast addresses are to be installed */
>         for (i = 0; i < nb_mc_addr; i++) {
> +               j = nix_find_mac_addr(eth_dev, NULL);
> +               if (j < 0) {
> +                       plt_err("Failed to find free mac address");
> +                       return -ENOSPC;
> +               }
> +
>                 index = roc_nix_mac_addr_add(nix, mc_addr_set[i].addr_bytes);
>                 if (index < 0) {
>                         plt_err("Failed to add mcast mac address, rc=%d",
> @@ -1183,9 +1226,17 @@ cnxk_nix_mc_addr_list_configure(struct rte_eth_dev *eth_dev, struct rte_ether_ad
>                         return index;
>                 }
>
> +               dev->dmac_idx_map[j] = index;
> +               plt_nix_dbg("Added mac address %02x:%02x:%02x:%02x:%02x:%02x at index %u(%d)",
> +                           mc_addr_set[i].addr_bytes[0], mc_addr_set[i].addr_bytes[1],
> +                           mc_addr_set[i].addr_bytes[2], mc_addr_set[i].addr_bytes[3],
> +                           mc_addr_set[i].addr_bytes[4], mc_addr_set[i].addr_bytes[5], j, index);
> +
>                 dev->dmac_filter_count++;
>                 /* Update address in NIC data structure */
> -               rte_ether_addr_copy(&mc_addr_set[i], &data->mac_addrs[index]);
> +               rte_ether_addr_copy(&mc_addr_set[i], &data->mac_addrs[j]);
> +               rte_ether_addr_copy(&mc_addr_set[i], &dev->dmac_addrs[j]);
> +               data->mac_pool_sel[j] = RTE_BIT64(0);
>         }
>
>         roc_nix_npc_promisc_ena_dis(nix, true);
> --
> 2.43.0
>