Re: [PATCH v5 00/15] net/zxdh: updated net zxdh driver

[Stephen Hemminger <stephen@networkplumber.org>]

On Mon, 23 Dec 2024 19:02:34 +0800
Junlong Wang <wang.junlong1@zte.com.cn> wrote:

> V5:
>   - Simplify the notify_data part in the zxdh_notify_queue function.
>   - Replace rte_zmalloc with rte_calloc in the rss_reta_update function.
>   - Remove unnecessary check in mtu_set function.
> 
> V4:
>   - resolved ci compile issues.
> 
> V3:
>   - use rte_zmalloc and rte_calloc to avoid memset.
>   - remove unnecessary initialization, which first usage will set.
>   - adjust some function which is always return 0, changed to void 
>     and skip the ASSERTION later.
>   - resolved some WARNING:MACRO_ARG_UNUSED issues.
>   - resolved some other issues.
> 
> V2:
>   - resolve code style and github-robot build issue.
> 
> V1:
>   - updated net zxdh driver
>     provided insert/delete/get table code funcs.
>     provided link/mac/vlan/promiscuous/rss/mtu ops.
> 
> Junlong Wang (15):
>   net/zxdh: zxdh np init implementation
>   net/zxdh: zxdh np uninit implementation
>   net/zxdh: port tables init implementations
>   net/zxdh: port tables unint implementations
>   net/zxdh: rx/tx queue setup and intr enable
>   net/zxdh: dev start/stop ops implementations
>   net/zxdh: provided dev simple tx implementations
>   net/zxdh: provided dev simple rx implementations
>   net/zxdh: link info update, set link up/down
>   net/zxdh: mac set/add/remove ops implementations
>   net/zxdh: promisc/allmulti ops implementations
>   net/zxdh: vlan filter/ offload ops implementations
>   net/zxdh: rss hash config/update, reta update/get
>   net/zxdh: basic stats ops implementations
>   net/zxdh: mtu update ops implementations
> 
>  doc/guides/nics/features/zxdh.ini  |   18 +
>  doc/guides/nics/zxdh.rst           |   17 +
>  drivers/net/zxdh/meson.build       |    4 +
>  drivers/net/zxdh/zxdh_common.c     |   24 +
>  drivers/net/zxdh/zxdh_common.h     |    1 +
>  drivers/net/zxdh/zxdh_ethdev.c     |  595 +++++++-
>  drivers/net/zxdh/zxdh_ethdev.h     |   40 +
>  drivers/net/zxdh/zxdh_ethdev_ops.c | 1573 +++++++++++++++++++++
>  drivers/net/zxdh/zxdh_ethdev_ops.h |   80 ++
>  drivers/net/zxdh/zxdh_msg.c        |  169 +++
>  drivers/net/zxdh/zxdh_msg.h        |  232 ++++
>  drivers/net/zxdh/zxdh_np.c         | 2060 ++++++++++++++++++++++++++++
>  drivers/net/zxdh/zxdh_np.h         |  579 ++++++++
>  drivers/net/zxdh/zxdh_pci.c        |   23 +-
>  drivers/net/zxdh/zxdh_pci.h        |    9 +-
>  drivers/net/zxdh/zxdh_queue.c      |  242 +++-
>  drivers/net/zxdh/zxdh_queue.h      |  144 +-
>  drivers/net/zxdh/zxdh_rxtx.c       |  804 +++++++++++
>  drivers/net/zxdh/zxdh_rxtx.h       |   20 +-
>  drivers/net/zxdh/zxdh_tables.c     |  794 +++++++++++
>  drivers/net/zxdh/zxdh_tables.h     |  231 ++++
>  21 files changed, 7613 insertions(+), 46 deletions(-)
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.c
>  create mode 100644 drivers/net/zxdh/zxdh_ethdev_ops.h
>  create mode 100644 drivers/net/zxdh/zxdh_np.c
>  create mode 100644 drivers/net/zxdh/zxdh_np.h
>  create mode 100644 drivers/net/zxdh/zxdh_rxtx.c
>  create mode 100644 drivers/net/zxdh/zxdh_tables.c
>  create mode 100644 drivers/net/zxdh/zxdh_tables.h
> 

This looks good, I saw a couple things that could be addressed later.
First, the log messages are a little inconsistent. Some have blanks before or after message,
others not. And some end with period but most don't. Easy to fix later.

The other minor thing is still some calls to free functions checking for null,
I can add this follow on patch after merging to address this.

diff --git a/drivers/net/zxdh/zxdh_ethdev.c b/drivers/net/zxdh/zxdh_ethdev.c
index e992c3f6cf..942435d318 100644
--- a/drivers/net/zxdh/zxdh_ethdev.c
+++ b/drivers/net/zxdh/zxdh_ethdev.c
@@ -944,19 +944,15 @@ zxdh_np_dtb_data_res_free(struct zxdh_hw *hw)
 		if (ret)
 			PMD_DRV_LOG(ERR, "%s dpp_np_online_uninstall failed", dev->data->name);
 
-		if (g_dtb_data.dtb_table_conf_mz)
-			rte_memzone_free(g_dtb_data.dtb_table_conf_mz);
+		rte_memzone_free(g_dtb_data.dtb_table_conf_mz);
+		g_dtb_data.dtb_table_conf_mz = NULL;
 
-		if (g_dtb_data.dtb_table_dump_mz) {
-			rte_memzone_free(g_dtb_data.dtb_table_dump_mz);
-			g_dtb_data.dtb_table_dump_mz = NULL;
-		}
+		rte_memzone_free(g_dtb_data.dtb_table_dump_mz);
+		g_dtb_data.dtb_table_dump_mz = NULL;
 
 		for (i = 0; i < ZXDH_MAX_BASE_DTB_TABLE_COUNT; i++) {
-			if (g_dtb_data.dtb_table_bulk_dump_mz[i]) {
-				rte_memzone_free(g_dtb_data.dtb_table_bulk_dump_mz[i]);
-				g_dtb_data.dtb_table_bulk_dump_mz[i] = NULL;
-			}
+			rte_memzone_free(g_dtb_data.dtb_table_bulk_dump_mz[i]);
+			g_dtb_data.dtb_table_bulk_dump_mz[i] = NULL;
 		}
 		g_dtb_data.init_done = 0;
 		g_dtb_data.bind_device = NULL;
@@ -1053,10 +1049,8 @@ zxdh_dev_close(struct rte_eth_dev *dev)
 
 	zxdh_bar_msg_chan_exit();
 
-	if (dev->data->mac_addrs != NULL) {
-		rte_free(dev->data->mac_addrs);
-		dev->data->mac_addrs = NULL;
-	}
+	rte_free(dev->data->mac_addrs);
+	dev->data->mac_addrs = NULL;
 
 	return ret;
 }
diff --git a/drivers/net/zxdh/zxdh_np.c b/drivers/net/zxdh/zxdh_np.c
index 42679635f4..11ad92e78f 100644
--- a/drivers/net/zxdh/zxdh_np.c
+++ b/drivers/net/zxdh/zxdh_np.c
@@ -873,8 +873,7 @@ zxdh_np_sdt_mgr_destroy(uint32_t dev_id)
 	p_sdt_tbl_temp = ZXDH_SDT_SOFT_TBL_GET(dev_id);
 	p_sdt_mgr = ZXDH_SDT_MGR_PTR_GET();
 
-	if (p_sdt_tbl_temp != NULL)
-		free(p_sdt_tbl_temp);
+	free(p_sdt_tbl_temp);
 
 	ZXDH_SDT_SOFT_TBL_GET(dev_id) = NULL;