[dpdk-dev] [PATCH 0/4] updates for hns3 PMD driver

[dpdk-dev] [PATCH 0/4] updates for hns3 PMD driver

[Wei Hu (Xavier)]

This series are updates for hns3 PMD driver.

Chengchang Tang (1):
  net/hns3: cleanup duplicate code when processing TSO in Tx

Lijun Ou (1):
  doc: update feature list of hns3 rst document

Wei Hu (Xavier) (2):
  net/hns3: check if registering mp action successfully
  net/hns3: fix VLAN tag inserted in wrong position in Tx

 doc/guides/nics/hns3.rst          |  11 +++-
 drivers/net/hns3/hns3_ethdev.c    |  22 +++++++-
 drivers/net/hns3/hns3_ethdev_vf.c |  20 ++++++-
 drivers/net/hns3/hns3_mp.c        |  34 ++++++++++--
 drivers/net/hns3/hns3_mp.h        |   4 +-
 drivers/net/hns3/hns3_rxtx.c      | 113 +++++++++++++++++++++++---------------
 6 files changed, 147 insertions(+), 57 deletions(-)

-- 
2.7.4

[dpdk-dev] [PATCH 1/4] net/hns3: check if registering mp action successfully

[Wei Hu (Xavier)]

Currently, there is a coverity defect warning about hns3 PMD driver, the
detail information as blow:
CID 289969 (#1 of 1): Unchecked return value (CHECKED_RETURN)
1. check_return: Calling rte_mp_action_register without checking return
   value (as is done elsewhere 11 out of 13 times).

The problem is that missing checking the return value of calling the API
rte_mp_action_register during initialization. If regitering an action
function for primary and secondary communication failed, the secondary
process can't work properly.

This patch fixes it by adding check return value of the API function
named rte_mp_action_register in the '.dev_init' implementation function
of hns3 PMD driver.

Coverity issue: 289969
Fixes: 23d4b61fee5d ("net/hns3: support multiple process")
Cc: stable@dpdk.org

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c    | 22 ++++++++++++++++++++--
 drivers/net/hns3/hns3_ethdev_vf.c | 20 ++++++++++++++++++--
 drivers/net/hns3/hns3_mp.c        | 34 +++++++++++++++++++++++++++++-----
 drivers/net/hns3/hns3_mp.h        |  4 ++--
 4 files changed, 69 insertions(+), 11 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 00ed3e2..265d620 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -5451,12 +5451,25 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 	hns3_set_rxtx_function(eth_dev);
 	eth_dev->dev_ops = &hns3_eth_dev_ops;
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		hns3_mp_init_secondary();
+		ret = hns3_mp_init_secondary();
+		if (ret) {
+			PMD_INIT_LOG(ERR, "Failed to init for secondary "
+				     "process, ret = %d", ret);
+			goto err_mp_init_secondary;
+		}
+
 		hw->secondary_cnt++;
 		return 0;
 	}
 
-	hns3_mp_init_primary();
+	ret = hns3_mp_init_primary();
+	if (ret) {
+		PMD_INIT_LOG(ERR,
+			     "Failed to init for primary process, ret = %d",
+			     ret);
+		goto err_mp_init_primary;
+	}
+
 	hw->adapter_state = HNS3_NIC_UNINITIALIZED;
 	hns->is_vf = false;
 	hw->data = eth_dev->data;
@@ -5517,7 +5530,12 @@ hns3_dev_init(struct rte_eth_dev *eth_dev)
 
 err_init_pf:
 	rte_free(hw->reset.wait_data);
+
 err_init_reset:
+	hns3_mp_uninit_primary();
+
+err_mp_init_primary:
+err_mp_init_secondary:
 	eth_dev->dev_ops = NULL;
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 3c5998a..54e5dac 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2524,12 +2524,24 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 	hns3_set_rxtx_function(eth_dev);
 	eth_dev->dev_ops = &hns3vf_eth_dev_ops;
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY) {
-		hns3_mp_init_secondary();
+		ret = hns3_mp_init_secondary();
+		if (ret) {
+			PMD_INIT_LOG(ERR, "Failed to init for secondary "
+					  "process, ret = %d", ret);
+			goto err_mp_init_secondary;
+		}
+
 		hw->secondary_cnt++;
 		return 0;
 	}
 
