IB/hfi1,core: Driver updates for 12/22/2017

PATCH In review Dennis Dalessandro 4 participants 11 messages Dec 22, 2017 – Jan 3, 2018 7 messages not in the archive

Subsystems (2)

InfiniBand HFI1 driver

lore

Share & tools

lore

Dennis Dalessandro

author

2017-12-22 16:45 UTC #0 lore raw

cover IB/hfi1,core: Driver updates for 12/22/2017

Hi Jason and Doug,

This includes a patch that Jason requsted, keeping the extended LID a consistent size in the kernel and changing it at the UAPI boundary. There is another 16B cleanup/improvement type patch from Don and an enhancement to our qp stats. I know this stat is ridiculously complicated to read and it's doubtful I'll take any other patches that make this worse, we are going to have to come up with something else. Plan to talk to Leon about adding this kind of stuff to the rdma tool.

Don Hiatt (2):
IB/core: Use rdma_cap_opa_mad to check for OPA IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit

Michael J. Ruhl (1):
IB/hfi1: Add RQ/SRQ information to QP stats

 drivers/infiniband/core/user_mad.c |    3 +--
 drivers/infiniband/hw/hfi1/hfi.h   |    2 +-
 drivers/infiniband/hw/hfi1/mad.c   |    6 +-----
 drivers/infiniband/hw/hfi1/qp.c    |   10 ++++++++--
 4 files changed, 11 insertions(+), 10 deletions(-)

--
-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171222164036.29731.43475.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org

Dennis Dalessandro

author

2017-12-22 16:45 UTC #2 lore raw

PATCH 1/3 IB/hfi1: Add RQ/SRQ information to QP stats

From: Michael J. Ruhl <[email protected]>

When debugging issues with RC QPs, it is useful to know if a QP has an associated RQ or SRQ, the size of the RQ, and any RNR timeout values.

Add the necessary information to the QP stats output.

Reviewed-by Mike Marciniszyn <[email protected]>

Signed-off-by Michael J. Ruhl <[email protected]>

Signed-off-by Dennis Dalessandro <[email protected]>

---
 drivers/infiniband/hw/hfi1/qp.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

drivers/infiniband/hw/hfi1/qp.c +8 -2

index 4b01ccd..5507910 100644
@@ -556,6 +556,8 @@ void qp_iter_print(struct seq_file *s, struct rvt_qp_iter *iter)
 	struct sdma_engine *sde;
 	struct send_context *send_context;
 	struct rvt_ack_entry *e = NULL;
+	struct rvt_srq *srq = qp->ibqp.srq ?
+		ibsrq_to_rvtsrq(qp->ibqp.srq) : NULL;
 
 	sde = qp_to_sdma_engine(qp, priv->s_sc);
 	wqe = rvt_get_swqe_ptr(qp, qp->s_last);

@@ -563,7 +565,7 @@ void qp_iter_print(struct seq_file *s, struct rvt_qp_iter *iter)
 	if (qp->s_ack_queue)
 		e = &qp->s_ack_queue[qp->s_tail_ack_queue];
 	seq_printf(s,
-		   "N %d %s QP %x R %u %s %u %u %u f=%x %u %u %u %u %u %u SPSN %x %x %x %x %x RPSN %x S(%u %u %u %u %u %u %u) R(%u %u %u) RQP %x LID %x SL %u MTU %u %u %u %u %u SDE %p,%u SC %p,%u SCQ %u %u PID %d OS %x %x E %x %x %x\n",
+		   "N %d %s QP %x R %u %s %u %u %u f=%x %u %u %u %u %u %u SPSN %x %x %x %x %x RPSN %x S(%u %u %u %u %u %u %u) R(%u %u %u) RQP %x LID %x SL %u MTU %u %u %u %u %u SDE %p,%u SC %p,%u SCQ %u %u PID %d OS %x %x E %x %x %x RNR %d %s %d\n",
 		   iter->n,
 		   qp_idle(qp) ? "I" : "B",
 		   qp->ibqp.qp_num,

@@ -610,7 +612,11 @@ void qp_iter_print(struct seq_file *s, struct rvt_qp_iter *iter)
 		   /* ack queue information */
 		   e ? e->opcode : 0,
 		   e ? e->psn : 0,
-		   e ? e->lpsn : 0);
+		   e ? e->lpsn : 0,
+		   qp->r_min_rnr_timer,
+		   srq ? "SRQ" : "RQ",
+		   srq ? srq->rq.size : qp->r_rq.size
+		);
 }
 
 void *qp_priv_alloc(struct rvt_dev_info *rdi, struct rvt_qp *qp)

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171222164540.29731.68770.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org

