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

[yanjun.zhu@linux.dev]

January 5, 2022 4:55 PM, "Leon Romanovsky" <leon@kernel.org> wrote:

> On Wed, Jan 05, 2022 at 04:27:38PM +0800, Zhu Yanjun wrote:
> 
>> On Wed, Jan 5, 2022 at 3:52 PM Leon Romanovsky <leon@kernel.org> wrote:
>> 
>> On Wed, Jan 05, 2022 at 05:12:36PM -0500, yanjun.zhu@linux.dev wrote:
>>> 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 | 6 ++++++
>>> 1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> index 0aa0d7e52773..42fa81b455de 100644
>>> --- a/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> +++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
>>> @@ -469,6 +469,12 @@ static int rxe_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>>> if (err)
>>> goto err1;
>>> 
>>> + if ((mask & IB_QP_AV) && (attr->ah_attr.ah_flags & IB_AH_GRH))
>> 
>> You are leaving src_port default and wired to same port as other QPs
>> without any randomization.
>> 
>> Hi,
>> 
>> I do not get you. Why do you think I am leaving src_pport default?
> 
> Because in original code, you randomized src_port without any relation
> to mask flags.
> 
> qp->src_port = RXE_ROCE_V2_SPORT +
> (hash_32_generic(qp_num(qp), 14) & 0x3fff);
> 
> After patch #5, if user doesn't pass "proper" mask, you will leave
> qp->src_port to be equal to RXE_ROCE_V2_SPORT, which is different from
> the current behaviour.

About the "proper" mask, please check this link https://patchwork.kernel.org/project/linux-rdma/patch/20211218204438.1345160-1-yanjun.zhu@linux.dev/

"the udp_sport is only set when address vector and dest qpn (IB_QP_AV and IB_QP_DEST_QPN) is provided."

Zhu Yanjun
> 
> Thanks
> 
>> Thanks.
>> 
>> Zhu Yanjun
>> 
>> Thanks
>> 
>>> + qp->src_port = rdma_get_udp_sport(attr->ah_attr.grh.flow_label,
>>> + qp->ibqp.qp_num,
>>> + qp->attr.dest_qp_num);
>>> +
>>> +
>>> return 0;
>>> 
>>> err1:
>>> --
>>> 2.27.0
>>>