-	hns3_mp_init_primary();
+	ret = hns3_mp_init_primary();
+	if (ret) {
+		PMD_INIT_LOG(ERR,
+			     "Failed to init for primary process, ret = %d",
+			     ret);
+		goto err_mp_init_primary;
+	}
 
 	hw->adapter_state = HNS3_NIC_UNINITIALIZED;
 	hns->is_vf = true;
@@ -2586,6 +2598,10 @@ hns3vf_dev_init(struct rte_eth_dev *eth_dev)
 	rte_free(hw->reset.wait_data);
 
 err_init_reset:
+	hns3_mp_uninit_primary();
+
+err_mp_init_primary:
+err_mp_init_secondary:
 	eth_dev->dev_ops = NULL;
 	eth_dev->rx_pkt_burst = NULL;
 	eth_dev->tx_pkt_burst = NULL;
diff --git a/drivers/net/hns3/hns3_mp.c b/drivers/net/hns3/hns3_mp.c
index 596c310..639f46c 100644
--- a/drivers/net/hns3/hns3_mp.c
+++ b/drivers/net/hns3/hns3_mp.c
@@ -14,6 +14,8 @@
 #include "hns3_rxtx.h"
 #include "hns3_mp.h"
 
+static bool hns3_inited;
+
 /*
  * Initialize IPC message.
  *
@@ -192,9 +194,20 @@ void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev)
 /*
  * Initialize by primary process.
  */
-void hns3_mp_init_primary(void)
+int hns3_mp_init_primary(void)
 {
-	rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
+	int ret;
+
+	if (!hns3_inited) {
+		/* primary is allowed to not support IPC */
+		ret = rte_mp_action_register(HNS3_MP_NAME, mp_primary_handle);
+		if (ret && rte_errno != ENOTSUP)
+			return ret;
+
+		hns3_inited = true;
+	}
+
+	return 0;
 }
 
 /*
@@ -202,13 +215,24 @@ void hns3_mp_init_primary(void)
  */
 void hns3_mp_uninit_primary(void)
 {
-	rte_mp_action_unregister(HNS3_MP_NAME);
+	if (hns3_inited)
+		rte_mp_action_unregister(HNS3_MP_NAME);
 }
 
 /*
  * Initialize by secondary process.
  */
-void hns3_mp_init_secondary(void)
+int hns3_mp_init_secondary(void)
 {
-	rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
+	int ret;
+
+	if (!hns3_inited) {
+		ret = rte_mp_action_register(HNS3_MP_NAME, mp_secondary_handle);
+		if (ret)
+			return ret;
+
+		hns3_inited = true;
+	}
+
+	return 0;
 }
diff --git a/drivers/net/hns3/hns3_mp.h b/drivers/net/hns3/hns3_mp.h
index aefbeb1..036546a 100644
--- a/drivers/net/hns3/hns3_mp.h
+++ b/drivers/net/hns3/hns3_mp.h
@@ -7,8 +7,8 @@
 
 void hns3_mp_req_start_rxtx(struct rte_eth_dev *dev);
 void hns3_mp_req_stop_rxtx(struct rte_eth_dev *dev);
-void hns3_mp_init_primary(void);
+int hns3_mp_init_primary(void);
 void hns3_mp_uninit_primary(void);
-void hns3_mp_init_secondary(void);
+int hns3_mp_init_secondary(void);
 
 #endif /* _HNS3_MP_H_ */
-- 
2.7.4

[dpdk-dev] [PATCH 2/4] net/hns3: cleanup duplicate code when processing TSO in Tx

[Wei Hu (Xavier)]

From: Chengchang Tang <tangchengchang@huawei.com>

This patch fixes up paylen calculation twice when processing TSO request
in the '.tx_pkt_burst' ops implementation function to avoid performance
loss.

Fixes: 6dca716c9e1d ("net/hns3: support TSO")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Hongbo Zheng <zhenghongbo3@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 8892fc1..07640db 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -1979,12 +1979,11 @@ hns3_pkt_is_tso(struct rte_mbuf *m)
 }
 
 static void
