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

Subsystems: infiniband subsystem, intel ethernet protocol driver for rdma, the rest

7 messages, 4 authors, 2022-01-04 · open the first message on its own page

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

From: <hidden>
Date: 2021-12-21 01:15:41

From: Zhu Yanjun <redacted>

Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
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 <redacted>
---
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;
+
+			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

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

From: Ismail, Mustafa <hidden>
Date: 2021-12-21 23:07:43

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

From: Zhu Yanjun <redacted>

Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
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 <redacted>
---
V2->V3: Move to the block of IB_QP_AV in the mask and IB_AH_GRH in
V2->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(+)

Reviewed-by: Mustafa Ismail <redacted>

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

From: Leon Romanovsky <leon@kernel.org>
Date: 2022-01-03 09:47:22

On Tue, Dec 21, 2021 at 12:39:13PM -0500, yanjun.zhu@linux.dev wrote:

From: Zhu Yanjun <redacted>

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.

quoted hunk

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 <redacted>
---
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.

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

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

From: <hidden>
Date: 2022-01-04 07:52:00

January 3, 2022 5:47 PM, "Leon Romanovsky" [off-list ref] wrote:

On Tue, Dec 21, 2021 at 12:39:13PM -0500, yanjun.zhu@linux.dev wrote:

quoted

From: Zhu Yanjun <redacted>

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.

quoted

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 <redacted>
---
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

quoted

+ 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

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

From: Leon Romanovsky <leon@kernel.org>
Date: 2022-01-04 08:44:43

On Tue, Jan 04, 2022 at 07:51:54AM +0000, yanjun.zhu@linux.dev wrote:

January 3, 2022 5:47 PM, "Leon Romanovsky" [off-list ref] wrote:

quoted

On Tue, Dec 21, 2021 at 12:39:13PM -0500, yanjun.zhu@linux.dev wrote:

quoted

From: Zhu Yanjun <redacted>

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.

quoted

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 <redacted>
---
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.

Yes and group all these patches in one series, please.

Thanks

Thanks.
Zhu Yanjun

quoted

Thanks

quoted

+ 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

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

From: Saleem, Shiraz <hidden>
Date: 2022-01-04 15:07:07

quoted hunk

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

From: Zhu Yanjun <redacted>

Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
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 <redacted>
---
V2->V3: Move to the block of IB_QP_AV in the mask and IB_AH_GRH in
V2->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;
+
+			udp_info->src_port = irdma_get_udp_sport(fl, lqp, rqp);

The flow label used in function is not right as udp_info->flow_label is only set a few lines below. Kindly fold this fix into your next revision.

 			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

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

From: Yanjun Zhu <hidden>
Date: 2022-01-04 15:35:06

在 2022/1/4 23:07, Saleem, Shiraz 写道:

quoted

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

From: Zhu Yanjun <redacted>

Based on the link https://www.spinics.net/lists/linux-rdma/msg73735.html,
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 <redacted>
---
V2->V3: Move to the block of IB_QP_AV in the mask and IB_AH_GRH in
V2->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;
+
+			udp_info->src_port = irdma_get_udp_sport(fl, lqp, rqp);

The flow label used in function is not right as udp_info->flow_label is only set a few lines below. Kindly fold this fix into your next revision.

Thanks.  " attr->ah_attr.grh.flow_label " should be OK.

Zhu Yanjun

quoted

  			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

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help