Re: [PATCH v3 3/3] net/hns3: fix Rx packet without CRC data

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 19 Jul 2024 17:04:15 +0800
Jie Hai <haijie1@huawei.com> wrote:

> 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>

> +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);
> +}

Ok, but minor nits for future.
Trying to phase out use of rte_memcpy(), rte_memcpy() has no advantage except
in certain cases of unaligned variable length copies mostly on older non x86 distros.
And regular memcpy() enables compiler to do more checking.

And you don't need a void * cast there, in C there is implicit cast to void * there.

Applied to next-net