> From: Dengdui Huang [mailto:huangdeng...@huawei.com]
> Sent: Friday, 24 January 2025 11.00
> 
> After discussion[1], the drivers do not include the CRC in the packet
> length calculation. This will cause users to be confused about whether
> the mbuf contains CRC data. This patch adds a packet offload Rx flag,
> indicating that CRC data exists at the end of the mbuf chain.
> 
> [1] https://inbox.dpdk.org/dev/20240206011030.2007689-1-
> haij...@huawei.com/
> 
> Signed-off-by: Dengdui Huang <huangdeng...@huawei.com>
> ---

Mbufs with F_RX_KEEP_CRC requires much more than this.

If the packet length omits the 4 byte Ethernet CRC, and the last segment only 
holds the CRC, rte_mbuf_check() will fail and cause panic in 
rte_mbuf_sanity_check().
And many functions working on segments, such as rte_pktmbuf_copy(), 
linearize(), etc. need to be patched to check for F_RX_KEEP_CRC when working on 
the packet. This will degrade performance, and we are also talking about 
frequently used dataplane functions.

Furthermore, if we really need to support KEEP_CRC with segmented packets, we 
need to add test cases with the CRC partially in the last segment, and with 
only the CRC in the last segment, for functions and libraries supporting 
segmented packets. Regardless if the packet length includes the 4 bytes CRC or 
not.

KEEP_CRC looks exotic to me, and am worried that full support for KEEP_CRC will 
impact performance and would be essentially untested in a bunch of libraries. I 
don't want exotic features impacting the performance of frequently used 
dataplane functions.
Can you please remind me of the use cases for KEEP_CRC?

Perhaps support for KEEP_CRC could be a build time option (default omitted, for 
performance and test coverage reasons)?

Alternatively, support for KEEP_CRC could be limited to non-segmented packets?

Or, how about a completely different approach:
Drivers supporting KEEP_CRC can strip the 4 byte CRC (like stripping a VLAN 
tag) and store it in an mbuf dynfield.

Reply via email to