DPDK patches and discussions
 help / color / mirror / Atom feed
From: Jie Hai <haijie1@huawei.com>
To: <dev@dpdk.org>, <stephen@networkplumber.org>,
	<ferruh.yigit@amd.com>, <mb@smartsharesystems.com>,
	<andrew.rybchenko@oktetlabs.ru>,
	Yisen Zhuang <yisen.zhuang@huawei.com>,
	"Wei Hu (Xavier)" <xavier.huwei@huawei.com>,
	"Min Hu (Connor)" <humin29@huawei.com>
Cc: <huangdengdui@huawei.com>, <fengchengwen@huawei.com>,
	<lihuisong@huawei.com>
Subject: [PATCH v3 3/3] net/hns3: fix Rx packet without CRC data
Date: Fri, 19 Jul 2024 17:04:15 +0800	[thread overview]
Message-ID: <20240719090415.1513301-4-haijie1@huawei.com> (raw)
In-Reply-To: <20240719090415.1513301-1-haijie1@huawei.com>

From: Dengdui Huang <huangdengdui@huawei.com>

When KEEP_CRC offload is enabled, the CRC data is still stripped
in following cases:
1. For HIP08 network engine, the packet type is TCP and the length
   is less than or equal to 60B.
2. For HIP09 network engine, the packet type is IP and the length
   is less than or equal to 60B.

So driver has to recaculate packet CRC for this rare scenarios.

In addition, to avoid impacting performance, KEEP_CRC is not
supported when NEON or SVE algorithm is used.

Fixes: 8973d7c4ca12 ("net/hns3: support keeping CRC")
Cc: stable@dpdk.org

Signed-off-by: Dengdui Huang <huangdengdui@huawei.com>
Acked-by: Huisong Li <lihuisong@huawei.com>
Acked-by: Jie Hai <haijie1@huawei.com>
---
 drivers/net/hns3/hns3_ethdev.c   |  5 ++
 drivers/net/hns3/hns3_ethdev.h   | 23 +++++++++
 drivers/net/hns3/hns3_rxtx.c     | 81 ++++++++++++++++++++++++++++++--
 drivers/net/hns3/hns3_rxtx.h     |  3 ++
 drivers/net/hns3/hns3_rxtx_vec.c |  3 +-
 5 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index ec1251cb7ead..5fdabba54794 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2739,6 +2739,7 @@ hns3_get_capability(struct hns3_hw *hw)
 		hw->udp_cksum_mode = HNS3_SPECIAL_PORT_SW_CKSUM_MODE;
 		pf->support_multi_tc_pause = false;
 		hw->rx_dma_addr_align = HNS3_RX_DMA_ADDR_ALIGN_64;
+		hw->strip_crc_ptype = HNS3_STRIP_CRC_PTYPE_TCP;
 		return 0;
 	}
 
@@ -2760,6 +2761,10 @@ hns3_get_capability(struct hns3_hw *hw)
 	hw->udp_cksum_mode = HNS3_SPECIAL_PORT_HW_CKSUM_MODE;
 	pf->support_multi_tc_pause = true;
 	hw->rx_dma_addr_align = HNS3_RX_DMA_ADDR_ALIGN_128;
+	if (hw->revision == PCI_REVISION_ID_HIP09_A)
+		hw->strip_crc_ptype = HNS3_STRIP_CRC_PTYPE_IP;
+	else
+		hw->strip_crc_ptype = HNS3_STRIP_CRC_PTYPE_NONE;
 
 	return 0;
 }
diff --git a/drivers/net/hns3/hns3_ethdev.h b/drivers/net/hns3/hns3_ethdev.h
index 799b61038a06..ef1f5ebd584c 100644
--- a/drivers/net/hns3/hns3_ethdev.h
+++ b/drivers/net/hns3/hns3_ethdev.h
@@ -56,6 +56,10 @@
 #define HNS3_SPECIAL_PORT_SW_CKSUM_MODE         0
 #define HNS3_SPECIAL_PORT_HW_CKSUM_MODE         1
 
+#define HNS3_STRIP_CRC_PTYPE_NONE         0
+#define HNS3_STRIP_CRC_PTYPE_TCP          1
+#define HNS3_STRIP_CRC_PTYPE_IP           2
+
 #define HNS3_UC_MACADDR_NUM		128
 #define HNS3_VF_UC_MACADDR_NUM		48
 #define HNS3_MC_MACADDR_NUM		128
