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

[Zhu Yanjun <yanjun.zhu@linux.dev>]

在 2024/8/23 16:23, zhenwei pi 写道:
>
>
> 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@linux.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.

Got it. Thanks a lot.

I am fine with your commit.

Best Regards,

Zhu Yanjun

>
>>>
>>> -- 
>>> zhenwei pi
>>>
>
-- 
Best Regards,
Yanjun.Zhu