packages/dpdk
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity
dpdk/0127-net-hns3-fix-flow-director-lock.patch
speech_white 3a8995b1ad Update DPDK baseline version 
Update DPDK version from 19.11 to 20.11 and also support
hns3 PMD for Kunpeng 920 and Kunpeng 930.

Signed-off-by: speech_white <humin29@huawei.com>
2021-06-28 00:52:34 +00:00

351 lines
12 KiB
Diff
Raw Blame History

From bdd72afbcc9fbd25812b4ba873875eca26f14b4b Mon Sep 17 00:00:00 2001
From: Chengwen Feng <fengchengwen@huawei.com>
Date: Sat, 17 Apr 2021 17:54:58 +0800
Subject: [PATCH 127/189] net/hns3: fix flow director lock
Currently, the fdir lock was used to protect concurrent access in
multiple processes, it has the following problems:
1) Lack of protection for fdir reset recover.
2) Only part of data is protected, eg. the filterlist is not protected.
We use the following scheme:
1) Del the fdir lock.
2) Add a flow lock and provides rte flow driver ops API-level
protection.
3) Declare support RTE_ETH_DEV_FLOW_OPS_THREAD_SAFE.
Fixes: fcba820d9b9e ("net/hns3: support flow director")
Cc: stable@dpdk.org
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
drivers/net/hns3/hns3_ethdev.c | 4 +-
drivers/net/hns3/hns3_ethdev.h | 4 ++
drivers/net/hns3/hns3_ethdev_vf.c | 3 +-
drivers/net/hns3/hns3_fdir.c | 28 ++++++------
drivers/net/hns3/hns3_fdir.h | 3 +-
drivers/net/hns3/hns3_flow.c | 96 ++++++++++++++++++++++++++++++++++++---
6 files changed, 111 insertions(+), 27 deletions(-)
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 4d63d12..ccafe60 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -7356,8 +7356,8 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
PMD_INIT_LOG(ERR, "Failed to alloc memory for process private");
return -ENOMEM;
}
- /* initialize flow filter lists */
- hns3_filterlist_init(eth_dev);
+
+ hns3_flow_init(eth_dev);
hns3_set_rxtx_function(eth_dev);
eth_dev->dev_ops = &hns3_eth_dev_ops;
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 271eadb..8191c53 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -5,6 +5,7 @@
#ifndef _HNS3_ETHDEV_H_
#define _HNS3_ETHDEV_H_
+#include <pthread.h>
#include <sys/time.h>
#include <rte_ethdev_driver.h>
#include <rte_byteorder.h>
@@ -624,6 +625,9 @@ struct hns3_hw {
uint8_t udp_cksum_mode;
struct hns3_port_base_vlan_config port_base_vlan_cfg;
+
+ pthread_mutex_t flows_lock; /* rte_flow ops lock */
+
/*
* PMD setup and configuration is not thread safe. Since it is not
* performance sensitive, it is better to guarantee thread-safety
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index a4577de..a278146 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2911,8 +2911,7 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
return -ENOMEM;
}
- /* initialize flow filter lists */
- hns3_filterlist_init(eth_dev);
+ hns3_flow_init(eth_dev);
hns3_set_rxtx_function(eth_dev);
eth_dev->dev_ops = &hns3vf_eth_dev_ops;
diff --git a/drivers/net/hns3/hns3_fdir.c b/drivers/net/hns3/hns3_fdir.c
index 857cc94..06c9a94 100644
--- a/drivers/net/hns3/hns3_fdir.c
+++ b/drivers/net/hns3/hns3_fdir.c
@@ -830,7 +830,6 @@ int hns3_fdir_filter_init(struct hns3_adapter *hns)
fdir_hash_params.socket_id = rte_socket_id();
TAILQ_INIT(&fdir_info->fdir_list);
- rte_spinlock_init(&fdir_info->flows_lock);
snprintf(fdir_hash_name, RTE_HASH_NAMESIZE, "%s", hns->hw.data->name);
fdir_info->hash_handle = rte_hash_create(&fdir_hash_params);
if (fdir_info->hash_handle == NULL) {
@@ -856,7 +855,6 @@ void hns3_fdir_filter_uninit(struct hns3_adapter *hns)
struct hns3_fdir_info *fdir_info = &pf->fdir;
struct hns3_fdir_rule_ele *fdir_filter;
- rte_spinlock_lock(&fdir_info->flows_lock);
if (fdir_info->hash_map) {
rte_free(fdir_info->hash_map);
fdir_info->hash_map = NULL;
@@ -865,7 +863,6 @@ void hns3_fdir_filter_uninit(struct hns3_adapter *hns)
rte_hash_free(fdir_info->hash_handle);
fdir_info->hash_handle = NULL;
}
- rte_spinlock_unlock(&fdir_info->flows_lock);
fdir_filter = TAILQ_FIRST(&fdir_info->fdir_list);
while (fdir_filter) {
@@ -891,10 +888,8 @@ static int hns3_fdir_filter_lookup(struct hns3_fdir_info *fdir_info,
hash_sig_t sig;
int ret;
- rte_spinlock_lock(&fdir_info->flows_lock);
sig = rte_hash_crc(key, sizeof(*key), 0);
ret = rte_hash_lookup_with_hash(fdir_info->hash_handle, key, sig);
- rte_spinlock_unlock(&fdir_info->flows_lock);
return ret;
}
@@ -908,11 +903,9 @@ static int hns3_insert_fdir_filter(struct hns3_hw *hw,
int ret;
key = &fdir_filter->fdir_conf.key_conf;
- rte_spinlock_lock(&fdir_info->flows_lock);
sig = rte_hash_crc(key, sizeof(*key), 0);
ret = rte_hash_add_key_with_hash(fdir_info->hash_handle, key, sig);
if (ret < 0) {
- rte_spinlock_unlock(&fdir_info->flows_lock);
hns3_err(hw, "Hash table full? err:%d(%s)!", ret,
strerror(-ret));
return ret;
@@ -920,7 +913,6 @@ static int hns3_insert_fdir_filter(struct hns3_hw *hw,
fdir_info->hash_map[ret] = fdir_filter;
TAILQ_INSERT_TAIL(&fdir_info->fdir_list, fdir_filter, entries);
- rte_spinlock_unlock(&fdir_info->flows_lock);
return ret;
}
@@ -933,11 +925,9 @@ static int hns3_remove_fdir_filter(struct hns3_hw *hw,
hash_sig_t sig;
int ret;
- rte_spinlock_lock(&fdir_info->flows_lock);
sig = rte_hash_crc(key, sizeof(*key), 0);
ret = rte_hash_del_key_with_hash(fdir_info->hash_handle, key, sig);
if (ret < 0) {
- rte_spinlock_unlock(&fdir_info->flows_lock);
hns3_err(hw, "Delete hash key fail ret=%d", ret);
return ret;
}
@@ -945,7 +935,6 @@ static int hns3_remove_fdir_filter(struct hns3_hw *hw,
fdir_filter = fdir_info->hash_map[ret];
fdir_info->hash_map[ret] = NULL;
TAILQ_REMOVE(&fdir_info->fdir_list, fdir_filter, entries);
- rte_spinlock_unlock(&fdir_info->flows_lock);
rte_free(fdir_filter);
@@ -1000,11 +989,9 @@ int hns3_fdir_filter_program(struct hns3_adapter *hns,
rule->location = ret;
node->fdir_conf.location = ret;
- rte_spinlock_lock(&fdir_info->flows_lock);
ret = hns3_config_action(hw, rule);
if (!ret)
ret = hns3_config_key(hns, rule);
- rte_spinlock_unlock(&fdir_info->flows_lock);
if (ret) {
hns3_err(hw, "Failed to config fdir: %u src_ip:%x dst_ip:%x "
"src_port:%u dst_port:%u ret = %d",
@@ -1029,9 +1016,7 @@ int hns3_clear_all_fdir_filter(struct hns3_adapter *hns)
int ret = 0;
/* flush flow director */
- rte_spinlock_lock(&fdir_info->flows_lock);
rte_hash_reset(fdir_info->hash_handle);
- rte_spinlock_unlock(&fdir_info->flows_lock);
fdir_filter = TAILQ_FIRST(&fdir_info->fdir_list);
while (fdir_filter) {
@@ -1059,6 +1044,17 @@ int hns3_restore_all_fdir_filter(struct hns3_adapter *hns)
bool err = false;
int ret;
+ /*
+ * 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)
@@ -1069,6 +1065,8 @@ 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_fdir.h b/drivers/net/hns3/hns3_fdir.h
index a5760a3..d64af85 100644
--- a/drivers/net/hns3/hns3_fdir.h
+++ b/drivers/net/hns3/hns3_fdir.h
@@ -199,7 +199,6 @@ struct hns3_process_private {
* A structure used to define fields of a FDIR related info.
*/
struct hns3_fdir_info {
- rte_spinlock_t flows_lock;
struct hns3_fdir_rule_list fdir_list;
struct hns3_fdir_rule_ele **hash_map;
struct rte_hash *hash_handle;
@@ -220,7 +219,7 @@ int hns3_fdir_filter_program(struct hns3_adapter *hns,
struct hns3_fdir_rule *rule, bool del);
int hns3_clear_all_fdir_filter(struct hns3_adapter *hns);
int hns3_get_count(struct hns3_hw *hw, uint32_t id, uint64_t *value);
-void hns3_filterlist_init(struct rte_eth_dev *dev);
+void hns3_flow_init(struct rte_eth_dev *dev);
int hns3_restore_all_fdir_filter(struct hns3_adapter *hns);
#endif /* _HNS3_FDIR_H_ */
diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c
index 3cca416..c07929f 100644
--- a/drivers/net/hns3/hns3_flow.c
+++ b/drivers/net/hns3/hns3_flow.c
@@ -1214,9 +1214,18 @@ hns3_parse_fdir_filter(struct rte_eth_dev *dev,
}
void
-hns3_filterlist_init(struct rte_eth_dev *dev)
+hns3_flow_init(struct rte_eth_dev *dev)
{
+ struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
struct hns3_process_private *process_list = dev->process_private;
+ pthread_mutexattr_t attr;
+
+ if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
+ 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(&process_list->fdir_list);
TAILQ_INIT(&process_list->filter_rss_list);
@@ -2002,12 +2011,87 @@ hns3_flow_query(struct rte_eth_dev *dev, struct rte_flow *flow,
return 0;
}
+static int
+hns3_flow_validate_wrap(struct rte_eth_dev *dev,
+ const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ 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);
+ ret = hns3_flow_validate(dev, attr, pattern, actions, error);
+ pthread_mutex_unlock(&hw->flows_lock);
+
+ return ret;
+}
+
+static struct rte_flow *
+hns3_flow_create_wrap(struct rte_eth_dev *dev, const struct rte_flow_attr *attr,
+ const struct rte_flow_item pattern[],
+ const struct rte_flow_action actions[],
+ struct rte_flow_error *error)
+{
+ struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+ struct rte_flow *flow;
+
+ pthread_mutex_lock(&hw->flows_lock);
+ flow = hns3_flow_create(dev, attr, pattern, actions, error);
+ pthread_mutex_unlock(&hw->flows_lock);
+
+ return flow;
+}
+
+static int
+hns3_flow_destroy_wrap(struct rte_eth_dev *dev, struct rte_flow *flow,
+ 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);
+ ret = hns3_flow_destroy(dev, flow, error);
+ pthread_mutex_unlock(&hw->flows_lock);
+
+ return ret;
+}
+
+static int
+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);
+ ret = hns3_flow_flush(dev, error);
+ pthread_mutex_unlock(&hw->flows_lock);
+
+ return ret;
+}
+
+static int
+hns3_flow_query_wrap(struct rte_eth_dev *dev, struct rte_flow *flow,
+ const struct rte_flow_action *actions, void *data,
+ 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);
+ ret = hns3_flow_query(dev, flow, actions, data, error);
+ pthread_mutex_unlock(&hw->flows_lock);
+
+ return ret;
+}
+
static const struct rte_flow_ops hns3_flow_ops = {
- .validate = hns3_flow_validate,
- .create = hns3_flow_create,
- .destroy = hns3_flow_destroy,
- .flush = hns3_flow_flush,
- .query = hns3_flow_query,
+ .validate = hns3_flow_validate_wrap,
+ .create = hns3_flow_create_wrap,
+ .destroy = hns3_flow_destroy_wrap,
+ .flush = hns3_flow_flush_wrap,
+ .query = hns3_flow_query_wrap,
.isolate = NULL,
};
--
2.7.4
Reference in New Issue View Git Blame Copy Permalink