From: Zhu Yanjun <yanjun.zhu@linux.dev>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: Eric Biggers <ebiggers@kernel.org>,
linux-rdma@vger.kernel.org,
Mustafa Ismail <mustafa.ismail@intel.com>,
Tatyana Nikolova <tatyana.e.nikolova@intel.com>,
Leon Romanovsky <leon@kernel.org>,
Zhu Yanjun <zyjzyj2000@gmail.com>,
Bernard Metzler <bmt@zurich.ibm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/6] RDMA/rxe: handle ICRC correctly on big endian systems
Date: Thu, 30 Jan 2025 08:27:44 +0100 [thread overview]
Message-ID: <d3feb12a-aca9-40b8-8a51-cc9fedce45e5@linux.dev> (raw)
In-Reply-To: <20250129183009.GC2120662@ziepe.ca>
在 2025/1/29 19:30, Jason Gunthorpe 写道:
> 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
> 31 significant bit of the most significant byte being the first
> bits in the 32 CRC calculation.
>
> In this context saying it is not "big endian" doesn't seem to be quite
> right..
>
> The spec gives a sample data packet (in offset/value pairs):
>
> 0 0xF0 15 0xB3 30 0x7A 45 0x8B
> 1 0x12 16 0x00 31 0x05 46 0xC0
> 2 0x37 17 0x0D 32 0x00 47 0x69
> 3 0x5C 18 0xEC 33 0x00 48 0x0E
> 4 0x00 19 0x2A 34 0x00 49 0xD4
> 5 0x0E 20 0x01 35 0x0E 50 0x00
> 6 0x17 21 0x71 36 0xBB 51 0x00
> 7 0xD2 22 0x0A 37 0x88
> 8 0x0A 23 0x1C 38 0x4D
> 9 0x20 24 0x01 39 0x85
> 10 0x24 25 0x5D 40 0xFD
> 11 0x87 26 0x40 41 0x5C
> 12 0xFF 27 0x02 42 0xFB
> 13 0x87 28 0x38 43 0xA4
> 14 0xB1 29 0xF2 44 0x72
>
> 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.
>
> Since rxe doesn't have any cpu_to_be32() on this path, I'm guessing
> the Linux CRC32 implementations gives a u32 with the
> value = 0x5AB72596 ie swapped.
>
> Probably the issue here is that the Linux CRC32 and the IBTA CRC32 are
> using a different mapping of LFSR bits. I love CRCs. So many different
> ways to implement the same thing.
>
> Thus, I guess, the code should really read:
> linux_crc32 = swab32(be32_to_cpu(val));
>
> Which is a NOP on x86 and requires a swap on BE.
>
> Zhu, can you check it for Eric? (this is page 229 in the spec).
Got it. In my IBTA spec, I can find what you mentioned in the IBTA spec.
Your explanation is in details and very nice.
Thanks a lot.
Zhu Yanjun
>
> I assume the Linux CRC32 always gives the same CPU value regardless of
> LE or BE?
>
> Jason
next prev parent reply other threads:[~2025-01-30 7:27 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-27 22:38 [PATCH 0/6] RDMA: switch to using CRC32 library functions Eric Biggers
2025-01-27 22:38 ` [PATCH 1/6] RDMA/rxe: handle ICRC correctly on big endian systems Eric Biggers
2025-01-29 9:44 ` Zhu Yanjun
2025-01-29 18:30 ` Jason Gunthorpe
2025-01-29 18:51 ` Eric Biggers
2025-01-29 19:43 ` Jason Gunthorpe
2025-01-29 20:25 ` Eric Biggers
2025-01-29 21:16 ` Jason Gunthorpe
2025-01-29 22:21 ` Eric Biggers
2025-01-30 1:29 ` Jason Gunthorpe
2025-01-30 2:04 ` Eric Biggers
2025-01-30 13:52 ` Jason Gunthorpe
2025-01-30 9:17 ` Zhu Yanjun
2025-01-30 7:27 ` Zhu Yanjun [this message]
2025-01-29 18:27 ` Zhu Yanjun
2025-01-29 19:02 ` Eric Biggers
2025-01-27 22:38 ` [PATCH 2/6] RDMA/rxe: consolidate code for calculating ICRC of packets Eric Biggers
2025-01-29 18:11 ` Zhu Yanjun
2025-01-30 2:15 ` Eric Biggers
2025-01-30 7:24 ` Zhu Yanjun
2025-01-31 2:42 ` Eric Biggers
2025-01-27 22:38 ` [PATCH 3/6] RDMA/rxe: switch to using the crc32 library Eric Biggers
2025-01-29 18:30 ` Zhu Yanjun
2025-01-27 22:38 ` [PATCH 4/6] RDMA/irdma: switch to using the crc32c library Eric Biggers
2025-01-27 22:38 ` [PATCH 5/6] RDMA/siw: fix type of CRC field Eric Biggers
2025-01-31 12:24 ` Bernard Metzler
2025-01-27 22:38 ` [PATCH 6/6] RDMA/siw: switch to using the crc32c library Eric Biggers
2025-01-31 14:17 ` Bernard Metzler
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d3feb12a-aca9-40b8-8a51-cc9fedce45e5@linux.dev \
--to=yanjun.zhu@linux.dev \
--cc=bmt@zurich.ibm.com \
--cc=ebiggers@kernel.org \
--cc=jgg@ziepe.ca \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rdma@vger.kernel.org \
--cc=mustafa.ismail@intel.com \
--cc=tatyana.e.nikolova@intel.com \
--cc=zyjzyj2000@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.