@@ -657,6 +661,25 @@ struct hns3_hw {
 	 */
 	uint8_t udp_cksum_mode;
 
+	/*
+	 * When KEEP_CRC offload is enabled, the CRC data of some type packets
+	 * whose length is less than or equal to HNS3_KEEP_CRC_OK_MIN_PKT_LEN
+	 * is still be stripped on some network engine. So here has to use this
+	 * field to distinguish the difference between different network engines.
+	 * value range:
+	 *  - HNS3_STRIP_CRC_PTYPE_TCP
+	 *     This value for HIP08 network engine.
+	 *     Indicates that only the IP-TCP packet type is stripped.
+	 *
+	 *  - HNS3_STRIP_CRC_PTYPE_IP
+	 *     This value for HIP09 network engine.
+	 *     Indicates that all IP packet types are stripped.
+	 *
+	 *  - HNS3_STRIP_CRC_PTYPE_NONE
+	 *     Indicates that all packet types are not stripped.
+	 */
+	uint8_t strip_crc_ptype;
+
 	struct hns3_port_base_vlan_config port_base_vlan_cfg;
 
 	pthread_mutex_t flows_lock; /* rte_flow ops lock */
diff --git a/drivers/net/hns3/hns3_rxtx.c b/drivers/net/hns3/hns3_rxtx.c
index 39ba9080ea15..d12463d0ec56 100644
--- a/drivers/net/hns3/hns3_rxtx.c
+++ b/drivers/net/hns3/hns3_rxtx.c
@@ -11,6 +11,7 @@
 #include <rte_io.h>
 #include <rte_net.h>
 #include <rte_malloc.h>
+#include <rte_net_crc.h>
 #if defined(RTE_ARCH_ARM64)
 #include <rte_cpuflags.h>
 #include <rte_vect.h>
@@ -1766,8 +1767,9 @@ hns3_rx_buf_len_calc(struct rte_mempool *mp, uint16_t *rx_buf_len)
 }
 
 static int
-hns3_rxq_conf_runtime_check(struct hns3_hw *hw, uint16_t buf_size,
-				uint16_t nb_desc)
+hns3_rxq_conf_runtime_check(struct hns3_hw *hw,
+			    const struct rte_eth_rxconf *conf,
+			    uint16_t buf_size, uint16_t nb_desc)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[hw->data->port_id];
 	eth_rx_burst_t pkt_burst = dev->rx_pkt_burst;
@@ -1800,6 +1802,14 @@ hns3_rxq_conf_runtime_check(struct hns3_hw *hw, uint16_t buf_size,
 			return -EINVAL;
 		}
 	}
