[PATCH 0/3] net/hns3: bugfix for hns3

[PATCH 0/3] net/hns3: bugfix for hns3

[Dengdui Huang]

This patchset fixes some bugs.

Chengwen Feng (1):
  net/hns3: fix overwrite mbuf in vector path

Dengdui Huang (2):
  net/hns3: fix inconsistent lock
  net/hns3: fix unrelease VLAN resource when init fail

 drivers/net/hns3/hns3_ethdev.c   | 31 +++++++++++------
 drivers/net/hns3/hns3_ethdev.h   |  1 -
 drivers/net/hns3/hns3_fdir.c     | 13 --------
 drivers/net/hns3/hns3_flow.c     | 57 ++++++++++++--------------------
 drivers/net/hns3/hns3_rxtx_vec.h |  4 +++
 5 files changed, 46 insertions(+), 60 deletions(-)

-- 
2.33.0

[PATCH 1/3] net/hns3: fix inconsistent lock

[Dengdui Huang]

The hns3 driver supports configuring RSS through both ops API and
rte_flow API. The ops API uses spink lock, while the rte_flow API uses
pthread mutex lock. When concurrent calls occur, issues may arise.
This patch replaces the lock in the flow API with spink lock.

Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.h |  1 -
 drivers/net/hns3/hns3_fdir.c   | 13 --------
 drivers/net/hns3/hns3_flow.c   | 57 +++++++++++++---------------------
 3 files changed, 22 insertions(+), 49 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index d602bfa02f..a63ab99edc 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -679,7 +679,6 @@ struct hns3_hw {
 
 	struct hns3_port_base_vlan_config port_base_vlan_cfg;
 
-	pthread_mutex_t flows_lock; /* rte_flow ops lock */
 	struct hns3_fdir_rule_list flow_fdir_list; /* flow fdir rule list */
 	struct hns3_rss_filter_list flow_rss_list; /* flow RSS rule list */
 	struct hns3_flow_mem_list flow_list;
diff --git a/drivers/net/hns3/hns3_fdir.c b/drivers/net/hns3/hns3_fdir.c
index aacad40e61..50572ae430 100644
--- a/drivers/net/hns3/hns3_fdir.c
+++ b/drivers/net/hns3/hns3_fdir.c
@@ -1145,17 +1145,6 @@ int hns3_restore_all_fdir_filter(struct hns3_adapter *hns)
 	if (hns->is_vf)
 		return 0;
 
-	/*
-	 * This API is called in the reset recovery process, the parent function
-	 * must hold hw->lock.
-	 * There maybe deadlock if acquire hw->flows_lock directly because rte
-	 * flow driver ops first acquire hw->flows_lock and then may acquire
-	 * hw->lock.
-	 * So here first release the hw->lock and then acquire the
-	 * hw->flows_lock to avoid deadlock.
-	 */
-	rte_spinlock_unlock(&hw->lock);
-	pthread_mutex_lock(&hw->flows_lock);
 	TAILQ_FOREACH(fdir_filter, &fdir_info->fdir_list, entries) {
 		ret = hns3_config_action(hw, &fdir_filter->fdir_conf);
 		if (!ret)
@@ -1166,8 +1155,6 @@ int hns3_restore_all_fdir_filter(struct hns3_adapter *hns)
 				break;
 		}
 	}
-	pthread_mutex_unlock(&hw->flows_lock);
-	rte_spinlock_lock(&hw->lock);
 
 	if (err) {
 		hns3_err(hw, "Fail to restore FDIR filter, ret = %d", ret);
diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index c0238d2bfa..458d4ffbf1 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -2210,18 +2210,6 @@ hns3_reconfig_all_rss_filter(struct hns3_hw *hw)
 	return 0;
 }
 
-static int
-hns3_restore_rss_filter(struct hns3_hw *hw)
-{
-	int ret;
-
-	pthread_mutex_lock(&hw->flows_lock);
-	ret = hns3_reconfig_all_rss_filter(hw);
-	pthread_mutex_unlock(&hw->flows_lock);
-
-	return ret;
-}
-
 int
 hns3_restore_filter(struct hns3_adapter *hns)
 {
@@ -2232,7 +2220,7 @@ hns3_restore_filter(struct hns3_adapter *hns)
 	if (ret != 0)
 		return ret;
 
-	return hns3_restore_rss_filter(hw);
+	return hns3_reconfig_all_rss_filter(hw);
 }
 
 static int
