* [PATCH 0/3] net/hns3: bugfix for hns3 @ 2025-08-13 7:33 Dengdui Huang 2025-08-13 7:33 ` [PATCH 1/3] net/hns3: fix inconsistent lock Dengdui Huang ` (3 more replies) 0 siblings, 4 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-13 7:33 UTC (permalink / raw) To: dev; +Cc: stephen, lihuisong, fengchengwen, liuyonglong 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [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-21 12:24 ` David Marchand 2025-08-13 7:33 ` [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang ` (2 subsequent siblings) 3 siblings, 2 replies; 20+ 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] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-08-13 7:33 ` [PATCH 1/3] net/hns3: fix inconsistent lock Dengdui Huang @ 2025-08-13 21:06 ` Stephen Hemminger 2025-08-21 12:20 ` huangdengdui 2025-09-08 11:53 ` Thomas Monjalon 2025-08-21 12:24 ` David Marchand 1 sibling, 2 replies; 20+ messages in thread From: Stephen Hemminger @ 2025-08-13 21:06 UTC (permalink / raw) To: Dengdui Huang; +Cc: dev, lihuisong, fengchengwen, liuyonglong 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); ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-08-13 21:06 ` Stephen Hemminger @ 2025-08-21 12:20 ` huangdengdui 2025-09-08 11:53 ` Thomas Monjalon 1 sibling, 0 replies; 20+ messages in thread From: huangdengdui @ 2025-08-21 12:20 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, lihuisong, fengchengwen, liuyonglong 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); ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-08-13 21:06 ` Stephen Hemminger 2025-08-21 12:20 ` huangdengdui @ 2025-09-08 11:53 ` Thomas Monjalon 2025-09-08 19:58 ` Stephen Hemminger 1 sibling, 1 reply; 20+ messages in thread From: Thomas Monjalon @ 2025-09-08 11:53 UTC (permalink / raw) To: Stephen Hemminger Cc: Dengdui Huang, dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-09-08 11:53 ` Thomas Monjalon @ 2025-09-08 19:58 ` Stephen Hemminger 2025-09-08 20:20 ` Thomas Monjalon 0 siblings, 1 reply; 20+ messages in thread From: Stephen Hemminger @ 2025-09-08 19:58 UTC (permalink / raw) To: Thomas Monjalon; +Cc: Dengdui Huang, dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-09-08 19:58 ` Stephen Hemminger @ 2025-09-08 20:20 ` Thomas Monjalon 2025-09-09 1:53 ` huangdengdui 0 siblings, 1 reply; 20+ messages in thread From: Thomas Monjalon @ 2025-09-08 20:20 UTC (permalink / raw) To: Stephen Hemminger Cc: Dengdui Huang, dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-09-08 20:20 ` Thomas Monjalon @ 2025-09-09 1:53 ` huangdengdui 0 siblings, 0 replies; 20+ messages in thread From: huangdengdui @ 2025-09-09 1:53 UTC (permalink / raw) To: Thomas Monjalon, Stephen Hemminger Cc: dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-08-13 7:33 ` [PATCH 1/3] net/hns3: fix inconsistent lock Dengdui Huang 2025-08-13 21:06 ` Stephen Hemminger @ 2025-08-21 12:24 ` David Marchand 2025-08-22 4:00 ` huangdengdui 1 sibling, 1 reply; 20+ messages in thread From: David Marchand @ 2025-08-21 12:24 UTC (permalink / raw) To: Dengdui Huang; +Cc: dev, stephen, lihuisong, fengchengwen, liuyonglong 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 1/3] net/hns3: fix inconsistent lock 2025-08-21 12:24 ` David Marchand @ 2025-08-22 4:00 ` huangdengdui 0 siblings, 0 replies; 20+ messages in thread From: huangdengdui @ 2025-08-22 4:00 UTC (permalink / raw) To: David Marchand; +Cc: dev, stephen, lihuisong, fengchengwen, liuyonglong 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 > > > > ^ permalink raw reply [flat|nested] 20+ 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 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang 3 siblings, 1 reply; 20+ 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] 20+ messages in thread
* Re: [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail 2025-08-13 7:33 ` [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang @ 2025-08-13 14:51 ` Stephen Hemminger 2025-08-21 12:16 ` huangdengdui 0 siblings, 1 reply; 20+ messages in thread From: Stephen Hemminger @ 2025-08-13 14:51 UTC (permalink / raw) To: Dengdui Huang; +Cc: dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail 2025-08-13 14:51 ` Stephen Hemminger @ 2025-08-21 12:16 ` huangdengdui 2025-09-10 16:03 ` Stephen Hemminger 0 siblings, 1 reply; 20+ messages in thread From: huangdengdui @ 2025-08-21 12:16 UTC (permalink / raw) To: Stephen Hemminger; +Cc: dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail 2025-08-21 12:16 ` huangdengdui @ 2025-09-10 16:03 ` Stephen Hemminger 0 siblings, 0 replies; 20+ messages in thread From: Stephen Hemminger @ 2025-09-10 16:03 UTC (permalink / raw) To: huangdengdui; +Cc: dev, lihuisong, fengchengwen, liuyonglong 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. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 3/3] net/hns3: fix overwrite mbuf in vector path 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 ` [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang @ 2025-08-13 7:33 ` Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang 3 siblings, 0 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-13 7:33 UTC (permalink / raw) To: dev; +Cc: stephen, lihuisong, fengchengwen, liuyonglong 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/4] net/hns3: bugfix for hns3 2025-08-13 7:33 [PATCH 0/3] net/hns3: bugfix for hns3 Dengdui Huang ` (2 preceding siblings ...) 2025-08-13 7:33 ` [PATCH 3/3] net/hns3: fix overwrite mbuf in vector path Dengdui Huang @ 2025-08-22 6:04 ` Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 1/4] net/hns3: fix inconsistent lock Dengdui Huang ` (3 more replies) 3 siblings, 4 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-22 6:04 UTC (permalink / raw) To: dev; +Cc: stephen, david.marchand, lihuisong, fengchengwen, liuyonglong 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/4] net/hns3: fix inconsistent lock 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang @ 2025-08-22 6:04 ` Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 2/4] net/hns3: enable lock annotations check Dengdui Huang ` (2 subsequent siblings) 3 siblings, 0 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-22 6:04 UTC (permalink / raw) To: dev; +Cc: stephen, david.marchand, 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 | 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/4] net/hns3: enable lock annotations check 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 1/4] net/hns3: fix inconsistent lock Dengdui Huang @ 2025-08-22 6:04 ` Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 3/4] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 4/4] net/hns3: fix overwrite mbuf in vector path Dengdui Huang 3 siblings, 0 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-22 6:04 UTC (permalink / raw) To: dev; +Cc: stephen, david.marchand, lihuisong, fengchengwen, liuyonglong 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/4] net/hns3: fix unrelease VLAN resource when init fail 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 1/4] net/hns3: fix inconsistent lock Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 2/4] net/hns3: enable lock annotations check Dengdui Huang @ 2025-08-22 6:04 ` Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 4/4] net/hns3: fix overwrite mbuf in vector path Dengdui Huang 3 siblings, 0 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-22 6:04 UTC (permalink / raw) To: dev; +Cc: stephen, david.marchand, 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] 20+ messages in thread
* [PATCH v2 4/4] net/hns3: fix overwrite mbuf in vector path 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang ` (2 preceding siblings ...) 2025-08-22 6:04 ` [PATCH v2 3/4] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang @ 2025-08-22 6:04 ` Dengdui Huang 3 siblings, 0 replies; 20+ messages in thread From: Dengdui Huang @ 2025-08-22 6:04 UTC (permalink / raw) To: dev; +Cc: stephen, david.marchand, lihuisong, fengchengwen, liuyonglong 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 ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2025-09-10 16:03 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 21:06 ` Stephen Hemminger 2025-08-21 12:20 ` huangdengdui 2025-09-08 11:53 ` Thomas Monjalon 2025-09-08 19:58 ` Stephen Hemminger 2025-09-08 20:20 ` Thomas Monjalon 2025-09-09 1:53 ` huangdengdui 2025-08-21 12:24 ` David Marchand 2025-08-22 4:00 ` huangdengdui 2025-08-13 7:33 ` [PATCH 2/3] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang 2025-08-13 14:51 ` Stephen Hemminger 2025-08-21 12:16 ` huangdengdui 2025-09-10 16:03 ` Stephen Hemminger 2025-08-13 7:33 ` [PATCH 3/3] net/hns3: fix overwrite mbuf in vector path Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 0/4] net/hns3: bugfix for hns3 Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 1/4] net/hns3: fix inconsistent lock Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 2/4] net/hns3: enable lock annotations check Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 3/4] net/hns3: fix unrelease VLAN resource when init fail Dengdui Huang 2025-08-22 6:04 ` [PATCH v2 4/4] net/hns3: fix overwrite mbuf in vector path Dengdui Huang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).