-hns3_set_tso(struct hns3_desc *desc,
-	     uint64_t ol_flags, struct rte_mbuf *rxm)
+hns3_set_tso(struct hns3_desc *desc, uint64_t ol_flags,
+		uint32_t paylen, struct rte_mbuf *rxm)
 {
-	uint32_t paylen, hdr_len;
-	uint32_t tmp;
 	uint8_t l2_len = rxm->l2_len;
+	uint32_t tmp;
 
 	if (!hns3_pkt_is_tso(rxm))
 		return;
@@ -1992,10 +1991,6 @@ hns3_set_tso(struct hns3_desc *desc,
 	if (hns3_tso_proc_tunnel(desc, ol_flags, rxm, &l2_len))
 		return;
 
-	hdr_len = rxm->l2_len + rxm->l3_len + rxm->l4_len;
-	hdr_len += (ol_flags & PKT_TX_TUNNEL_MASK) ?
-		    rxm->outer_l2_len + rxm->outer_l3_len : 0;
-	paylen = rxm->pkt_len - hdr_len;
 	if (paylen <= rxm->tso_segsz)
 		return;
 
@@ -2036,7 +2031,7 @@ fill_desc(struct hns3_tx_queue *txq, uint16_t tx_desc_id, struct rte_mbuf *rxm,
 			   rxm->outer_l2_len + rxm->outer_l3_len : 0;
 		paylen = rxm->pkt_len - hdr_len;
 		desc->tx.paylen = rte_cpu_to_le_32(paylen);
-		hns3_set_tso(desc, ol_flags, rxm);
+		hns3_set_tso(desc, ol_flags, paylen, rxm);
 	}
 
 	hns3_set_bit(rrcfv, HNS3_TXD_FE_B, frag_end);
-- 
2.7.4

[dpdk-dev] [PATCH 3/4] net/hns3: fix VLAN tag inserted in wrong position in Tx

[Wei Hu (Xavier)]

Based on hns3 network engine, in order to configure hardware VLAN insert
offload in Tx direction, PMD driver reads the VLAN tags from the
vlan_tci_outer and vlan_tci of the structure rte_mbuf, fills them into the
Tx Buffer Descriptor and sets the related offload flag for every packet.

Currently, there are two VLAN related problems in the 'tx_pkt_burst' ops
implementation function:
1) When setting the related offload flag, PMD driver inserts the VLAN tag
   into the position that close to L3 header. So, when upper application
   sends a packet with a VLAN tag in the data buffer, the VLAN offloaded
   by hardware will be added to the wrong position. It is supposed to add
   the VLAN tag from the rte_mbuf to the position close to the MAC header
   in the packet when using VLAN insertion.

   And when PF PVID is enabled by calling the API function named
   rte_eth_dev_set_vlan_pvid or VF PVID is enabled by hns3 PF kernel ether
   driver, the VLAN tag from the structure rte_mbuf to enable the VLAN
   insertion should be filled into the position that close to L3 header to
   avoid to be overwittern by the PVID which will always be inserted in the
   position that close to the MAC address.

2) When sending multiple segment packets, VLAN information is required to
   be filled into the first Tx Buffer descriptor. However, currently hns3
   PMD driver incorrectly placed it in the last Tx Buffer Descriptor. This
   results in VLAN insert offload failure when sending multiple segment
   packets.

This patch fixed them by filling the VLAN information into the position of
the Tx Buffer Descriptor.

Fixes: bba636698316 ("net/hns3: support Rx/Tx and related operations")
Cc: stable@dpdk.org

Signed-off-by: Chengchang Tang <tangchengchang@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 drivers/net/hns3/hns3_rxtx.c | 102 +++++++++++++++++++++++++++----------------
 1 file changed, 65 insertions(+), 37 deletions(-)

diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 07640db..b0253d5 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -2007,51 +2007,58 @@ hns3_set_tso(struct hns3_desc *desc, uint64_t ol_flags,
 	desc->tx.mss = rte_cpu_to_le_16(rxm->tso_segsz);
 }
 
+static inline void
+hns3_fill_per_desc(struct hns3_desc *desc, struct rte_mbuf *rxm)
+{
+	desc->addr = rte_mbuf_data_iova(rxm);
+	desc->tx.send_size = rte_cpu_to_le_16(rte_pktmbuf_data_len(rxm));
+	desc->tx.tp_fe_sc_vld_ra_ri = rte_cpu_to_le_16(BIT(HNS3_TXD_VLD_B));
+}
+
 static void
