Re: [PATCH 1/6] RDMA/rxe: handle ICRC correctly on big endian systems

[Zhu Yanjun <yanjun.zhu@linux.dev>]

On 29.01.25 20:43, Jason Gunthorpe wrote:
> On Wed, Jan 29, 2025 at 06:51:15PM +0000, Eric Biggers wrote:
>> Hi Jason,
>>
>> On Wed, Jan 29, 2025 at 02:30:09PM -0400, Jason Gunthorpe wrote:
>>> On Wed, Jan 29, 2025 at 10:44:39AM +0100, Zhu Yanjun wrote:
>>>> 在 2025/1/27 23:38, Eric Biggers 写道:
>>>>> From: Eric Biggers <ebiggers@google.com>
>>>>>
>>>>> The usual big endian convention of InfiniBand does not apply to the
>>>>> ICRC field, whose transmission is specified in terms of the CRC32
>>>>> polynomial coefficients.
>>>
>>> This patch is on to something but this is not a good explanation.
>>>
>>> The CRC32 in IB is stored as big endian and computed in big endian,
>>> the spec says so explicitly:
>>>
>>> 2) The CRC calculation is done in big endian byte order with the least
>>>     significant bit of the most significant byte being the first
>>>     bits in the CRC calculation.
>>
>> (2) just specifies the order in which the bits are passed to the CRC.  It says
>> nothing about how the CRC value is stored; that's in (4) instead.
> 
> It could be. The other parts of the spec are more definitive.
>   
>> The mention of "big endian" seems to refer to the bytes being passed
>> from first to last, which is the nearly universal convention.
> 
> The LFSR Figure 57 shows how the numerical representation the spec
> uses (ie the 0x9625B75A) maps to the LFSR, and it could be called big
> endian.
> 
> Regardless, table 29 makes this crystal clear, it shows how the
> numerical representation of 0x9625B75A is placed on the wire, it is
> big endian as all IBTA values are - byte 52 is 0x96, byte 55 is 0x5A.
> 
>> (I would not have used the term "big endian" here, as it's
>> confusing.)
> 
> It could be read as going byte-by-byte, or it could be referring to
> the layout of the numerical representation..
> 
>>> If you feed that to the CRC32 you should get 0x9625B75A in a CPU
>>> register u32.
>>>
>>> cpu_to_be32() will put it in the right order for on the wire.
>>
>> I think cpu_to_be32() would put it in the wrong order.  Refer to the following:
> 
> It is fully explicit in table 29, 0x9625B75A is stored in big
> endian format starting at byte 52.
> 
> It is easy to answer without guessing, Zhu should just run the sample
> packet through the new crc library code and determine what the u32
> value is. It is either 0x9625B75A or swab(0x9625B75A) and that will
> tell what the code should be.

Got it. I will run the sample packet through the new crc library code 
and determine what the u32 value is.

Zhu Yanjun

> 
> Jason