[PATCH 0/8] RDMA/bnxt_re: Misc fixes for bnxt_re

[PATCH 0/8] RDMA/bnxt_re: Misc fixes for bnxt_re

[Somnath Kotur]

This patch series includes bug fixes for bnxt_re.
Doug,
	Please review apply for 4.14.
Thanks
Somnath

Devesh Sharma (2):
  bnxt_re: Fix update of qplib_qp.mtu when modified
  bnxt_re: Fix compare and swap atomic operands

Selvin Xavier (1):
  bnxt_re: Fix memory leak in FRMR path

Somnath Kotur (5):
  bnxt_re: Stop issuing further cmds to FW once a cmd times out
  bnxt_re: Free up devices in module_exit path
  bnxt_re: Fix race between the netdev register and unregister events
  bnxt_re: Remove RTNL lock dependency in bnxt_re_query_port
  bnxt_re: Don't issue cmd to delete GID for QP1 GID entry before the QP
    is destroyed

 drivers/infiniband/hw/bnxt_re/bnxt_re.h    | 14 +++++++----
 drivers/infiniband/hw/bnxt_re/ib_verbs.c   | 40 +++++++++++++++++++++---------
 drivers/infiniband/hw/bnxt_re/main.c       | 28 +++++++++++++++++++++
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c |  4 +++
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  3 ++-
 5 files changed, 71 insertions(+), 18 deletions(-)

-- 
1.8.3.1

--
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 1/8] bnxt_re: Fix update of qplib_qp.mtu when modified

[Somnath Kotur]

From: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

The MTU value in the qplib_qp.mtu should be
consistent with whatever mtu was set during
INIT to RTR.The Next PSN and number of packets
are calculated based on this member in the qplib_qp structure.

Signed-off-by: Narender Reddy <narender.reddy-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 43b3dee..6631d6e 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -1438,11 +1438,14 @@ int bnxt_re_modify_qp(struct ib_qp *ib_qp, struct ib_qp_attr *qp_attr,
 		qp->qplib_qp.modify_flags |=
 				CMDQ_MODIFY_QP_MODIFY_MASK_PATH_MTU;
 		qp->qplib_qp.path_mtu = __from_ib_mtu(qp_attr->path_mtu);
+		qp->qplib_qp.mtu = ib_mtu_enum_to_int(qp_attr->path_mtu);
 	} else if (qp_attr->qp_state == IB_QPS_RTR) {
 		qp->qplib_qp.modify_flags |=
 			CMDQ_MODIFY_QP_MODIFY_MASK_PATH_MTU;
 		qp->qplib_qp.path_mtu =
 			__from_ib_mtu(iboe_get_mtu(rdev->netdev->mtu));
+		qp->qplib_qp.mtu =
+			ib_mtu_enum_to_int(iboe_get_mtu(rdev->netdev->mtu));
 	}
 
 	if (qp_attr_mask & IB_QP_TIMEOUT) {
-- 
1.8.3.1

--
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 2/8] bnxt_re: Stop issuing further cmds to FW once a cmd times out

[Somnath Kotur]

Once a cmd to FW times out(after 20s) it is reasonable to
assume the FW or atleast the control path is dead.
No point issuing further cmds to the FW as each subsequent cmd
with another 20s timeout will cascade resulting in unnecessary
traces and/or NMI Lockups.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.c | 4 ++++
 drivers/infiniband/hw/bnxt_re/qplib_rcfw.h | 3 ++-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
index 391bb70..2bdb156 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
@@ -107,6 +107,9 @@ static int __send_message(struct bnxt_qplib_rcfw *rcfw, struct cmdq_base *req,
 		return -EINVAL;
 	}
 
+	if (test_bit(FIRMWARE_TIMED_OUT, &rcfw->flags))
+		return -ETIMEDOUT;
+
 	/* Cmdq are in 16-byte units, each request can consume 1 or more
 	 * cmdqe
 	 */
@@ -226,6 +229,7 @@ int bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
 		/* timed out */
 		dev_err(&rcfw->pdev->dev, "QPLIB: cmdq[%#x]=%#x timedout (%d)msec",
 			cookie, opcode, RCFW_CMD_WAIT_TIME_MS);
+		set_bit(FIRMWARE_TIMED_OUT, &rcfw->flags);
 		return rc;
 	}
 
diff --git a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
index 0ed312f..85b16da 100644
--- a/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
+++ b/drivers/infiniband/hw/bnxt_re/qplib_rcfw.h
@@ -162,8 +162,9 @@ struct bnxt_qplib_rcfw {
 	unsigned long		*cmdq_bitmap;
 	u32			bmap_size;
 	unsigned long		flags;
-#define FIRMWARE_INITIALIZED_FLAG	1
+#define FIRMWARE_INITIALIZED_FLAG	BIT(0)
 #define FIRMWARE_FIRST_FLAG		BIT(31)
+#define FIRMWARE_TIMED_OUT		BIT(3)
 	wait_queue_head_t	waitq;
 	int			(*aeq_handler)(struct bnxt_qplib_rcfw *,
 					       struct creq_func_event *);
-- 
1.8.3.1

--
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 3/8] bnxt_re: Fix compare and swap atomic operands

[Somnath Kotur]

From: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

Driver must assign the user supplied compare/swap values in
the wqe to successfully complete the atomic compare and
swap operation.

Signed-off-by: Devesh Sharma <devesh.sharma-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 6631d6e..712a62b 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -1913,6 +1913,7 @@ static int bnxt_re_build_atomic_wqe(struct ib_send_wr *wr,
 	switch (wr->opcode) {
 	case IB_WR_ATOMIC_CMP_AND_SWP:
 		wqe->type = BNXT_QPLIB_SWQE_TYPE_ATOMIC_CMP_AND_SWP;
+		wqe->atomic.cmp_data = atomic_wr(wr)->compare_add;
 		wqe->atomic.swap_data = atomic_wr(wr)->swap;
 		break;
 	case IB_WR_ATOMIC_FETCH_AND_ADD:
-- 
1.8.3.1

--
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 4/8] bnxt_re: Free up devices in module_exit path

[Somnath Kotur]

Clean up all devices added to the bnxt_re_dev_list in the
module_exit entry point.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/main.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 82d1cbc..00a3b74 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1375,6 +1375,22 @@ static int __init bnxt_re_mod_init(void)
 
 static void __exit bnxt_re_mod_exit(void)
 {
+	struct bnxt_re_dev *rdev;
+	LIST_HEAD(to_be_deleted);
+
+	mutex_lock(&bnxt_re_dev_lock);
+	/* Free all adapter allocated resources */
+	if (!list_empty(&bnxt_re_dev_list))
+		list_splice_init(&bnxt_re_dev_list, &to_be_deleted);
+	mutex_unlock(&bnxt_re_dev_lock);
+
+	list_for_each_entry(rdev, &to_be_deleted, list) {
+		dev_info(rdev_to_dev(rdev), "Unregistering Device");
+		bnxt_re_dev_stop(rdev);
+		bnxt_re_ib_unreg(rdev, true);
+		bnxt_re_remove_one(rdev);
+		bnxt_re_dev_unreg(rdev);
+	}
 	unregister_netdevice_notifier(&bnxt_re_netdev_notifier);
 	if (bnxt_re_wq)
 		destroy_workqueue(bnxt_re_wq);
-- 
1.8.3.1

--
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 4/8] bnxt_re: Free up devices in module_exit path

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1736 bytes --]

On Thu, Aug 31, 2017 at 09:27:31AM +0530, Somnath Kotur wrote:
> Clean up all devices added to the bnxt_re_dev_list in the
> module_exit entry point.
>
> Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/infiniband/hw/bnxt_re/main.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> index 82d1cbc..00a3b74 100644
> --- a/drivers/infiniband/hw/bnxt_re/main.c
> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> @@ -1375,6 +1375,22 @@ static int __init bnxt_re_mod_init(void)
>
>  static void __exit bnxt_re_mod_exit(void)
>  {
> +	struct bnxt_re_dev *rdev;
> +	LIST_HEAD(to_be_deleted);
> +
> +	mutex_lock(&bnxt_re_dev_lock);
> +	/* Free all adapter allocated resources */
> +	if (!list_empty(&bnxt_re_dev_list))

Hi Somnath,

How is it possible to have the bnxt_re_dev_list to be empty at this
stage? Doesn't the call to bnxt_re_mod_exit mean that at least one entry
exists?

Thanks

> +		list_splice_init(&bnxt_re_dev_list, &to_be_deleted);
> +	mutex_unlock(&bnxt_re_dev_lock);
> +
> +	list_for_each_entry(rdev, &to_be_deleted, list) {
> +		dev_info(rdev_to_dev(rdev), "Unregistering Device");
> +		bnxt_re_dev_stop(rdev);
> +		bnxt_re_ib_unreg(rdev, true);
> +		bnxt_re_remove_one(rdev);
> +		bnxt_re_dev_unreg(rdev);
> +	}
>  	unregister_netdevice_notifier(&bnxt_re_netdev_notifier);
>  	if (bnxt_re_wq)
>  		destroy_workqueue(bnxt_re_wq);
> --
> 1.8.3.1
>
> --
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/8] bnxt_re: Free up devices in module_exit path

[Somnath Kotur]

Hi Leon,

On Wed, Sep 13, 2017 at 11:12 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Aug 31, 2017 at 09:27:31AM +0530, Somnath Kotur wrote:
>> Clean up all devices added to the bnxt_re_dev_list in the
>> module_exit entry point.
>>
>> Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
>> ---
>>  drivers/infiniband/hw/bnxt_re/main.c | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
>> index 82d1cbc..00a3b74 100644
>> --- a/drivers/infiniband/hw/bnxt_re/main.c
>> +++ b/drivers/infiniband/hw/bnxt_re/main.c
>> @@ -1375,6 +1375,22 @@ static int __init bnxt_re_mod_init(void)
>>
>>  static void __exit bnxt_re_mod_exit(void)
>>  {
>> +     struct bnxt_re_dev *rdev;
>> +     LIST_HEAD(to_be_deleted);
>> +
>> +     mutex_lock(&bnxt_re_dev_lock);
>> +     /* Free all adapter allocated resources */
>> +     if (!list_empty(&bnxt_re_dev_list))
>
> Hi Somnath,
>
> How is it possible to have the bnxt_re_dev_list to be empty at this
> stage? Doesn't the call to bnxt_re_mod_exit mean that at least one entry
> exists?
Well, there is a possibility that NETDEV_UNREGISTER got invoked (not
via rmmod , but via the pci-sysfs route)
and this list is empty by the time rmmod of the bnxt_re was invoked.

Thanks
Som
--
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 4/8] bnxt_re: Free up devices in module_exit path

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 1665 bytes --]

On Thu, Sep 14, 2017 at 09:23:53AM +0530, Somnath Kotur wrote:
> Hi Leon,
>
> On Wed, Sep 13, 2017 at 11:12 PM, Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Thu, Aug 31, 2017 at 09:27:31AM +0530, Somnath Kotur wrote:
> >> Clean up all devices added to the bnxt_re_dev_list in the
> >> module_exit entry point.
> >>
> >> Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
> >> ---
> >>  drivers/infiniband/hw/bnxt_re/main.c | 16 ++++++++++++++++
> >>  1 file changed, 16 insertions(+)
> >>
> >> diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
> >> index 82d1cbc..00a3b74 100644
> >> --- a/drivers/infiniband/hw/bnxt_re/main.c
> >> +++ b/drivers/infiniband/hw/bnxt_re/main.c
> >> @@ -1375,6 +1375,22 @@ static int __init bnxt_re_mod_init(void)
> >>
> >>  static void __exit bnxt_re_mod_exit(void)
> >>  {
> >> +     struct bnxt_re_dev *rdev;
> >> +     LIST_HEAD(to_be_deleted);
> >> +
> >> +     mutex_lock(&bnxt_re_dev_lock);
> >> +     /* Free all adapter allocated resources */
> >> +     if (!list_empty(&bnxt_re_dev_list))
> >
> > Hi Somnath,
> >
> > How is it possible to have the bnxt_re_dev_list to be empty at this
> > stage? Doesn't the call to bnxt_re_mod_exit mean that at least one entry
> > exists?
> Well, there is a possibility that NETDEV_UNREGISTER got invoked (not
> via rmmod , but via the pci-sysfs route)
> and this list is empty by the time rmmod of the bnxt_re was invoked.

I was under impression that calls to bnxt_re_ib_unreg() and bnxt_re_remove_one()
in NETDEV_UNREGISTER should eliminate such scenario.

Thanks

>
> Thanks
> Som

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 5/8] bnxt_re: Fix race between the netdev register and unregister events

[Somnath Kotur]

Upon receipt of the NETDEV_REGISTER event from the netdev notifier chain,
the IB stack registration is spawned off to a workqueue since that also
requires an rtnl lock.
There could be 2 kinds of races between the NETDEV_REGISTER and the
NETDEV_UNREGISTER event handling.
a)The NETDEV_UNREGISTER event is received in rapid succession after
the NETDEV_REGISTER event even before the work queue got a chance to run.
b)The NETDEV_UNREGISTER event is received while the workqueue that handles
registration with the IB stack is still in progress.
Handle both the races with a bit flag that is set just before the work item
is queued and cleared in the workqueue after the event is handled just
before the workqueue item is freed.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/bnxt_re.h | 12 +++++++-----
 drivers/infiniband/hw/bnxt_re/main.c    |  8 ++++++++
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
index b3ad37f..a25f9d2 100644
--- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
+++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
@@ -93,11 +93,13 @@ struct bnxt_re_dev {
 	struct ib_device		ibdev;
 	struct list_head		list;
 	unsigned long			flags;
-#define BNXT_RE_FLAG_NETDEV_REGISTERED	0
-#define BNXT_RE_FLAG_IBDEV_REGISTERED	1
-#define BNXT_RE_FLAG_GOT_MSIX		2
-#define BNXT_RE_FLAG_RCFW_CHANNEL_EN	8
-#define BNXT_RE_FLAG_QOS_WORK_REG	16
+#define BNXT_RE_FLAG_NETDEV_REGISTERED		0
+#define BNXT_RE_FLAG_IBDEV_REGISTERED		1
+#define BNXT_RE_FLAG_GOT_MSIX			2
+#define BNXT_RE_FLAG_HAVE_L2_REF		3
+#define BNXT_RE_FLAG_RCFW_CHANNEL_EN		4
+#define BNXT_RE_FLAG_QOS_WORK_REG		5
+#define BNXT_RE_FLAG_TASK_IN_PROG		6
 	struct net_device		*netdev;
 	unsigned int			version, major, minor;
 	struct bnxt_en_dev		*en_dev;
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 00a3b74..29c3d7e 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1259,6 +1259,8 @@ static void bnxt_re_task(struct work_struct *work)
 	default:
 		break;
 	}
+	smp_mb__before_atomic();
+	clear_bit(BNXT_RE_FLAG_TASK_IN_PROG, &rdev->flags);
 	kfree(re_work);
 }
 
@@ -1317,6 +1319,11 @@ static int bnxt_re_netdev_event(struct notifier_block *notifier,
 		break;
 
 	case NETDEV_UNREGISTER:
+		/* netdev notifier will call NETDEV_UNREGISTER again later since
+		 * we are still holding the reference to the netdev
+		 */
+		if (test_bit(BNXT_RE_FLAG_TASK_IN_PROG, &rdev->flags))
+			goto exit;
 		bnxt_re_ib_unreg(rdev, false);
 		bnxt_re_remove_one(rdev);
 		bnxt_re_dev_unreg(rdev);
@@ -1335,6 +1342,7 @@ static int bnxt_re_netdev_event(struct notifier_block *notifier,
 			re_work->vlan_dev = (real_dev == netdev ?
 					     NULL : netdev);
 			INIT_WORK(&re_work->work, bnxt_re_task);
+			set_bit(BNXT_RE_FLAG_TASK_IN_PROG, &rdev->flags);
 			queue_work(bnxt_re_wq, &re_work->work);
 		}
 	}
-- 
1.8.3.1

--
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 5/8] bnxt_re: Fix race between the netdev register and unregister events

[Doug Ledford]

On Thu, 2017-08-31 at 09:27 +0530, Somnath Kotur wrote:
> diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> index b3ad37f..a25f9d2 100644
> --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> @@ -93,11 +93,13 @@ struct bnxt_re_dev {
>         struct ib_device                ibdev;
>         struct list_head                list;
>         unsigned long                   flags;
> -#define BNXT_RE_FLAG_NETDEV_REGISTERED 0
> -#define BNXT_RE_FLAG_IBDEV_REGISTERED  1
> -#define BNXT_RE_FLAG_GOT_MSIX          2
> -#define BNXT_RE_FLAG_RCFW_CHANNEL_EN   8
> -#define BNXT_RE_FLAG_QOS_WORK_REG      16
> +#define BNXT_RE_FLAG_NETDEV_REGISTERED         0
> +#define BNXT_RE_FLAG_IBDEV_REGISTERED          1
> +#define BNXT_RE_FLAG_GOT_MSIX                  2
> +#define BNXT_RE_FLAG_HAVE_L2_REF               3
> +#define BNXT_RE_FLAG_RCFW_CHANNEL_EN           4
> +#define BNXT_RE_FLAG_QOS_WORK_REG              5
> +#define BNXT_RE_FLAG_TASK_IN_PROG              6
>         struct net_device               *netdev;

So, this is making two changes in once and one of the changes is not
documented.  In particular, it looks like the usage of your flags
elements was originally the actual value of 1 << bit, which is the
wrong thing to pass to the *_bit() operations that want a bit number. 
So you're both adding a new used bit in your flags, but also changing
your flags to be better numbered for use with the *_bit operations.  I
did a quick check and the BNXT_RE_FLAGS* items are never used without
using the bit operations, so this change is safe.  I'm going to update
the commit log when I take this to make that clear.  Generally though,
this should have been two patches.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

--
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 5/8] bnxt_re: Fix race between the netdev register and unregister events

[Somnath Kotur]

On Fri, Sep 22, 2017 at 11:25 PM, Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
>
> On Thu, 2017-08-31 at 09:27 +0530, Somnath Kotur wrote:
> > diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > index b3ad37f..a25f9d2 100644
> > --- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > +++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
> > @@ -93,11 +93,13 @@ struct bnxt_re_dev {
> >         struct ib_device                ibdev;
> >         struct list_head                list;
> >         unsigned long                   flags;
> > -#define BNXT_RE_FLAG_NETDEV_REGISTERED 0
> > -#define BNXT_RE_FLAG_IBDEV_REGISTERED  1
> > -#define BNXT_RE_FLAG_GOT_MSIX          2
> > -#define BNXT_RE_FLAG_RCFW_CHANNEL_EN   8
> > -#define BNXT_RE_FLAG_QOS_WORK_REG      16
> > +#define BNXT_RE_FLAG_NETDEV_REGISTERED         0
> > +#define BNXT_RE_FLAG_IBDEV_REGISTERED          1
> > +#define BNXT_RE_FLAG_GOT_MSIX                  2
> > +#define BNXT_RE_FLAG_HAVE_L2_REF               3
> > +#define BNXT_RE_FLAG_RCFW_CHANNEL_EN           4
> > +#define BNXT_RE_FLAG_QOS_WORK_REG              5
> > +#define BNXT_RE_FLAG_TASK_IN_PROG              6
> >         struct net_device               *netdev;
>
> So, this is making two changes in once and one of the changes is not
> documented.  In particular, it looks like the usage of your flags
> elements was originally the actual value of 1 << bit, which is the
> wrong thing to pass to the *_bit() operations that want a bit number.
> So you're both adding a new used bit in your flags, but also changing
> your flags to be better numbered for use with the *_bit operations.  I
> did a quick check and the BNXT_RE_FLAGS* items are never used without
> using the bit operations, so this change is safe.  I'm going to update
> the commit log when I take this to make that clear.  Generally though,
> this should have been two patches.
Yes, the intention was to fix the incorrect usage and yes it should
have found a mention in the commit log, missed it somehow.
Thanks for taking care of that Doug

-Som


>
>
> --
> Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
>     GPG KeyID: B826A3330E572FDD
>     Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD
>
--
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 6/8] bnxt_re: Remove RTNL lock dependency in bnxt_re_query_port

[Somnath Kotur]

When there is a NETDEV_UNREGISTER event, bnxt_re driver calls
ib_unregister_device() (RTNL lock held).
ib_unregister_device attempts to flush a worker queue scheduled by
ib_core and that queue might have a pending ib_query_port().
ib_query_port in turn calls bnxt_re_query_port(), which while querying the
link speed using ib_get_eth_speed(), tries to acquire the rtnl_lock() which
was already held by NETDEV_UNREGISTER.
Fixing the issue by removing the link speed query from bnxt_re_query_port()
Now the speed is queried post a successful ib_register_device or whenever
there is a NETDEV_CHANGE event.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/bnxt_re.h  |  2 ++
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 11 +++--------
 drivers/infiniband/hw/bnxt_re/main.c     |  4 ++++
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/bnxt_re.h b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
index a25f9d2..ecbac91 100644
--- a/drivers/infiniband/hw/bnxt_re/bnxt_re.h
+++ b/drivers/infiniband/hw/bnxt_re/bnxt_re.h
@@ -110,6 +110,8 @@ struct bnxt_re_dev {
 
 	struct delayed_work		worker;
 	u8				cur_prio_map;
+	u8				active_speed;
+	u8				active_width;
 
 	/* FP Notification Queue (CQ & SRQ) */
 	struct tasklet_struct		nq_task;
diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 712a62b..cf0440c 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -259,14 +259,9 @@ int bnxt_re_query_port(struct ib_device *ibdev, u8 port_num,
 	port_attr->sm_sl = 0;
 	port_attr->subnet_timeout = 0;
 	port_attr->init_type_reply = 0;
-	/* call the underlying netdev's ethtool hooks to query speed settings
-	 * for which we acquire rtnl_lock _only_ if it's registered with
-	 * IB stack to avoid race in the NETDEV_UNREG path
-	 */
-	if (test_bit(BNXT_RE_FLAG_IBDEV_REGISTERED, &rdev->flags))
-		if (ib_get_eth_speed(ibdev, port_num, &port_attr->active_speed,
-				     &port_attr->active_width))
-			return -EINVAL;
+	port_attr->active_speed = rdev->active_speed;
+	port_attr->active_width = rdev->active_width;
+
 	return 0;
 }
 
diff --git a/drivers/infiniband/hw/bnxt_re/main.c b/drivers/infiniband/hw/bnxt_re/main.c
index 29c3d7e..e7450ea 100644
--- a/drivers/infiniband/hw/bnxt_re/main.c
+++ b/drivers/infiniband/hw/bnxt_re/main.c
@@ -1161,6 +1161,8 @@ static int bnxt_re_ib_reg(struct bnxt_re_dev *rdev)
 		}
 	}
 	set_bit(BNXT_RE_FLAG_IBDEV_REGISTERED, &rdev->flags);
+	ib_get_eth_speed(&rdev->ibdev, 1, &rdev->active_speed,
+			 &rdev->active_width);
 	bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1, IB_EVENT_PORT_ACTIVE);
 	bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1, IB_EVENT_GID_CHANGE);
 
@@ -1255,6 +1257,8 @@ static void bnxt_re_task(struct work_struct *work)
 		else if (netif_carrier_ok(rdev->netdev))
 			bnxt_re_dispatch_event(&rdev->ibdev, NULL, 1,
 					       IB_EVENT_PORT_ACTIVE);
+		ib_get_eth_speed(&rdev->ibdev, 1, &rdev->active_speed,
+				 &rdev->active_width);
 		break;
 	default:
 		break;
-- 
1.8.3.1

--
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 7/8] bnxt_re: Fix memory leak in FRMR path

[Somnath Kotur]

From: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>

This patch fixes a memory leak issue when alloc_mr is used.
mr->pages and mr->npages are used only in alloc_mr path. mr->pages
is allocated when alloc_mr is called or in the case of FRMR, while
creating the MR. mr->npages is updated only when the MR created
is used i.e. after invoking map_mr_sg verb, before data transfer.
In the dereg_mr path, if mr->npages is 0, driver ends up not freeing
the memory created.
Removing the npages check from the dereg_mr path for kernel consumers.

Signed-off-by: Selvin Xavier <selvin.xavier-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index cf0440c..94ab9e1 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -3063,7 +3063,7 @@ int bnxt_re_dereg_mr(struct ib_mr *ib_mr)
 		return rc;
 	}
 
-	if (mr->npages && mr->pages) {
+	if (mr->pages) {
 		rc = bnxt_qplib_free_fast_reg_page_list(&rdev->qplib_res,
 							&mr->qplib_frpl);
 		kfree(mr->pages);
-- 
1.8.3.1

--
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 8/8] bnxt_re: Don't issue cmd to delete GID for QP1 GID entry before the QP is destroyed

[Somnath Kotur]

FW needs the 0th GID Entry in the Table to be preserved before
it's corresponding QP1 is deleted, else it will fail the cmd.
Check for the same and return to prevent error msg being logged for
cmd failure.

Signed-off-by: Somnath Kotur <somnath.kotur-dY08KVG/lbpWk0Htik3J/w@public.gmane.org>
---
 drivers/infiniband/hw/bnxt_re/ib_verbs.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/bnxt_re/ib_verbs.c b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
index 94ab9e1..a5f1ada 100644
--- a/drivers/infiniband/hw/bnxt_re/ib_verbs.c
+++ b/drivers/infiniband/hw/bnxt_re/ib_verbs.c
@@ -314,6 +314,7 @@ int bnxt_re_del_gid(struct ib_device *ibdev, u8 port_num,
 	struct bnxt_re_gid_ctx *ctx, **ctx_tbl;
 	struct bnxt_re_dev *rdev = to_bnxt_re_dev(ibdev, ibdev);
 	struct bnxt_qplib_sgid_tbl *sgid_tbl = &rdev->qplib_res.sgid_tbl;
+	struct bnxt_qplib_gid *gid_to_del;
 
 	/* Delete the entry from the hardware */
 	ctx = *context;
@@ -323,11 +324,25 @@ int bnxt_re_del_gid(struct ib_device *ibdev, u8 port_num,
 	if (sgid_tbl && sgid_tbl->active) {
 		if (ctx->idx >= sgid_tbl->max)
 			return -EINVAL;
+		gid_to_del = &sgid_tbl->tbl[ctx->idx];
+		/* DEL_GID is called in WQ context(netdevice_event_work_handler)
+		 * or via the ib_unregister_device path. In the former case QP1
+		 * may not be destroyed yet, in which case just return as FW
+		 * needs that entry to be present and will fail it's deletion.
+		 * We could get invoked again after QP1 is destroyed OR get an
+		 * ADD_GID call with a different GID value for the same index
+		 * where we issue MODIFY_GID cmd to update the GID entry -- TBD
+		 */
+		if (ctx->idx == 0 &&
+		    rdma_link_local_addr((struct in6_addr *)gid_to_del) &&
+		    ctx->refcnt == 1 && rdev->qp1_sqp) {
+			dev_dbg(rdev_to_dev(rdev),
+				"Trying to delete GID0 while QP1 is alive\n");
+			return -EFAULT;
+		}
 		ctx->refcnt--;
 		if (!ctx->refcnt) {
-			rc = bnxt_qplib_del_sgid(sgid_tbl,
-						 &sgid_tbl->tbl[ctx->idx],
-						 true);
+			rc = bnxt_qplib_del_sgid(sgid_tbl, gid_to_del, true);
 			if (rc) {
 				dev_err(rdev_to_dev(rdev),
 					"Failed to remove GID: %#x", rc);
@@ -811,6 +826,8 @@ int bnxt_re_destroy_qp(struct ib_qp *ib_qp)
 
 		kfree(rdev->sqp_ah);
 		kfree(rdev->qp1_sqp);
+		rdev->qp1_sqp = NULL;
+		rdev->sqp_ah = NULL;
 	}
 
 	if (!IS_ERR_OR_NULL(qp->rumem))
-- 
1.8.3.1

--
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 0/8] RDMA/bnxt_re: Misc fixes for bnxt_re

[Doug Ledford]

On Thu, 2017-08-31 at 09:27 +0530, Somnath Kotur wrote:
> This patch series includes bug fixes for bnxt_re.
> Doug,
> 	Please review apply for 4.14.
> Thanks
> Somnath
> 
> Devesh Sharma (2):
>   bnxt_re: Fix update of qplib_qp.mtu when modified
>   bnxt_re: Fix compare and swap atomic operands
> 
> Selvin Xavier (1):
>   bnxt_re: Fix memory leak in FRMR path
> 
> Somnath Kotur (5):
>   bnxt_re: Stop issuing further cmds to FW once a cmd times out
>   bnxt_re: Free up devices in module_exit path
>   bnxt_re: Fix race between the netdev register and unregister events
>   bnxt_re: Remove RTNL lock dependency in bnxt_re_query_port
>   bnxt_re: Don't issue cmd to delete GID for QP1 GID entry before the
> QP
>     is destroyed
> 
>  drivers/infiniband/hw/bnxt_re/bnxt_re.h    | 14 +++++++----
>  drivers/infiniband/hw/bnxt_re/ib_verbs.c   | 40
> +++++++++++++++++++++---------
>  drivers/infiniband/hw/bnxt_re/main.c       | 28
> +++++++++++++++++++++
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.c |  4 +++
>  drivers/infiniband/hw/bnxt_re/qplib_rcfw.h |  3 ++-
>  5 files changed, 71 insertions(+), 18 deletions(-)

Thanks, series (with fixed commit message in 5/8) applied.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

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