Skip to contentSkip to conversation

some bugs fix

PATCH In review Xiaoyun wang 2 participants 7 messages Sep 14, 2020 – Sep 22, 2020 Network drivers

lore
Share & tools
lore
#0 lore raw

cover some bugs fix

These patches fix return value of null not checked, fix TCAM filter set failed process, reset rx_mbuf_alloc_failed to 0 when get stats to avoid adding everytime when this ops is called, get default cos from chip, and Sync the repair of patch("fix compile error for old glibc caused by CLOCK_MONOTONIC_RAW").

--

v1:
- fix return value of null not checked
- fix TCAM filter set failed
- fix Rx nombuf stats
- get default cos from chip
- fix clock definition with glibc version

Xiaoyun wang (5):
net/hinic: fix return value of null not checked net/hinic: fix TCAM filter set failed net/hinic: fix Rx nombuf stats
net/hinic/base: get default cos from chip net/hinic/base: fix clock definition with glibc version

 drivers/net/hinic/base/hinic_compat.h  |  8 ++++++-
 drivers/net/hinic/base/hinic_pmd_cfg.c |  7 ++++--
 drivers/net/hinic/base/hinic_pmd_cfg.h |  1 +
 drivers/net/hinic/hinic_pmd_ethdev.c   | 41 +++++++++++++++++++++++++++++-----
 drivers/net/hinic/hinic_pmd_flow.c     | 27 +++++++++++++++++-----
 5 files changed, 69 insertions(+), 15 deletions(-)
Signature · 1 line
1.8.3.1
#1 lore raw

PATCH 1/5 net/hinic: fix return value of null not checked

If rte_zmalloc failed, pmd driver should also delete the ntuple filter or ethertype filter or normal and tcam filter that already added before.

Fixes d7964ce192e7 ("net/hinic: check memory allocations in flow creation")
---
 drivers/net/hinic/hinic_pmd_flow.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)
drivers/net/hinic/hinic_pmd_flow.c +15 -4
index 70fd445..9888a87 100644
@@ -694,6 +694,7 @@ static int hinic_ntuple_item_check_end(const struct rte_flow_item *item,
 			item, "Not supported by ntuple filter");
 		return -rte_errno;
 	}
+
 	return 0;
 }
 
