linux-kernel - Re: Re: [PATCH 1/3] RDMA/rxe: Use sizeof instead of hard code number

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: <10810dec-37b5-463f-bc7b-f88d2e4385d9@bytedance.com>
Date: Fri, 23 Aug 2024 16:23:03 +0800
From: zhenwei pi <pizhenwei@...edance.com>
To: Zhu Yanjun <yanjun.zhu@...ux.dev>
Cc: linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
 Jason Gunthorpe <jgg@...dia.com>, zyjzyj2000@...il.com, leonro@...dia.com
Subject: Re: Re: [PATCH 1/3] RDMA/rxe: Use sizeof instead of hard code number



On 8/23/24 13:56, Zhu Yanjun wrote:
> 
> 在 2024/8/23 10:30, zhenwei pi 写道:
>>
>> On 8/22/24 20:36, Jason Gunthorpe wrote:
>> > On Thu, Aug 22, 2024 at 07:59:32PM +0800, Zhu Yanjun wrote:
>> >> 在 2024/8/22 14:52, zhenwei pi 写道:
>> >>> Use 'sizeof(union rdma_network_hdr)' instead of hard code GRH length
>> >>> for GSI and UD.
>> >>>
>> >>> Signed-off-by: zhenwei pi
>> >>> ---
>> >>> drivers/infiniband/sw/rxe/rxe_resp.c | 2 +-
>> >>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>>
>> >>> diff --git a/drivers/infiniband/sw/rxe/rxe_resp.c b/drivers/ 
>> infiniband/sw/rxe/rxe_resp.c
>> >>> index 6596a85723c9..bf8f4bc8c5c8 100644
>> >>> --- a/drivers/infiniband/sw/rxe/rxe_resp.c
>> >>> +++ b/drivers/infiniband/sw/rxe/rxe_resp.c
>> >>> @@ -351,7 +351,7 @@ static enum resp_states 
>> rxe_resp_check_length(struct rxe_qp *qp,
>> >>> for (i = 0; i < qp->resp.wqe->dma.num_sge; i++)
>> >>> recv_buffer_len += qp->resp.wqe->dma.sge[i].length;
>> >>> - if (payload + 40 > recv_buffer_len) {
>> >>> + if (payload + sizeof(union rdma_network_hdr) > recv_buffer_len) {
>> >>
>> >> The definition of union rdma_network_hdr is as below
>> >>
>> >> 797 union rdma_network_hdr {
>> >> 798 struct ib_grh ibgrh;
>> >> 799 struct {
>> >> 800 /* The IB spec states that if it's IPv4, the header
>> >> 801 * is located in the last 20 bytes of the header.
>> >> 802 */
>> >> 803 u8 reserved[20];
>> >> 804 struct iphdr roce4grh;
>> >> 805 };
>> >> 806 };
>> >>
>> >> The length is 40 byte.
>> >
>> > This looks like the right struct to me if this is talking about the
>> > special 40 byte blob that is placed in front of UD verbs completions.
>> >
>> > Jason
>>
>> Yes, this is the front part(40 bytes) of UD/GSI verbs completion.
>>
> When running, you can print the value of the front part (40 bytes) of 
> UD/GSI to confirm that these 40 bytes are the union rdma_network_hdr.
> 
> If these 40 bytes are the union rdma_network_hdr,
> 
> Reviewed-by: Zhu Yanjun <yanjun.zhu@...ux.dev>
> 
> Best Regards,
> 
> Zhu Yanjun
> 

Hi Yanjun,

I test on mlx-cx5 by command: ibv_ud_pingpong -d mlx5_2 -g 3 -r 1 -n 1

Dump the front 64 bytes:
00000000000000000000000000000000000000004502043436f340000111448c
0a0b73120a0b73107b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b7b

Byte[0, 20) (rdma_network_hdr::reserved[20]) are filled by zero, 
byte[20,40) (rdma_network_hdr::roce4grh) are filled by IPv4 header.

>>
>> -- 
>> zhenwei pi
>>