[PATCH v4] net/hns3: fix Rx packet without CRC data
Jie Hai
haijie1 at huawei.com
Fri Nov 29 02:36:43 CET 2024
Hi, stephen,
Kindly ping for review.
Thanks,
Jie Hai
On 2024/11/27 18:08, Jie Hai wrote:
> From: Dengdui Huang <huangdengdui at 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 at dpdk.org
>
> Signed-off-by: Dengdui Huang <huangdengdui at huawei.com>
> Acked-by: Huisong Li <lihuisong at huawei.com>
> Acked-by: Jie Hai <haijie1 at huawei.com>
> ---
> drivers/net/hns3/hns3_ethdev.c | 5 ++
> drivers/net/hns3/hns3_ethdev.h | 23 +++++
> drivers/net/hns3/hns3_rxtx.c | 121 +++++++++++++++++++++-----
> drivers/net/hns3/hns3_rxtx.h | 3 +
> drivers/net/hns3/hns3_rxtx_vec.c | 3 +-
> drivers/net/hns3/hns3_rxtx_vec_neon.h | 19 ----
> drivers/net/hns3/hns3_rxtx_vec_sve.c | 3 +-
> 7 files changed, 132 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 72d1c30a7b2e..b3bd439d0dd5 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 7824503bb89f..01d473fd2e66 100644
> --- a/drivers/net/hns3/hns3_ethdev.h
> +++ b/drivers/net/hns3/hns3_ethdev.h
> @@ -54,6 +54,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
> @@ -655,6 +659,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 03bbbc435fac..75fd4f55e73a 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>
> @@ -1768,8 +1769,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;
> @@ -1802,6 +1804,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;
> }
>
> @@ -1838,7 +1848,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;
> @@ -1959,6 +1969,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);
> @@ -2435,6 +2447,55 @@ 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;
> +}
> +
> +/*
> + * 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.
> + */
> +static_assert(HNS3_KEEP_CRC_OK_MIN_PKT_LEN < HNS3_MIN_BD_BUF_SIZE,
> + "buffer size too small to insert CRC");
> +
> +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);
> +
> + /*
> + * 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.
> + */
> + append_data = rte_pktmbuf_append(m, RTE_ETHER_CRC_LEN);
> +
> + /* CRC data is binary data and does not care about the byte order. */
> + memcpy(append_data, &crc, RTE_ETHER_CRC_LEN);
> +}
> +
> uint16_t
> hns3_recv_pkts_simple(void *rx_queue,
> struct rte_mbuf **rx_pkts,
> @@ -2505,8 +2566,7 @@ hns3_recv_pkts_simple(void *rx_queue,
> rxdp->rx.bd_base_info = 0;
>
> rxm->data_off = RTE_PKTMBUF_HEADROOM;
> - rxm->pkt_len = (uint16_t)(rte_le_to_cpu_16(rxd.rx.pkt_len)) -
> - rxq->crc_len;
> + rxm->pkt_len = (uint16_t)(rte_le_to_cpu_16(rxd.rx.pkt_len));
> rxm->data_len = rxm->pkt_len;
> rxm->port = rxq->port_id;
> rxm->hash.rss = rte_le_to_cpu_32(rxd.rx.rss_hash);
> @@ -2531,6 +2591,12 @@ 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);
> + rxm->pkt_len -= rxq->crc_len;
> + rxm->data_len -= rxq->crc_len;
> +
> hns3_rxd_to_vlan_tci(rxq, rxm, l234_info, &rxd);
>
> /* Increment bytes counter */
> @@ -2697,10 +2763,10 @@ hns3_recv_scattered_pkts(void *rx_queue,
>
> rxm->data_off = RTE_PKTMBUF_HEADROOM;
> rxm->data_len = rte_le_to_cpu_16(rxd.rx.size);
> + rxm->next = NULL;
>
> if (!(bd_base_info & BIT(HNS3_RXD_FE_B))) {
> last_seg = rxm;
> - rxm->next = NULL;
> continue;
> }
>
> @@ -2715,23 +2781,6 @@ hns3_recv_scattered_pkts(void *rx_queue,
> */
> first_seg->pkt_len = rte_le_to_cpu_16(rxd.rx.pkt_len);
>
> - /*
> - * This is the last buffer of the received packet. If the CRC
> - * is not stripped by the hardware:
> - * - Subtract the CRC length from the total packet length.
> - * - If the last buffer only contains the whole CRC or a part
> - * of it, free the mbuf associated to the last buffer. If part
> - * of the CRC is also contained in the previous mbuf, subtract
> - * the length of that CRC part from the data length of the
> - * previous mbuf.
> - */
> - rxm->next = NULL;
> - if (unlikely(rxq->crc_len > 0)) {
> - first_seg->pkt_len -= rxq->crc_len;
> - recalculate_data_len(first_seg, last_seg, rxm, rxq,
> - rxm->data_len);
> - }
> -
> first_seg->port = rxq->port_id;
> first_seg->hash.rss = rte_le_to_cpu_32(rxd.rx.rss_hash);
> first_seg->ol_flags |= RTE_MBUF_F_RX_RSS_HASH;
> @@ -2760,6 +2809,32 @@ 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;
> + /*
> + * This is the last buffer of the received packet. If the CRC
> + * is not stripped by the hardware:
> + * - Subtract the CRC length from the total packet length.
> + * - If the last buffer only contains the whole CRC or a part
> + * of it, free the mbuf associated to the last buffer. If part
> + * of the CRC is also contained in the previous mbuf, subtract
> + * the length of that CRC part from the data length of the
> + * previous mbuf.
> + *
> + * In addition, 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)) {
> + if (hns3_need_recalculate_crc(rxq, first_seg))
> + hns3_recalculate_crc(first_seg);
> + first_seg->pkt_len -= rxq->crc_len;
> + recalculate_data_len(first_seg, last_seg, rxm, rxq,
> + rxm->data_len);
> + }
>
> hns3_rxd_to_vlan_tci(rxq, first_seg, l234_info, &rxd);
>
> 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;
> diff --git a/drivers/net/hns3/hns3_rxtx_vec_neon.h b/drivers/net/hns3/hns3_rxtx_vec_neon.h
> index bbb5478015dd..86063a8def12 100644
> --- a/drivers/net/hns3/hns3_rxtx_vec_neon.h
> +++ b/drivers/net/hns3/hns3_rxtx_vec_neon.h
> @@ -150,14 +150,6 @@ hns3_recv_burst_vec(struct hns3_rx_queue *__restrict rxq,
> 8, 9, 10, 11, /* rx.rss_hash to rte_mbuf.hash.rss */
> };
>
> - uint16x8_t crc_adjust = {
> - 0, 0, /* ignore pkt_type field */
> - rxq->crc_len, /* sub crc on pkt_len */
> - 0, /* ignore high-16bits of pkt_len */
> - rxq->crc_len, /* sub crc on data_len */
> - 0, 0, 0, /* ignore non-length fields */
> - };
> -
> /* compile-time verifies the shuffle mask */
> RTE_BUILD_BUG_ON(offsetof(struct rte_mbuf, pkt_len) !=
> offsetof(struct rte_mbuf, rx_descriptor_fields1) + 4);
> @@ -173,7 +165,6 @@ hns3_recv_burst_vec(struct hns3_rx_queue *__restrict rxq,
> uint8x16_t pkt_mb1, pkt_mb2, pkt_mb3, pkt_mb4;
> uint64x2_t mbp1, mbp2;
> uint16x4_t bd_vld = {0};
> - uint16x8_t tmp;
> uint64_t stat;
>
> /* calc how many bd valid */
> @@ -227,16 +218,6 @@ hns3_recv_burst_vec(struct hns3_rx_queue *__restrict rxq,
> pkt_mb3 = vqtbl2q_u8(pkt_mbuf3, shuf_desc_fields_msk);
> pkt_mb4 = vqtbl2q_u8(pkt_mbuf4, shuf_desc_fields_msk);
>
> - /* 4 packets remove crc */
> - tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb1), crc_adjust);
> - pkt_mb1 = vreinterpretq_u8_u16(tmp);
> - tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb2), crc_adjust);
> - pkt_mb2 = vreinterpretq_u8_u16(tmp);
> - tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb3), crc_adjust);
> - pkt_mb3 = vreinterpretq_u8_u16(tmp);
> - tmp = vsubq_u16(vreinterpretq_u16_u8(pkt_mb4), crc_adjust);
> - pkt_mb4 = vreinterpretq_u8_u16(tmp);
> -
> /* save packet info to rx_pkts mbuf */
> vst1q_u8((void *)&sw_ring[pos + 0].mbuf->rx_descriptor_fields1,
> pkt_mb1);
> diff --git a/drivers/net/hns3/hns3_rxtx_vec_sve.c b/drivers/net/hns3/hns3_rxtx_vec_sve.c
> index 8aa4448558cf..67c87f570e8a 100644
> --- a/drivers/net/hns3/hns3_rxtx_vec_sve.c
> +++ b/drivers/net/hns3/hns3_rxtx_vec_sve.c
> @@ -36,8 +36,7 @@ hns3_desc_parse_field_sve(struct hns3_rx_queue *rxq,
> /* init rte_mbuf.rearm_data last 64-bit */
> rx_pkts[i]->ol_flags = RTE_MBUF_F_RX_RSS_HASH;
> rx_pkts[i]->hash.rss = rxdp[i].rx.rss_hash;
> - rx_pkts[i]->pkt_len = rte_le_to_cpu_16(rxdp[i].rx.pkt_len) -
> - rxq->crc_len;
> + rx_pkts[i]->pkt_len = rte_le_to_cpu_16(rxdp[i].rx.pkt_len);
> rx_pkts[i]->data_len = rx_pkts[i]->pkt_len;
>
> l234_info = rxdp[i].rx.l234_info;
More information about the dev
mailing list