CRC offload from application's POV

[Viacheslav Galaktionov]

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.