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

[huangdengdui <huangdengdui@huawei.com>]

On 2024/11/26 1:45, Stephen Hemminger wrote:
> On Fri, 19 Jul 2024 17:04:15 +0800
> Jie Hai <haijie1@huawei.com> wrote:
> 
>> +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);
>> +}
>> +
> 
> There is a bug here. If there is no space at end of mbuf (tailroom) then
> rte_pktmbuf_append will return NULL. This would only happen if mbuf pool
> was sized such that there was space of a full size packet without CRC
> and then the code to add kept CRC was executed.
> 
> You need to check for NULL return and figure out some kind of unwind
> path such as making it a receive error and dropping the packet.

This situation has been described in the comments.
The hns3 driver requires that mbuf size must be at least 512B.[1]
When CRC needs to be recalculated, the packet length must be less than 60.
So the space of the mbuf must be enough to insert the CRC.
[1]:https://elixir.bootlin.com/dpdk/v23.11/source/drivers/net/hns3/hns3_rxtx.c#L1723