Skip to contentSkip to conversation

Fix and add txq xstats

PATCH In review Xiaoyun wang 2 participants 5 messages Apr 10, 2020 Network drivers

lore
Share & tools
lore

Xiaoyun wang

author
#0 lore raw

v4 cover Fix and add txq xstats

This patch fixes PF firmware hotactive problem, optimizes log style and adds txq xstats members.

--
v3->v4:
- fix PF firmware hotactive problem
- optimize log style
- adds txq xstats members

v2->v3:
- fix FW hotactive problem
- optimize log style

v1->v2:
- Fix LRO problems
- Fix hotupdate firmware problem
- Optimize doorbell area initialization
- Remove 40GE Mezz card id
- Add Fdir filter type
- Optimize log files
- Support pause flow control

v1:
- Allocate IO memory with socketid

Xiaoyun wang (3):
net/hinic/base: fix PF firmware hotactive problem net/hinic/base: optimize log style net/hinic: adds txq xstats member

 drivers/net/hinic/base/hinic_pmd_cmdq.c   |  7 ++++--
 drivers/net/hinic/base/hinic_pmd_hwdev.c  |  8 +++----
 drivers/net/hinic/base/hinic_pmd_hwif.c   |  4 ++--
 drivers/net/hinic/base/hinic_pmd_mgmt.c   | 38 +++++++++++++++++--------------
 drivers/net/hinic/base/hinic_pmd_mgmt.h   |  2 ++
 drivers/net/hinic/base/hinic_pmd_niccfg.c |  3 +--
 drivers/net/hinic/base/hinic_pmd_nicio.c  |  2 +-
 drivers/net/hinic/hinic_pmd_ethdev.c      | 17 ++++++--------
 drivers/net/hinic/hinic_pmd_flow.c        |  5 ++--
 drivers/net/hinic/hinic_pmd_tx.c          | 31 +++++++++++++++++++++++++
 drivers/net/hinic/hinic_pmd_tx.h          |  2 ++
 11 files changed, 78 insertions(+), 41 deletions(-)
Signature · 1 line
1.8.3.1
author
#1 lore raw

PATCH v4 1/3 net/hinic/base: fix PF firmware hotactive problem

When FW is hotactive which means updating the FW but not needs to reboot OS, FW returns HINIC_DEV_BUSY_ACTIVE_FW for pf driver because firmware is being reinitialized, at which point the cmdq initialization that relies on the fw channel will fail, so driver should reinit the cmdq when port start.

Fixes cdf7f7ce1423 ("net/hinic/base: fix port start during FW hotupdate")
Cc [email protected]
Signed-off-by Xiaoyun wang <[email protected]>
---
 drivers/net/hinic/base/hinic_pmd_cmdq.c  |  7 ++++--
 drivers/net/hinic/base/hinic_pmd_hwdev.c |  4 ++--
 drivers/net/hinic/base/hinic_pmd_mgmt.c  | 40 ++++++++++++++++++--------------
 drivers/net/hinic/base/hinic_pmd_mgmt.h  |  2 ++
 drivers/net/hinic/base/hinic_pmd_nicio.c |  2 +-
 drivers/net/hinic/hinic_pmd_ethdev.c     |  7 +-----
 6 files changed, 33 insertions(+), 29 deletions(-)
drivers/net/hinic/base/hinic_pmd_cmdq.c +5 -2
index 685498e..2e98b9c 100644
@@ -440,9 +440,12 @@ static int hinic_set_cmdq_ctxts(struct hinic_hwdev *hwdev)
 					     cmdq_ctxt, in_size, NULL,
 					     NULL, 0);
 		if (err) {
-			if (err == HINIC_MBOX_PF_BUSY_ACTIVE_FW)
+			if (err == HINIC_MBOX_PF_BUSY_ACTIVE_FW ||
+				err == HINIC_DEV_BUSY_ACTIVE_FW) {
 				cmdqs->status |= HINIC_CMDQ_SET_FAIL;
-			PMD_DRV_LOG(ERR, "Set cmdq ctxt failed");
+				PMD_DRV_LOG(ERR, "PF or VF fw is hot active");
+			}
+			PMD_DRV_LOG(ERR, "Set cmdq ctxt failed, err: %d", err);
 			return -EFAULT;
 		}
 	}