Jason Gunthorpe

2018-01-03 21:22 UTC #4 lore raw

Re: [PATCH for-next 1/3] IB/hfi1: Add RQ/SRQ information to QP stats

Quoted 6 lines · Dennis Dalessandro

From: Michael J. Ruhl <[email protected]>
When debugging issues with RC QPs, it is useful to know if a QP has an associated RQ or SRQ, the size of the RQ, and any RNR timeout values.

Quoted 7 lines

Add the necessary information to the QP stats output.
Reviewed-by Mike Marciniszyn <[email protected]>
Signed-off-by Michael J. Ruhl <[email protected]>
Signed-off-by Dennis Dalessandro <[email protected]>
 drivers/infiniband/hw/hfi1/qp.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

Okay, applied to for-next, lets use netlink in future.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Dennis Dalessandro

author

2017-12-22 16:45 UTC #5 lore raw

PATCH 2/3 IB/core: Use rdma_cap_opa_mad to check for OPA

In reply to a message not in the archive

From: Don Hiatt <[email protected]>

Use rdma_cap_opa_mad() to check for OPA to promote code reuse.

Signed-off-by Don Hiatt <[email protected]>

Signed-off-by Dennis Dalessandro <[email protected]>

---
 drivers/infiniband/core/user_mad.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

drivers/infiniband/core/user_mad.c +1 -2

index 4b64dd0..33d5e8e 100644
@@ -233,8 +233,7 @@ static void recv_handler(struct ib_mad_agent *agent,
 	 * On OPA devices it is okay to lose the upper 16 bits of LID as this
 	 * information is obtained elsewhere. Mask off the upper 16 bits.
 	 */
-	if (agent->device->port_immutable[agent->port_num].core_cap_flags &
-	    RDMA_CORE_PORT_INTEL_OPA)
+	if (rdma_cap_opa_mad(agent->device, agent->port_num))
 		packet->mad.hdr.lid = ib_lid_be16(0xFFFF &
 						  mad_recv_wc->wc->slid);
 	else

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Dennis Dalessandro

author

2017-12-22 16:46 UTC #6 lore raw

PATCH 3/3 IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit

In reply to a message not in the archive

From: Don Hiatt <[email protected]>

Change the slid arg to ingress_pkey_table_fail() to a full 32Bits and do not convert to 16Bits in caller. This is so we can keep everything 32bit in the kernel and only change to 16bit at the uapi boundary.

Signed-off-by Don Hiatt <[email protected]>

Signed-off-by Dennis Dalessandro <[email protected]>

---
 drivers/infiniband/hw/hfi1/hfi.h |    2 +-
 drivers/infiniband/hw/hfi1/mad.c |    6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)

drivers/infiniband/hw/hfi1/hfi.h +1 -1