-fill_desc(struct hns3_tx_queue *txq, uint16_t tx_desc_id, struct rte_mbuf *rxm,
-	  bool first, int offset)
+hns3_fill_first_desc(struct hns3_tx_queue *txq, struct hns3_desc *desc,
+		     struct rte_mbuf *rxm)
 {
-	struct hns3_desc *tx_ring = txq->tx_ring;
-	struct hns3_desc *desc = &tx_ring[tx_desc_id];
-	uint8_t frag_end = rxm->next == NULL ? 1 : 0;
 	uint64_t ol_flags = rxm->ol_flags;
-	uint16_t size = rxm->data_len;
-	uint16_t rrcfv = 0;
 	uint32_t hdr_len;
 	uint32_t paylen;
-	uint32_t tmp;
 
-	desc->addr = rte_mbuf_data_iova(rxm) + offset;
-	desc->tx.send_size = rte_cpu_to_le_16(size);
-	hns3_set_bit(rrcfv, HNS3_TXD_VLD_B, 1);
-
-	if (first) {
-		hdr_len = rxm->l2_len + rxm->l3_len + rxm->l4_len;
-		hdr_len += (ol_flags & PKT_TX_TUNNEL_MASK) ?
+	hdr_len = rxm->l2_len + rxm->l3_len + rxm->l4_len;
+	hdr_len += (ol_flags & PKT_TX_TUNNEL_MASK) ?
 			   rxm->outer_l2_len + rxm->outer_l3_len : 0;
-		paylen = rxm->pkt_len - hdr_len;
-		desc->tx.paylen = rte_cpu_to_le_32(paylen);
-		hns3_set_tso(desc, ol_flags, paylen, rxm);
-	}
-
-	hns3_set_bit(rrcfv, HNS3_TXD_FE_B, frag_end);
-	desc->tx.tp_fe_sc_vld_ra_ri = rte_cpu_to_le_16(rrcfv);
-
-	if (frag_end) {
-		if (ol_flags & (PKT_TX_VLAN_PKT | PKT_TX_QINQ_PKT)) {
-			tmp = rte_le_to_cpu_32(desc->tx.type_cs_vlan_tso_len);
-			hns3_set_bit(tmp, HNS3_TXD_VLAN_B, 1);
-			desc->tx.type_cs_vlan_tso_len = rte_cpu_to_le_32(tmp);
-			desc->tx.vlan_tag = rte_cpu_to_le_16(rxm->vlan_tci);
-		}
+	paylen = rxm->pkt_len - hdr_len;
+	desc->tx.paylen = rte_cpu_to_le_32(paylen);
+	hns3_set_tso(desc, ol_flags, paylen, rxm);
 
-		if (ol_flags & PKT_TX_QINQ_PKT) {
-			tmp = rte_le_to_cpu_32(desc->tx.ol_type_vlan_len_msec);
-			hns3_set_bit(tmp, HNS3_TXD_OVLAN_B, 1);
-			desc->tx.ol_type_vlan_len_msec = rte_cpu_to_le_32(tmp);
+	/*
+	 * Currently, hardware doesn't support more than two layers VLAN offload
+	 * in Tx direction based on hns3 network engine. So when the number of
+	 * VLANs in the packets represented by rxm plus the number of VLAN
+	 * offload by hardware such as PVID etc, exceeds two, the packets will
+	 * be discarded or the original VLAN of the packets will be overwitted
+	 * by hardware. When the PF PVID is enabled by calling the API function
+	 * named rte_eth_dev_set_vlan_pvid or the VF PVID is enabled by the hns3
+	 * PF kernel ether driver, the outer VLAN tag will always be the PVID.
+	 * To avoid the VLAN of Tx descriptor is overwritten by PVID, it should
+	 * be added to the position close to the IP header when PVID is enabled.
+	 */
+	if (!txq->pvid_state && ol_flags & (PKT_TX_VLAN_PKT |
+				PKT_TX_QINQ_PKT)) {
+		desc->tx.ol_type_vlan_len_msec |=
+				rte_cpu_to_le_32(BIT(HNS3_TXD_OVLAN_B));
+		if (ol_flags & PKT_TX_QINQ_PKT)
 			desc->tx.outer_vlan_tag =
-				rte_cpu_to_le_16(rxm->vlan_tci_outer);
-		}
+					rte_cpu_to_le_16(rxm->vlan_tci_outer);
+		else
+			desc->tx.outer_vlan_tag =
+					rte_cpu_to_le_16(rxm->vlan_tci);
+	}
+
+	if (ol_flags & PKT_TX_QINQ_PKT ||
+	    ((ol_flags & PKT_TX_VLAN_PKT) && txq->pvid_state)) {
+		desc->tx.type_cs_vlan_tso_len |=
+					rte_cpu_to_le_32(BIT(HNS3_TXD_VLAN_B));
+		desc->tx.vlan_tag = rte_cpu_to_le_16(rxm->vlan_tci);
 	}
 }
 
@@ -2628,8 +2635,10 @@ hns3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	struct rte_net_hdr_lens hdr_lens = {0};
 	struct hns3_tx_queue *txq = tx_queue;
 	struct hns3_entry *tx_bak_pkt;
+	struct hns3_desc *tx_ring;
 	struct rte_mbuf *tx_pkt;
 	struct rte_mbuf *m_seg;
+	struct hns3_desc *desc;
 	uint32_t nb_hold = 0;
 	uint16_t tx_next_use;
 	uint16_t tx_pkt_num;
@@ -2644,6 +2653,7 @@ hns3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	tx_next_use   = txq->next_to_use;
 	tx_bd_max     = txq->nb_tx_desc;
 	tx_pkt_num = nb_pkts;
+	tx_ring = txq->tx_ring;
 
 	/* send packets */
 	tx_bak_pkt = &txq->sw_ring[tx_next_use];
@@ -2688,8 +2698,22 @@ hns3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 			goto end_of_tx;
 
 		i = 0;
+		desc = &tx_ring[tx_next_use];
+
+		/*
+		 * If the packet is divided into multiple Tx Buffer Descriptors,
+		 * only need to fill vlan, paylen and tso into the first Tx
+		 * Buffer Descriptor.
+		 */
+		hns3_fill_first_desc(txq, desc, m_seg);
+
 		do {
-			fill_desc(txq, tx_next_use, m_seg, (i == 0), 0);
+			desc = &tx_ring[tx_next_use];
+			/*
+			 * Fill valid bits, DMA address and data length for each
+			 * Tx Buffer Descriptor.
+			 */
+			hns3_fill_per_desc(desc, m_seg);
 			tx_bak_pkt->mbuf = m_seg;
 			m_seg = m_seg->next;
 			tx_next_use++;
@@ -2702,6 +2726,10 @@ hns3_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 			i++;
 		} while (m_seg != NULL);
 
+		/* Add end flag for the last Tx Buffer Descriptor */
+		desc->tx.tp_fe_sc_vld_ra_ri |=
+				 rte_cpu_to_le_16(BIT(HNS3_TXD_FE_B));
+
 		nb_hold += i;
 		txq->next_to_use = tx_next_use;
 		txq->tx_bd_ready -= i;
-- 
2.7.4

[dpdk-dev] [PATCH 4/4] doc: update feature list of hns3 rst document

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun@huawei.com>

This patch updates the feature list for hns3 PMD driver document.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 doc/guides/nics/hns3.rst | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/doc/guides/nics/hns3.rst b/doc/guides/nics/hns3.rst
index ae3c5f6..a62fcfd 100644
--- a/doc/guides/nics/hns3.rst
+++ b/doc/guides/nics/hns3.rst
@@ -25,7 +25,16 @@ Features of the HNS3 PMD are:
 - Jumbo frames
 - Link state information
 - Interrupt mode for RX
-- VLAN stripping
+- VLAN stripping and inserting
+- QinQ inserting
+- DCB
+- Scattered and gather for TX and RX
+- Flow director
+- Dump register
+- SR-IOV VF
+- Multi-process
+- MAC/VLAN filter
+- MTU update
 - NUMA support
 
 Prerequisites
-- 
2.7.4

Re: [dpdk-dev] [PATCH 0/4] updates for hns3 PMD driver

[Ferruh Yigit]

On 7/4/2020 11:09 AM, Wei Hu (Xavier) wrote:
> This series are updates for hns3 PMD driver.
> 
> Chengchang Tang (1):
>   net/hns3: cleanup duplicate code when processing TSO in Tx
> 
> Lijun Ou (1):
>   doc: update feature list of hns3 rst document
> 
> Wei Hu (Xavier) (2):
>   net/hns3: check if registering mp action successfully
>   net/hns3: fix VLAN tag inserted in wrong position in Tx
> 

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