Re: [PATCH 4/4] RDMA/rxe: Use the standard method to produce udp source port

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

在 2022/1/5 8:33, Yanjun Zhu 写道:
> 在 2022/1/5 1:17, Saleem, Shiraz 写道:
>>> Subject: [PATCH 4/4] RDMA/rxe: Use the standard method to produce 
>>> udp source
>>> port
>>>
>>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>>>
>>> Use the standard method to produce udp source port.
>>>
>>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>>> ---
>>>   drivers/infiniband/sw/rxe/rxe_verbs.c | 10 ++++++++++
>>>   1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> index 0aa0d7e52773..f30d98ad13cd 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> @@ -469,6 +469,16 @@ static int rxe_modify_qp(struct ib_qp *ibqp, 
>>> struct
>>> ib_qp_attr *attr,
>>>       if (err)
>>>           goto err1;
>>>
>>> +    if (mask & IB_QP_AV) {
>>> +        if (attr->ah_attr.ah_flags & IB_AH_GRH) {
>>> +            u32 fl = attr->ah_attr.grh.flow_label;
>>> +            u32 lqp = qp->ibqp.qp_num;
>>> +            u32 rqp = qp->attr.dest_qp_num;
>>> +
>> Isn't the randomization for src_port done in rxe_qp_init_req 
>> redundant then?
>>
>> https://elixir.bootlin.com/linux/v5.16-rc8/source/drivers/infiniband/sw/rxe/rxe_qp.c#L220 
>>
>>
>> Can we remove it?

Based on the discussion with Leon Romanovsky, when in-subnet 
communications, GRH is optional.

So It is possible that GRH is not set. Without the randomization for 
src_port done in rxe_qp_init_req, udp source

port will be 0xC000 in that case.

So it is better to keep this randomization for src_port done in 
rxe_qp_init_req.

Zhu Yanjun

>
> Yes. We can remove it.
> Because this "randomization for src_port done in  rxe_qp_init_req" is 
> replaced by rdma_get_udp_sport in rxe_modify_qp, I do not remove it.
>
> I will remove it in the latest commits soon.
>
> Zhu Yanjun
>
>>
>>> +            qp->src_port = rdma_get_udp_sport(fl, lqp, rqp);
>>> +        }
>>> +    }
>>> +
>>>       return 0;
>>>
>>>   err1:
>>> -- 
>>> 2.27.0
>>
>