Re: [PATCHv3 1/1] RDMA/irdma: Make the source udp port vary

[yanjun.zhu@linux.dev]

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

> On Tue, Dec 21, 2021 at 12:39:13PM -0500, yanjun.zhu@linux.dev wrote:
> 
>> From: Zhu Yanjun <yanjun.zhu@linux.dev>
>> 
>> Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
> 
> Please use lore.kernel.org links. They have all chances to outlive spinics.
> 
>> get the source udp port number for a QP based on the grh.flow_label or
>> lqpn/rqrpn. This provides a better spread of traffic across NIC RX queues.
>> The method in the commit 2b880b2e5e03 ("RDMA/mlx5: Define RoCEv2 udp
>> source port when set path") is a standard way. So it is also adopted in
>> this commit.
>> 
>> Signed-off-by: Zhu Yanjun <yanjun.zhu@linux.dev>
>> ---
>> V2->V3: Move to the block of IB_QP_AV in the mask and IB_AH_GRH in ah_flags
>> V1->V2: Adopt a standard method to get udp source port.
>> ---
>> drivers/infiniband/hw/irdma/verbs.c | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>> 
>> diff --git a/drivers/infiniband/hw/irdma/verbs.c b/drivers/infiniband/hw/irdma/verbs.c
>> index 8cd5f9261692..31039b295206 100644
>> --- a/drivers/infiniband/hw/irdma/verbs.c
>> +++ b/drivers/infiniband/hw/irdma/verbs.c
>> @@ -1094,6 +1094,15 @@ static int irdma_query_pkey(struct ib_device *ibdev, u32 port, u16 index,
>> return 0;
>> }
>> 
>> +
>> +static u16 irdma_get_udp_sport(u32 fl, u32 lqpn, u32 rqpn)
>> +{
>> + if (!fl)
>> + fl = rdma_calc_flow_label(lqpn, rqpn);
>> +
>> + return rdma_flow_label_to_udp_sport(fl);
>> +}
>> +
>> /**
>> * irdma_modify_qp_roce - modify qp request
>> * @ibqp: qp's pointer for modify
>> @@ -1167,6 +1176,11 @@ int irdma_modify_qp_roce(struct ib_qp *ibqp, struct ib_qp_attr *attr,
>> 
>> memset(&iwqp->roce_ah, 0, sizeof(iwqp->roce_ah));
>> if (attr->ah_attr.ah_flags & IB_AH_GRH) {
>> + u32 fl = udp_info->flow_label;
>> + u32 lqp = ibqp->qp_num;
>> + u32 rqp = roce_info->dest_qp;
>> +
> 
> I don't see too much value in these extra variables and extra function
> that is the same as get_udp_sport() from hns.
> 
> It is worth to add new function to ib_verbs.h and reuse in both drivers.

Do you mean the following function should be added into ib_verbs.h?

"
static inline u16 rdma_get_udp_sport(u32 fl, u32 lqpn, u32 rqpn)
{
        if (!fl)
                fl = rdma_calc_flow_label(lqpn, rqpn);

        return rdma_flow_label_to_udp_sport(fl);
}
"
Then in hns, rxe and irdma, this function is called to get udp source port?
If so, I will send new patches.

Thanks.
Zhu Yanjun

> 
> Thanks
> 
>> + udp_info->src_port = irdma_get_udp_sport(fl, lqp, rqp);
>> udp_info->ttl = attr->ah_attr.grh.hop_limit;
>> udp_info->flow_label = attr->ah_attr.grh.flow_label;
>> udp_info->tos = attr->ah_attr.grh.traffic_class;
>> --
>> 2.27.0