Re: [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads

From: Doug Ledford <dledford@redhat.com>
To: Tom Talpey <tom@talpey.com>, Leon Romanovsky <leon@kernel.org>
Cc: Bart Van Assche <bvanassche@acm.org>,
	Steve Wise <larrystevenwise@gmail.com>,
	linux-rdma@vger.kernel.org, 3100102071@zju.edu.cn
Subject: Re: [PATCH 2/2] rxe: correctly calculate iCRC for unaligned payloads
Date: Thu, 12 Dec 2019 17:06:32 -0500	[thread overview]
Message-ID: <55d97fa9d8efd296ecc308a67e043db3714cfcf3.camel@redhat.com> (raw)
In-Reply-To: <5f98ecdc-44d1-e185-19e9-7710c2c7c991@talpey.com>

[-- Attachment #1: Type: text/plain, Size: 1968 bytes --]

On Wed, 2019-12-11 at 09:42 -0500, Tom Talpey wrote:
> On 12/10/2019 11:24 PM, Doug Ledford wrote:
> > On Tue, 2019-12-10 at 08:54 +0200, Leon Romanovsky wrote:
> > > On Mon, Dec 09, 2019 at 02:07:06PM -0500, Doug Ledford wrote:
> > > > 
> > > Do you find this implementation needed? I see RXE as a development
> > > platform and in my view it is unlikely that someone will run RXE
> > > in
> > > production with mixture of different kernel versions, which
> > > requires
> > > such compatibility fallback.
> > 
> > It's not a requirement, that's why I took the patches as they
> > were.  It
> > would just be a "nice to have".
> 
> The counterargument to this is that it only extends the protocol bug
> into the future, and for one single RoCE implementation.

It just allows buggy implementations to talk to newer soft-RoCE
(although not to hardware).

>  No hardware
> implementation will do this, as it violates the protocol.

Right.

>  And, it
> potentially opens a silent data corruption, by accepting packets which
> don't actually pass the checksum.

This, I highly doubt.  For packets without padding, it would be the
same.  For packets with padding, it would only allow packets where the
data bytes had a correct CRC, so it's not like it just allows anything
to come through.  And it would only allow it if the flag was set, it's
not like we would allow two different CRCs on every packet with padding,
it's either on or off, and the check still covers all data bytes.  I
find it highly unlikely that this would introduce any sort of data
consistency problems for the specific case of old soft-RoCE talking to
new soft-RoCE.

> Personally, I'd say it "nice to avoid", i.e. don't apply such a patch.

No one has submitted a patch, so we seem to be good regardless ;-)

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]