@@ -2981,6 +2982,8 @@ static struct rte_flow *hinic_flow_create(struct rte_eth_dev *dev,
 				sizeof(struct hinic_ntuple_filter_ele), 0);
 			if (ntuple_filter_ptr == NULL) {
 				PMD_DRV_LOG(ERR, "Failed to allocate ntuple_filter_ptr");
+				(void)hinic_add_del_ntuple_filter(dev,
+							&ntuple_filter, FALSE);
 				goto out;
 			}
 			rte_memcpy(&ntuple_filter_ptr->filter_info,
@@ -3011,6 +3014,8 @@ static struct rte_flow *hinic_flow_create(struct rte_eth_dev *dev,
 				sizeof(struct hinic_ethertype_filter_ele), 0);
 			if (ethertype_filter_ptr == NULL) {
 				PMD_DRV_LOG(ERR, "Failed to allocate ethertype_filter_ptr");
+				(void)hinic_add_del_ethertype_filter(dev,
+						&ethertype_filter, FALSE);
 				goto out;
 			}
 			rte_memcpy(&ethertype_filter_ptr->filter_info,
@@ -3034,11 +3039,10 @@ static struct rte_flow *hinic_flow_create(struct rte_eth_dev *dev,
 				      actions, &fdir_rule, error);
 	if (!ret) {
 		if (fdir_rule.mode == HINIC_FDIR_MODE_NORMAL) {
-			ret = hinic_add_del_fdir_filter(dev,
-					&fdir_rule, TRUE);
+			ret = hinic_add_del_fdir_filter(dev, &fdir_rule, TRUE);
 		} else if (fdir_rule.mode == HINIC_FDIR_MODE_TCAM) {
-			ret = hinic_add_del_tcam_fdir_filter(dev,
-					&fdir_rule, TRUE);
+			ret = hinic_add_del_tcam_fdir_filter(dev, &fdir_rule,
+							     TRUE);
 		}  else {
 			PMD_DRV_LOG(INFO, "flow fdir rule create failed, rule mode wrong");
 			goto out;
@@ -3048,6 +3052,13 @@ static struct rte_flow *hinic_flow_create(struct rte_eth_dev *dev,
 				sizeof(struct hinic_fdir_rule_ele), 0);
 			if (fdir_rule_ptr == NULL) {
 				PMD_DRV_LOG(ERR, "Failed to allocate fdir_rule_ptr");
+				if (fdir_rule.mode == HINIC_FDIR_MODE_NORMAL)
+					hinic_add_del_fdir_filter(dev,
+						&fdir_rule, FALSE);
+				else if (fdir_rule.mode == HINIC_FDIR_MODE_TCAM)
+					hinic_add_del_tcam_fdir_filter(dev,
+						&fdir_rule, FALSE);
+
 				goto out;
 			}
 			rte_memcpy(&fdir_rule_ptr->filter_info, &fdir_rule,
Signature · 1 line
1.8.3.1
#2 lore raw

PATCH 2/5 net/hinic: fix TCAM filter set failed

In reply to Xiaoyun wang

hinic supports two methods: linear table and tcam table, if tcam filter enables failed but linear table is ok, which also needs to enable filter, so for this scene, driver should not close fdir switch.

Fixes f4ca3fd54c4d ("net/hinic: create and destroy flow director filter")
---
 drivers/net/hinic/hinic_pmd_flow.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/net/hinic/hinic_pmd_flow.c +6 -2
index 9888a87..d71a42a 100644
@@ -2809,8 +2809,12 @@ static int hinic_add_tcam_filter(struct rte_eth_dev *dev,
 
 		rc = hinic_set_fdir_tcam_rule_filter(nic_dev->hwdev, true);
 		if (rc && rc != HINIC_MGMT_CMD_UNSUPPORTED) {
-			(void)hinic_set_fdir_filter(nic_dev->hwdev, 0, 0, 0,
-						false);
+			/*
+			 * hinic supports two methods: linear table and tcam
+			 * table, if tcam filter enables failed but linear table
+			 * is ok, which also needs to enable filter, so for this
+			 * scene, driver should not close fdir switch.
+			 */
 			(void)hinic_del_tcam_rule(nic_dev->hwdev,
 						fdir_tcam_rule->index);
 			return rc;
Signature · 1 line
1.8.3.1
#3 lore raw

PATCH 3/5 net/hinic: fix Rx nombuf stats

In reply to Xiaoyun wang

rx_mbuf_alloc_failed value is not set to 0 when get stats from driver, which may cause this counter added every time when call this ops.

Fixes cb7b6606ebff ("net/hinic: add RSS stats and promiscuous ops")
---
 drivers/net/hinic/hinic_pmd_ethdev.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/hinic/hinic_pmd_ethdev.c +2 -0
index 67e6afc..b2c8a51 100644
@@ -1320,6 +1320,8 @@ static int hinic_set_dev_promiscuous(struct hinic_nic_dev *nic_dev, bool enable)
 		return err;
 	}
 
+	dev->data->rx_mbuf_alloc_failed = 0;
+
 	/* rx queue stats */
 	q_num = (nic_dev->num_rq < RTE_ETHDEV_QUEUE_STAT_CNTRS) ?
 			nic_dev->num_rq : RTE_ETHDEV_QUEUE_STAT_CNTRS;
Signature · 1 line
1.8.3.1
#4 lore raw

PATCH 4/5 net/hinic/base: get default cos from chip

In reply to Xiaoyun wang

Get default cos of pf driver from chip configuration file.

Fixes 6691acef0d3d ("net/hinic: support VF")
---
 drivers/net/hinic/base/hinic_pmd_cfg.c |  7 ++++--
 drivers/net/hinic/base/hinic_pmd_cfg.h |  1 +
 drivers/net/hinic/hinic_pmd_ethdev.c   | 39 ++++++++++++++++++++++++++++------
 3 files changed, 39 insertions(+), 8 deletions(-)
drivers/net/hinic/base/hinic_pmd_cfg.c +5 -2
index a0cc16d..837734f 100644
@@ -112,6 +112,7 @@ static void hinic_parse_pub_res_cap(struct service_cap *cap,
 	cap->host_id = dev_cap->host_id;
 	cap->ep_id = dev_cap->ep_id;
 	cap->max_cos_id = dev_cap->max_cos_id;
+	cap->valid_cos_bitmap = dev_cap->valid_cos_bitmap;
 	cap->er_id = dev_cap->er_id;
 	cap->port_id = dev_cap->port_id;
 
@@ -134,9 +135,11 @@ static void hinic_parse_pub_res_cap(struct service_cap *cap,
 	cap->host_oq_id_mask_val = dev_cap->host_oq_id_mask_val;
 
 	PMD_DRV_LOG(INFO, "Get public resource capability:");
-	PMD_DRV_LOG(INFO, "host_id: 0x%x, ep_id: 0x%x, intr_type: 0x%x, max_cos_id: 0x%x, er_id: 0x%x, port_id: 0x%x",
+	PMD_DRV_LOG(INFO, "host_id: 0x%x, ep_id: 0x%x, intr_type: 0x%x, "
+		    "max_cos_id: 0x%x, cos_bitmap: 0x%x, er_id: 0x%x, port_id: 0x%x",
 		    cap->host_id, cap->ep_id, cap->intr_chip_en,
-		    cap->max_cos_id, cap->er_id, cap->port_id);
+		    cap->max_cos_id, cap->valid_cos_bitmap, cap->er_id,
+		    cap->port_id);
 	PMD_DRV_LOG(INFO, "host_total_function: 0x%x, host_oq_id_mask_val: 0x%x, max_vf: 0x%x",
 		    cap->host_total_function, cap->host_oq_id_mask_val,
 		    cap->max_vf);
drivers/net/hinic/base/hinic_pmd_cfg.h +1 -0
index 1741ca4..6e76e1d 100644
@@ -54,6 +54,7 @@ struct service_cap {
 	u8 ep_id;
 	u8 intr_chip_en;
 	u8 max_cos_id;	/* PF/VF's max cos id */
+	u8 valid_cos_bitmap;
 	u8 er_id;	/* PF/VF's ER */
 	u8 port_id;	/* PF/VF's physical port */
 	u8 max_vf;	/* max VF number that PF supported */
drivers/net/hinic/hinic_pmd_ethdev.c +33 -6
index b2c8a51..6fd16e6 100644
@@ -2583,26 +2583,53 @@ static int hinic_set_default_dcb_feature(struct hinic_nic_dev *nic_dev)
 					up_pgid, up_bw, up_strict);
 }
 
+static int hinic_pf_get_default_cos(struct hinic_hwdev *hwdev, u8 *cos_id)
+{
+	u8 default_cos = 0;
+	u8 valid_cos_bitmap;
+	u8 i;
+
+	valid_cos_bitmap = hwdev->cfg_mgmt->svc_cap.valid_cos_bitmap;
+	if (!valid_cos_bitmap) {
+		PMD_DRV_LOG(ERR, "PF has none cos to support\n");
+		return -EFAULT;
+	}
+
+	for (i = 0; i < NR_MAX_COS; i++) {
+		if (valid_cos_bitmap & BIT(i))
+			default_cos = i; /* Find max cos id as default cos */
+	}
+
+	*cos_id = default_cos;
+
+	return 0;
+}
+
 static int hinic_init_default_cos(struct hinic_nic_dev *nic_dev)
 {
 	u8 cos_id = 0;
 	int err;
 
 	if (!HINIC_IS_VF(nic_dev->hwdev)) {
-		nic_dev->default_cos =
-				(hinic_global_func_id(nic_dev->hwdev) +
-						DEFAULT_BASE_COS) % NR_MAX_COS;
+		err = hinic_pf_get_default_cos(nic_dev->hwdev, &cos_id);
+		if (err) {
+			PMD_DRV_LOG(ERR, "Get PF default cos failed, err: %d",
+				    err);
+			return HINIC_ERROR;
+		}
 	} else {
 		err = hinic_vf_get_default_cos(nic_dev->hwdev, &cos_id);
 		if (err) {
 			PMD_DRV_LOG(ERR, "Get VF default cos failed, err: %d",
-					err);
+				    err);
 			return HINIC_ERROR;
 		}
-
-		nic_dev->default_cos = cos_id;
 	}
 
+	nic_dev->default_cos = cos_id;
+
+	PMD_DRV_LOG(INFO, "Default cos %d", nic_dev->default_cos);
+
 	return 0;
 }
 
Signature · 1 line
1.8.3.1
#5 lore raw

PATCH 5/5 net/hinic/base: fix clock definition with glibc version

In reply to Xiaoyun wang

Sync the repair of patch("fix compile error for old glibc caused by CLOCK_MONOTONIC_RAW") in the community.

Fixes efeed0894e9c ("net/hinic/base: avoid system time jump")
---
 drivers/net/hinic/base/hinic_compat.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
drivers/net/hinic/base/hinic_compat.h +7 -1
index 7036b03..6dd210e 100644
@@ -166,11 +166,17 @@ static inline u32 readl(const volatile void *addr)
 #define spin_lock(spinlock_prt)		rte_spinlock_lock(spinlock_prt)
 #define spin_unlock(spinlock_prt)	rte_spinlock_unlock(spinlock_prt)
 
+#ifdef CLOCK_MONOTONIC_RAW /* Defined in glibc bits/time.h */
+#define CLOCK_TYPE CLOCK_MONOTONIC_RAW
+#else
+#define CLOCK_TYPE CLOCK_MONOTONIC
+#endif
+
 static inline unsigned long clock_gettime_ms(void)
 {
 	struct timespec tv;
 
-	(void)clock_gettime(CLOCK_MONOTONIC, &tv);
+	(void)clock_gettime(CLOCK_TYPE, &tv);
 
 	return (unsigned long)tv.tv_sec * 1000 +
 	       (unsigned long)tv.tv_nsec / 1000000;
Signature · 1 line
1.8.3.1
#6 lore raw
In reply to Xiaoyun wang
Quoted 16 lines · Xiaoyun wang

These patches fix return value of null not checked, fix TCAM filter set failed process, reset rx_mbuf_alloc_failed to 0 when get stats to avoid adding everytime when this ops is called, get default cos from chip, and Sync the repair of patch("fix compile error for old glibc caused by CLOCK_MONOTONIC_RAW").

--

v1:
- fix return value of null not checked
- fix TCAM filter set failed
- fix Rx nombuf stats
- get default cos from chip
- fix clock definition with glibc version

Quoted 7 lines

Xiaoyun wang (5):
net/hinic: fix return value of null not checked net/hinic: fix TCAM filter set failed net/hinic: fix Rx nombuf stats
net/hinic/base: get default cos from chip net/hinic/base: fix clock definition with glibc version

Series applied to dpdk-next-net/main, thanks.