* [PATCH 1/3] net/hns3: fix inconsistent lock
2025-08-13 7:33 [PATCH 0/3] net/hns3: bugfix for hns3 Dengdui Huang
@ 2025-08-13 7:33 ` Dengdui Huang
2025-08-13 21:06 ` Stephen Hemminger
2025-08-13 7:33 ` [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang
2025-08-13 7:33 ` [PATCH 3/3] net/hns3: fix overwrite mbuf in vector path Dengdui Huang
2 siblings, 1 reply; 7+ messages in thread
From: Dengdui Huang @ 2025-08-13 7:33 UTC (permalink / raw)
To: dev; +Cc: stephen, lihuisong, fengchengwen, liuyonglong
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
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail
2025-08-13 7:33 [PATCH 0/3] net/hns3: bugfix for hns3 Dengdui Huang
2025-08-13 7:33 ` [PATCH 1/3] net/hns3: fix inconsistent lock Dengdui Huang
@ 2025-08-13 7:33 ` Dengdui Huang
2025-08-13 14:51 ` Stephen Hemminger
2025-08-13 7:33 ` [PATCH 3/3] net/hns3: fix overwrite mbuf in vector path Dengdui Huang
2 siblings, 1 reply; 7+ messages in thread
From: Dengdui Huang @ 2025-08-13 7:33 UTC (permalink / raw)
To: dev; +Cc: stephen, lihuisong, fengchengwen, liuyonglong
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
^ permalink raw reply [flat|nested] 7+ messages in thread