On Tue, 26 Nov 2024 10:40:27 +0800
huangdengdui <huangdeng...@huawei.com> wrote:
> 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
Ok, but maybe add an RTE_ASSERT() to be safe.