@@ -2624,10 +2612,10 @@ hns3_flow_validate_wrap(struct rte_eth_dev *dev,
 	struct hns3_filter_info filter_info = {0};
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_validate(dev, attr, pattern, actions, error,
 				 &filter_info);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2641,9 +2629,9 @@ hns3_flow_create_wrap(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_flow *flow;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	flow = hns3_flow_create(dev, attr, pattern, actions, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return flow;
 }
@@ -2655,9 +2643,9 @@ hns3_flow_destroy_wrap(struct rte_eth_dev *dev, struct rte_flow *flow,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_destroy(dev, flow, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2668,9 +2656,9 @@ hns3_flow_flush_wrap(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_flush(dev, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2683,9 +2671,9 @@ hns3_flow_query_wrap(struct rte_eth_dev *dev, struct rte_flow *flow,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_query(dev, flow, actions, data, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2733,7 +2721,7 @@ hns3_flow_action_create(struct rte_eth_dev *dev,
 	if (hns3_check_indir_action(conf, action, error))
 		return NULL;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 
 	act_count = (const struct rte_flow_action_count *)action->conf;
 	if (act_count->id >= pf->fdir.fd_cfg.cnt_num[HNS3_FD_STAGE_1]) {
@@ -2758,11 +2746,11 @@ hns3_flow_action_create(struct rte_eth_dev *dev,
 	handle.indirect_type = HNS3_INDIRECT_ACTION_TYPE_COUNT;
 	handle.counter_id = counter->id;
 
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return (struct rte_flow_action_handle *)handle.val64;
 
 err_exit:
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return NULL;
 }
 
@@ -2775,11 +2763,11 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 	struct rte_flow_action_handle indir;
 	struct hns3_flow_counter *counter;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 
 	indir.val64 = (uint64_t)handle;
 	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					handle, "Invalid indirect type");
@@ -2787,14 +2775,14 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 
 	counter = hns3_counter_lookup(dev, indir.counter_id);
 	if (counter == NULL) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 				handle, "Counter id not exist");
 	}
 
 	if (counter->ref_cnt > 1) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EBUSY,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				handle, "Counter id in use");
@@ -2802,7 +2790,7 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 
 	(void)hns3_counter_release(dev, indir.counter_id);
 
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return 0;
 }
 
@@ -2817,11 +2805,11 @@ hns3_flow_action_query(struct rte_eth_dev *dev,
 	struct rte_flow flow;
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 
 	indir.val64 = (uint64_t)handle;
 	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					handle, "Invalid indirect type");
@@ -2831,7 +2819,7 @@ hns3_flow_action_query(struct rte_eth_dev *dev,
 	flow.counter_id = indir.counter_id;
 	ret = hns3_counter_query(dev, &flow,
 				 (struct rte_flow_query_count *)data, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return ret;
 }
 
@@ -2872,7 +2860,6 @@ hns3_flow_init(struct rte_eth_dev *dev)
 
 	pthread_mutexattr_init(&attr);
 	pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
-	pthread_mutex_init(&hw->flows_lock, &attr);
 	dev->data->dev_flags |= RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE;
 
 	TAILQ_INIT(&hw->flow_fdir_list);
-- 
2.33.0

[PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail

[Dengdui Huang]

In the initialization process, it is necessary to release VLAN resources
on the failure branch.

Additionally, encapsulate a function hns3_uninit_hardware() to release
the resources allocated by the hns3_init_hardware() function.

Fixes: 411d23b9eafb ("net/hns3: support VLAN")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index a809a47423..b2eab7e1c5 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4366,25 +4366,25 @@ hns3_init_hardware(struct hns3_adapter *hns)
 	ret = hns3_dcb_init(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init dcb: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_init_fd_config(hns);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init flow director: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_config_tso(hw, HNS3_TSO_MSS_MIN, HNS3_TSO_MSS_MAX);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to config tso: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_config_gro(hw, false);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to config gro: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	/*
@@ -4396,22 +4396,33 @@ hns3_init_hardware(struct hns3_adapter *hns)
 	ret = hns3_init_ring_with_vector(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init ring intr vector: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_ptp_init(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init PTP, ret = %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	return 0;
-
+rm_vlan_table:
+	hns3_rm_all_vlan_table(hns, true);
 err_mac_init:
 	hns3_uninit_umv_space(hw);
 	return ret;
 }
 
+static void
+hns3_uninit_hardware(struct hns3_hw *hw)
+{
+	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+
+	(void)hns3_uninit_umv_space(hw);
+	hns3_ptp_uninit(hw);
+	hns3_rm_all_vlan_table(hns, true);
+}
+
 static int
 hns3_clear_hw(struct hns3_hw *hw)
 {
@@ -4623,8 +4634,7 @@ hns3_init_pf(struct rte_eth_dev *eth_dev)
 err_enable_intr:
 	hns3_fdir_filter_uninit(hns);
 err_fdir:
-	hns3_uninit_umv_space(hw);
-	hns3_ptp_uninit(hw);
+	hns3_uninit_hardware(hw);
 err_init_hw:
 	hns3_stats_uninit(hw);
 err_get_config:
@@ -4659,8 +4669,7 @@ hns3_uninit_pf(struct rte_eth_dev *eth_dev)
 	hns3_promisc_uninit(hw);
 	hns3_flow_uninit(eth_dev);
 	hns3_fdir_filter_uninit(hns);
-	hns3_uninit_umv_space(hw);
-	hns3_ptp_uninit(hw);
+	hns3_uninit_hardware(hw);
 	hns3_stats_uninit(hw);
 	hns3_config_mac_tnl_int(hw, false);
 	hns3_pf_disable_irq0(hw);
-- 
2.33.0

[PATCH 3/3] net/hns3: fix overwrite mbuf in vector path

[Dengdui Huang]

From: Chengwen Feng <fengchengwen@huawei.com>

The vector path processes multiple descriptors at a time and modifies
the corresponding mbufs accordingly. When the mbufs cannot be applied
for, the previous patch [1] only clears the first descriptor's valid
bit, which is not enough. In this patch, the mbufs points to the
fake_mbuf to prevent overwrite.

[1] 'commit 01843ab2f2fc ("net/hns3: fix crash for NEON and SVE")'

Fixes: a3d4f4d291d7 ("net/hns3: support NEON Rx")
Fixes: 01843ab2f2fc ("net/hns3: fix crash for NEON and SVE")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_rxtx_vec.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx_vec.h b/drivers/net/hns3/hns3_rxtx_vec.h
index 9018e79c2f..6afa2fb814 100644
--- a/drivers/net/hns3/hns3_rxtx_vec.h
+++ b/drivers/net/hns3/hns3_rxtx_vec.h
@@ -109,8 +109,12 @@ hns3_rxq_rearm_mbuf(struct hns3_rx_queue *rxq)
 		/*
 		 * Clear VLD bit for the first descriptor rearmed in case
 		 * of going to receive packets later.
+		 * And also point mbufs to fake_mbuf to prevent modification
+		 * of the mbuf field during vector packet receiving.
 		 */
 		rxdp[0].rx.bd_base_info = 0;
+		for (i = 0; i < HNS3_VECTOR_RX_OFFSET_TABLE_LEN; i++)
+			rxep[i].mbuf = &rxq->fake_mbuf;
 		rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
 		return;
 	}
-- 
2.33.0

Re: [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail

[Stephen Hemminger]

On Wed, 13 Aug 2025 15:33:16 +0800
Dengdui Huang <huangdengdui@huawei.com> wrote:

> +static void
> +hns3_uninit_hardware(struct hns3_hw *hw)
> +{
> +	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
> +
> +	(void)hns3_uninit_umv_space(hw);

Overall, the patch looks good. But why the (void) cast here.
The compiler allows ignoring return value unless the must check attribute is set.

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[Stephen Hemminger]

On Wed, 13 Aug 2025 15:33:15 +0800
Dengdui Huang <huangdengdui@huawei.com> wrote:

> The hns3 driver supports configuring RSS through both ops API and
> rte_flow API. The ops API uses spink lock, while the rte_flow API uses
> pthread mutex lock. When concurrent calls occur, issues may arise.
> This patch replaces the lock in the flow API with spink lock.
> 
> Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---

With that mutex removed, you can also go farther and remove all references to pthread.

$ git diff
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index a63ab99edc..f6bb1b5d43 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -5,7 +5,6 @@
 #ifndef HNS3_ETHDEV_H
 #define HNS3_ETHDEV_H
 
-#include <pthread.h>
 #include <ethdev_driver.h>
 #include <rte_byteorder.h>
 #include <rte_io.h>
diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index 458d4ffbf1..f2d1e4ec3a 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -2853,13 +2853,10 @@ void
 hns3_flow_init(struct rte_eth_dev *dev)
 {
        struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-       pthread_mutexattr_t attr;
 
        if (rte_eal_process_type() != RTE_PROC_PRIMARY)
                return;
 
-       pthread_mutexattr_init(&attr);
-       pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
        dev->data->dev_flags |= RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE;
 
        TAILQ_INIT(&hw->flow_fdir_list);

Re: [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail

[huangdengdui]

On 2025/8/13 22:51, Stephen Hemminger wrote:
> On Wed, 13 Aug 2025 15:33:16 +0800
> Dengdui Huang <huangdengdui@huawei.com> wrote:
> 
>> +static void
>> +hns3_uninit_hardware(struct hns3_hw *hw)
>> +{
>> +	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
>> +
>> +	(void)hns3_uninit_umv_space(hw);
> 
> Overall, the patch looks good. But why the (void) cast here.
> The compiler allows ignoring return value unless the must check attribute is set.

Our internal coding standards require that when we do not check the return value of a function,
we use `(void)` to indicate that we have thoroughly considered that it is unnecessary to handle the return value here.

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[huangdengdui]

On 2025/8/14 5:06, Stephen Hemminger wrote:
> On Wed, 13 Aug 2025 15:33:15 +0800
> Dengdui Huang <huangdengdui@huawei.com> wrote:
> 
>> The hns3 driver supports configuring RSS through both ops API and
>> rte_flow API. The ops API uses spink lock, while the rte_flow API uses
>> pthread mutex lock. When concurrent calls occur, issues may arise.
>> This patch replaces the lock in the flow API with spink lock.
>>
>> Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
> 
> With that mutex removed, you can also go farther and remove all references to pthread.


I made a mistake, and I will remove these references in the next version.

> 
> $ git diff
> diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
> index a63ab99edc..f6bb1b5d43 100644
> --- a/drivers/net/hns3/hns3_ethdev.h
> +++ b/drivers/net/hns3/hns3_ethdev.h
> @@ -5,7 +5,6 @@
>  #ifndef HNS3_ETHDEV_H
>  #define HNS3_ETHDEV_H
>  
> -#include <pthread.h>
>  #include <ethdev_driver.h>
>  #include <rte_byteorder.h>
>  #include <rte_io.h>
> diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
> index 458d4ffbf1..f2d1e4ec3a 100644
> --- a/drivers/net/hns3/hns3_flow.c
> +++ b/drivers/net/hns3/hns3_flow.c
> @@ -2853,13 +2853,10 @@ void
>  hns3_flow_init(struct rte_eth_dev *dev)
>  {
>         struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> -       pthread_mutexattr_t attr;
>  
>         if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>                 return;
>  
> -       pthread_mutexattr_init(&attr);
> -       pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
>         dev->data->dev_flags |= RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE;
>  
>         TAILQ_INIT(&hw->flow_fdir_list);

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[David Marchand]

Hello,

On Wed, Aug 13, 2025 at 9:33 AM Dengdui Huang <huangdengdui@huawei.com> wrote:
>
> The hns3 driver supports configuring RSS through both ops API and
> rte_flow API. The ops API uses spink lock, while the rte_flow API uses
> pthread mutex lock. When concurrent calls occur, issues may arise.
> This patch replaces the lock in the flow API with spink lock.
>
> Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
> Cc: stable@dpdk.org
>
> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> ---
>  drivers/net/hns3/hns3_ethdev.h |  1 -
>  drivers/net/hns3/hns3_fdir.c   | 13 --------
>  drivers/net/hns3/hns3_flow.c   | 57 +++++++++++++---------------------
>  3 files changed, 22 insertions(+), 49 deletions(-)

With this change, is it possible to enable the lock annotations check?

I mean:
$ git diff
diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index 53a9dd6f39..36ce93ccde 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -35,8 +35,6 @@ sources = files(

 require_iova_in_mbuf = false

-annotate_locks = false
-
 deps += ['hash']

 cflags += no_wvla_cflag




-- 
David Marchand

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[huangdengdui]

On 2025/8/21 20:24, David Marchand wrote:
> Hello,
> 
> On Wed, Aug 13, 2025 at 9:33 AM Dengdui Huang <huangdengdui@huawei.com> wrote:
>>
>> The hns3 driver supports configuring RSS through both ops API and
>> rte_flow API. The ops API uses spink lock, while the rte_flow API uses
>> pthread mutex lock. When concurrent calls occur, issues may arise.
>> This patch replaces the lock in the flow API with spink lock.
>>
>> Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>> ---
>>  drivers/net/hns3/hns3_ethdev.h |  1 -
>>  drivers/net/hns3/hns3_fdir.c   | 13 --------
>>  drivers/net/hns3/hns3_flow.c   | 57 +++++++++++++---------------------
>>  3 files changed, 22 insertions(+), 49 deletions(-)
> 
> With this change, is it possible to enable the lock annotations check?
> 

This is fine. I will enable it in the next version.

> I mean:
> $ git diff
> diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
> index 53a9dd6f39..36ce93ccde 100644
> --- a/drivers/net/hns3/meson.build
> +++ b/drivers/net/hns3/meson.build
> @@ -35,8 +35,6 @@ sources = files(
> 
>  require_iova_in_mbuf = false
> 
> -annotate_locks = false
> -
>  deps += ['hash']
> 
>  cflags += no_wvla_cflag
> 
> 
> 
> 

[PATCH v2 0/4] net/hns3: bugfix for hns3

[Dengdui Huang]

This patchset fixes some bugs.

Change log:
v2:
1. remove all references to pthread.
2. enable lock annotations check.

Chengwen Feng (1):
  net/hns3: fix overwrite mbuf in vector path

Dengdui Huang (3):
  net/hns3: fix inconsistent lock
  net/hns3: enable lock annotations check
  net/hns3: fix unrelease VLAN resource when init fail

 drivers/net/hns3/hns3_ethdev.c   | 31 +++++++++++------
 drivers/net/hns3/hns3_ethdev.h   |  2 --
 drivers/net/hns3/hns3_fdir.c     | 13 -------
 drivers/net/hns3/hns3_flow.c     | 60 ++++++++++++--------------------
 drivers/net/hns3/hns3_rxtx_vec.h |  4 +++
 drivers/net/hns3/meson.build     |  2 --
 6 files changed, 46 insertions(+), 66 deletions(-)

-- 
2.33.0

[PATCH v2 1/4] net/hns3: fix inconsistent lock

[Dengdui Huang]

The hns3 driver supports configuring RSS through both ops API and
rte_flow API. The ops API uses spink lock, while the rte_flow API uses
pthread mutex lock. When concurrent calls occur, issues may arise.
This patch replaces the lock in the flow API with spink lock.

Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.h |  2 --
 drivers/net/hns3/hns3_fdir.c   | 13 --------
 drivers/net/hns3/hns3_flow.c   | 60 +++++++++++++---------------------
 3 files changed, 22 insertions(+), 53 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index d602bfa02f..f6bb1b5d43 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -5,7 +5,6 @@
 #ifndef HNS3_ETHDEV_H
 #define HNS3_ETHDEV_H
 
-#include <pthread.h>
 #include <ethdev_driver.h>
 #include <rte_byteorder.h>
 #include <rte_io.h>
@@ -679,7 +678,6 @@ struct hns3_hw {
 
 	struct hns3_port_base_vlan_config port_base_vlan_cfg;
 
-	pthread_mutex_t flows_lock; /* rte_flow ops lock */
 	struct hns3_fdir_rule_list flow_fdir_list; /* flow fdir rule list */
 	struct hns3_rss_filter_list flow_rss_list; /* flow RSS rule list */
 	struct hns3_flow_mem_list flow_list;
diff --git a/drivers/net/hns3/hns3_fdir.c b/drivers/net/hns3/hns3_fdir.c
index aacad40e61..50572ae430 100644
--- a/drivers/net/hns3/hns3_fdir.c
+++ b/drivers/net/hns3/hns3_fdir.c
@@ -1145,17 +1145,6 @@ int hns3_restore_all_fdir_filter(struct hns3_adapter *hns)
 	if (hns->is_vf)
 		return 0;
 
-	/*
-	 * This API is called in the reset recovery process, the parent function
-	 * must hold hw->lock.
-	 * There maybe deadlock if acquire hw->flows_lock directly because rte
-	 * flow driver ops first acquire hw->flows_lock and then may acquire
-	 * hw->lock.
-	 * So here first release the hw->lock and then acquire the
-	 * hw->flows_lock to avoid deadlock.
-	 */
-	rte_spinlock_unlock(&hw->lock);
-	pthread_mutex_lock(&hw->flows_lock);
 	TAILQ_FOREACH(fdir_filter, &fdir_info->fdir_list, entries) {
 		ret = hns3_config_action(hw, &fdir_filter->fdir_conf);
 		if (!ret)
@@ -1166,8 +1155,6 @@ int hns3_restore_all_fdir_filter(struct hns3_adapter *hns)
 				break;
 		}
 	}
-	pthread_mutex_unlock(&hw->flows_lock);
-	rte_spinlock_lock(&hw->lock);
 
 	if (err) {
 		hns3_err(hw, "Fail to restore FDIR filter, ret = %d", ret);
diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index c0238d2bfa..f2d1e4ec3a 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -2210,18 +2210,6 @@ hns3_reconfig_all_rss_filter(struct hns3_hw *hw)
 	return 0;
 }
 
-static int
-hns3_restore_rss_filter(struct hns3_hw *hw)
-{
-	int ret;
-
-	pthread_mutex_lock(&hw->flows_lock);
-	ret = hns3_reconfig_all_rss_filter(hw);
-	pthread_mutex_unlock(&hw->flows_lock);
-
-	return ret;
-}
-
 int
 hns3_restore_filter(struct hns3_adapter *hns)
 {
@@ -2232,7 +2220,7 @@ hns3_restore_filter(struct hns3_adapter *hns)
 	if (ret != 0)
 		return ret;
 
-	return hns3_restore_rss_filter(hw);
+	return hns3_reconfig_all_rss_filter(hw);
 }
 
 static int
@@ -2624,10 +2612,10 @@ hns3_flow_validate_wrap(struct rte_eth_dev *dev,
 	struct hns3_filter_info filter_info = {0};
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_validate(dev, attr, pattern, actions, error,
 				 &filter_info);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2641,9 +2629,9 @@ hns3_flow_create_wrap(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_flow *flow;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	flow = hns3_flow_create(dev, attr, pattern, actions, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return flow;
 }
@@ -2655,9 +2643,9 @@ hns3_flow_destroy_wrap(struct rte_eth_dev *dev, struct rte_flow *flow,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_destroy(dev, flow, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2668,9 +2656,9 @@ hns3_flow_flush_wrap(struct rte_eth_dev *dev, struct rte_flow_error *error)
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_flush(dev, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2683,9 +2671,9 @@ hns3_flow_query_wrap(struct rte_eth_dev *dev, struct rte_flow *flow,
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 	ret = hns3_flow_query(dev, flow, actions, data, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 
 	return ret;
 }
@@ -2733,7 +2721,7 @@ hns3_flow_action_create(struct rte_eth_dev *dev,
 	if (hns3_check_indir_action(conf, action, error))
 		return NULL;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 
 	act_count = (const struct rte_flow_action_count *)action->conf;
 	if (act_count->id >= pf->fdir.fd_cfg.cnt_num[HNS3_FD_STAGE_1]) {
@@ -2758,11 +2746,11 @@ hns3_flow_action_create(struct rte_eth_dev *dev,
 	handle.indirect_type = HNS3_INDIRECT_ACTION_TYPE_COUNT;
 	handle.counter_id = counter->id;
 
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return (struct rte_flow_action_handle *)handle.val64;
 
 err_exit:
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return NULL;
 }
 
@@ -2775,11 +2763,11 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 	struct rte_flow_action_handle indir;
 	struct hns3_flow_counter *counter;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 
 	indir.val64 = (uint64_t)handle;
 	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					handle, "Invalid indirect type");
@@ -2787,14 +2775,14 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 
 	counter = hns3_counter_lookup(dev, indir.counter_id);
 	if (counter == NULL) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EINVAL,
 				RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 				handle, "Counter id not exist");
 	}
 
 	if (counter->ref_cnt > 1) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EBUSY,
 				RTE_FLOW_ERROR_TYPE_HANDLE,
 				handle, "Counter id in use");
@@ -2802,7 +2790,7 @@ hns3_flow_action_destroy(struct rte_eth_dev *dev,
 
 	(void)hns3_counter_release(dev, indir.counter_id);
 
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return 0;
 }
 
@@ -2817,11 +2805,11 @@ hns3_flow_action_query(struct rte_eth_dev *dev,
 	struct rte_flow flow;
 	int ret;
 
-	pthread_mutex_lock(&hw->flows_lock);
+	rte_spinlock_lock(&hw->lock);
 
 	indir.val64 = (uint64_t)handle;
 	if (indir.indirect_type != HNS3_INDIRECT_ACTION_TYPE_COUNT) {
-		pthread_mutex_unlock(&hw->flows_lock);
+		rte_spinlock_unlock(&hw->lock);
 		return rte_flow_error_set(error, EINVAL,
 					RTE_FLOW_ERROR_TYPE_ACTION_CONF,
 					handle, "Invalid indirect type");
@@ -2831,7 +2819,7 @@ hns3_flow_action_query(struct rte_eth_dev *dev,
 	flow.counter_id = indir.counter_id;
 	ret = hns3_counter_query(dev, &flow,
 				 (struct rte_flow_query_count *)data, error);
-	pthread_mutex_unlock(&hw->flows_lock);
+	rte_spinlock_unlock(&hw->lock);
 	return ret;
 }
 
@@ -2865,14 +2853,10 @@ void
 hns3_flow_init(struct rte_eth_dev *dev)
 {
 	struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	pthread_mutexattr_t attr;
 
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return;
 
-	pthread_mutexattr_init(&attr);
-	pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
-	pthread_mutex_init(&hw->flows_lock, &attr);
 	dev->data->dev_flags |= RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE;
 
 	TAILQ_INIT(&hw->flow_fdir_list);
-- 
2.33.0

[PATCH v2 2/4] net/hns3: enable lock annotations check

[Dengdui Huang]

The hns3 PMD can be compiled with the lock annotations check.
so this patch enable it.

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/meson.build | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/hns3/meson.build b/drivers/net/hns3/meson.build
index 53a9dd6f39..36ce93ccde 100644
--- a/drivers/net/hns3/meson.build
+++ b/drivers/net/hns3/meson.build
@@ -35,8 +35,6 @@ sources = files(
 
 require_iova_in_mbuf = false
 
-annotate_locks = false
-
 deps += ['hash']
 
 cflags += no_wvla_cflag
-- 
2.33.0

[PATCH v2 3/4] net/hns3: fix unrelease VLAN resource when init fail

[Dengdui Huang]

In the initialization process, it is necessary to release VLAN resources
on the failure branch.

Additionally, encapsulate a function hns3_uninit_hardware() to release
the resources allocated by the hns3_init_hardware() function.

Fixes: 411d23b9eafb ("net/hns3: support VLAN")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index a809a47423..b2eab7e1c5 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -4366,25 +4366,25 @@ hns3_init_hardware(struct hns3_adapter *hns)
 	ret = hns3_dcb_init(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init dcb: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_init_fd_config(hns);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init flow director: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_config_tso(hw, HNS3_TSO_MSS_MIN, HNS3_TSO_MSS_MAX);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to config tso: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_config_gro(hw, false);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to config gro: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	/*
@@ -4396,22 +4396,33 @@ hns3_init_hardware(struct hns3_adapter *hns)
 	ret = hns3_init_ring_with_vector(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init ring intr vector: %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	ret = hns3_ptp_init(hw);
 	if (ret) {
 		PMD_INIT_LOG(ERR, "Failed to init PTP, ret = %d", ret);
-		goto err_mac_init;
+		goto rm_vlan_table;
 	}
 
 	return 0;
-
+rm_vlan_table:
+	hns3_rm_all_vlan_table(hns, true);
 err_mac_init:
 	hns3_uninit_umv_space(hw);
 	return ret;
 }
 
+static void
+hns3_uninit_hardware(struct hns3_hw *hw)
+{
+	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
+
+	(void)hns3_uninit_umv_space(hw);
+	hns3_ptp_uninit(hw);
+	hns3_rm_all_vlan_table(hns, true);
+}
+
 static int
 hns3_clear_hw(struct hns3_hw *hw)
 {
@@ -4623,8 +4634,7 @@ hns3_init_pf(struct rte_eth_dev *eth_dev)
 err_enable_intr:
 	hns3_fdir_filter_uninit(hns);
 err_fdir:
-	hns3_uninit_umv_space(hw);
-	hns3_ptp_uninit(hw);
+	hns3_uninit_hardware(hw);
 err_init_hw:
 	hns3_stats_uninit(hw);
 err_get_config:
@@ -4659,8 +4669,7 @@ hns3_uninit_pf(struct rte_eth_dev *eth_dev)
 	hns3_promisc_uninit(hw);
 	hns3_flow_uninit(eth_dev);
 	hns3_fdir_filter_uninit(hns);
-	hns3_uninit_umv_space(hw);
-	hns3_ptp_uninit(hw);
+	hns3_uninit_hardware(hw);
 	hns3_stats_uninit(hw);
 	hns3_config_mac_tnl_int(hw, false);
 	hns3_pf_disable_irq0(hw);
-- 
2.33.0

[PATCH v2 4/4] net/hns3: fix overwrite mbuf in vector path

[Dengdui Huang]

From: Chengwen Feng <fengchengwen@huawei.com>

The vector path processes multiple descriptors at a time and modifies
the corresponding mbufs accordingly. When the mbufs cannot be applied
for, the previous patch [1] only clears the first descriptor's valid
bit, which is not enough. In this patch, the mbufs points to the
fake_mbuf to prevent overwrite.

[1] 'commit 01843ab2f2fc ("net/hns3: fix crash for NEON and SVE")'

Fixes: a3d4f4d291d7 ("net/hns3: support NEON Rx")
Fixes: 01843ab2f2fc ("net/hns3: fix crash for NEON and SVE")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
---
 drivers/net/hns3/hns3_rxtx_vec.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/hns3/hns3_rxtx_vec.h b/drivers/net/hns3/hns3_rxtx_vec.h
index 9018e79c2f..6afa2fb814 100644
--- a/drivers/net/hns3/hns3_rxtx_vec.h
+++ b/drivers/net/hns3/hns3_rxtx_vec.h
@@ -109,8 +109,12 @@ hns3_rxq_rearm_mbuf(struct hns3_rx_queue *rxq)
 		/*
 		 * Clear VLD bit for the first descriptor rearmed in case
 		 * of going to receive packets later.
+		 * And also point mbufs to fake_mbuf to prevent modification
+		 * of the mbuf field during vector packet receiving.
 		 */
 		rxdp[0].rx.bd_base_info = 0;
+		for (i = 0; i < HNS3_VECTOR_RX_OFFSET_TABLE_LEN; i++)
+			rxep[i].mbuf = &rxq->fake_mbuf;
 		rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed++;
 		return;
 	}
-- 
2.33.0

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[Thomas Monjalon]

13/08/2025 23:06, Stephen Hemminger:
> On Wed, 13 Aug 2025 15:33:15 +0800
> Dengdui Huang <huangdengdui@huawei.com> wrote:
> 
> > The hns3 driver supports configuring RSS through both ops API and
> > rte_flow API. The ops API uses spink lock, while the rte_flow API uses
> > pthread mutex lock. When concurrent calls occur, issues may arise.
> > This patch replaces the lock in the flow API with spink lock.
> > 
> > Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
> > Cc: stable@dpdk.org
> > 
> > Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> > ---
> 
> With that mutex removed, you can also go farther and remove all references to pthread.

This patch was (probably by mistake) in the next-net/for-main branch.
I've dropped it when pulling in main.

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[Stephen Hemminger]

On Mon, 08 Sep 2025 13:53:38 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> 13/08/2025 23:06, Stephen Hemminger:
> > On Wed, 13 Aug 2025 15:33:15 +0800
> > Dengdui Huang <huangdengdui@huawei.com> wrote:
> >   
> > > The hns3 driver supports configuring RSS through both ops API and
> > > rte_flow API. The ops API uses spink lock, while the rte_flow API uses
> > > pthread mutex lock. When concurrent calls occur, issues may arise.
> > > This patch replaces the lock in the flow API with spink lock.
> > > 
> > > Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
> > > Cc: stable@dpdk.org
> > > 
> > > Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> > > ---  
> > 
> > With that mutex removed, you can also go farther and remove all references to pthread.  
> 
> This patch was (probably by mistake) in the next-net/for-main branch.
> I've dropped it when pulling in main.
> 

I went ahead and fixed the original patch to drop pthread references
and fix wording in commit message; and put it back in next-net.

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[Thomas Monjalon]

08/09/2025 21:58, Stephen Hemminger:
> On Mon, 08 Sep 2025 13:53:38 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 13/08/2025 23:06, Stephen Hemminger:
> > > On Wed, 13 Aug 2025 15:33:15 +0800
> > > Dengdui Huang <huangdengdui@huawei.com> wrote:
> > >   
> > > > The hns3 driver supports configuring RSS through both ops API and
> > > > rte_flow API. The ops API uses spink lock, while the rte_flow API uses
> > > > pthread mutex lock. When concurrent calls occur, issues may arise.
> > > > This patch replaces the lock in the flow API with spink lock.
> > > > 
> > > > Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
> > > > Cc: stable@dpdk.org
> > > > 
> > > > Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
> > > > ---  
> > > 
> > > With that mutex removed, you can also go farther and remove all references to pthread.  
> > 
> > This patch was (probably by mistake) in the next-net/for-main branch.
> > I've dropped it when pulling in main.
> > 
> 
> I went ahead and fixed the original patch to drop pthread references
> and fix wording in commit message; and put it back in next-net.

There are 2 other patches to consider in this series.
We should not break series if no urgency.

Re: [PATCH 1/3] net/hns3: fix inconsistent lock

[huangdengdui]

On 2025/9/9 4:20, Thomas Monjalon wrote:
> 08/09/2025 21:58, Stephen Hemminger:
>> On Mon, 08 Sep 2025 13:53:38 +0200
>> Thomas Monjalon <thomas@monjalon.net> wrote:
>>
>>> 13/08/2025 23:06, Stephen Hemminger:
>>>> On Wed, 13 Aug 2025 15:33:15 +0800
>>>> Dengdui Huang <huangdengdui@huawei.com> wrote:
>>>>   
>>>>> The hns3 driver supports configuring RSS through both ops API and
>>>>> rte_flow API. The ops API uses spink lock, while the rte_flow API uses
>>>>> pthread mutex lock. When concurrent calls occur, issues may arise.
>>>>> This patch replaces the lock in the flow API with spink lock.
>>>>>
>>>>> Fixes: 1bdcca8006e4 ("net/hns3: fix flow director lock")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
>>>>> ---  
>>>>
>>>> With that mutex removed, you can also go farther and remove all references to pthread.  
>>>
>>> This patch was (probably by mistake) in the next-net/for-main branch.
>>> I've dropped it when pulling in main.
>>>
>>
>> I went ahead and fixed the original patch to drop pthread references
>> and fix wording in commit message; and put it back in next-net.
> 
> There are 2 other patches to consider in this series.
> We should not break series if no urgency.
> 
> 

I also found this patch in the next-net/for-main branch, and I created the V2 patch after reverting.

Re: [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail

[Stephen Hemminger]

On Thu, 21 Aug 2025 20:16:25 +0800
huangdengdui <huangdengdui@huawei.com> wrote:

> On 2025/8/13 22:51, Stephen Hemminger wrote:
> > On Wed, 13 Aug 2025 15:33:16 +0800
> > Dengdui Huang <huangdengdui@huawei.com> wrote:
> >   
> >> +static void
> >> +hns3_uninit_hardware(struct hns3_hw *hw)
> >> +{
> >> +	struct hns3_adapter *hns = HNS3_DEV_HW_TO_ADAPTER(hw);
> >> +
> >> +	(void)hns3_uninit_umv_space(hw);  
> > 
> > Overall, the patch looks good. But why the (void) cast here.
> > The compiler allows ignoring return value unless the must check attribute is set.  
> 
> Our internal coding standards require that when we do not check the return value of a function,
> we use `(void)` to indicate that we have thoroughly considered that it is unnecessary to handle the return value here.

Having a close or uninit function return an error code is rarely useful.
The application is left with "what now scenario".

This style goes back to 90's BSD style. Probably when your coding standards were written..

PS: if you want to force checking of return value, there are attributes for that.