+
+	if ((conf->offloads & RTE_ETH_RX_OFFLOAD_KEEP_CRC) &&
+	    pkt_burst != hns3_recv_pkts_simple &&
+	    pkt_burst != hns3_recv_scattered_pkts) {
+		hns3_err(hw, "KEEP_CRC offload is not supported in the current rx function.");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 
@@ -1836,7 +1846,7 @@ hns3_rx_queue_conf_check(struct hns3_hw *hw, const struct rte_eth_rxconf *conf,
 	}
 
 	if (hw->data->dev_started) {
-		ret = hns3_rxq_conf_runtime_check(hw, *buf_size, nb_desc);
+		ret = hns3_rxq_conf_runtime_check(hw, conf, *buf_size, nb_desc);
 		if (ret) {
 			hns3_err(hw, "Rx queue runtime setup fail.");
 			return ret;
@@ -1956,6 +1966,8 @@ hns3_rx_queue_setup(struct rte_eth_dev *dev, uint16_t idx, uint16_t nb_desc,
 	else
 		rxq->crc_len = 0;
 
+	rxq->keep_crc_fail_ptype = hw->strip_crc_ptype;
+
 	rxq->bulk_mbuf_num = 0;
 
 	rte_spinlock_lock(&hw->lock);
@@ -2415,6 +2427,50 @@ hns3_rx_ptp_timestamp_handle(struct hns3_rx_queue *rxq, struct rte_mbuf *mbuf,
 	pf->rx_timestamp = timestamp;
 }
 
+static inline bool
+hns3_need_recalculate_crc(struct hns3_rx_queue *rxq, struct rte_mbuf *m)
+{
+	uint32_t ptype = m->packet_type;
+
+	if (rxq->keep_crc_fail_ptype == HNS3_STRIP_CRC_PTYPE_NONE)
+		return false;
+
+	if (m->pkt_len > HNS3_KEEP_CRC_OK_MIN_PKT_LEN)
+		return false;
+
+	if (!(RTE_ETH_IS_IPV4_HDR(ptype) || RTE_ETH_IS_IPV6_HDR(ptype)))
+		return false;
+
+	if (rxq->keep_crc_fail_ptype == HNS3_STRIP_CRC_PTYPE_TCP)
+		return (ptype & RTE_PTYPE_L4_MASK) == RTE_PTYPE_L4_TCP;
+
+	return true;
+}
+
+static inline void
+hns3_recalculate_crc(struct rte_mbuf *m)
+{
+	char *append_data;
+	uint32_t crc;
+
+	crc = rte_net_crc_calc(rte_pktmbuf_mtod(m, void *),
+			       m->data_len, RTE_NET_CRC32_ETH);
+
+	/*
+	 * The hns3 driver requires that mbuf size must be at least 512B.
+	 * When CRC is stripped by hardware, the pkt_len must be less than
+	 * or equal to 60B. Therefore, the space of the mbuf is enough
+	 * to insert the CRC.
+	 *
+	 * In addition, after CRC is stripped by hardware, pkt_len and data_len
+	 * do not contain the CRC length. Therefore, after CRC data is appended
+	 * by PMD again, both pkt_len and data_len add the CRC length.
+	 */
+	append_data = rte_pktmbuf_append(m, RTE_NET_CRC32_ETH);
+	/* The CRC data is binary data and does not care about the byte order. */
+	rte_memcpy(append_data, (void *)&crc, RTE_NET_CRC32_ETH);
+}
+
 uint16_t
 hns3_recv_pkts_simple(void *rx_queue,
 		      struct rte_mbuf **rx_pkts,
@@ -2510,6 +2566,10 @@ hns3_recv_pkts_simple(void *rx_queue,
 		if (rxm->packet_type == RTE_PTYPE_L2_ETHER_TIMESYNC)
 			rxm->ol_flags |= RTE_MBUF_F_RX_IEEE1588_PTP;
 
+		if (unlikely(rxq->crc_len > 0) &&
+		    hns3_need_recalculate_crc(rxq, rxm))
+			hns3_recalculate_crc(rxm);
+
 		hns3_rxd_to_vlan_tci(rxq, rxm, l234_info, &rxd);
 
 		/* All byte-related statistics do not include Ethernet FCS */
@@ -2718,6 +2778,21 @@ hns3_recv_scattered_pkts(void *rx_queue,
 		if (first_seg->packet_type == RTE_PTYPE_L2_ETHER_TIMESYNC)
 			rxm->ol_flags |= RTE_MBUF_F_RX_IEEE1588_PTP;
 
+		/*
+		 * If KEEP_CRC offload is enabled, the network engine reserves
+		 * the CRC data in the packet. But the CRC is still stripped
+		 * for a kind of packets in hns3 NIC:
+		 * 1. All IP-TCP packet whose the length is less than and equal
+		 *    to 60 Byte (no CRC) on HIP08 network engine.
+		 * 2. All IP packet whose the length is less than and equal to
+		 *    60 Byte (no CRC) on HIP09 network engine.
+		 * In this case, the PMD calculates the CRC and appends it to
+		 * mbuf.
+		 */
+		if (unlikely(rxq->crc_len > 0) &&
+		    hns3_need_recalculate_crc(rxq, first_seg))
+			hns3_recalculate_crc(first_seg);
+
 		hns3_rxd_to_vlan_tci(rxq, first_seg, l234_info, &rxd);
 
 		/* All byte-related statistics do not include Ethernet FCS */
diff --git a/drivers/net/hns3/hns3_rxtx.h b/drivers/net/hns3/hns3_rxtx.h
index e975cd151a7e..0eb9796fe053 100644
--- a/drivers/net/hns3/hns3_rxtx.h
+++ b/drivers/net/hns3/hns3_rxtx.h
@@ -178,6 +178,8 @@
 		(HNS3_TXD_VLD_CMD | HNS3_TXD_FE_CMD | HNS3_TXD_DEFAULT_BDTYPE)
 #define HNS3_TXD_SEND_SIZE_SHIFT	16
 
+#define HNS3_KEEP_CRC_OK_MIN_PKT_LEN	60
+
 enum hns3_pkt_l2t_type {
 	HNS3_L2_TYPE_UNICAST,
 	HNS3_L2_TYPE_MULTICAST,
@@ -341,6 +343,7 @@ struct hns3_rx_queue {
 	 */
 	uint8_t pvid_sw_discard_en:1;
 	uint8_t ptype_en:1;          /* indicate if the ptype field enabled */
+	uint8_t keep_crc_fail_ptype:2;
 
 	uint64_t mbuf_initializer; /* value to init mbufs used with vector rx */
 	/* offset_table: used for vector, to solve execute re-order problem */
diff --git a/drivers/net/hns3/hns3_rxtx_vec.c b/drivers/net/hns3/hns3_rxtx_vec.c
index 9708ec614e02..bf37ce51b1ad 100644
--- a/drivers/net/hns3/hns3_rxtx_vec.c
+++ b/drivers/net/hns3/hns3_rxtx_vec.c
@@ -185,7 +185,8 @@ hns3_rx_check_vec_support(struct rte_eth_dev *dev)
 	struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	uint64_t offloads_mask = RTE_ETH_RX_OFFLOAD_TCP_LRO |
 				 RTE_ETH_RX_OFFLOAD_VLAN |
-				 RTE_ETH_RX_OFFLOAD_TIMESTAMP;
+				 RTE_ETH_RX_OFFLOAD_TIMESTAMP |
+				 RTE_ETH_RX_OFFLOAD_KEEP_CRC;
 
 	if (dev->data->scattered_rx)
 		return -ENOTSUP;
-- 
2.33.0


  parent reply	other threads:[~2024-07-19  9:11 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-06  1:10 [PATCH] net/hns3: fix Rx packet truncation when KEEP CRC enabled Jie Hai
2024-02-07 14:15 ` Ferruh Yigit
2024-02-20  3:58   ` Jie Hai
2024-02-23 13:53     ` Ferruh Yigit
2024-02-26  3:16       ` Jie Hai
2024-02-26 16:43         ` Ferruh Yigit
2024-02-28  2:27           ` huangdengdui
2024-02-28 13:07             ` Ferruh Yigit
2024-02-29  3:58               ` huangdengdui
2024-02-29  9:25                 ` Ferruh Yigit
2024-03-01  6:55                   ` huangdengdui
2024-03-01 11:10                     ` Ferruh Yigit
2024-03-08 11:36                       ` Jie Hai
2024-03-22  6:28                         ` Jie Hai
2024-06-03  1:38                       ` Jie Hai
2024-06-03  2:33                         ` Stephen Hemminger
2024-06-03  5:24                           ` Morten Brørup
2024-06-03  7:07                           ` Andrew Rybchenko
2024-07-18 11:48 ` [PATCH v2 0/3] bugfix about KEEP CRC offload Jie Hai
2024-07-18 11:48   ` [PATCH v2 1/3] ethdev: add description for " Jie Hai
2024-07-18 11:57     ` Morten Brørup
2024-07-18 11:48   ` [PATCH v2 2/3] net/hns3: fix packet length do not contain CRC data length Jie Hai
2024-07-18 11:48   ` [PATCH v2 3/3] net/hns3: fix Rx packet without CRC data Jie Hai
2024-07-18 12:35   ` [PATCH v2 0/3] bugfix about KEEP CRC offload lihuisong (C)
2024-07-19  9:04 ` [PATCH v3 " Jie Hai
2024-07-19  9:04   ` [PATCH v3 1/3] ethdev: add description for " Jie Hai
2024-09-05  6:33     ` Andrew Rybchenko
2024-07-19  9:04   ` [PATCH v3 2/3] net/hns3: fix packet length do not contain CRC data length Jie Hai
2024-07-19  9:04   ` Jie Hai [this message]
2024-07-19  9:49   ` [PATCH v3 0/3] bugfix about KEEP CRC offload fengchengwen
2024-08-09  9:21   ` Jie Hai
2024-09-05  2:53   ` Jie Hai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240719090415.1513301-4-haijie1@huawei.com \
    --to=haijie1@huawei.com \
    --cc=andrew.rybchenko@oktetlabs.ru \
    --cc=dev@dpdk.org \
    --cc=fengchengwen@huawei.com \
    --cc=ferruh.yigit@amd.com \
    --cc=huangdengdui@huawei.com \
    --cc=humin29@huawei.com \
    --cc=lihuisong@huawei.com \
    --cc=mb@smartsharesystems.com \
    --cc=stephen@networkplumber.org \
    --cc=xavier.huwei@huawei.com \
    --cc=yisen.zhuang@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).