index 869c2bf..dc5b133 100644
@@ -1627,7 +1627,7 @@ static int ingress_pkey_table_search(struct hfi1_pportdata *ppd, u16 pkey)
  * the 'error info' for this failure.
  */
 static void ingress_pkey_table_fail(struct hfi1_pportdata *ppd, u16 pkey,
-				    u16 slid)
+				    u32 slid)
 {
 	struct hfi1_devdata *dd = ppd->dd;

drivers/infiniband/hw/hfi1/mad.c +1 -5

index cf8dba3..34547a4 100644
@@ -4348,11 +4348,7 @@ static int opa_local_smp_check(struct hfi1_ibport *ibp,
 	 */
 	if (pkey == LIM_MGMT_P_KEY || pkey == FULL_MGMT_P_KEY)
 		return 0;
-	/*
-	 * On OPA devices it is okay to lose the upper 16 bits of LID as this
-	 * information is obtained elsewhere. Mask off the upper 16 bits.
-	 */
-	ingress_pkey_table_fail(ppd, pkey, ib_lid_cpu16(0xFFFF & in_wc->slid));
+	ingress_pkey_table_fail(ppd, pkey, in_wc->slid);
 	return 1;
 }

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171222164557.29731.16649.stgit-9QXIwq+3FY+1XWohqUldA0EOCMrvLtNR@public.gmane.org

Jason Gunthorpe

2017-12-22 16:56 UTC #8 lore raw

Re: [PATCH for-next 3/3] IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit

Quoted 30 lines · Dennis Dalessandro

From: Don Hiatt <[email protected]>
Change the slid arg to ingress_pkey_table_fail() to a full 32Bits and do not convert to 16Bits in caller. This is so we can keep everything 32bit in the kernel and only change to 16bit at the uapi boundary.
Signed-off-by Don Hiatt <[email protected]>
Signed-off-by Dennis Dalessandro <[email protected]>
 drivers/infiniband/hw/hfi1/hfi.h |    2 +-
 drivers/infiniband/hw/hfi1/mad.c |    6 +-----
 2 files changed, 2 insertions(+), 6 deletions(-)
drivers/infiniband/hw/hfi1/hfi.h +2 -1
index 869c2bf..dc5b133 100644
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -1627,7 +1627,7 @@ static int ingress_pkey_table_search(struct hfi1_pportdata *ppd, u16 pkey)
  * the 'error info' for this failure.
  */
 static void ingress_pkey_table_fail(struct hfi1_pportdata *ppd, u16 pkey,
-				    u16 slid)
+				    u32 slid)
 {
 	struct hfi1_devdata *dd = ppd->dd;
 
drivers/infiniband/hw/hfi1/mad.c +1 -0
index cf8dba3..34547a4 100644
+++ b/drivers/infiniband/hw/hfi1/mad.c
@@ -4348,11 +4348,7 @@ static int opa_local_smp_check(struct hfi1_ibport *ibp,
 	 */

drivers/infiniband/hw/hfi1/mad.c static int opa_local_smp_check(struct hfi1_ibport *ibp, · line 4353

Quoted 10 lines

drivers/infiniband/hw/hfi1/mad.c +1 -5

 	if (pkey == LIM_MGMT_P_KEY || pkey == FULL_MGMT_P_KEY)
 		return 0;
-	/*
-	 * On OPA devices it is okay to lose the upper 16 bits of LID as this
-	 * information is obtained elsewhere. Mask off the upper 16 bits.
-	 */
-	ingress_pkey_table_fail(ppd, pkey, ib_lid_cpu16(0xFFFF & in_wc->slid));
+	ingress_pkey_table_fail(ppd, pkey, in_wc->slid);
 	return 1;
 }

I'm confused by this patch.

It seems to totally revert the -stable patch 4988be5813ff2afdc0d8bfa315ef34a577d3efbf, with the exception that ingress_pkey_table_fail now accepts a u32.

Is this saying the correct version of 4988be5813ff2afdc0d8bfa315ef34a577d3efbf was to just change that one type??

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171222165600.GE30884-uk2M96/[email protected]

Don Hiatt

2017-12-22 17:28 UTC #10 lore raw

Re: [PATCH for-next 3/3] IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit

Hi Jason,

Quoted 47 lines · Jason Gunthorpe

Quoted 30 lines · Dennis Dalessandro
From: Don Hiatt <[email protected]>
Change the slid arg to ingress_pkey_table_fail() to a full 32Bits and do not convert to 16Bits in caller. This is so we can keep everything 32bit in the kernel and only change to 16bit at the uapi boundary.
Signed-off-by Don Hiatt <[email protected]>
Signed-off-by Dennis Dalessandro <[email protected]>
  drivers/infiniband/hw/hfi1/hfi.h |    2 +-
  drivers/infiniband/hw/hfi1/mad.c |    6 +-----
  2 files changed, 2 insertions(+), 6 deletions(-)
drivers/infiniband/hw/hfi1/hfi.h +2 -1
index 869c2bf..dc5b133 100644
+++ b/drivers/infiniband/hw/hfi1/hfi.h
@@ -1627,7 +1627,7 @@ static int ingress_pkey_table_search(struct hfi1_pportdata *ppd, u16 pkey)
   * the 'error info' for this failure.
   */
  static void ingress_pkey_table_fail(struct hfi1_pportdata *ppd, u16 pkey,
-				    u16 slid)
+				    u32 slid)
  {
  	struct hfi1_devdata *dd = ppd->dd;
 
drivers/infiniband/hw/hfi1/mad.c +1 -0
index cf8dba3..34547a4 100644
+++ b/drivers/infiniband/hw/hfi1/mad.c
@@ -4348,11 +4348,7 @@ static int opa_local_smp_check(struct hfi1_ibport *ibp,
  	 */
drivers/infiniband/hw/hfi1/mad.c static int opa_local_smp_check(struct hfi1_ibport *ibp, · line 4353
Quoted 10 lines
drivers/infiniband/hw/hfi1/mad.c +1 -5
  	if (pkey == LIM_MGMT_P_KEY || pkey == FULL_MGMT_P_KEY)
  		return 0;
-	/*
-	 * On OPA devices it is okay to lose the upper 16 bits of LID as this
-	 * information is obtained elsewhere. Mask off the upper 16 bits.
-	 */
-	ingress_pkey_table_fail(ppd, pkey, ib_lid_cpu16(0xFFFF & in_wc->slid));
+	ingress_pkey_table_fail(ppd, pkey, in_wc->slid);
  	return 1;
  }
I'm confused by this patch.
It seems to totally revert the -stable patch 4988be5813ff2afdc0d8bfa315ef34a577d3efbf, with the exception that ingress_pkey_table_fail now accepts a u32.

Quoted 5 lines

Is this saying the correct version of 4988be5813ff2afdc0d8bfa315ef34a577d3efbf was to just change that one type??
Jason

Yes, the correct version was to just change the type. When I made the original fix I didn't notice that was the easy/proper way of doing it until your comments. Our original implementation had the masking off and I've been just carrying it along without much thought until you commented. Thanks again!

don

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

Jason Gunthorpe

2017-12-22 23:37 UTC #11 lore raw

In reply to a message not in the archive

Quoted 3 lines · Dennis Dalessandro

Don Hiatt (2):
IB/core: Use rdma_cap_opa_mad to check for OPA IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit

For now I took these two to for-next the other should sit on the list for a bit.

Thanks
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171222233738.GB17198-uk2M96/[email protected]

Leon Romanovsky

2017-12-25 10:46 UTC #13 lore raw

Quoted 7 lines · Jason Gunthorpe

Quoted 3 lines · Dennis Dalessandro
Don Hiatt (2):
IB/core: Use rdma_cap_opa_mad to check for OPA IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit
For now I took these two to for-next the other should sit on the list for a bit.

Just to emphasize how easy to add new field in nldev:

drivers/infiniband/core/nldev.c +4 -0

index 6b22f1f2d084..ca07f845a6ae 100644
@@ -70,6 +70,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_RES_PID]		= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_RES_PID_COMM]	= { .type = NLA_NUL_STRING,
 						    .len = TASK_COMM_LEN },
+	[RDMA_NLDEV_ATTR_RES_RNR_TIMEOUT]	= { .type = NLA_U8 },
 };
 
 static int fill_nldev_handle(struct sk_buff *msg, struct ib_device *device)

