On 2024/11/26 1:45, Stephen Hemminger wrote:
> On Fri, 19 Jul 2024 17:04:15 +0800
> Jie Hai <haij...@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
