[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

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 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 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

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 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.

[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

[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