@@ -283,6 +284,9 @@ static int fill_res_qp_entry(struct sk_buff *msg,
 			   RDMA_NLDEV_ATTR_RES_PID_COMM, qp->res.task_comm))
 		goto err;
 
+	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_RES_RNR_TIMEOUT, qp_attr.min_rnr_timer))
+		goto err;
+
 	nla_nest_end(msg, entry_attr);
 
 	return 0;

include/uapi/rdma/rdma_netlink.h +2 -0

index 9a90cd9f614e..241a9184a9f7 100644
@@ -357,6 +357,8 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_RES_PID_COMM,		/* string */
 
+	RDMA_NLDEV_ATTR_RES_RNR_TIMEOUT,	/* u8 */
+
 	RDMA_NLDEV_ATTR_MAX
 };
 #endif /* _UAPI_RDMA_NETLINK_H */

Quoted 7 lines

Thanks
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 20171225104615.GH2942-U/[email protected]

Dennis Dalessandro

author

2018-01-03 15:13 UTC #15 lore raw

Quoted 10 lines · Leon Romanovsky

Quoted 7 lines · Jason Gunthorpe
Quoted 3 lines · Dennis Dalessandro
Don Hiatt (2):
IB/core: Use rdma_cap_opa_mad to check for OPA IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit
For now I took these two to for-next the other should sit on the list for a bit.
Just to emphasize how easy to add new field in nldev:

Agree. I'm going to look into that when I can. For now, the patch is minor and adds something important for debugging. I've made folks aware internally this is the last patch like this, so I'd like to see this go in for the current cycle and then we'll get to work on the new implementation. Does that sound reasonable?

-Denny
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): 2517cf89-211d-0f40-17e5-eae670ff429e-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org

Leon Romanovsky

2018-01-03 15:25 UTC #17 lore raw

Quoted 10 lines · Dennis Dalessandro

Quoted 9 lines · Leon Romanovsky
Quoted 7 lines · Jason Gunthorpe
Quoted 3 lines · Dennis Dalessandro
Don Hiatt (2):
IB/core: Use rdma_cap_opa_mad to check for OPA IB/hfi1: Change slid arg in ingress_pkey_table_fail to 32bit
For now I took these two to for-next the other should sit on the list for a bit.

Quoted 7 lines

Quoted 1 line
Just to emphasize how easy to add new field in nldev:
Agree. I'm going to look into that when I can. For now, the patch is minor and adds something important for debugging. I've made folks aware internally this is the last patch like this, so I'd like to see this go in for the current cycle and then we'll get to work on the new implementation. Does that sound reasonable?

It will be great.

Thanks

Quoted 2 lines

-Denny