On 10/12/2022 9:18 AM, Wu, Wenjun1 wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@amd.com>
Sent: Wednesday, October 12, 2022 4:07 PM
To: Wu, Wenjun1 <wenjun1...@intel.com>; Viacheslav Galaktionov
<viacheslav.galaktio...@arknetworks.am>; Su, Simei <simei...@intel.com>
Cc: Denis Pryazhennikov <denis.pryazhenni...@arknetworks.am>;
dev@dpdk.org
Subject: Re: CRC offload from application's POV
On 10/12/2022 3:29 AM, Wu, Wenjun1 wrote:
-----Original Message-----
From: Ferruh Yigit <ferruh.yi...@amd.com>
Sent: Tuesday, October 11, 2022 9:46 PM
To: Viacheslav Galaktionov <viacheslav.galaktio...@arknetworks.am>;
Su, Simei <simei...@intel.com>; Wu, Wenjun1 <wenjun1...@intel.com>
Cc: Denis Pryazhennikov <denis.pryazhenni...@arknetworks.am>;
dev@dpdk.org
Subject: Re: CRC offload from application's POV
On 10/11/2022 12:54 PM, Viacheslav Galaktionov wrote:
On 10/11/22 15:36, Ferruh Yigit wrote:
On 10/11/2022 11:48 AM, Viacheslav Galaktionov wrote:
Hi!
We're looking to implement CRC offload in our driver and we're
having difficulties understanding what the feature changes from
the application's point of view. If we enable the KEEP_CRC
offload, then the NIC is supposed to preserve the CRC in the
packet, that much is clear. But we checked other drivers and it
seems common for PMDs to remove the CRC from the final mbufs.
Why is that?
We couldn't find any place where the CRC would be stored after
removal, so it looks like the application doesn't have access to
this piece of data. And if so, what's the point of having this
feature if the CRC is discarded either way?
We're probably missing something and would really appreciate any
help with this.
Hi Viacheslav,
As you said default behavior is to strip the CRC from packet, even
some devices doesn't support having CRC in the packet it is removed
by HW automatically. In this case application can't access to the CRC.
For the devices that has capability to keep CRC, KEEP_CRC offload
should enable having CRC as part of the packet. There is no special
field to store the CRC.
I'm asking because I'm seeing a common pattern in the code base: if
the hardware didn't remove the CRC, the driver does this itself.
Grepping the code for "crc_len" will show you what I mean. One of
the most apparent examples of this happening can be seen in
drivers/net/e1000/em_rxtx.c:
/*
* This is the last buffer of the received packet.
* If the CRC is not stripped by the hardware:
* - Subtract the CRC length from the total packet length.
* - If the last buffer only contains the whole CRC or a part
* of it, free the mbuf associated to the last buffer.
* If part of the CRC is also contained in the previous
* mbuf, subtract the length of that CRC part from the
* data length of the previous mbuf.
*/
I don't understand why this is necessary, and whether this is just a
particularity of this driver or how the feature is supposed to be
implemented everywhere. I haven't checked every driver, but it seems
like a lot of them do something similar to this.
That looks wrong to me too, cc'ed maintainers for comment.
That piece of code seems remaining from first upstream of the driver
(2012), it is before KEEP_CRC change, looks like it is missed.
CRC should be kept in the packet if driver supports it and user
requested KEEP_CRC offload.
But Rx stats should not include CRC, as it is common to use 'm->pkt_len'
for received packet stat, when CRC is in packet that should taken
into account for stats.
I agree with Ferruh. PMD will advertise KEEP_CRC offload in
rte_eth_dev_info->rx_offload_capa. If it is supported, CRC will always
keep in the packets. If user request KEEP_CRC offload in
rte_eth_rxmode, the driver will subtract CRC length from data length
to remove CRC from packet data, and user can get the CRC after the end of
the packet.
Hi Wenjun,
What we said is slightly different, it is OK to subtract the length from
*stats*,
but I think driver shouldn't remove the CRC from packet data.
You are right, the driver will never remove the CRC from packet data.
Just to be sure we are on same page, driver won't remove the CRC only
when KEEP_CRC is requested by user. Otherwise it will remove the CRC by
default.
