public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
From: Jason Gunthorpe <jgg@ziepe.ca>
To: Eric Biggers <ebiggers@kernel.org>
Cc: Zhu Yanjun <yanjun.zhu@linux.dev>,
	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: Wed, 29 Jan 2025 15:43:44 -0400	[thread overview]
Message-ID: <20250129194344.GD2120662@ziepe.ca> (raw)
In-Reply-To: <20250129185115.GA2619178@google.com>

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.

Jason

  reply	other threads:[~2025-01-29 19:43 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 [this message]
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
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=20250129194344.GD2120662@ziepe.ca \
    --to=jgg@ziepe.ca \
    --cc=bmt@zurich.ibm.com \
    --cc=ebiggers@kernel.org \
    --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=yanjun.zhu@linux.dev \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox