[PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge window

[PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge window

[Dennis Dalessandro]

Hi Doug,

Here is the latest set of patches I have that we'd like to land in 4.15. There
are some driver fixes, including a qib that is marked as stable. We also have
a CM and SA patch thrown in the mix.

As always patches available for browsing at:
https://github.com/ddalessa/kernel/tree/for-4.15

---

Dennis Dalessandro (1):
      IB/hfi1: Initialize bth1 in 16B rc ack builder

Don Hiatt (2):
      IB/hfi1: Mask the path bits with the LMC for 16B RC Acks
      IB/CM: Change sgid to IB GID when handling CM request

Jan Sokolowski (1):
      IB/hfi1: Use 4096 for default active MTU in query_qp

Mike Marciniszyn (1):
      IB/qib: Fix comparison error with qperf compare/swap test

Venkata Sandeep Dhanalakota (1):
      IB/SA: Check dlid before SA agent queries for ClassPortInfo


 drivers/infiniband/core/cm.c       |   38 ++++++++++++++++++++++++++++++++++--
 drivers/infiniband/core/sa_query.c |   10 +++++++++
 drivers/infiniband/hw/hfi1/rc.c    |   10 +++++----
 drivers/infiniband/hw/hfi1/verbs.c |    2 +-
 drivers/infiniband/hw/qib/qib_rc.c |    6 +++---
 5 files changed, 55 insertions(+), 11 deletions(-)

[PATCH for-next 1/6] IB/hfi1: Mask the path bits with the LMC for 16B RC Acks

[Dennis Dalessandro]

From: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

16B packets require that the path bits are masked with the LMC.
This mask is done correctly in all 16B header creation but was
left out for the RC Acknowledge.

Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/rc.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index fd01a76..e6fa936 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -843,11 +843,11 @@ static inline void hfi1_make_rc_ack_16B(struct rvt_qp *qp,
 	/* Convert dwords to flits */
 	len = (*hwords + *nwords) >> 1;
 
-	hfi1_make_16b_hdr(hdr,
-			  ppd->lid | rdma_ah_get_path_bits(&qp->remote_ah_attr),
+	hfi1_make_16b_hdr(hdr, ppd->lid |
+			  (rdma_ah_get_path_bits(&qp->remote_ah_attr) &
+			  ((1 << ppd->lmc) - 1)),
 			  opa_get_lid(rdma_ah_get_dlid(&qp->remote_ah_attr),
-				      16B),
-			  len, pkey, becn, 0, l4, sc5);
+				      16B), len, pkey, becn, 0, l4, sc5);
 
 	bth0 = pkey | (OP(ACKNOWLEDGE) << 24);
 	bth0 |= extra_bytes << 20;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [for-next, 1/6] IB/hfi1: Mask the path bits with the LMC for 16B RC Acks

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 04:34:16AM -0800, Dennis Dalessandro wrote:
> From: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> 16B packets require that the path bits are masked with the LMC.
> This mask is done correctly in all 16B header creation but was
> left out for the RC Acknowledge.
> 
> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>  drivers/infiniband/hw/hfi1/rc.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)

Applied 1,4,5,6 of this series to for-next

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 2/6] IB/hfi1: Initialize bth1 in 16B rc ack builder

[Dennis Dalessandro]

It is possible the bth1 variable could be used uninitialized so going
ahead and giving it a default value.

Fixes: 5b6cabb0db77 ("IB/hfi1: Add 16B RC/UC support")
Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/rc.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/rc.c b/drivers/infiniband/hw/hfi1/rc.c
index e6fa936..68d5c3c 100644
--- a/drivers/infiniband/hw/hfi1/rc.c
+++ b/drivers/infiniband/hw/hfi1/rc.c
@@ -814,7 +814,7 @@ static inline void hfi1_make_rc_ack_16B(struct rvt_qp *qp,
 	struct hfi1_pportdata *ppd = ppd_from_ibp(ibp);
 	struct hfi1_16b_header *hdr = &opa_hdr->opah;
 	struct ib_other_headers *ohdr;
-	u32 bth0, bth1;
+	u32 bth0, bth1 = 0;
 	u16 len, pkey;
 	u8 becn = !!is_fecn;
 	u8 l4 = OPA_16B_L4_IB_LOCAL;

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [for-next,2/6] IB/hfi1: Initialize bth1 in 16B rc ack builder

[Jason Gunthorpe]

On Tue, Nov 14, 2017 at 04:34:23AM -0800, Dennis Dalessandro wrote:
> It is possible the bth1 variable could be used uninitialized so going
> ahead and giving it a default value.
> 
> Fixes: 5b6cabb0db77 ("IB/hfi1: Add 16B RC/UC support")
> Reviewed-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>  drivers/infiniband/hw/hfi1/rc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)

Applied to for-rc
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 3/6] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Dennis Dalessandro]

From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

SA queries SM for class port info when there is a LID_CHANGE event.

When a base lid is configured before fm is started ie when smlid is
not yet assigned, SA handles the LID_CHANGE event and tries query SM
with lid 0. This will cause an hang.

[ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
[ 1106.965082] Tainted: G O 4.12.0+ #1
[ 1106.969602] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
 this message.
[ 1106.977227] kworker/2:0 D 0 23 2 0x00000000
[ 1106.977250] Workqueue: infiniband update_ib_cpi [ib_core]
[ 1106.977261] Call Trace:
[ 1106.977273] __schedule+0x28e/0x860
[ 1106.977285] schedule+0x36/0x80
[ 1106.977298] schedule_timeout+0x1a3/0x2e0
[ 1106.977310] ? radix_tree_iter_tag_clear+0x1b/0x20
[ 1106.977322] ? idr_alloc+0x64/0x90
[ 1106.977334] wait_for_completion+0xe3/0x140
[ 1106.977347] ? wake_up_q+0x80/0x80
[ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core]
[ 1106.977381] process_one_work+0x147/0x370
[ 1106.977394] worker_thread+0x4a/0x390
[ 1106.977406] kthread+0x109/0x140
[ 1106.977418] ? process_one_work+0x370/0x370
[ 1106.977430] ? kthread_park+0x60/0x60
[ 1106.977443] ret_from_fork+0x22/0x30

Always ensure a proper smlid is assigned before querying SM for cpi.

Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port info")
Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/sa_query.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index ab5e102..24b9b5f 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1345,6 +1345,7 @@ int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
 
 static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
 {
+	struct rdma_ah_attr ah_attr;
 	unsigned long flags;
 
 	spin_lock_irqsave(&query->port->ah_lock, flags);
@@ -1356,6 +1357,15 @@ static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
 	query->sm_ah = query->port->sm_ah;
 	spin_unlock_irqrestore(&query->port->ah_lock, flags);
 
+	/*
+	 * Always check if sm_ah has valid dlid assigned,
+	 * before querying for class port info
+	 */
+	if ((rdma_query_ah(query->sm_ah->ah, &ah_attr) < 0) ||
+	    !rdma_ah_get_dlid(&ah_attr)) {
+		kref_put(&query->sm_ah->ref, free_sm_ah);
+		return -EAGAIN;
+	}
 	query->mad_buf = ib_create_send_mad(query->port->agent, 1,
 					    query->sm_ah->pkey_index,
 					    0, IB_MGMT_SA_HDR, IB_MGMT_SA_DATA,

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 3/6] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Hal Rosenstock]

On 11/14/2017 7:34 AM, Dennis Dalessandro wrote:
> From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> SA queries SM for class port info when there is a LID_CHANGE event.
> 
> When a base lid is configured before fm is started ie when smlid is
> not yet assigned, SA handles the LID_CHANGE event and tries query SM
> with lid 0. This will cause an hang.
> 
> [ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
> [ 1106.965082] Tainted: G O 4.12.0+ #1
> [ 1106.969602] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
>  this message.
> [ 1106.977227] kworker/2:0 D 0 23 2 0x00000000
> [ 1106.977250] Workqueue: infiniband update_ib_cpi [ib_core]
> [ 1106.977261] Call Trace:
> [ 1106.977273] __schedule+0x28e/0x860
> [ 1106.977285] schedule+0x36/0x80
> [ 1106.977298] schedule_timeout+0x1a3/0x2e0
> [ 1106.977310] ? radix_tree_iter_tag_clear+0x1b/0x20
> [ 1106.977322] ? idr_alloc+0x64/0x90
> [ 1106.977334] wait_for_completion+0xe3/0x140
> [ 1106.977347] ? wake_up_q+0x80/0x80
> [ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core]
> [ 1106.977381] process_one_work+0x147/0x370
> [ 1106.977394] worker_thread+0x4a/0x390
> [ 1106.977406] kthread+0x109/0x140
> [ 1106.977418] ? process_one_work+0x370/0x370
> [ 1106.977430] ? kthread_park+0x60/0x60
> [ 1106.977443] ret_from_fork+0x22/0x30
> 
> Always ensure a proper smlid is assigned before querying SM for cpi.
> 
> Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port info")
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/infiniband/core/sa_query.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
> index ab5e102..24b9b5f 100644
> --- a/drivers/infiniband/core/sa_query.c
> +++ b/drivers/infiniband/core/sa_query.c
> @@ -1345,6 +1345,7 @@ int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
>  
>  static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
>  {
> +	struct rdma_ah_attr ah_attr;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&query->port->ah_lock, flags);
> @@ -1356,6 +1357,15 @@ static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
>  	query->sm_ah = query->port->sm_ah;
>  	spin_unlock_irqrestore(&query->port->ah_lock, flags);
>  
> +	/*
> +	 * Always check if sm_ah has valid dlid assigned,
> +	 * before querying for class port info
> +	 */
> +	if ((rdma_query_ah(query->sm_ah->ah, &ah_attr) < 0) ||
> +	    !rdma_ah_get_dlid(&ah_attr)) {

Isn't valid SM LID more complex than just != 0 ?

For IB, invalid SM LID is != 0 and not in multicast LID range (>= 0xC000
which includes the permissive LID). At a minimum (for at least IB), this
check should also include the permissive LID.

-- Hal

> +		kref_put(&query->sm_ah->ref, free_sm_ah);
> +		return -EAGAIN;
> +	}
>  	query->mad_buf = ib_create_send_mad(query->port->agent, 1,
>  					    query->sm_ah->pkey_index,
>  					    0, IB_MGMT_SA_HDR, IB_MGMT_SA_DATA,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH for-next 3/6] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Dhanalakota, Venkata S]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4390 bytes --]

Hi Hal,

The patch intends to only check if smlid is assigned or not. It does not intend to check if smlid is a valid lid or not. Validity of lid is taken care by the individual drivers, during create_ah().

Thanks,
Sandeep

-----Original Message-----
From: Hal Rosenstock [mailto:hal@dev.mellanox.co.il] 
Sent: Tuesday, November 14, 2017 5:12 AM
To: Dalessandro, Dennis <dennis.dalessandro@intel.com>; dledford@redhat.com
Cc: linux-rdma@vger.kernel.org; Marciniszyn, Mike <mike.marciniszyn@intel.com>; Dhanalakota, Venkata S <venkata.s.dhanalakota@intel.com>; Weiny, Ira <ira.weiny@intel.com>
Subject: Re: [PATCH for-next 3/6] IB/SA: Check dlid before SA agent queries for ClassPortInfo
Importance: High

On 11/14/2017 7:34 AM, Dennis Dalessandro wrote:
> From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota@intel.com>
> 
> SA queries SM for class port info when there is a LID_CHANGE event.
> 
> When a base lid is configured before fm is started ie when smlid is 
> not yet assigned, SA handles the LID_CHANGE event and tries query SM 
> with lid 0. This will cause an hang.
> 
> [ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
> [ 1106.965082] Tainted: G O 4.12.0+ #1 [ 1106.969602] "echo 0 > 
> /proc/sys/kernel/hung_task_timeout_secs" disables  this message.
> [ 1106.977227] kworker/2:0 D 0 23 2 0x00000000 [ 1106.977250] 
> Workqueue: infiniband update_ib_cpi [ib_core] [ 1106.977261] Call 
> Trace:
> [ 1106.977273] __schedule+0x28e/0x860
> [ 1106.977285] schedule+0x36/0x80
> [ 1106.977298] schedule_timeout+0x1a3/0x2e0 [ 1106.977310] ? 
> radix_tree_iter_tag_clear+0x1b/0x20
> [ 1106.977322] ? idr_alloc+0x64/0x90
> [ 1106.977334] wait_for_completion+0xe3/0x140 [ 1106.977347] ? 
> wake_up_q+0x80/0x80 [ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core] 
> [ 1106.977381] process_one_work+0x147/0x370 [ 1106.977394] 
> worker_thread+0x4a/0x390 [ 1106.977406] kthread+0x109/0x140 [ 
> 1106.977418] ? process_one_work+0x370/0x370 [ 1106.977430] ? 
> kthread_park+0x60/0x60 [ 1106.977443] ret_from_fork+0x22/0x30
> 
> Always ensure a proper smlid is assigned before querying SM for cpi.
> 
> Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port 
> info")
> Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Venkata Sandeep Dhanalakota 
> <venkata.s.dhanalakota@intel.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> ---
>  drivers/infiniband/core/sa_query.c |   10 ++++++++++
>  1 files changed, 10 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/infiniband/core/sa_query.c 
> b/drivers/infiniband/core/sa_query.c
> index ab5e102..24b9b5f 100644
> --- a/drivers/infiniband/core/sa_query.c
> +++ b/drivers/infiniband/core/sa_query.c
> @@ -1345,6 +1345,7 @@ int ib_init_ah_from_path(struct ib_device 
> *device, u8 port_num,
>  
>  static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)  {
> +	struct rdma_ah_attr ah_attr;
>  	unsigned long flags;
>  
>  	spin_lock_irqsave(&query->port->ah_lock, flags); @@ -1356,6 +1357,15 
> @@ static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
>  	query->sm_ah = query->port->sm_ah;
>  	spin_unlock_irqrestore(&query->port->ah_lock, flags);
>  
> +	/*
> +	 * Always check if sm_ah has valid dlid assigned,
> +	 * before querying for class port info
> +	 */
> +	if ((rdma_query_ah(query->sm_ah->ah, &ah_attr) < 0) ||
> +	    !rdma_ah_get_dlid(&ah_attr)) {

Isn't valid SM LID more complex than just != 0 ?

For IB, invalid SM LID is != 0 and not in multicast LID range (>= 0xC000 which includes the permissive LID). At a minimum (for at least IB), this check should also include the permissive LID.

-- Hal

> +		kref_put(&query->sm_ah->ref, free_sm_ah);
> +		return -EAGAIN;
> +	}
>  	query->mad_buf = ib_create_send_mad(query->port->agent, 1,
>  					    query->sm_ah->pkey_index,
>  					    0, IB_MGMT_SA_HDR, IB_MGMT_SA_DATA,
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" 
> in the body of a message to majordomo@vger.kernel.org More majordomo 
> info at  http://vger.kernel.org/majordomo-info.html
> 
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH for-next 3/6] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Jason Gunthorpe]

On Mon, Nov 20, 2017 at 07:11:09PM +0000, Dhanalakota, Venkata S wrote:
> Hi Hal,
> 
> The patch intends to only check if smlid is assigned or not. It does
> not intend to check if smlid is a valid lid or not. Validity of lid
> is taken care by the individual drivers, during create_ah().

I think that is Hal's point - there are multiple ways to say 'not
assigned' for SMLID, including == 0 and == 0xFFFF

It is best to use !valid as a test for an unassigned SM lid.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 for-next] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Dennis Dalessandro]

From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

SA queries SM for class port info when there is a LID_CHANGE event.

When a base lid is configured before fm is started ie when smlid is
not yet assigned, SA handles the LID_CHANGE event and tries query SM
with lid 0. This will cause an hang.

[ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
[ 1106.965082] Tainted: G O 4.12.0+ #1
[ 1106.969602] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
 this message.
[ 1106.977227] kworker/2:0 D 0 23 2 0x00000000
[ 1106.977250] Workqueue: infiniband update_ib_cpi [ib_core]
[ 1106.977261] Call Trace:
[ 1106.977273] __schedule+0x28e/0x860
[ 1106.977285] schedule+0x36/0x80
[ 1106.977298] schedule_timeout+0x1a3/0x2e0
[ 1106.977310] ? radix_tree_iter_tag_clear+0x1b/0x20
[ 1106.977322] ? idr_alloc+0x64/0x90
[ 1106.977334] wait_for_completion+0xe3/0x140
[ 1106.977347] ? wake_up_q+0x80/0x80
[ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core]
[ 1106.977381] process_one_work+0x147/0x370
[ 1106.977394] worker_thread+0x4a/0x390
[ 1106.977406] kthread+0x109/0x140
[ 1106.977418] ? process_one_work+0x370/0x370
[ 1106.977430] ? kthread_park+0x60/0x60
[ 1106.977443] ret_from_fork+0x22/0x30

Always ensure a proper smlid is assigned before querying SM for cpi.

Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port info")
Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

---
changes since v1:
	Add inline function to check for valid lid
---
 drivers/infiniband/core/sa_query.c |   10 ++++++++++
 include/rdma/opa_addr.h            |   16 ++++++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/core/sa_query.c b/drivers/infiniband/core/sa_query.c
index ab5e102..62d7def 100644
--- a/drivers/infiniband/core/sa_query.c
+++ b/drivers/infiniband/core/sa_query.c
@@ -1345,6 +1345,7 @@ int ib_init_ah_from_path(struct ib_device *device, u8 port_num,
 
 static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
 {
+	struct rdma_ah_attr ah_attr;
 	unsigned long flags;
 
 	spin_lock_irqsave(&query->port->ah_lock, flags);
@@ -1356,6 +1357,15 @@ static int alloc_mad(struct ib_sa_query *query, gfp_t gfp_mask)
 	query->sm_ah = query->port->sm_ah;
 	spin_unlock_irqrestore(&query->port->ah_lock, flags);
 
+	/*
+	 * Always check if sm_ah has valid dlid assigned,
+	 * before querying for class port info
+	 */
+	if ((rdma_query_ah(query->sm_ah->ah, &ah_attr) < 0) ||
+	    !rdma_is_valid_unicast_lid(&ah_attr)) {
+		kref_put(&query->sm_ah->ref, free_sm_ah);
+		return -EAGAIN;
+	}
 	query->mad_buf = ib_create_send_mad(query->port->agent, 1,
 					    query->sm_ah->pkey_index,
 					    0, IB_MGMT_SA_HDR, IB_MGMT_SA_DATA,
diff --git a/include/rdma/opa_addr.h b/include/rdma/opa_addr.h
index f68fca2..2bbb7a6 100644
--- a/include/rdma/opa_addr.h
+++ b/include/rdma/opa_addr.h
@@ -114,4 +114,20 @@ static inline u32 opa_get_mcast_base(u32 nr_top_bits)
 	return (be32_to_cpu(OPA_LID_PERMISSIVE) << (32 - nr_top_bits));
 }
 
+/* Check for a valid unicast LID for non-SM traffic types */
+static inline bool rdma_is_valid_unicast_lid(struct rdma_ah_attr *attr)
+{
+	if (attr->type == RDMA_AH_ATTR_TYPE_IB) {
+		if (!rdma_ah_get_dlid(attr) ||
+		    rdma_ah_get_dlid(attr) >=
+		    be32_to_cpu(IB_MULTICAST_LID_BASE))
+			return false;
+	} else if (attr->type == RDMA_AH_ATTR_TYPE_OPA) {
+		if (!rdma_ah_get_dlid(attr) ||
+		    rdma_ah_get_dlid(attr) >=
+		    opa_get_mcast_base(OPA_MCAST_NR))
+			return false;
+	}
+	return true;
+}
 #endif /* OPA_ADDR_H */

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 for-next] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Jason Gunthorpe]

On Mon, Dec 18, 2017 at 07:26:58PM -0800, Dennis Dalessandro wrote:
> From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> SA queries SM for class port info when there is a LID_CHANGE event.
> 
> When a base lid is configured before fm is started ie when smlid is
> not yet assigned, SA handles the LID_CHANGE event and tries query SM
> with lid 0. This will cause an hang.
> 
> [ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
> [ 1106.965082] Tainted: G O 4.12.0+ #1
> [ 1106.969602] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
>  this message.
> [ 1106.977227] kworker/2:0 D 0 23 2 0x00000000
> [ 1106.977250] Workqueue: infiniband update_ib_cpi [ib_core]
> [ 1106.977261] Call Trace:
> [ 1106.977273] __schedule+0x28e/0x860
> [ 1106.977285] schedule+0x36/0x80
> [ 1106.977298] schedule_timeout+0x1a3/0x2e0
> [ 1106.977310] ? radix_tree_iter_tag_clear+0x1b/0x20
> [ 1106.977322] ? idr_alloc+0x64/0x90
> [ 1106.977334] wait_for_completion+0xe3/0x140
> [ 1106.977347] ? wake_up_q+0x80/0x80
> [ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core]
> [ 1106.977381] process_one_work+0x147/0x370
> [ 1106.977394] worker_thread+0x4a/0x390
> [ 1106.977406] kthread+0x109/0x140
> [ 1106.977418] ? process_one_work+0x370/0x370
> [ 1106.977430] ? kthread_park+0x60/0x60
> [ 1106.977443] ret_from_fork+0x22/0x30
> 
> Always ensure a proper smlid is assigned before querying SM for cpi.

Shouldn't we do something about this long hang as well?

> Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port info")
> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Otherwise seems like a reasonable step to me..

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 for-next] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Jason Gunthorpe]

On Mon, Dec 18, 2017 at 07:26:58PM -0800, Dennis Dalessandro wrote:
> From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> SA queries SM for class port info when there is a LID_CHANGE event.
> 
> When a base lid is configured before fm is started ie when smlid is
> not yet assigned, SA handles the LID_CHANGE event and tries query SM
> with lid 0. This will cause an hang.
> 
> [ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
> [ 1106.965082] Tainted: G O 4.12.0+ #1
> [ 1106.969602] "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables
>  this message.
> [ 1106.977227] kworker/2:0 D 0 23 2 0x00000000
> [ 1106.977250] Workqueue: infiniband update_ib_cpi [ib_core]
> [ 1106.977261] Call Trace:
> [ 1106.977273] __schedule+0x28e/0x860
> [ 1106.977285] schedule+0x36/0x80
> [ 1106.977298] schedule_timeout+0x1a3/0x2e0
> [ 1106.977310] ? radix_tree_iter_tag_clear+0x1b/0x20
> [ 1106.977322] ? idr_alloc+0x64/0x90
> [ 1106.977334] wait_for_completion+0xe3/0x140
> [ 1106.977347] ? wake_up_q+0x80/0x80
> [ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core]
> [ 1106.977381] process_one_work+0x147/0x370
> [ 1106.977394] worker_thread+0x4a/0x390
> [ 1106.977406] kthread+0x109/0x140
> [ 1106.977418] ? process_one_work+0x370/0x370
> [ 1106.977430] ? kthread_park+0x60/0x60
> [ 1106.977443] ret_from_fork+0x22/0x30
> 
> Always ensure a proper smlid is assigned before querying SM for cpi.
> 
> Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port info")
> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Took it to for-next

Although I am unclear why a bad SM lid should cause a hang? Doesn't
like seem like a bug unto itself?

Thanks,
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Re: [PATCH v2 for-next] IB/SA: Check dlid before SA agent queries for ClassPortInfo

[Dhanalakota, Venkata S]

Hi Jason,
Yes it is a bug to hang on bad SM lid. While root causing the hang, I thought it is better to add some logic that will prevent sending the packets with bad dlid in first place.

Thank you,
Sandeep
On Mon, Dec 18, 2017 at 07:26:58PM -0800, Dennis Dalessandro wrote:
> From: Venkata Sandeep Dhanalakota <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> 
> SA queries SM for class port info when there is a LID_CHANGE event.
> 
> When a base lid is configured before fm is started ie when smlid is 
> not yet assigned, SA handles the LID_CHANGE event and tries query SM 
> with lid 0. This will cause an hang.
> 
> [ 1106.958820] INFO: task kworker/2:0:23 blocked for more than 120 seconds.
> [ 1106.965082] Tainted: G O 4.12.0+ #1 [ 1106.969602] "echo 0 > 
> /proc/sys/kernel/hung_task_timeout_secs" disables  this message.
> [ 1106.977227] kworker/2:0 D 0 23 2 0x00000000 [ 1106.977250] 
> Workqueue: infiniband update_ib_cpi [ib_core] [ 1106.977261] Call 
> Trace:
> [ 1106.977273] __schedule+0x28e/0x860
> [ 1106.977285] schedule+0x36/0x80
> [ 1106.977298] schedule_timeout+0x1a3/0x2e0 [ 1106.977310] ? 
> radix_tree_iter_tag_clear+0x1b/0x20
> [ 1106.977322] ? idr_alloc+0x64/0x90
> [ 1106.977334] wait_for_completion+0xe3/0x140 [ 1106.977347] ? 
> wake_up_q+0x80/0x80 [ 1106.977369] update_ib_cpi+0x163/0x210 [ib_core] 
> [ 1106.977381] process_one_work+0x147/0x370 [ 1106.977394] 
> worker_thread+0x4a/0x390 [ 1106.977406] kthread+0x109/0x140 [ 
> 1106.977418] ? process_one_work+0x370/0x370 [ 1106.977430] ? 
> kthread_park+0x60/0x60 [ 1106.977443] ret_from_fork+0x22/0x30
> 
> Always ensure a proper smlid is assigned before querying SM for cpi.
> 
> Fixes: ee1c60b1bff ("IB/SA: Modify SA to implicitly cache Class Port 
> info")
> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Venkata Sandeep Dhanalakota 
> <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Took it to for-next

Although I am unclear why a bad SM lid should cause a hang? Doesn't like seem like a bug unto itself?

Thanks,
Jason

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling CM request

[Dennis Dalessandro]

From: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

ULPs do not understand OPA GIDs and will reject CM requests
if the sgid does not match the local_gid. In order to
fix this behavior we convert the OPA GID back to an IB GID.

Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/core/cm.c |   38 ++++++++++++++++++++++++++++++++++++--
 1 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c
index 5927ee4..f9f6f5e 100644
--- a/drivers/infiniband/core/cm.c
+++ b/drivers/infiniband/core/cm.c
@@ -1560,6 +1560,37 @@ static u16 cm_get_bth_pkey(struct cm_work *work)
 	return pkey;
 }
 
+/**
+ * Convert OPA SGID to IB SGID
+ * ULPs (such as IPoIB) do not understand OPA GIDs and will
+ * reject them as the local_gid will not match the sgid. Therefore,
+ * change the pathrec's SGID to an IB SGID.
+ *
+ * @work: Work completion
+ * @path: Path record
+ */
+static void cm_opa_to_ib_sgid(struct cm_work *work,
+			      struct sa_path_rec *path)
+{
+	struct ib_device *dev = work->port->cm_dev->ib_device;
+	struct ib_gid_attr gid_attr;
+	u8 port_num = work->port->port_num;
+
+	if (rdma_cap_opa_ah(dev, port_num) &&
+	    (ib_is_opa_gid(&path->sgid))) {
+		union ib_gid sgid;
+
+		if (ib_get_cached_gid(dev, port_num, 0,
+				      &sgid, &gid_attr)) {
+			dev_warn(&dev->dev,
+				 "Error updating sgid in CM request\n");
+			return;
+		}
+
+		path->sgid = sgid;
+	}
+}
+
 static void cm_format_req_event(struct cm_work *work,
 				struct cm_id_private *cm_id_priv,
 				struct ib_cm_id *listen_id)
@@ -1573,10 +1604,13 @@ static void cm_format_req_event(struct cm_work *work,
 	param->bth_pkey = cm_get_bth_pkey(work);
 	param->port = cm_id_priv->av.port->port_num;
 	param->primary_path = &work->path[0];
-	if (req_msg->alt_local_lid)
+	cm_opa_to_ib_sgid(work, param->primary_path);
+	if (cm_req_has_alt_path(req_msg)) {
 		param->alternate_path = &work->path[1];
-	else
+		cm_opa_to_ib_sgid(work, param->alternate_path);
+	} else {
 		param->alternate_path = NULL;
+	}
 	param->remote_ca_guid = req_msg->local_ca_guid;
 	param->remote_qkey = be32_to_cpu(req_msg->local_qkey);
 	param->remote_qpn = be32_to_cpu(cm_req_get_local_qpn(req_msg));

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling CM request

[Parav Pandit]

Hi Dennis,

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Dennis Dalessandro
> Sent: Tuesday, November 14, 2017 6:35 AM
> To: dledford@redhat.com
> Cc: linux-rdma@vger.kernel.org; Don Hiatt <don.hiatt@intel.com>; Ira Weiny
> <ira.weiny@intel.com>
> Subject: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling CM
> request
> 
> From: Don Hiatt <don.hiatt@intel.com>
> 
> ULPs do not understand OPA GIDs and will reject CM requests if the sgid does
> not match the local_gid. In order to fix this behavior we convert the OPA GID
> back to an IB GID.
> 
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> ---
>  drivers/infiniband/core/cm.c |   38
> ++++++++++++++++++++++++++++++++++++--
>  1 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index
> 5927ee4..f9f6f5e 100644
> --- a/drivers/infiniband/core/cm.c
> +++ b/drivers/infiniband/core/cm.c
> @@ -1560,6 +1560,37 @@ static u16 cm_get_bth_pkey(struct cm_work *work)
>  	return pkey;
>  }
> 
> +/**
> + * Convert OPA SGID to IB SGID
> + * ULPs (such as IPoIB) do not understand OPA GIDs and will
> + * reject them as the local_gid will not match the sgid. Therefore,
> + * change the pathrec's SGID to an IB SGID.
> + *
Address and other conversions are in ib_sa.h such as path_conv_opa_to_ib().
Do path_conv_opa_to_ib function also need such opa to ib gid conversion?
Can you place cm_opa_to_ib_sgid function in core/addr.c as it deals with addresses.

While replying back to CM request, is it ok to reply back with IB GID and reverse conversion not needed or it replies with OPA GID?

> + * @work: Work completion
> + * @path: Path record
> + */
> +static void cm_opa_to_ib_sgid(struct cm_work *work,
> +			      struct sa_path_rec *path)
> +{
> +	struct ib_device *dev = work->port->cm_dev->ib_device;
> +	struct ib_gid_attr gid_attr;
> +	u8 port_num = work->port->port_num;
> +
> +	if (rdma_cap_opa_ah(dev, port_num) &&
> +	    (ib_is_opa_gid(&path->sgid))) {
> +		union ib_gid sgid;
> +
> +		if (ib_get_cached_gid(dev, port_num, 0,
Only gid 0 needs conversion?

> +				      &sgid, &gid_attr)) {
Since gid attribute is not used here and applicable to RoCE and its optional, can you pass NULL and avoid gid_attr?

> +			dev_warn(&dev->dev,
> +				 "Error updating sgid in CM request\n");
> +			return;
> +		}
> +
> +		path->sgid = sgid;
> +	}
> +}
> +
>  static void cm_format_req_event(struct cm_work *work,
>  				struct cm_id_private *cm_id_priv,
>  				struct ib_cm_id *listen_id)
> @@ -1573,10 +1604,13 @@ static void cm_format_req_event(struct cm_work
> *work,
>  	param->bth_pkey = cm_get_bth_pkey(work);
>  	param->port = cm_id_priv->av.port->port_num;
>  	param->primary_path = &work->path[0];
> -	if (req_msg->alt_local_lid)
> +	cm_opa_to_ib_sgid(work, param->primary_path);
> +	if (cm_req_has_alt_path(req_msg)) {
>  		param->alternate_path = &work->path[1];
> -	else
> +		cm_opa_to_ib_sgid(work, param->alternate_path);
> +	} else {
>  		param->alternate_path = NULL;
> +	}
>  	param->remote_ca_guid = req_msg->local_ca_guid;
>  	param->remote_qkey = be32_to_cpu(req_msg->local_qkey);
>  	param->remote_qpn = be32_to_cpu(cm_req_get_local_qpn(req_msg));
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling CM request

[Don Hiatt]

Hi Parav,

On 11/14/2017 10:50 AM, Parav Pandit wrote:
> Hi Dennis,
>
>> -----Original Message-----
>> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
>> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Dennis Dalessandro
>> Sent: Tuesday, November 14, 2017 6:35 AM
>> To: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
>> Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Ira Weiny
>> <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Subject: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling CM
>> request
>>
>> From: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>>
>> ULPs do not understand OPA GIDs and will reject CM requests if the sgid does
>> not match the local_gid. In order to fix this behavior we convert the OPA GID
>> back to an IB GID.
>>
>> Reviewed-by: Ira Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>>   drivers/infiniband/core/cm.c |   38
>> ++++++++++++++++++++++++++++++++++++--
>>   1 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/infiniband/core/cm.c b/drivers/infiniband/core/cm.c index
>> 5927ee4..f9f6f5e 100644
>> --- a/drivers/infiniband/core/cm.c
>> +++ b/drivers/infiniband/core/cm.c
>> @@ -1560,6 +1560,37 @@ static u16 cm_get_bth_pkey(struct cm_work *work)
>>   	return pkey;
>>   }
>>
>> +/**
>> + * Convert OPA SGID to IB SGID
>> + * ULPs (such as IPoIB) do not understand OPA GIDs and will
>> + * reject them as the local_gid will not match the sgid. Therefore,
>> + * change the pathrec's SGID to an IB SGID.
>> + *
> Address and other conversions are in ib_sa.h such as path_conv_opa_to_ib().
> Do path_conv_opa_to_ib function also need such opa to ib gid conversion?
> Can you place cm_opa_to_ib_sgid function in core/addr.c as it deals with addresses.
Since this function is only used by the CM (and is only necessary 
because the responder is not performing a path record query itself, 
while the requester has already done query for us) there is no point in 
moving it to core/addr.c. Further, if we do move the function there, we 
will have to include all the required include files (pathrec, wc, helper 
functions) . Given the limited applicability of this function I prefer 
to keep it as a static function as it is.
> While replying back to CM request, is it ok to reply back with IB GID and reverse conversion not needed or it replies with OPA GID?

Yes, it is okay to reply back with an IB GID and we do not need the 
reverse conversion (effectively they are the same thing as the requester 
in this case has already done a pathrec query and does nothing with the 
reply, and, even if it does the requester already has the IB gid and 
will know how to resolve it).
>> + * @work: Work completion
>> + * @path: Path record
>> + */
>> +static void cm_opa_to_ib_sgid(struct cm_work *work,
>> +			      struct sa_path_rec *path)
>> +{
>> +	struct ib_device *dev = work->port->cm_dev->ib_device;
>> +	struct ib_gid_attr gid_attr;
>> +	u8 port_num = work->port->port_num;
>> +
>> +	if (rdma_cap_opa_ah(dev, port_num) &&
>> +	    (ib_is_opa_gid(&path->sgid))) {
>> +		union ib_gid sgid;
>> +
>> +		if (ib_get_cached_gid(dev, port_num, 0,
> Only gid 0 needs conversion?
Yes,  because gid 1 is the OPA gid (per the OPA spec there is only one 
gid per port so gid 1
is a caching point for the opa gid).
>> +				      &sgid, &gid_attr)) {
> Since gid attribute is not used here and applicable to RoCE and its optional, can you pass NULL and avoid gid_attr?
Will do.
>> +			dev_warn(&dev->dev,
>> +				 "Error updating sgid in CM request\n");
>> +			return;
>> +		}
>> +
>> +		path->sgid = sgid;
>> +	}
>> +}
>> +
>>   static void cm_format_req_event(struct cm_work *work,
>>   				struct cm_id_private *cm_id_priv,
>>   				struct ib_cm_id *listen_id)
>> @@ -1573,10 +1604,13 @@ static void cm_format_req_event(struct cm_work
>> *work,
>>   	param->bth_pkey = cm_get_bth_pkey(work);
>>   	param->port = cm_id_priv->av.port->port_num;
>>   	param->primary_path = &work->path[0];
>> -	if (req_msg->alt_local_lid)
>> +	cm_opa_to_ib_sgid(work, param->primary_path);
>> +	if (cm_req_has_alt_path(req_msg)) {
>>   		param->alternate_path = &work->path[1];
>> -	else
>> +		cm_opa_to_ib_sgid(work, param->alternate_path);
>> +	} else {
>>   		param->alternate_path = NULL;
>> +	}
>>   	param->remote_ca_guid = req_msg->local_ca_guid;
>>   	param->remote_qkey = be32_to_cpu(req_msg->local_qkey);
>>   	param->remote_qpn = be32_to_cpu(cm_req_get_local_qpn(req_msg));
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body
>> of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling CM request

[Parav Pandit]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 4339 bytes --]

Hi Don,

> -----Original Message-----
> From: Don Hiatt [mailto:don.hiatt@intel.com]
> Sent: Thursday, November 30, 2017 5:40 PM
> To: Parav Pandit <parav@mellanox.com>; Dennis Dalessandro
> <dennis.dalessandro@intel.com>; dledford@redhat.com
> Cc: linux-rdma@vger.kernel.org; Ira Weiny <ira.weiny@intel.com>;
> don.hiatt@intel.com
> Subject: Re: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when handling
> CM request
> 
> Hi Parav,
> 
> On 11/14/2017 10:50 AM, Parav Pandit wrote:
> > Hi Dennis,
> >
> >> -----Original Message-----
> >> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> >> owner@vger.kernel.org] On Behalf Of Dennis Dalessandro
> >> Sent: Tuesday, November 14, 2017 6:35 AM
> >> To: dledford@redhat.com
> >> Cc: linux-rdma@vger.kernel.org; Don Hiatt <don.hiatt@intel.com>; Ira
> >> Weiny <ira.weiny@intel.com>
> >> Subject: [PATCH for-next 4/6] IB/CM: Change sgid to IB GID when
> >> handling CM request
> >>
> >> From: Don Hiatt <don.hiatt@intel.com>
> >>
> >> ULPs do not understand OPA GIDs and will reject CM requests if the
> >> sgid does not match the local_gid. In order to fix this behavior we
> >> convert the OPA GID back to an IB GID.
> >>
> >> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> >> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> >> Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> >> ---
> >>   drivers/infiniband/core/cm.c |   38
> >> ++++++++++++++++++++++++++++++++++++--
> >>   1 files changed, 36 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/core/cm.c
> >> b/drivers/infiniband/core/cm.c index 5927ee4..f9f6f5e 100644
> >> --- a/drivers/infiniband/core/cm.c
> >> +++ b/drivers/infiniband/core/cm.c
> >> @@ -1560,6 +1560,37 @@ static u16 cm_get_bth_pkey(struct cm_work
> *work)
> >>   	return pkey;
> >>   }
> >>
> >> +/**
> >> + * Convert OPA SGID to IB SGID
> >> + * ULPs (such as IPoIB) do not understand OPA GIDs and will
> >> + * reject them as the local_gid will not match the sgid. Therefore,
> >> + * change the pathrec's SGID to an IB SGID.
> >> + *
> > Address and other conversions are in ib_sa.h such as path_conv_opa_to_ib().
> > Do path_conv_opa_to_ib function also need such opa to ib gid conversion?
> > Can you place cm_opa_to_ib_sgid function in core/addr.c as it deals with
> addresses.
> Since this function is only used by the CM (and is only necessary because the
> responder is not performing a path record query itself, while the requester has
> already done query for us) there is no point in moving it to core/addr.c. Further,
> if we do move the function there, we will have to include all the required include
> files (pathrec, wc, helper
> functions) . Given the limited applicability of this function I prefer to keep it as a
> static function as it is.
> > While replying back to CM request, is it ok to reply back with IB GID and
> reverse conversion not needed or it replies with OPA GID?
> 
> Yes, it is okay to reply back with an IB GID and we do not need the reverse
> conversion (effectively they are the same thing as the requester in this case has
> already done a pathrec query and does nothing with the reply, and, even if it
> does the requester already has the IB gid and will know how to resolve it).
Ok.

> >> + * @work: Work completion
> >> + * @path: Path record
> >> + */
> >> +static void cm_opa_to_ib_sgid(struct cm_work *work,
> >> +			      struct sa_path_rec *path)
> >> +{
> >> +	struct ib_device *dev = work->port->cm_dev->ib_device;
> >> +	struct ib_gid_attr gid_attr;
> >> +	u8 port_num = work->port->port_num;
> >> +
> >> +	if (rdma_cap_opa_ah(dev, port_num) &&
> >> +	    (ib_is_opa_gid(&path->sgid))) {
> >> +		union ib_gid sgid;
> >> +
> >> +		if (ib_get_cached_gid(dev, port_num, 0,
> > Only gid 0 needs conversion?
> Yes,  because gid 1 is the OPA gid (per the OPA spec there is only one gid per port
> so gid 1 is a caching point for the opa gid).
> >> +				      &sgid, &gid_attr)) {
> > Since gid attribute is not used here and applicable to RoCE and its optional, can
> you pass NULL and avoid gid_attr?
> Will do.
Ok. Thanks.

N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

[PATCH for-next 5/6] IB/hfi1: Use 4096 for default active MTU in query_qp

[Dennis Dalessandro]

From: Jan Sokolowski <jan.sokolowski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Currently, if a port is queried that has an invalid
Maximum Transmission Unit, driver reports default MTU of 2048.
This in incorrect.

Use default value of 4096 if invalid.

Reviewed-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Jan Sokolowski <jan.sokolowski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/verbs.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/verbs.c b/drivers/infiniband/hw/hfi1/verbs.c
index a38785e..6d27c85 100644
--- a/drivers/infiniband/hw/hfi1/verbs.c
+++ b/drivers/infiniband/hw/hfi1/verbs.c
@@ -1486,7 +1486,7 @@ static int query_port(struct rvt_dev_info *rdi, u8 port_num,
 	props->max_mtu = mtu_to_enum((!valid_ib_mtu(hfi1_max_mtu) ?
 				      4096 : hfi1_max_mtu), IB_MTU_4096);
 	props->active_mtu = !valid_ib_mtu(ppd->ibmtu) ? props->max_mtu :
-		mtu_to_enum(ppd->ibmtu, IB_MTU_2048);
+		mtu_to_enum(ppd->ibmtu, IB_MTU_4096);
 
 	/*
 	 * sm_lid of 0xFFFF needs special handling so that it can

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge window

[Dennis Dalessandro]

On 11/14/2017 7:34 AM, Dennis Dalessandro wrote:
> Hi Doug,
> 
> Here is the latest set of patches I have that we'd like to land in 4.15. There
> are some driver fixes, including a qib that is marked as stable. We also have
> a CM and SA patch thrown in the mix.
> 
> As always patches available for browsing at:
> https://github.com/ddalessa/kernel/tree/for-4.15
> 
> ---
> 
> Dennis Dalessandro (1):
>        IB/hfi1: Initialize bth1 in 16B rc ack builder

This is probably the only one that really needs to go for the rc. The 
rest can push off till 4.16 I think. This is trivial and prevents use of 
an uninitialized variable.

> Don Hiatt (2):
>        IB/hfi1: Mask the path bits with the LMC for 16B RC Acks
>        IB/CM: Change sgid to IB GID when handling CM request

These two fix problems introduced in 4.13

> Jan Sokolowski (1):
>        IB/hfi1: Use 4096 for default active MTU in query_qp

This is not a critical bug fix.

> Mike Marciniszyn (1):
>        IB/qib: Fix comparison error with qperf compare/swap test

This one is an older bug in qib.

> Venkata Sandeep Dhanalakota (1):
>        IB/SA: Check dlid before SA agent queries for ClassPortInfo

Still under discussion/rework.

Jason, if you want to pull:
https://patchwork.kernel.org/patch/10057487/
for the -rc I can submit a v2 of the other 5 once Sandeep's rework is ready.

-Denny


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge window

[Parav Pandit]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2623 bytes --]

Hi Dennis,

> -----Original Message-----
> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
> owner@vger.kernel.org] On Behalf Of Dennis Dalessandro
> Sent: Wednesday, November 29, 2017 12:43 PM
> To: dledford@redhat.com; Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>; Jan Sokolowski
> <jan.sokolowski@intel.com>; linux-rdma@vger.kernel.org; Michael J. Ruhl
> <michael.j.ruhl@intel.com>; Venkata Sandeep Dhanalakota
> <venkata.s.dhanalakota@intel.com>; Don Hiatt <don.hiatt@intel.com>; Ira
> Weiny <ira.weiny@intel.com>
> Subject: Re: [PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge
> window
> 
> On 11/14/2017 7:34 AM, Dennis Dalessandro wrote:
> > Hi Doug,
> >
> > Here is the latest set of patches I have that we'd like to land in
> > 4.15. There are some driver fixes, including a qib that is marked as
> > stable. We also have a CM and SA patch thrown in the mix.
> >
> > As always patches available for browsing at:
> > https://github.com/ddalessa/kernel/tree/for-4.15
> >
> > ---
> >
> > Dennis Dalessandro (1):
> >        IB/hfi1: Initialize bth1 in 16B rc ack builder
> 
> This is probably the only one that really needs to go for the rc. The rest can push
> off till 4.16 I think. This is trivial and prevents use of an uninitialized variable.
> 
> > Don Hiatt (2):
> >        IB/hfi1: Mask the path bits with the LMC for 16B RC Acks
> >        IB/CM: Change sgid to IB GID when handling CM request
> 
There are couple of queries and inputs to amend above patch of IB/CM.
Did you get a chance to address them?


> These two fix problems introduced in 4.13
> 
> > Jan Sokolowski (1):
> >        IB/hfi1: Use 4096 for default active MTU in query_qp
> 
> This is not a critical bug fix.
> 
> > Mike Marciniszyn (1):
> >        IB/qib: Fix comparison error with qperf compare/swap test
> 
> This one is an older bug in qib.
> 
> > Venkata Sandeep Dhanalakota (1):
> >        IB/SA: Check dlid before SA agent queries for ClassPortInfo
> 
> Still under discussion/rework.
> 
> Jason, if you want to pull:
> https://patchwork.kernel.org/patch/10057487/
> for the -rc I can submit a v2 of the other 5 once Sandeep's rework is ready.
> 
> -Denny
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body
> of a message to majordomo@vger.kernel.org More majordomo info at
> http://vger.kernel.org/majordomo-info.html
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±­ÙšŠ{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš

Re: [PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge window

[Don Hiatt]

On 11/29/2017 10:47 AM, Parav Pandit wrote:
> Hi Dennis,
>
>> -----Original Message-----
>> From: linux-rdma-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org [mailto:linux-rdma-
>> owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of Dennis Dalessandro
>> Sent: Wednesday, November 29, 2017 12:43 PM
>> To: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Jason Gunthorpe <jgg-uk2M96/98Pc@public.gmane.org>
>> Cc: Mike Marciniszyn <mike.marciniszyn-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Jan Sokolowski
>> <jan.sokolowski-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Michael J. Ruhl
>> <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Venkata Sandeep Dhanalakota
>> <venkata.s.dhanalakota-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Don Hiatt <don.hiatt-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>; Ira
>> Weiny <ira.weiny-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> Subject: Re: [PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge
>> window
>>
>> On 11/14/2017 7:34 AM, Dennis Dalessandro wrote:
>>> Hi Doug,
>>>
>>> Here is the latest set of patches I have that we'd like to land in
>>> 4.15. There are some driver fixes, including a qib that is marked as
>>> stable. We also have a CM and SA patch thrown in the mix.
>>>
>>> As always patches available for browsing at:
>>> https://github.com/ddalessa/kernel/tree/for-4.15
>>>
>>> ---
>>>
>>> Dennis Dalessandro (1):
>>>         IB/hfi1: Initialize bth1 in 16B rc ack builder
>> This is probably the only one that really needs to go for the rc. The rest can push
>> off till 4.16 I think. This is trivial and prevents use of an uninitialized variable.
>>
>>> Don Hiatt (2):
>>>         IB/hfi1: Mask the path bits with the LMC for 16B RC Acks
>>>         IB/CM: Change sgid to IB GID when handling CM request
> There are couple of queries and inputs to amend above patch of IB/CM.
> Did you get a chance to address them?
>
Hi Parva,

Sorry, I totally missed your comments. I'll look at the archive and 
address them.

Thanks,

don


>> These two fix problems introduced in 4.13
>>
>>> Jan Sokolowski (1):
>>>         IB/hfi1: Use 4096 for default active MTU in query_qp
>> This is not a critical bug fix.
>>
>>> Mike Marciniszyn (1):
>>>         IB/qib: Fix comparison error with qperf compare/swap test
>> This one is an older bug in qib.
>>
>>> Venkata Sandeep Dhanalakota (1):
>>>         IB/SA: Check dlid before SA agent queries for ClassPortInfo
>> Still under discussion/rework.
>>
>> Jason, if you want to pull:
>> https://patchwork.kernel.org/patch/10057487/
>> for the -rc I can submit a v2 of the other 5 once Sandeep's rework is ready.
>>
>> -Denny
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body
>> of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at
>> http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 0/6] IB/hfi1, qib, cm, sa: Last set for 4.15 merge window

[Jason Gunthorpe]

On Wed, Nov 29, 2017 at 01:43:03PM -0500, Dennis Dalessandro wrote:

> Jason, if you want to pull:
> https://patchwork.kernel.org/patch/10057487/
> for the -rc I can submit a v2 of the other 5 once Sandeep's rework is ready.

Okay thanks, patchworks reflects this now.

Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 6/6] IB/qib: Fix comparison error with qperf compare/swap test

[Dennis Dalessandro]

From: Mike Marciniszyn <mike.marciniszyn@intel.com>

This failure exists with qib:

ver_rc_compare_swap:
mismatch, sequence 2, expected 123456789abcdef, got 0

The request builder was using the incorrect inlines to
build the request header resulting in incorrect data
in the atomic header.

Fix by using the appropriate inlines to create the request.

Cc: <stable@vger.kernel.org> # 4.9.x+
Fixes: 261a4351844b ("IB/qib,IB/hfi: Use core common header file")
Reviewed-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
Signed-off-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
---
 drivers/infiniband/hw/qib/qib_rc.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/qib/qib_rc.c b/drivers/infiniband/hw/qib/qib_rc.c
index 8f5754f..e4a9ba1 100644
--- a/drivers/infiniband/hw/qib/qib_rc.c
+++ b/drivers/infiniband/hw/qib/qib_rc.c
@@ -434,13 +434,13 @@ int qib_make_rc_req(struct rvt_qp *qp, unsigned long *flags)
 				qp->s_state = OP(COMPARE_SWAP);
 				put_ib_ateth_swap(wqe->atomic_wr.swap,
 						  &ohdr->u.atomic_eth);
-				put_ib_ateth_swap(wqe->atomic_wr.compare_add,
-						  &ohdr->u.atomic_eth);
+				put_ib_ateth_compare(wqe->atomic_wr.compare_add,
+						     &ohdr->u.atomic_eth);
 			} else {
 				qp->s_state = OP(FETCH_ADD);
 				put_ib_ateth_swap(wqe->atomic_wr.compare_add,
 						  &ohdr->u.atomic_eth);
-				put_ib_ateth_swap(0, &ohdr->u.atomic_eth);
+				put_ib_ateth_compare(0, &ohdr->u.atomic_eth);
 			}
 			put_ib_ateth_vaddr(wqe->atomic_wr.remote_addr,
 					   &ohdr->u.atomic_eth);