drivers/net/hinic/base/hinic_pmd_hwdev.c +2 -2
index cf2a970..fc11ecd 100644
@@ -529,7 +529,7 @@ static int hinic_vf_rx_tx_flush(struct hinic_hwdev *hwdev)
 
 	err = hinic_reinit_cmdq_ctxts(hwdev);
 	if (err)
-		PMD_DRV_LOG(WARNING, "Reinit cmdq failed");
+		PMD_DRV_LOG(WARNING, "Reinit cmdq failed when vf flush");
 
 	return err;
 }

@@ -587,7 +587,7 @@ static int hinic_pf_rx_tx_flush(struct hinic_hwdev *hwdev)
 
 	err = hinic_reinit_cmdq_ctxts(hwdev);
 	if (err)
-		PMD_DRV_LOG(WARNING, "Reinit cmdq failed");
+		PMD_DRV_LOG(WARNING, "Reinit cmdq failed when pf flush");
 
 	return 0;
 }
drivers/net/hinic/base/hinic_pmd_mgmt.c +22 -18
index addc9d2..06c9b68 100644
@@ -248,6 +248,19 @@ static void free_msg_buf(struct hinic_msg_pf_to_mgmt *pf_to_mgmt)
 	free_recv_msg(&pf_to_mgmt->recv_msg_from_mgmt);
 }
 
+static int hinic_get_mgmt_channel_status(void *hwdev)
+{
+	struct hinic_hwif *hwif = ((struct hinic_hwdev *)hwdev)->hwif;
+	u32 val;
+
+	if (hinic_func_type((struct hinic_hwdev *)hwdev) == TYPE_VF)
+		return false;
+
+	val = hinic_hwif_read_reg(hwif, HINIC_ICPL_RESERVD_ADDR);
+
+	return HINIC_GET_MGMT_CHANNEL_STATUS(val, MGMT_CHANNEL_STATUS);
+}
+
 /**
  * send_msg_to_mgmt_async - send async message
  * @pf_to_mgmt: PF to MGMT channel

@@ -309,6 +322,14 @@ static int send_msg_to_mgmt_sync(struct hinic_msg_pf_to_mgmt *pf_to_mgmt,
 	u64 header;
 	u16 cmd_size = mgmt_msg_len(msg_len);
 
+	/* If fw is hot active, return failed */
+	if (hinic_get_mgmt_channel_status(pf_to_mgmt->hwdev)) {
+		if (mod == HINIC_MOD_COMM || mod == HINIC_MOD_L2NIC)
+			return HINIC_DEV_BUSY_ACTIVE_FW;
+		else
+			return -EBUSY;
+	}
+
 	if (direction == HINIC_MSG_RESPONSE)
 		prepare_header(pf_to_mgmt, &header, msg_len, mod, ack_type,
 			       direction, cmd, resp_msg_id);

@@ -449,7 +470,7 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
 			       recv_msg->msg_len);
 			*out_size = recv_msg->msg_len;
 		} else {
-			PMD_DRV_LOG(ERR, "Mgmt rsp's msg len: %u overflow.",
+			PMD_DRV_LOG(ERR, "Mgmt rsp's msg len:%u overflow.",
 				recv_msg->msg_len);
 			err = -ERANGE;
 		}

@@ -462,19 +483,6 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
 	return err;
 }
 
-static int hinic_get_mgmt_channel_status(void *hwdev)
-{
-	struct hinic_hwif *hwif = ((struct hinic_hwdev *)hwdev)->hwif;
-	u32 val;
-
-	if (hinic_func_type((struct hinic_hwdev *)hwdev) == TYPE_VF)
-		return false;
-
-	val = hinic_hwif_read_reg(hwif, HINIC_ICPL_RESERVD_ADDR);
-
-	return HINIC_GET_MGMT_CHANNEL_STATUS(val, MGMT_CHANNEL_STATUS);
-}
-
 int hinic_msg_to_mgmt_sync(void *hwdev, enum hinic_mod_type mod, u8 cmd,
 			   void *buf_in, u16 in_size,
 			   void *buf_out, u16 *out_size, u32 timeout)

