linux-kernel - Re: [PATCH 2/6] RDMA/rxe: consolidate code for calculating ICRC of packets

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0043edb8-8bba-4675-b0b6-fdb70fb2e091@linux.dev>
Date: Thu, 30 Jan 2025 08:24:59 +0100
From: Zhu Yanjun <yanjun.zhu@...ux.dev>
To: Eric Biggers <ebiggers@...nel.org>
Cc: linux-rdma@...r.kernel.org, Mustafa Ismail <mustafa.ismail@...el.com>,
 Tatyana Nikolova <tatyana.e.nikolova@...el.com>,
 Jason Gunthorpe <jgg@...pe.ca>, Leon Romanovsky <leon@...nel.org>,
 Zhu Yanjun <zyjzyj2000@...il.com>, Bernard Metzler <bmt@...ich.ibm.com>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/6] RDMA/rxe: consolidate code for calculating ICRC of
 packets

在 2025/1/30 3:15, Eric Biggers 写道:
> On Wed, Jan 29, 2025 at 07:11:35PM +0100, Zhu Yanjun wrote:
>> 在 2025/1/27 23:38, Eric Biggers 写道:
>>> From: Eric Biggers <ebiggers@...gle.com>
>>>
>>> Since rxe_icrc_hdr() is always immediately followed by updating the CRC
>>> with the packet's payload, just rename it to rxe_icrc() and make it
>>> include the payload in the CRC, so it now handles the entire packet.
>>>
>>> This is a refactor with no change in behavior.
>>
>> In this commit, currently the entire packet are checked while the header is
>> checked in the original source code.
>>
>> Now it can work between RXE <----> RXE.
>> I am not sure whether RXE <---> MLX can work or not.
>>
>> If it can work well, I am fine with this patch.
>>
>> Reviewed-by: Zhu Yanjun <yanjun.zhu@...ux.dev>
>>
> 
> Both the header and payload are checksummed both before and after this patch.
> Can you elaborate on why you think this patch changed any behavior?

 From the source code, it seems that only the header is checked. And RXE 
can connect to MLX RDMA NIC. That is, the CRC of the header can be 
verified both in RXE and MLX RDMA NIC.

Now in your commit, the header and payload are checked. Thus, the CRC 
value in RDMA header may be different from the CRC of the header(that 
CRC can be verified in RXE and MLX RDMA NIC). Finally the CRC of the 
header and payload will not be verified in MLX RDMA NIC?

IMO, after your patchset is applied, if RXE can connect to MLX RDMA NIC, 
I am fine with it.

In the function rxe_rcv as below,
"
...
     err = rxe_icrc_check(skb, pkt);
     if (unlikely(err))
         goto drop;
...
"
rxe_icrc_check is called to check the RDMA packet. In your commit, the 
icrc is changed. I am not sure whether this icrc can also be verified 
correctly in MLX RDMA NIC or not.

Because RXE can connect to MLX RDMA NIC, after your patchset is applied, 
hope that RXE can also connect to MLX RDMA NIC successfully.

Thanks,
Zhu Yanjun


> 
> - Eric