@@ -484,10 +492,6 @@ int hinic_msg_to_mgmt_sync(void *hwdev, enum hinic_mod_type mod, u8 cmd,
 	if (!hwdev || in_size > HINIC_MSG_TO_MGMT_MAX_LEN)
 		return -EINVAL;
 
-	/* If status is hot upgrading, don't send message to mgmt */
-	if (hinic_get_mgmt_channel_status(hwdev))
-		return -EPERM;
-
 	if (hinic_func_type(hwdev) == TYPE_VF) {
 		rc = hinic_mbox_to_pf(hwdev, mod, cmd, buf_in, in_size,
 					buf_out, out_size, timeout);
drivers/net/hinic/base/hinic_pmd_mgmt.h +2 -0
index cc18843..52b319e 100644
@@ -34,6 +34,8 @@
 #define HINIC_MSG_HEADER_P2P_IDX_MASK				0xF
 #define HINIC_MSG_HEADER_MSG_ID_MASK				0x3FF
 
+#define HINIC_DEV_BUSY_ACTIVE_FW				0xFE
+
 #define HINIC_MSG_HEADER_GET(val, member)			\
 		(((val) >> HINIC_MSG_HEADER_##member##_SHIFT) & \
 		HINIC_MSG_HEADER_##member##_MASK)
drivers/net/hinic/base/hinic_pmd_nicio.c +1 -1
index fd34b03..60c4e14 100644
@@ -537,7 +537,7 @@ int hinic_init_qp_ctxts(struct hinic_hwdev *hwdev)
 	if (hwdev->cmdqs->status & HINIC_CMDQ_SET_FAIL) {
 		err = hinic_reinit_cmdq_ctxts(hwdev);
 		if (err) {
-			PMD_DRV_LOG(ERR, "Reinit cmdq context failed, rc: %d",
+			PMD_DRV_LOG(ERR, "Reinit cmdq context failed when dev start, err: %d",
 				err);
 			return err;
 		}
drivers/net/hinic/hinic_pmd_ethdev.c +1 -6
index 23724a0..239a78c 100644
@@ -1862,11 +1862,6 @@ static int hinic_flow_ctrl_get(struct rte_eth_dev *dev,
 	else
 		fc_conf->mode = RTE_FC_NONE;
 
-	PMD_DRV_LOG(INFO, "Get pause options, tx: %s, rx: %s, auto: %s\n",
-		nic_pause.tx_pause ? "on" : "off",
-		nic_pause.rx_pause ? "on" : "off",
-		nic_pause.auto_neg ? "on" : "off");
-
 	return 0;
 }
 

@@ -1900,7 +1895,7 @@ static int hinic_flow_ctrl_set(struct rte_eth_dev *dev,
 	nic_dev->nic_pause.rx_pause = nic_pause.rx_pause;
 	nic_dev->nic_pause.tx_pause = nic_pause.tx_pause;
 
-	PMD_DRV_LOG(INFO, "Get pause options, tx: %s, rx: %s, auto: %s\n",
+	PMD_DRV_LOG(INFO, "Set pause options, tx: %s, rx: %s, auto: %s\n",
 		nic_pause.tx_pause ? "on" : "off",
 		nic_pause.rx_pause ? "on" : "off",
 		nic_pause.auto_neg ? "on" : "off");
Signature · 1 line
1.8.3.1
author
#2 lore raw

PATCH v4 2/3 net/hinic/base: optimize log style

In reply to Xiaoyun wang

The patch adds space between descriptors and variables in log files.

Signed-off-by Xiaoyun wang <[email protected]>
---
 drivers/net/hinic/base/hinic_pmd_hwdev.c  | 4 ++--
 drivers/net/hinic/base/hinic_pmd_hwif.c   | 4 ++--
 drivers/net/hinic/base/hinic_pmd_mgmt.c   | 2 +-
 drivers/net/hinic/base/hinic_pmd_niccfg.c | 3 +--
 drivers/net/hinic/hinic_pmd_ethdev.c      | 8 ++++----
 drivers/net/hinic/hinic_pmd_flow.c        | 5 ++---
 6 files changed, 12 insertions(+), 14 deletions(-)
drivers/net/hinic/base/hinic_pmd_hwdev.c +2 -2
index fc11ecd..cc42076 100644
@@ -298,7 +298,7 @@ void dma_pool_destroy(struct dma_pool *pool)
 		return;
 
 	if (rte_atomic32_read(&pool->inuse) != 0) {
-		PMD_DRV_LOG(ERR, "Leak memory, dma_pool:%s, inuse_count:%d",
+		PMD_DRV_LOG(ERR, "Leak memory, dma_pool: %s, inuse_count: %d",
 			    pool->name, rte_atomic32_read(&pool->inuse));
 	}
 

@@ -1068,7 +1068,7 @@ int hinic_l2nic_reset(struct hinic_hwdev *hwdev)
 		watchdog_info->is_overflow, watchdog_info->stack_top,
 		watchdog_info->stack_bottom);
 
-	PMD_DRV_LOG(ERR, "Mgmt pc: 0x%08x, lr: 0x%08x, cpsr:0x%08x",
+	PMD_DRV_LOG(ERR, "Mgmt pc: 0x%08x, lr: 0x%08x, cpsr: 0x%08x",
 		watchdog_info->pc, watchdog_info->lr, watchdog_info->cpsr);
 
 	PMD_DRV_LOG(ERR, "Mgmt register info");
drivers/net/hinic/base/hinic_pmd_hwif.c +2 -2
index 0fced5b..63fba0d 100644
@@ -450,8 +450,8 @@ static void hinic_parse_hwif_attr(struct hinic_hwdev *hwdev)
 	struct hinic_hwif *hwif = hwdev->hwif;
 
 	PMD_DRV_LOG(INFO, "Device %s hwif attribute:", hwdev->pcidev_hdl->name);
-	PMD_DRV_LOG(INFO, "func_idx:%u, p2p_idx:%u, pciintf_idx:%u, "
-		    "vf_in_pf:%u, ppf_idx:%u, global_vf_id:%u, func_type:%u",
+	PMD_DRV_LOG(INFO, "func_idx: %u, p2p_idx: %u, pciintf_idx: %u, "
+		    "vf_in_pf: %u, ppf_idx: %u, global_vf_id: %u, func_type: %u",
 		    hwif->attr.func_global_idx,
 		    hwif->attr.port_to_port_idx, hwif->attr.pci_intf_idx,
 		    hwif->attr.vf_in_pf, hwif->attr.ppf_idx,
drivers/net/hinic/base/hinic_pmd_mgmt.c +1 -1
index 06c9b68..94bc45f 100644
@@ -470,7 +470,7 @@ static void hinic_pf_to_mgmt_free(struct hinic_hwdev *hwdev)
 			       recv_msg->msg_len);
 			*out_size = recv_msg->msg_len;
 		} else {
-			PMD_DRV_LOG(ERR, "Mgmt rsp's msg len:%u overflow.",
+			PMD_DRV_LOG(ERR, "Mgmt rsp's msg len: %u overflow.",
 				recv_msg->msg_len);
 			err = -ERANGE;
 		}
drivers/net/hinic/base/hinic_pmd_niccfg.c +1 -2
index 0899a9c..c5663df 100644
@@ -1610,8 +1610,7 @@ int hinic_set_link_status_follow(void *hwdev,
 		return 0;
 
 	if (status >= HINIC_LINK_FOLLOW_STATUS_MAX) {
-		PMD_DRV_LOG(ERR,
-			"Invalid link follow status: %d", status);
+		PMD_DRV_LOG(ERR, "Invalid link follow status: %d", status);
 		return -EINVAL;
 	}
drivers/net/hinic/hinic_pmd_ethdev.c +4 -4
index 239a78c..4091cf1 100644
@@ -477,7 +477,7 @@ static int hinic_rx_queue_setup(struct rte_eth_dev *dev, uint16_t queue_idx,
 	/* alloc rx_cqe and prepare rq_wqe */
 	rc = hinic_setup_rx_resources(rxq);
 	if (rc) {
-		PMD_DRV_LOG(ERR, "Setup rxq[%d] rx_resources failed, dev_name:%s",
+		PMD_DRV_LOG(ERR, "Setup rxq[%d] rx_resources failed, dev_name: %s",
 			    queue_idx, dev->data->name);
 		goto setup_rx_res_err;
 	}

@@ -1014,7 +1014,7 @@ static int hinic_dev_start(struct rte_eth_dev *dev)
 	/* init txq and rxq context */
 	rc = hinic_init_qp_ctxts(nic_dev->hwdev);
 	if (rc) {
-		PMD_DRV_LOG(ERR, "Initialize qp context failed, dev_name:%s",
+		PMD_DRV_LOG(ERR, "Initialize qp context failed, dev_name: %s",
 			    name);
 		goto init_qp_fail;
 	}

@@ -1056,7 +1056,7 @@ static int hinic_dev_start(struct rte_eth_dev *dev)
 	/* open physical port and start packet receiving */
 	rc = hinic_set_port_enable(nic_dev->hwdev, true);
 	if (rc) {
-		PMD_DRV_LOG(ERR, "Enable physical port failed, dev_name:%s",
+		PMD_DRV_LOG(ERR, "Enable physical port failed, dev_name: %s",
 			    name);
 		goto en_port_fail;
 	}

@@ -1203,7 +1203,7 @@ static void hinic_dev_stop(struct rte_eth_dev *dev)
 
 	rc = hinic_set_vport_enable(nic_dev->hwdev, false);
 	if (rc)
-		PMD_DRV_LOG(WARNING, "Disable vport failed, error: %d, dev_name:%s, port_id:%d",
+		PMD_DRV_LOG(WARNING, "Disable vport failed, error: %d, dev_name: %s, port_id: %d",
 			  rc, name, port_id);
 
 	/* Clear recorded link status */
drivers/net/hinic/hinic_pmd_flow.c +2 -3
index c1f86ef..e620df0 100644
@@ -2285,8 +2285,7 @@ static int hinic_add_del_ntuple_filter(struct rte_eth_dev *dev,
 			case RTE_ETHER_TYPE_SLOW:
 				(void)hinic_clear_fdir_tcam(nic_dev->hwdev,
 								TCAM_PKT_LACP);
-				PMD_DRV_LOG(INFO,
-					"Del lacp tcam succeed");
+				PMD_DRV_LOG(INFO, "Del lacp tcam succeed");
 				break;
 			default:
 				break;

@@ -2935,7 +2934,7 @@ static int hinic_flow_destroy(struct rte_eth_dev *dev, struct rte_flow *flow,
 		}
 		break;
 	default:
-		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
+		PMD_DRV_LOG(WARNING, "Filter type (%d) is not supported",
 			filter_type);
 		ret = -EINVAL;
 		break;
Signature · 1 line
1.8.3.1
author
#3 lore raw

PATCH v4 3/3 net/hinic: adds txq xstats member

In reply to Xiaoyun wang

Because some APPs may pass illegal parameters, driver increases checks on illegal parameters and DFX statistics, which includes sge_len0 and mbuf_null txq xstats member.

Signed-off-by Xiaoyun wang <[email protected]>
---
 drivers/net/hinic/hinic_pmd_ethdev.c |  2 ++
 drivers/net/hinic/hinic_pmd_tx.c     | 31 +++++++++++++++++++++++++++++++
 drivers/net/hinic/hinic_pmd_tx.h     |  2 ++
 3 files changed, 35 insertions(+)
drivers/net/hinic/hinic_pmd_ethdev.c +2 -0
index 4091cf1..cfbca64 100644
@@ -225,6 +225,8 @@ struct hinic_xstats_name_off {
 	{"copy_pkts", offsetof(struct hinic_txq_stats, cpy_pkts)},
 	{"rl_drop", offsetof(struct hinic_txq_stats, rl_drop)},
 	{"burst_pkts", offsetof(struct hinic_txq_stats, burst_pkts)},
+	{"sge_len0", offsetof(struct hinic_txq_stats, sge_len0)},
+	{"mbuf_null", offsetof(struct hinic_txq_stats, mbuf_null)},
 };
 
 #define HINIC_TXQ_XSTATS_NUM (sizeof(hinic_txq_stats_strings) / \
drivers/net/hinic/hinic_pmd_tx.c +31 -0
index f24e3e4..258f2c1 100644
@@ -334,7 +334,16 @@ static inline bool hinic_mbuf_dma_map_sge(struct hinic_txq *txq,
 		i = 0;
 		for (sge_idx = sges; (u64)sge_idx <= txq->sq_bot_sge_addr;
 		     sge_idx++) {
+			if (unlikely(mbuf == NULL)) {
+				txq->txq_stats.mbuf_null++;
+				return false;
+			}
+
 			dma_addr = rte_mbuf_data_iova(mbuf);
+			if (unlikely(mbuf->data_len == 0)) {
+				txq->txq_stats.sge_len0++;
+				return false;
+			}
 			hinic_set_sge((struct hinic_sge *)sge_idx, dma_addr,
 				      mbuf->data_len);
 			mbuf = mbuf->next;

@@ -345,7 +354,16 @@ static inline bool hinic_mbuf_dma_map_sge(struct hinic_txq *txq,
 		sge_idx = (struct hinic_sq_bufdesc *)
 				((void *)txq->sq_head_addr);
 		for (; i < nb_segs; i++) {
+			if (unlikely(mbuf == NULL)) {
+				txq->txq_stats.mbuf_null++;
+				return false;
+			}
+
 			dma_addr = rte_mbuf_data_iova(mbuf);
+			if (unlikely(mbuf->data_len == 0)) {
+				txq->txq_stats.sge_len0++;
+				return false;
+			}
 			hinic_set_sge((struct hinic_sge *)sge_idx, dma_addr,
 				      mbuf->data_len);
 			mbuf = mbuf->next;

@@ -357,7 +375,16 @@ static inline bool hinic_mbuf_dma_map_sge(struct hinic_txq *txq,
 	} else {
 		/* wqe is in continuous space */
 		for (i = 0; i < nb_segs; i++) {
+			if (unlikely(mbuf == NULL)) {
+				txq->txq_stats.mbuf_null++;
+				return false;
+			}
+
 			dma_addr = rte_mbuf_data_iova(mbuf);
+			if (unlikely(mbuf->data_len == 0)) {
+				txq->txq_stats.sge_len0++;
+				return false;
+			}
 			hinic_set_sge((struct hinic_sge *)sge_idx, dma_addr,
 				      mbuf->data_len);
 			mbuf = mbuf->next;

@@ -378,6 +405,10 @@ static inline bool hinic_mbuf_dma_map_sge(struct hinic_txq *txq,
 
 		/* deal with the last mbuf */
 		dma_addr = rte_mbuf_data_iova(mbuf);
+		if (unlikely(mbuf->data_len == 0)) {
+			txq->txq_stats.sge_len0++;
+			return false;
+		}
 		hinic_set_sge((struct hinic_sge *)sge_idx, dma_addr,
 			      mbuf->data_len);
 		if (unlikely(sqe_info->around))
drivers/net/hinic/hinic_pmd_tx.h +2 -0
index dabbc6c..d98abad 100644
@@ -93,6 +93,8 @@ struct hinic_txq_stats {
 	u64 off_errs;
 	u64 cpy_pkts;
 	u64 burst_pkts;
+	u64 sge_len0;
+	u64 mbuf_null;
 };
 
 struct hinic_tx_info {
Signature · 1 line
1.8.3.1
#4 lore raw
In reply to Xiaoyun wang
Quoted 22 lines · Xiaoyun wang

This patch fixes PF firmware hotactive problem, optimizes log style and adds txq xstats members.

--
v3->v4:
- fix PF firmware hotactive problem
- optimize log style
- adds txq xstats members

v2->v3:
- fix FW hotactive problem
- optimize log style

v1->v2:
- Fix LRO problems
- Fix hotupdate firmware problem
- Optimize doorbell area initialization
- Remove 40GE Mezz card id
- Add Fdir filter type
- Optimize log files
- Support pause flow control

Quoted 8 lines

v1:
- Allocate IO memory with socketid

Xiaoyun wang (3):
net/hinic/base: fix PF firmware hotactive problem net/hinic/base: optimize log style net/hinic: adds txq xstats member

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