[PATCH rdma-next 00/14] RAW format dumps through RDMAtool

[PATCH rdma-next 00/14] RAW format dumps through RDMAtool

[Leon Romanovsky]

From: Leon Romanovsky <leonro@mellanox.com>

From Maor:

Hi,

The following series adds support to get the RDMA resource data in RAW
format. The main motivation for doing this is to enable vendors to return
the entire QP/CQ/MR data without a need from the vendor to set each field
separately.

Thanks

Maor Gottlieb (14):
  net/mlx5: Export resource dump interface
  net/mlx5: Add support in query QP, CQ and MKEY segments
  RDMA/core: Fix double put of resource
  RDMA/core: Allow to override device op
  RDMA/core: Don't call fill_res_entry for PD
  RDMA/core: Add restrack dummy ops
  RDMA: Add dedicated MR resource tracker function
  RDMA: Add a dedicated CQ resource tracker function
  RDMA: Add a dedicated QP resource tracker function
  RDMA: Add dedicated cm id resource tracker function
  RDMA: Add support to dump resource tracker in RAW format
  RDMA/mlx5: Add support to get QP resource in raw format
  RDMA/mlx5: Add support to get CQ resource in RAW format
  RDMA/mlx5: Add support to get MR resource in RAW format

 drivers/infiniband/core/device.c              |  16 ++-
 drivers/infiniband/core/nldev.c               | 136 ++++++++----------
 drivers/infiniband/core/restrack.c            |  32 +++++
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h        |   7 +-
 drivers/infiniband/hw/cxgb4/provider.c        |  11 +-
 drivers/infiniband/hw/cxgb4/restrack.c        |  33 ++---
 drivers/infiniband/hw/hns/hns_roce_device.h   |   4 +-
 drivers/infiniband/hw/hns/hns_roce_main.c     |   2 +-
 drivers/infiniband/hw/hns/hns_roce_restrack.c |  17 +--
 drivers/infiniband/hw/mlx5/main.c             |   6 +-
 drivers/infiniband/hw/mlx5/mlx5_ib.h          |  11 +-
 drivers/infiniband/hw/mlx5/restrack.c         | 105 +++++++++++---
 .../mellanox/mlx5/core/diag/rsc_dump.c        |   6 +
 .../mellanox/mlx5/core/diag/rsc_dump.h        |  33 +----
 .../diag => include/linux/mlx5}/rsc_dump.h    |  25 ++--
 include/rdma/ib_verbs.h                       |  13 +-
 include/uapi/rdma/rdma_netlink.h              |   2 +
 17 files changed, 258 insertions(+), 201 deletions(-)
 copy {drivers/net/ethernet/mellanox/mlx5/core/diag => include/linux/mlx5}/rsc_dump.h (68%)

--
2.26.2

[PATCH mlx5-next 01/14] net/mlx5: Export resource dump interface

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Export some of the resource dump API, so it could be
used by the mlx5_ib driver as well.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 .../mellanox/mlx5/core/diag/rsc_dump.c        |  3 ++
 .../mellanox/mlx5/core/diag/rsc_dump.h        | 33 +------------------
 .../diag => include/linux/mlx5}/rsc_dump.h    | 22 ++++---------
 3 files changed, 10 insertions(+), 48 deletions(-)
 copy {drivers/net/ethernet/mellanox/mlx5/core/diag => include/linux/mlx5}/rsc_dump.h (68%)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c
index 17ab7efe693d..10218c2324cc 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c
@@ -130,11 +130,13 @@ struct mlx5_rsc_dump_cmd *mlx5_rsc_dump_cmd_create(struct mlx5_core_dev *dev,
 	cmd->mem_size = key->size;
 	return cmd;
 }
+EXPORT_SYMBOL(mlx5_rsc_dump_cmd_create);
 
 void mlx5_rsc_dump_cmd_destroy(struct mlx5_rsc_dump_cmd *cmd)
 {
 	kfree(cmd);
 }
+EXPORT_SYMBOL(mlx5_rsc_dump_cmd_destroy);
 
 int mlx5_rsc_dump_next(struct mlx5_core_dev *dev, struct mlx5_rsc_dump_cmd *cmd,
 		       struct page *page, int *size)
@@ -155,6 +157,7 @@ int mlx5_rsc_dump_next(struct mlx5_core_dev *dev, struct mlx5_rsc_dump_cmd *cmd,
 
 	return more_dump;
 }
+EXPORT_SYMBOL(mlx5_rsc_dump_next);
 
 #define MLX5_RSC_DUMP_MENU_SEGMENT 0xffff
 static int mlx5_rsc_dump_menu(struct mlx5_core_dev *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h b/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h
index 148270073e71..64c4956db6d2 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h
@@ -4,41 +4,10 @@
 #ifndef __MLX5_RSC_DUMP_H
 #define __MLX5_RSC_DUMP_H
 
+#include <linux/mlx5/rsc_dump.h>
 #include <linux/mlx5/driver.h>
 #include "mlx5_core.h"
 
-enum mlx5_sgmt_type {
-	MLX5_SGMT_TYPE_HW_CQPC,
-	MLX5_SGMT_TYPE_HW_SQPC,
-	MLX5_SGMT_TYPE_HW_RQPC,
-	MLX5_SGMT_TYPE_FULL_SRQC,
-	MLX5_SGMT_TYPE_FULL_CQC,
-	MLX5_SGMT_TYPE_FULL_EQC,
-	MLX5_SGMT_TYPE_FULL_QPC,
-	MLX5_SGMT_TYPE_SND_BUFF,
-	MLX5_SGMT_TYPE_RCV_BUFF,
-	MLX5_SGMT_TYPE_SRQ_BUFF,
-	MLX5_SGMT_TYPE_CQ_BUFF,
-	MLX5_SGMT_TYPE_EQ_BUFF,
-	MLX5_SGMT_TYPE_SX_SLICE,
-	MLX5_SGMT_TYPE_SX_SLICE_ALL,
-	MLX5_SGMT_TYPE_RDB,
-	MLX5_SGMT_TYPE_RX_SLICE_ALL,
-	MLX5_SGMT_TYPE_MENU,
-	MLX5_SGMT_TYPE_TERMINATE,
-
-	MLX5_SGMT_TYPE_NUM, /* Keep last */
-};
-
-struct mlx5_rsc_key {
-	enum mlx5_sgmt_type rsc;
-	int index1;
-	int index2;
-	int num_of_obj1;
-	int num_of_obj2;
-	int size;
-};
-
 #define MLX5_RSC_DUMP_ALL 0xFFFF
 struct mlx5_rsc_dump_cmd;
 struct mlx5_rsc_dump;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h b/include/linux/mlx5/rsc_dump.h
similarity index 68%
copy from drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h
copy to include/linux/mlx5/rsc_dump.h
index 148270073e71..87415fa754fe 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.h
+++ b/include/linux/mlx5/rsc_dump.h
@@ -1,11 +1,10 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* Copyright (c) 2019 Mellanox Technologies. */
-
-#ifndef __MLX5_RSC_DUMP_H
-#define __MLX5_RSC_DUMP_H
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2020 Mellanox Technologies inc. */
 
 #include <linux/mlx5/driver.h>
-#include "mlx5_core.h"
+
+#ifndef __MLX5_RSC_DUMP
+#define __MLX5_RSC_DUMP
 
 enum mlx5_sgmt_type {
 	MLX5_SGMT_TYPE_HW_CQPC,
@@ -39,20 +38,11 @@ struct mlx5_rsc_key {
 	int size;
 };
 
-#define MLX5_RSC_DUMP_ALL 0xFFFF
 struct mlx5_rsc_dump_cmd;
-struct mlx5_rsc_dump;
-
-struct mlx5_rsc_dump *mlx5_rsc_dump_create(struct mlx5_core_dev *dev);
-void mlx5_rsc_dump_destroy(struct mlx5_core_dev *dev);
-
-int mlx5_rsc_dump_init(struct mlx5_core_dev *dev);
-void mlx5_rsc_dump_cleanup(struct mlx5_core_dev *dev);
 
 struct mlx5_rsc_dump_cmd *mlx5_rsc_dump_cmd_create(struct mlx5_core_dev *dev,
 						   struct mlx5_rsc_key *key);
 void mlx5_rsc_dump_cmd_destroy(struct mlx5_rsc_dump_cmd *cmd);
-
 int mlx5_rsc_dump_next(struct mlx5_core_dev *dev, struct mlx5_rsc_dump_cmd *cmd,
 		       struct page *page, int *size);
-#endif
+#endif /* __MLX5_RSC_DUMP */
-- 
2.26.2

Re: [PATCH mlx5-next 01/14] net/mlx5: Export resource dump interface

[Jason Gunthorpe]

On Wed, May 13, 2020 at 12:50:21PM +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@mellanox.com>
> 
> Export some of the resource dump API, so it could be
> used by the mlx5_ib driver as well.

This description doesn't really match the patch, is this other stuff
dead code?

Jason

Re: [PATCH mlx5-next 01/14] net/mlx5: Export resource dump interface

[Maor Gottlieb]

On 5/25/2020 5:24 PM, Jason Gunthorpe wrote:
> On Wed, May 13, 2020 at 12:50:21PM +0300, Leon Romanovsky wrote:
>> From: Maor Gottlieb <maorg@mellanox.com>
>>
>> Export some of the resource dump API, so it could be
>> used by the mlx5_ib driver as well.
> This description doesn't really match the patch, is this other stuff
> dead code?
>
> Jason

It is used in later patch, I can clarify it better in the commit message.

[PATCH mlx5-next 02/14] net/mlx5: Add support in query QP, CQ and MKEY segments

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Introduce new resource dump segments - PRM_QUERY_QP,
PRM_QUERY_CQ and PRM_QUERY_MKEY. These segments contains the resource
dump in PRM query format.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c | 3 +++
 include/linux/mlx5/rsc_dump.h                           | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c
index 10218c2324cc..4924a5658853 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/rsc_dump.c
@@ -23,6 +23,9 @@ static const char *const mlx5_rsc_sgmt_name[] = {
 	MLX5_SGMT_STR_ASSING(SX_SLICE_ALL),
 	MLX5_SGMT_STR_ASSING(RDB),
 	MLX5_SGMT_STR_ASSING(RX_SLICE_ALL),
+	MLX5_SGMT_STR_ASSING(PRM_QUERY_QP),
+	MLX5_SGMT_STR_ASSING(PRM_QUERY_CQ),
+	MLX5_SGMT_STR_ASSING(PRM_QUERY_MKEY),
 };
 
 struct mlx5_rsc_dump {
diff --git a/include/linux/mlx5/rsc_dump.h b/include/linux/mlx5/rsc_dump.h
index 87415fa754fe..d11c0b228620 100644
--- a/include/linux/mlx5/rsc_dump.h
+++ b/include/linux/mlx5/rsc_dump.h
@@ -23,6 +23,9 @@ enum mlx5_sgmt_type {
 	MLX5_SGMT_TYPE_SX_SLICE_ALL,
 	MLX5_SGMT_TYPE_RDB,
 	MLX5_SGMT_TYPE_RX_SLICE_ALL,
+	MLX5_SGMT_TYPE_PRM_QUERY_QP,
+	MLX5_SGMT_TYPE_PRM_QUERY_CQ,
+	MLX5_SGMT_TYPE_PRM_QUERY_MKEY,
 	MLX5_SGMT_TYPE_MENU,
 	MLX5_SGMT_TYPE_TERMINATE,
 
-- 
2.26.2

[PATCH rdma-next 03/14] RDMA/core: Fix double put of resource

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Avoid decrease reference count of resource tracker object in the error
flow of res_get_common_doit.

Fixes: c5dfe0ea6ffa ("RDMA/nldev: Add resource tracker doit callback")
Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 9eec26d10d7b..e16105be2eb2 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -1292,11 +1292,10 @@ static int res_get_common_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	has_cap_net_admin = netlink_capable(skb, CAP_NET_ADMIN);
 
 	ret = fill_func(msg, has_cap_net_admin, res, port);
-
-	rdma_restrack_put(res);
 	if (ret)
 		goto err_free;
 
+	rdma_restrack_put(res);
 	nlmsg_end(msg, nlh);
 	ib_device_put(device);
 	return rdma_nl_unicast(sock_net(skb->sk), msg, NETLINK_CB(skb).portid);
-- 
2.26.2

[PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Current device ops implementation allows only two stages "set"/"not set"
and requires caller to check if function pointer exists before
calling it.

In order to simplify this repetitive task, let's give an option to
overwrite those pointers. This will allow us to set dummy functions
for the specific function pointers.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index d9f565a779df..9486e60b42cc 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
 void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
 {
 	struct ib_device_ops *dev_ops = &dev->ops;
-#define SET_DEVICE_OP(ptr, name)                                               \
-	do {                                                                   \
-		if (ops->name)                                                 \
-			if (!((ptr)->name))				       \
-				(ptr)->name = ops->name;                       \
+#define SET_DEVICE_OP(ptr, name)					\
+	do {								\
+		if (ops->name)						\
+			(ptr)->name = ops->name;			\
 	} while (0)
 
 #define SET_OBJ_SIZE(ptr, name) SET_DEVICE_OP(ptr, size_##name)
-- 
2.26.2

Re: [PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Jason Gunthorpe]

On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@mellanox.com>
> 
> Current device ops implementation allows only two stages "set"/"not set"
> and requires caller to check if function pointer exists before
> calling it.
> 
> In order to simplify this repetitive task, let's give an option to
> overwrite those pointers. This will allow us to set dummy functions
> for the specific function pointers.
> 
> Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  drivers/infiniband/core/device.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index d9f565a779df..9486e60b42cc 100644
> --- a/drivers/infiniband/core/device.c
> +++ b/drivers/infiniband/core/device.c
> @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
>  void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
>  {
>  	struct ib_device_ops *dev_ops = &dev->ops;
> -#define SET_DEVICE_OP(ptr, name)                                               \
> -	do {                                                                   \
> -		if (ops->name)                                                 \
> -			if (!((ptr)->name))				       \
> -				(ptr)->name = ops->name;                       \
> +#define SET_DEVICE_OP(ptr, name)					\
> +	do {								\
> +		if (ops->name)						\
> +			(ptr)->name = ops->name;			\
>  	} while (0)

Did you carefully check every driver to be sure it is OK with this?

Maybe Kamal remembers why it was like this?

Jason

Re: [PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Leon Romanovsky]

On Mon, May 25, 2020 at 11:26:41AM -0300, Jason Gunthorpe wrote:
> On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> > From: Maor Gottlieb <maorg@mellanox.com>
> >
> > Current device ops implementation allows only two stages "set"/"not set"
> > and requires caller to check if function pointer exists before
> > calling it.
> >
> > In order to simplify this repetitive task, let's give an option to
> > overwrite those pointers. This will allow us to set dummy functions
> > for the specific function pointers.
> >
> > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  drivers/infiniband/core/device.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index d9f565a779df..9486e60b42cc 100644
> > --- a/drivers/infiniband/core/device.c
> > +++ b/drivers/infiniband/core/device.c
> > @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
> >  void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> >  {
> >  	struct ib_device_ops *dev_ops = &dev->ops;
> > -#define SET_DEVICE_OP(ptr, name)                                               \
> > -	do {                                                                   \
> > -		if (ops->name)                                                 \
> > -			if (!((ptr)->name))				       \
> > -				(ptr)->name = ops->name;                       \
> > +#define SET_DEVICE_OP(ptr, name)					\
> > +	do {								\
> > +		if (ops->name)						\
> > +			(ptr)->name = ops->name;			\
> >  	} while (0)
>
> Did you carefully check every driver to be sure it is OK with this?

We will recheck.

Thanks

Re: [PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Kamal Heib]

On Mon, May 25, 2020 at 11:26:41AM -0300, Jason Gunthorpe wrote:
> On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> > From: Maor Gottlieb <maorg@mellanox.com>
> > 
> > Current device ops implementation allows only two stages "set"/"not set"
> > and requires caller to check if function pointer exists before
> > calling it.
> > 
> > In order to simplify this repetitive task, let's give an option to
> > overwrite those pointers. This will allow us to set dummy functions
> > for the specific function pointers.
> > 
> > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  drivers/infiniband/core/device.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index d9f565a779df..9486e60b42cc 100644
> > --- a/drivers/infiniband/core/device.c
> > +++ b/drivers/infiniband/core/device.c
> > @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
> >  void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> >  {
> >  	struct ib_device_ops *dev_ops = &dev->ops;
> > -#define SET_DEVICE_OP(ptr, name)                                               \
> > -	do {                                                                   \
> > -		if (ops->name)                                                 \
> > -			if (!((ptr)->name))				       \
> > -				(ptr)->name = ops->name;                       \
> > +#define SET_DEVICE_OP(ptr, name)					\
> > +	do {								\
> > +		if (ops->name)						\
> > +			(ptr)->name = ops->name;			\
> >  	} while (0)
> 
> Did you carefully check every driver to be sure it is OK with this?
> 
> Maybe Kamal remembers why it was like this?
> 
> Jason

The idea was to set a specific op only once by the provider when there
is a valid function for the op, this was done to make sure that if
the op isn't supported by the provider then it will be set to NULL.

I think it will be more cleaner from the provider point of view to
see which ops are supported or not supported in the provider code. by
overriding the ops in the core this will make things more confusing.

Thanks,
Kamal

Re: [PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Leon Romanovsky]

On Tue, May 26, 2020 at 02:21:25AM +0300, Kamal Heib wrote:
> On Mon, May 25, 2020 at 11:26:41AM -0300, Jason Gunthorpe wrote:
> > On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> > > From: Maor Gottlieb <maorg@mellanox.com>
> > >
> > > Current device ops implementation allows only two stages "set"/"not set"
> > > and requires caller to check if function pointer exists before
> > > calling it.
> > >
> > > In order to simplify this repetitive task, let's give an option to
> > > overwrite those pointers. This will allow us to set dummy functions
> > > for the specific function pointers.
> > >
> > > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > ---
> > >  drivers/infiniband/core/device.c | 9 ++++-----
> > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > index d9f565a779df..9486e60b42cc 100644
> > > --- a/drivers/infiniband/core/device.c
> > > +++ b/drivers/infiniband/core/device.c
> > > @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
> > >  void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> > >  {
> > >  	struct ib_device_ops *dev_ops = &dev->ops;
> > > -#define SET_DEVICE_OP(ptr, name)                                               \
> > > -	do {                                                                   \
> > > -		if (ops->name)                                                 \
> > > -			if (!((ptr)->name))				       \
> > > -				(ptr)->name = ops->name;                       \
> > > +#define SET_DEVICE_OP(ptr, name)					\
> > > +	do {								\
> > > +		if (ops->name)						\
> > > +			(ptr)->name = ops->name;			\
> > >  	} while (0)
> >
> > Did you carefully check every driver to be sure it is OK with this?
> >
> > Maybe Kamal remembers why it was like this?
> >
> > Jason
>
> The idea was to set a specific op only once by the provider when there
> is a valid function for the op, this was done to make sure that if
> the op isn't supported by the provider then it will be set to NULL.

This is not changed.

>
> I think it will be more cleaner from the provider point of view to
> see which ops are supported or not supported in the provider code. by
> overriding the ops in the core this will make things more confusing.

Actually the patch does quite opposite, set defaults by IB/core and
overwrite it by the provider later and not vice versa. The IB/core
won't overwrite defined by the provider ops. From provider point of view
no change.

Thanks

>
> Thanks,
> Kamal

Re: [PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Kamal Heib]

On Tue, May 26, 2020 at 08:53:42AM +0300, Leon Romanovsky wrote:
> On Tue, May 26, 2020 at 02:21:25AM +0300, Kamal Heib wrote:
> > On Mon, May 25, 2020 at 11:26:41AM -0300, Jason Gunthorpe wrote:
> > > On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> > > > From: Maor Gottlieb <maorg@mellanox.com>
> > > >
> > > > Current device ops implementation allows only two stages "set"/"not set"
> > > > and requires caller to check if function pointer exists before
> > > > calling it.
> > > >
> > > > In order to simplify this repetitive task, let's give an option to
> > > > overwrite those pointers. This will allow us to set dummy functions
> > > > for the specific function pointers.
> > > >
> > > > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > > ---
> > > >  drivers/infiniband/core/device.c | 9 ++++-----
> > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > > index d9f565a779df..9486e60b42cc 100644
> > > > --- a/drivers/infiniband/core/device.c
> > > > +++ b/drivers/infiniband/core/device.c
> > > > @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
> > > >  void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> > > >  {
> > > >  	struct ib_device_ops *dev_ops = &dev->ops;
> > > > -#define SET_DEVICE_OP(ptr, name)                                               \
> > > > -	do {                                                                   \
> > > > -		if (ops->name)                                                 \
> > > > -			if (!((ptr)->name))				       \
> > > > -				(ptr)->name = ops->name;                       \
> > > > +#define SET_DEVICE_OP(ptr, name)					\
> > > > +	do {								\
> > > > +		if (ops->name)						\
> > > > +			(ptr)->name = ops->name;			\
> > > >  	} while (0)
> > >
> > > Did you carefully check every driver to be sure it is OK with this?
> > >
> > > Maybe Kamal remembers why it was like this?
> > >
> > > Jason
> >
> > The idea was to set a specific op only once by the provider when there
> > is a valid function for the op, this was done to make sure that if
> > the op isn't supported by the provider then it will be set to NULL.
> 
> This is not changed.
>

Well, This is changed in patch #6 as dummy functions are now allowed...,
Instead of setting NULL when the op isn't supported by the provider.

I'm wondering why allow only fill_res_entry() and fill_stat_entry() to
be a dummy function as there is multiple ops that can be dummy too,
and why this has to be in this patch set instead in a seperiate patch
set for all the ops that can be dummy? 

Thanks,
Kamal

> >
> > I think it will be more cleaner from the provider point of view to
> > see which ops are supported or not supported in the provider code. by
> > overriding the ops in the core this will make things more confusing.
> 
> Actually the patch does quite opposite, set defaults by IB/core and
> overwrite it by the provider later and not vice versa. The IB/core
> won't overwrite defined by the provider ops. From provider point of view
> no change.
> 
> Thanks
> 
> >
> > Thanks,
> > Kamal

Re: [PATCH rdma-next 04/14] RDMA/core: Allow to override device op

[Leon Romanovsky]

On Tue, May 26, 2020 at 11:07:51AM +0300, Kamal Heib wrote:
> On Tue, May 26, 2020 at 08:53:42AM +0300, Leon Romanovsky wrote:
> > On Tue, May 26, 2020 at 02:21:25AM +0300, Kamal Heib wrote:
> > > On Mon, May 25, 2020 at 11:26:41AM -0300, Jason Gunthorpe wrote:
> > > > On Wed, May 13, 2020 at 12:50:24PM +0300, Leon Romanovsky wrote:
> > > > > From: Maor Gottlieb <maorg@mellanox.com>
> > > > >
> > > > > Current device ops implementation allows only two stages "set"/"not set"
> > > > > and requires caller to check if function pointer exists before
> > > > > calling it.
> > > > >
> > > > > In order to simplify this repetitive task, let's give an option to
> > > > > overwrite those pointers. This will allow us to set dummy functions
> > > > > for the specific function pointers.
> > > > >
> > > > > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > > > > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > > > > ---
> > > > >  drivers/infiniband/core/device.c | 9 ++++-----
> > > > >  1 file changed, 4 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > > > > index d9f565a779df..9486e60b42cc 100644
> > > > > --- a/drivers/infiniband/core/device.c
> > > > > +++ b/drivers/infiniband/core/device.c
> > > > > @@ -2542,11 +2542,10 @@ EXPORT_SYMBOL(ib_get_net_dev_by_params);
> > > > >  void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> > > > >  {
> > > > >  	struct ib_device_ops *dev_ops = &dev->ops;
> > > > > -#define SET_DEVICE_OP(ptr, name)                                               \
> > > > > -	do {                                                                   \
> > > > > -		if (ops->name)                                                 \
> > > > > -			if (!((ptr)->name))				       \
> > > > > -				(ptr)->name = ops->name;                       \
> > > > > +#define SET_DEVICE_OP(ptr, name)					\
> > > > > +	do {								\
> > > > > +		if (ops->name)						\
> > > > > +			(ptr)->name = ops->name;			\
> > > > >  	} while (0)
> > > >
> > > > Did you carefully check every driver to be sure it is OK with this?
> > > >
> > > > Maybe Kamal remembers why it was like this?
> > > >
> > > > Jason
> > >
> > > The idea was to set a specific op only once by the provider when there
> > > is a valid function for the op, this was done to make sure that if
> > > the op isn't supported by the provider then it will be set to NULL.
> >
> > This is not changed.
> >
>
> Well, This is changed in patch #6 as dummy functions are now allowed...,
> Instead of setting NULL when the op isn't supported by the provider.

From provider point of view nothing is changed, if it is not supported,
the ops won't be set. Everything will be handled in the IB/core exactly
like we do in the rdma-core.

>
> I'm wondering why allow only fill_res_entry() and fill_stat_entry() to
> be a dummy function as there is multiple ops that can be dummy too,
> and why this has to be in this patch set instead in a seperiate patch
> set for all the ops that can be dummy?

We started from fill_res_entry() as a perfect example of code
duplication and primary beneficiary of this approach. Inside nldev.c, we
want to be able to call function without need to check that function
pointer exists.

But let's drop this change. It is not important for main feature
implemented in this patch set.

Thanks

>
> Thanks,
> Kamal
>
> > >
> > > I think it will be more cleaner from the provider point of view to
> > > see which ops are supported or not supported in the provider code. by
> > > overriding the ops in the core this will make things more confusing.
> >
> > Actually the patch does quite opposite, set defaults by IB/core and
> > overwrite it by the provider later and not vice versa. The IB/core
> > won't overwrite defined by the provider ops. From provider point of view
> > no change.
> >
> > Thanks
> >
> > >
> > > Thanks,
> > > Kamal

[PATCH rdma-next 05/14] RDMA/core: Don't call fill_res_entry for PD

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

None of the vendor implement it, remove it.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index e16105be2eb2..8548f09746ab 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -653,7 +653,6 @@ static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,
 			     struct rdma_restrack_entry *res, uint32_t port)
 {
 	struct ib_pd *pd = container_of(res, struct ib_pd, res);
-	struct ib_device *dev = pd->device;
 
 	if (has_cap_net_admin) {
 		if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LOCAL_DMA_LKEY,
@@ -676,13 +675,7 @@ static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,
 			pd->uobject->context->res.id))
 		goto err;
 
-	if (fill_res_name_pid(msg, res))
-		goto err;
-
-	if (fill_res_entry(dev, msg, res))
-		goto err;
-
-	return 0;
+	return fill_res_name_pid(msg, res);
 
 err:	return -EMSGSIZE;
 }
-- 
2.26.2

[PATCH rdma-next 06/14] RDMA/core: Add restrack dummy ops

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

When fill_res_entry is not implemented by the vendor, then we just
need to return 0. Reduce some code and make it more clear by
set dummy ops.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c    | 46 ++++--------------------------
 drivers/infiniband/core/restrack.c | 13 +++++++++
 2 files changed, 19 insertions(+), 40 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 8548f09746ab..8b4115bc26b2 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -446,22 +446,6 @@ static int fill_res_name_pid(struct sk_buff *msg,
 	return err ? -EMSGSIZE : 0;
 }
 
-static bool fill_res_entry(struct ib_device *dev, struct sk_buff *msg,
-			   struct rdma_restrack_entry *res)
-{
-	if (!dev->ops.fill_res_entry)
-		return false;
-	return dev->ops.fill_res_entry(msg, res);
-}
-
-static bool fill_stat_entry(struct ib_device *dev, struct sk_buff *msg,
-			    struct rdma_restrack_entry *res)
-{
-	if (!dev->ops.fill_stat_entry)
-		return false;
-	return dev->ops.fill_stat_entry(msg, res);
-}
-
 static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
 			     struct rdma_restrack_entry *res, uint32_t port)
 {
@@ -515,10 +499,7 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	if (fill_res_entry(dev, msg, res))
-		goto err;
-
-	return 0;
+	return dev->ops.fill_res_entry(msg, res);
 
 err:	return -EMSGSIZE;
 }
@@ -568,10 +549,7 @@ static int fill_res_cm_id_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	if (fill_res_entry(dev, msg, res))
-		goto err;
-
-	return 0;
+	return dev->ops.fill_res_entry(msg, res);
 
 err: return -EMSGSIZE;
 }
@@ -606,10 +584,7 @@ static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	if (fill_res_entry(dev, msg, res))
-		goto err;
-
-	return 0;
+	return dev->ops.fill_res_entry(msg, res);
 
 err:	return -EMSGSIZE;
 }
@@ -641,10 +616,7 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	if (fill_res_entry(dev, msg, res))
-		goto err;
-
-	return 0;
+	return dev->ops.fill_res_entry(msg, res);
 
 err:	return -EMSGSIZE;
 }
@@ -784,15 +756,9 @@ static int fill_stat_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	struct ib_device *dev = mr->pd->device;
 
 	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_MRN, res->id))
-		goto err;
-
-	if (fill_stat_entry(dev, msg, res))
-		goto err;
-
-	return 0;
+		return -EMSGSIZE;
 
-err:
-	return -EMSGSIZE;
+	return dev->ops.fill_stat_entry(msg, res);
 }
 
 static int fill_stat_counter_hwcounters(struct sk_buff *msg,
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 62fbb0ae9cb4..093b27c0bbe6 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -14,6 +14,17 @@
 #include "cma_priv.h"
 #include "restrack.h"
 
+static int fill_res_dummy(struct sk_buff *msg,
+			  struct rdma_restrack_entry *entry)
+{
+	return 0;
+}
+
+static const struct ib_device_ops restrack_dummy_ops = {
+	.fill_res_entry = fill_res_dummy,
+	.fill_stat_entry = fill_res_dummy,
+};
+
 /**
  * rdma_restrack_init() - initialize and allocate resource tracking
  * @dev:  IB device
@@ -34,6 +45,8 @@ int rdma_restrack_init(struct ib_device *dev)
 	for (i = 0; i < RDMA_RESTRACK_MAX; i++)
 		xa_init_flags(&rt[i].xa, XA_FLAGS_ALLOC);
 
+	ib_set_device_ops(dev, &restrack_dummy_ops);
+
 	return 0;
 }
 
-- 
2.26.2

Re: [PATCH rdma-next 06/14] RDMA/core: Add restrack dummy ops

[Jason Gunthorpe]

On Wed, May 13, 2020 at 12:50:26PM +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@mellanox.com>
> 
> When fill_res_entry is not implemented by the vendor, then we just
> need to return 0. Reduce some code and make it more clear by
> set dummy ops.
> 
> Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> ---
>  drivers/infiniband/core/nldev.c    | 46 ++++--------------------------
>  drivers/infiniband/core/restrack.c | 13 +++++++++
>  2 files changed, 19 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 8548f09746ab..8b4115bc26b2 100644
> --- a/drivers/infiniband/core/nldev.c
> +++ b/drivers/infiniband/core/nldev.c
> @@ -446,22 +446,6 @@ static int fill_res_name_pid(struct sk_buff *msg,
>  	return err ? -EMSGSIZE : 0;
>  }
>  
> -static bool fill_res_entry(struct ib_device *dev, struct sk_buff *msg,
> -			   struct rdma_restrack_entry *res)
> -{
> -	if (!dev->ops.fill_res_entry)
> -		return false;
> -	return dev->ops.fill_res_entry(msg, res);
> -}
> -
> -static bool fill_stat_entry(struct ib_device *dev, struct sk_buff *msg,
> -			    struct rdma_restrack_entry *res)
> -{
> -	if (!dev->ops.fill_stat_entry)
> -		return false;
> -	return dev->ops.fill_stat_entry(msg, res);
> -}
> -
>  static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  			     struct rdma_restrack_entry *res, uint32_t port)
>  {
> @@ -515,10 +499,7 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  	if (fill_res_name_pid(msg, res))
>  		goto err;
>  
> -	if (fill_res_entry(dev, msg, res))
> -		goto err;
> -
> -	return 0;
> +	return dev->ops.fill_res_entry(msg, res);
>  
>  err:	return -EMSGSIZE;
>  }
> @@ -568,10 +549,7 @@ static int fill_res_cm_id_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  	if (fill_res_name_pid(msg, res))
>  		goto err;
>  
> -	if (fill_res_entry(dev, msg, res))
> -		goto err;
> -
> -	return 0;
> +	return dev->ops.fill_res_entry(msg, res);
>  
>  err: return -EMSGSIZE;
>  }
> @@ -606,10 +584,7 @@ static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  	if (fill_res_name_pid(msg, res))
>  		goto err;
>  
> -	if (fill_res_entry(dev, msg, res))
> -		goto err;
> -
> -	return 0;
> +	return dev->ops.fill_res_entry(msg, res);
>  
>  err:	return -EMSGSIZE;
>  }
> @@ -641,10 +616,7 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  	if (fill_res_name_pid(msg, res))
>  		goto err;
>  
> -	if (fill_res_entry(dev, msg, res))
> -		goto err;
> -
> -	return 0;
> +	return dev->ops.fill_res_entry(msg, res);
>  
>  err:	return -EMSGSIZE;
>  }
> @@ -784,15 +756,9 @@ static int fill_stat_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  	struct ib_device *dev = mr->pd->device;
>  
>  	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_MRN, res->id))
> -		goto err;
> -
> -	if (fill_stat_entry(dev, msg, res))
> -		goto err;
> -
> -	return 0;
> +		return -EMSGSIZE;
>  
> -err:
> -	return -EMSGSIZE;
> +	return dev->ops.fill_stat_entry(msg, res);
>  }
>  
>  static int fill_stat_counter_hwcounters(struct sk_buff *msg,
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index 62fbb0ae9cb4..093b27c0bbe6 100644
> --- a/drivers/infiniband/core/restrack.c
> +++ b/drivers/infiniband/core/restrack.c
> @@ -14,6 +14,17 @@
>  #include "cma_priv.h"
>  #include "restrack.h"
>  
> +static int fill_res_dummy(struct sk_buff *msg,
> +			  struct rdma_restrack_entry *entry)
> +{
> +	return 0;
> +}
> +
> +static const struct ib_device_ops restrack_dummy_ops = {
> +	.fill_res_entry = fill_res_dummy,
> +	.fill_stat_entry = fill_res_dummy,
> +};

If you are going to do this then you should do it for substantially
everything, as we did in rdma-core. I don't want to see easy stuff
like this half done..

And this should be a broken out series or two as it really has nothing
to do with 'raw format dumps'

Jason

Re: [PATCH rdma-next 06/14] RDMA/core: Add restrack dummy ops

[Leon Romanovsky]

On Mon, May 25, 2020 at 11:36:50AM -0300, Jason Gunthorpe wrote:
> On Wed, May 13, 2020 at 12:50:26PM +0300, Leon Romanovsky wrote:
> > From: Maor Gottlieb <maorg@mellanox.com>
> >
> > When fill_res_entry is not implemented by the vendor, then we just
> > need to return 0. Reduce some code and make it more clear by
> > set dummy ops.
> >
> > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> > ---
> >  drivers/infiniband/core/nldev.c    | 46 ++++--------------------------
> >  drivers/infiniband/core/restrack.c | 13 +++++++++
> >  2 files changed, 19 insertions(+), 40 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> > index 8548f09746ab..8b4115bc26b2 100644
> > --- a/drivers/infiniband/core/nldev.c
> > +++ b/drivers/infiniband/core/nldev.c
> > @@ -446,22 +446,6 @@ static int fill_res_name_pid(struct sk_buff *msg,
> >  	return err ? -EMSGSIZE : 0;
> >  }
> >
> > -static bool fill_res_entry(struct ib_device *dev, struct sk_buff *msg,
> > -			   struct rdma_restrack_entry *res)
> > -{
> > -	if (!dev->ops.fill_res_entry)
> > -		return false;
> > -	return dev->ops.fill_res_entry(msg, res);
> > -}
> > -
> > -static bool fill_stat_entry(struct ib_device *dev, struct sk_buff *msg,
> > -			    struct rdma_restrack_entry *res)
> > -{
> > -	if (!dev->ops.fill_stat_entry)
> > -		return false;
> > -	return dev->ops.fill_stat_entry(msg, res);
> > -}
> > -
> >  static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  			     struct rdma_restrack_entry *res, uint32_t port)
> >  {
> > @@ -515,10 +499,7 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  	if (fill_res_name_pid(msg, res))
> >  		goto err;
> >
> > -	if (fill_res_entry(dev, msg, res))
> > -		goto err;
> > -
> > -	return 0;
> > +	return dev->ops.fill_res_entry(msg, res);
> >
> >  err:	return -EMSGSIZE;
> >  }
> > @@ -568,10 +549,7 @@ static int fill_res_cm_id_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  	if (fill_res_name_pid(msg, res))
> >  		goto err;
> >
> > -	if (fill_res_entry(dev, msg, res))
> > -		goto err;
> > -
> > -	return 0;
> > +	return dev->ops.fill_res_entry(msg, res);
> >
> >  err: return -EMSGSIZE;
> >  }
> > @@ -606,10 +584,7 @@ static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  	if (fill_res_name_pid(msg, res))
> >  		goto err;
> >
> > -	if (fill_res_entry(dev, msg, res))
> > -		goto err;
> > -
> > -	return 0;
> > +	return dev->ops.fill_res_entry(msg, res);
> >
> >  err:	return -EMSGSIZE;
> >  }
> > @@ -641,10 +616,7 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  	if (fill_res_name_pid(msg, res))
> >  		goto err;
> >
> > -	if (fill_res_entry(dev, msg, res))
> > -		goto err;
> > -
> > -	return 0;
> > +	return dev->ops.fill_res_entry(msg, res);
> >
> >  err:	return -EMSGSIZE;
> >  }
> > @@ -784,15 +756,9 @@ static int fill_stat_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  	struct ib_device *dev = mr->pd->device;
> >
> >  	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_MRN, res->id))
> > -		goto err;
> > -
> > -	if (fill_stat_entry(dev, msg, res))
> > -		goto err;
> > -
> > -	return 0;
> > +		return -EMSGSIZE;
> >
> > -err:
> > -	return -EMSGSIZE;
> > +	return dev->ops.fill_stat_entry(msg, res);
> >  }
> >
> >  static int fill_stat_counter_hwcounters(struct sk_buff *msg,
> > diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> > index 62fbb0ae9cb4..093b27c0bbe6 100644
> > --- a/drivers/infiniband/core/restrack.c
> > +++ b/drivers/infiniband/core/restrack.c
> > @@ -14,6 +14,17 @@
> >  #include "cma_priv.h"
> >  #include "restrack.h"
> >
> > +static int fill_res_dummy(struct sk_buff *msg,
> > +			  struct rdma_restrack_entry *entry)
> > +{
> > +	return 0;
> > +}
> > +
> > +static const struct ib_device_ops restrack_dummy_ops = {
> > +	.fill_res_entry = fill_res_dummy,
> > +	.fill_stat_entry = fill_res_dummy,
> > +};
>
> If you are going to do this then you should do it for substantially
> everything, as we did in rdma-core. I don't want to see easy stuff
> like this half done..
>
> And this should be a broken out series or two as it really has nothing
> to do with 'raw format dumps'

Maor will be committed to do it. Can we merge this series because I see it is
an independent task?

Thanks

>
> Jason

[PATCH rdma-next 07/14] RDMA: Add dedicated MR resource tracker function

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

In order to avoid double multiplexing of the resource when it's MR,
add a dedicated callback function.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c       |  3 ++-
 drivers/infiniband/core/nldev.c        |  4 ++--
 drivers/infiniband/core/restrack.c     | 11 +++++++++-
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h |  1 +
 drivers/infiniband/hw/cxgb4/provider.c |  1 +
 drivers/infiniband/hw/cxgb4/restrack.c |  5 +----
 drivers/infiniband/hw/mlx5/main.c      |  4 ++--
 drivers/infiniband/hw/mlx5/mlx5_ib.h   |  6 ++----
 drivers/infiniband/hw/mlx5/restrack.c  | 28 ++++----------------------
 include/rdma/ib_verbs.h                |  4 ++--
 10 files changed, 27 insertions(+), 40 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 9486e60b42cc..7f8da01b313f 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2617,7 +2617,8 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
 	SET_DEVICE_OP(dev_ops, drain_sq);
 	SET_DEVICE_OP(dev_ops, enable_driver);
 	SET_DEVICE_OP(dev_ops, fill_res_entry);
-	SET_DEVICE_OP(dev_ops, fill_stat_entry);
+	SET_DEVICE_OP(dev_ops, fill_res_mr_entry);
+	SET_DEVICE_OP(dev_ops, fill_stat_mr_entry);
 	SET_DEVICE_OP(dev_ops, get_dev_fw_str);
 	SET_DEVICE_OP(dev_ops, get_dma_mr);
 	SET_DEVICE_OP(dev_ops, get_hw_stats);
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 8b4115bc26b2..bc7e6786b71d 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -616,7 +616,7 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	return dev->ops.fill_res_entry(msg, res);
+	return dev->ops.fill_res_mr_entry(msg, mr);
 
 err:	return -EMSGSIZE;
 }
@@ -758,7 +758,7 @@ static int fill_stat_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_MRN, res->id))
 		return -EMSGSIZE;
 
-	return dev->ops.fill_stat_entry(msg, res);
+	return dev->ops.fill_stat_mr_entry(msg, mr);
 }
 
 static int fill_stat_counter_hwcounters(struct sk_buff *msg,
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 093b27c0bbe6..3bd4cb6ce698 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -14,15 +14,24 @@
 #include "cma_priv.h"
 #include "restrack.h"
 
+#define FILL_DUMMY(type) \
+	static int fill_res_##type(struct sk_buff *msg,		\
+				   struct ib_##type *res)	\
+	{							\
+		return 0;					\
+	}
+
 static int fill_res_dummy(struct sk_buff *msg,
 			  struct rdma_restrack_entry *entry)
 {
 	return 0;
 }
 
+FILL_DUMMY(mr);
 static const struct ib_device_ops restrack_dummy_ops = {
 	.fill_res_entry = fill_res_dummy,
-	.fill_stat_entry = fill_res_dummy,
+	.fill_res_mr_entry = fill_res_mr,
+	.fill_stat_mr_entry = fill_res_mr,
 };
 
 /**
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index e8e11bd95e42..5b9884ca2f5e 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -1055,6 +1055,7 @@ struct c4iw_wr_wait *c4iw_alloc_wr_wait(gfp_t gfp);
 
 typedef int c4iw_restrack_func(struct sk_buff *msg,
 			       struct rdma_restrack_entry *res);
+int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr);
 extern c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX];
 
 #endif
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index ba83d942997c..36eeb595d41c 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -486,6 +486,7 @@ static const struct ib_device_ops c4iw_dev_ops = {
 	.destroy_qp = c4iw_destroy_qp,
 	.destroy_srq = c4iw_destroy_srq,
 	.fill_res_entry = fill_res_entry,
+	.fill_res_mr_entry = c4iw_fill_res_mr_entry,
 	.get_dev_fw_str = get_dev_fw_str,
 	.get_dma_mr = c4iw_get_dma_mr,
 	.get_hw_stats = c4iw_get_mib,
diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
index f82d46ed969d..9a5ca9192c1c 100644
--- a/drivers/infiniband/hw/cxgb4/restrack.c
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -433,10 +433,8 @@ static int fill_res_cq_entry(struct sk_buff *msg,
 	return -EMSGSIZE;
 }
 
-static int fill_res_mr_entry(struct sk_buff *msg,
-			     struct rdma_restrack_entry *res)
+int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr)
 {
-	struct ib_mr *ibmr = container_of(res, struct ib_mr, res);
 	struct c4iw_mr *mhp = to_c4iw_mr(ibmr);
 	struct c4iw_dev *dev = mhp->rhp;
 	u32 stag = mhp->attr.stag;
@@ -497,5 +495,4 @@ c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX] = {
 	[RDMA_RESTRACK_QP]	= fill_res_qp_entry,
 	[RDMA_RESTRACK_CM_ID]	= fill_res_ep_entry,
 	[RDMA_RESTRACK_CQ]	= fill_res_cq_entry,
-	[RDMA_RESTRACK_MR]	= fill_res_mr_entry,
 };
diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index ab551403d5e0..fdc9ab990f6b 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -6614,8 +6614,8 @@ static const struct ib_device_ops mlx5_ib_dev_ops = {
 	.drain_rq = mlx5_ib_drain_rq,
 	.drain_sq = mlx5_ib_drain_sq,
 	.enable_driver = mlx5_ib_enable_driver,
-	.fill_res_entry = mlx5_ib_fill_res_entry,
-	.fill_stat_entry = mlx5_ib_fill_stat_entry,
+	.fill_res_mr_entry = mlx5_ib_fill_res_mr_entry,
+	.fill_stat_mr_entry = mlx5_ib_fill_stat_mr_entry,
 	.get_dev_fw_str = get_dev_fw_str,
 	.get_dma_mr = mlx5_ib_get_dma_mr,
 	.get_link_layer = mlx5_ib_port_link_layer,
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 482b54eb9764..d2b36f4ce508 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -1383,10 +1383,8 @@ struct mlx5_core_dev *mlx5_ib_get_native_port_mdev(struct mlx5_ib_dev *dev,
 						   u8 *native_port_num);
 void mlx5_ib_put_native_port_mdev(struct mlx5_ib_dev *dev,
 				  u8 port_num);
-int mlx5_ib_fill_res_entry(struct sk_buff *msg,
-			   struct rdma_restrack_entry *res);
-int mlx5_ib_fill_stat_entry(struct sk_buff *msg,
-			    struct rdma_restrack_entry *res);
+int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr);
+int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr);
 
 extern const struct uapi_definition mlx5_ib_devx_defs[];
 extern const struct uapi_definition mlx5_ib_flow_defs[];
diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
index 8f6c04f12531..598a09796d09 100644
--- a/drivers/infiniband/hw/mlx5/restrack.c
+++ b/drivers/infiniband/hw/mlx5/restrack.c
@@ -8,10 +8,9 @@
 #include <rdma/restrack.h>
 #include "mlx5_ib.h"
 
-static int fill_stat_mr_entry(struct sk_buff *msg,
-			      struct rdma_restrack_entry *res)
+int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg,
+			       struct ib_mr *ibmr)
 {
-	struct ib_mr *ibmr = container_of(res, struct ib_mr, res);
 	struct mlx5_ib_mr *mr = to_mmr(ibmr);
 	struct nlattr *table_attr;
 
@@ -41,10 +40,9 @@ static int fill_stat_mr_entry(struct sk_buff *msg,
 	return -EMSGSIZE;
 }
 
-static int fill_res_mr_entry(struct sk_buff *msg,
-			     struct rdma_restrack_entry *res)
+int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg,
+			      struct ib_mr *ibmr)
 {
-	struct ib_mr *ibmr = container_of(res, struct ib_mr, res);
 	struct mlx5_ib_mr *mr = to_mmr(ibmr);
 	struct nlattr *table_attr;
 
@@ -70,21 +68,3 @@ static int fill_res_mr_entry(struct sk_buff *msg,
 	nla_nest_cancel(msg, table_attr);
 	return -EMSGSIZE;
 }
-
-int mlx5_ib_fill_res_entry(struct sk_buff *msg,
-			   struct rdma_restrack_entry *res)
-{
-	if (res->type == RDMA_RESTRACK_MR)
-		return fill_res_mr_entry(msg, res);
-
-	return 0;
-}
-
-int mlx5_ib_fill_stat_entry(struct sk_buff *msg,
-			    struct rdma_restrack_entry *res)
-{
-	if (res->type == RDMA_RESTRACK_MR)
-		return fill_stat_mr_entry(msg, res);
-
-	return 0;
-}
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 88b69715d3dd..34c9f278c3cd 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2569,6 +2569,7 @@ struct ib_device_ops {
 	 */
 	int (*fill_res_entry)(struct sk_buff *msg,
 			      struct rdma_restrack_entry *entry);
+	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
 
 	/* Device lifecycle callbacks */
 	/*
@@ -2623,8 +2624,7 @@ struct ib_device_ops {
 	 * Allows rdma drivers to add their own restrack attributes
 	 * dumped via 'rdma stat' iproute2 command.
 	 */
-	int (*fill_stat_entry)(struct sk_buff *msg,
-			       struct rdma_restrack_entry *entry);
+	int (*fill_stat_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
 
 	DECLARE_RDMA_OBJ_SIZE(ib_ah);
 	DECLARE_RDMA_OBJ_SIZE(ib_cq);
-- 
2.26.2

[PATCH rdma-next 08/14] RDMA: Add a dedicated CQ resource tracker function

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

In order to avoid double multiplexing of the resource when it's CQ,
add a dedicated callback function.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c              |  1 +
 drivers/infiniband/core/nldev.c               |  2 +-
 drivers/infiniband/core/restrack.c            |  3 +++
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h        |  1 +
 drivers/infiniband/hw/cxgb4/provider.c        |  1 +
 drivers/infiniband/hw/cxgb4/restrack.c        |  5 +----
 drivers/infiniband/hw/hns/hns_roce_device.h   |  4 ++--
 drivers/infiniband/hw/hns/hns_roce_main.c     |  2 +-
 drivers/infiniband/hw/hns/hns_roce_restrack.c | 14 ++------------
 include/rdma/ib_verbs.h                       |  1 +
 10 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 7f8da01b313f..1f9f44e62e49 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2616,6 +2616,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
 	SET_DEVICE_OP(dev_ops, drain_rq);
 	SET_DEVICE_OP(dev_ops, drain_sq);
 	SET_DEVICE_OP(dev_ops, enable_driver);
+	SET_DEVICE_OP(dev_ops, fill_res_cq_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_mr_entry);
 	SET_DEVICE_OP(dev_ops, fill_stat_mr_entry);
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index bc7e6786b71d..6207b68453a1 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -584,7 +584,7 @@ static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	return dev->ops.fill_res_entry(msg, res);
+	return dev->ops.fill_res_cq_entry(msg, cq);
 
 err:	return -EMSGSIZE;
 }
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 3bd4cb6ce698..031a4f94400e 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -28,7 +28,10 @@ static int fill_res_dummy(struct sk_buff *msg,
 }
 
 FILL_DUMMY(mr);
+FILL_DUMMY(cq);
+
 static const struct ib_device_ops restrack_dummy_ops = {
+	.fill_res_cq_entry = fill_res_cq,
 	.fill_res_entry = fill_res_dummy,
 	.fill_res_mr_entry = fill_res_mr,
 	.fill_stat_mr_entry = fill_res_mr,
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 5b9884ca2f5e..18a2c1a44dcc 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -1056,6 +1056,7 @@ struct c4iw_wr_wait *c4iw_alloc_wr_wait(gfp_t gfp);
 typedef int c4iw_restrack_func(struct sk_buff *msg,
 			       struct rdma_restrack_entry *res);
 int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr);
+int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq);
 extern c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX];
 
 #endif
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index 36eeb595d41c..d6b20aa314a0 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -485,6 +485,7 @@ static const struct ib_device_ops c4iw_dev_ops = {
 	.destroy_cq = c4iw_destroy_cq,
 	.destroy_qp = c4iw_destroy_qp,
 	.destroy_srq = c4iw_destroy_srq,
+	.fill_res_cq_entry = c4iw_fill_res_cq_entry,
 	.fill_res_entry = fill_res_entry,
 	.fill_res_mr_entry = c4iw_fill_res_mr_entry,
 	.get_dev_fw_str = get_dev_fw_str,
diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
index 9a5ca9192c1c..ead2cd08793d 100644
--- a/drivers/infiniband/hw/cxgb4/restrack.c
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -372,10 +372,8 @@ static int fill_swcqes(struct sk_buff *msg, struct t4_cq *cq,
 	return -EMSGSIZE;
 }
 
-static int fill_res_cq_entry(struct sk_buff *msg,
-			     struct rdma_restrack_entry *res)
+int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq)
 {
-	struct ib_cq *ibcq = container_of(res, struct ib_cq, res);
 	struct c4iw_cq *chp = to_c4iw_cq(ibcq);
 	struct nlattr *table_attr;
 	struct t4_cqe hwcqes[2];
@@ -494,5 +492,4 @@ int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr)
 c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX] = {
 	[RDMA_RESTRACK_QP]	= fill_res_qp_entry,
 	[RDMA_RESTRACK_CM_ID]	= fill_res_ep_entry,
-	[RDMA_RESTRACK_CQ]	= fill_res_cq_entry,
 };
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 4fcd608ee55f..556449a143ed 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -1257,6 +1257,6 @@ void hns_roce_handle_device_err(struct hns_roce_dev *hr_dev);
 int hns_roce_init(struct hns_roce_dev *hr_dev);
 void hns_roce_exit(struct hns_roce_dev *hr_dev);
 
-int hns_roce_fill_res_entry(struct sk_buff *msg,
-			    struct rdma_restrack_entry *res);
+int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
+			       struct ib_cq *ib_cq);
 #endif /* _HNS_ROCE_DEVICE_H */
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index fd3581efe9a8..0ab40e771cf0 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -429,7 +429,7 @@ static const struct ib_device_ops hns_roce_dev_ops = {
 	.destroy_ah = hns_roce_destroy_ah,
 	.destroy_cq = hns_roce_destroy_cq,
 	.disassociate_ucontext = hns_roce_disassociate_ucontext,
-	.fill_res_entry = hns_roce_fill_res_entry,
+	.fill_res_cq_entry = hns_roce_fill_res_cq_entry,
 	.get_dma_mr = hns_roce_get_dma_mr,
 	.get_link_layer = hns_roce_get_link_layer,
 	.get_port_immutable = hns_roce_port_immutable,
diff --git a/drivers/infiniband/hw/hns/hns_roce_restrack.c b/drivers/infiniband/hw/hns/hns_roce_restrack.c
index 06871731ac43..259444c0a630 100644
--- a/drivers/infiniband/hw/hns/hns_roce_restrack.c
+++ b/drivers/infiniband/hw/hns/hns_roce_restrack.c
@@ -76,10 +76,9 @@ static int hns_roce_fill_cq(struct sk_buff *msg,
 	return -EMSGSIZE;
 }
 
-static int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
-				      struct rdma_restrack_entry *res)
+int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
+			       struct ib_cq *ib_cq)
 {
-	struct ib_cq *ib_cq = container_of(res, struct ib_cq, res);
 	struct hns_roce_dev *hr_dev = to_hr_dev(ib_cq->device);
 	struct hns_roce_cq *hr_cq = to_hr_cq(ib_cq);
 	struct hns_roce_v2_cq_context *context;
@@ -119,12 +118,3 @@ static int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
 	kfree(context);
 	return ret;
 }
-
-int hns_roce_fill_res_entry(struct sk_buff *msg,
-			    struct rdma_restrack_entry *res)
-{
-	if (res->type == RDMA_RESTRACK_CQ)
-		return hns_roce_fill_res_cq_entry(msg, res);
-
-	return 0;
-}
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 34c9f278c3cd..61f27cacc120 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2570,6 +2570,7 @@ struct ib_device_ops {
 	int (*fill_res_entry)(struct sk_buff *msg,
 			      struct rdma_restrack_entry *entry);
 	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
+	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq);
 
 	/* Device lifecycle callbacks */
 	/*
-- 
2.26.2

[PATCH rdma-next 09/14] RDMA: Add a dedicated QP resource tracker function

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

In order to avoid double multiplexing of the resource when it's QP,
add a dedicated callback function.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c       | 1 +
 drivers/infiniband/core/nldev.c        | 2 +-
 drivers/infiniband/core/restrack.c     | 2 ++
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 1 +
 drivers/infiniband/hw/cxgb4/restrack.c | 5 +----
 include/rdma/ib_verbs.h                | 1 +
 6 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 1f9f44e62e49..23af3cc27ee1 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2619,6 +2619,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
 	SET_DEVICE_OP(dev_ops, fill_res_cq_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_mr_entry);
+	SET_DEVICE_OP(dev_ops, fill_res_qp_entry);
 	SET_DEVICE_OP(dev_ops, fill_stat_mr_entry);
 	SET_DEVICE_OP(dev_ops, get_dev_fw_str);
 	SET_DEVICE_OP(dev_ops, get_dma_mr);
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 6207b68453a1..8c748888bf28 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -499,7 +499,7 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	return dev->ops.fill_res_entry(msg, res);
+	return dev->ops.fill_res_qp_entry(msg, qp);
 
 err:	return -EMSGSIZE;
 }
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 031a4f94400e..33d7c0888753 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -29,11 +29,13 @@ static int fill_res_dummy(struct sk_buff *msg,
 
 FILL_DUMMY(mr);
 FILL_DUMMY(cq);
+FILL_DUMMY(qp);
 
 static const struct ib_device_ops restrack_dummy_ops = {
 	.fill_res_cq_entry = fill_res_cq,
 	.fill_res_entry = fill_res_dummy,
 	.fill_res_mr_entry = fill_res_mr,
+	.fill_res_qp_entry = fill_res_qp,
 	.fill_stat_mr_entry = fill_res_mr,
 };
 
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 18a2c1a44dcc..c84aa7c937f1 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -1057,6 +1057,7 @@ typedef int c4iw_restrack_func(struct sk_buff *msg,
 			       struct rdma_restrack_entry *res);
 int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr);
 int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq);
+int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp);
 extern c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX];
 
 #endif
diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
index ead2cd08793d..5144d3b67293 100644
--- a/drivers/infiniband/hw/cxgb4/restrack.c
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -134,10 +134,8 @@ static int fill_swsqes(struct sk_buff *msg, struct t4_sq *sq,
 	return -EMSGSIZE;
 }
 
-static int fill_res_qp_entry(struct sk_buff *msg,
-			     struct rdma_restrack_entry *res)
+int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp)
 {
-	struct ib_qp *ibqp = container_of(res, struct ib_qp, res);
 	struct t4_swsqe *fsp = NULL, *lsp = NULL;
 	struct c4iw_qp *qhp = to_c4iw_qp(ibqp);
 	u16 first_sq_idx = 0, last_sq_idx = 0;
@@ -490,6 +488,5 @@ int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr)
 }
 
 c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX] = {
-	[RDMA_RESTRACK_QP]	= fill_res_qp_entry,
 	[RDMA_RESTRACK_CM_ID]	= fill_res_ep_entry,
 };
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 61f27cacc120..194a15b71498 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2571,6 +2571,7 @@ struct ib_device_ops {
 			      struct rdma_restrack_entry *entry);
 	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
 	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq);
+	int (*fill_res_qp_entry)(struct sk_buff *msg, struct ib_qp *ibqp);
 
 	/* Device lifecycle callbacks */
 	/*
-- 
2.26.2

Re: [PATCH rdma-next 09/14] RDMA: Add a dedicated QP resource tracker function

[Jason Gunthorpe]

On Wed, May 13, 2020 at 12:50:29PM +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@mellanox.com>
> 
> In order to avoid double multiplexing of the resource when it's QP,
> add a dedicated callback function.
> 
> Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/infiniband/core/device.c       | 1 +
>  drivers/infiniband/core/nldev.c        | 2 +-
>  drivers/infiniband/core/restrack.c     | 2 ++
>  drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 1 +
>  drivers/infiniband/hw/cxgb4/restrack.c | 5 +----
>  include/rdma/ib_verbs.h                | 1 +
>  6 files changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> index 1f9f44e62e49..23af3cc27ee1 100644
> +++ b/drivers/infiniband/core/device.c
> @@ -2619,6 +2619,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
>  	SET_DEVICE_OP(dev_ops, fill_res_cq_entry);
>  	SET_DEVICE_OP(dev_ops, fill_res_entry);
>  	SET_DEVICE_OP(dev_ops, fill_res_mr_entry);
> +	SET_DEVICE_OP(dev_ops, fill_res_qp_entry);
>  	SET_DEVICE_OP(dev_ops, fill_stat_mr_entry);
>  	SET_DEVICE_OP(dev_ops, get_dev_fw_str);
>  	SET_DEVICE_OP(dev_ops, get_dma_mr);
> diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> index 6207b68453a1..8c748888bf28 100644
> +++ b/drivers/infiniband/core/nldev.c
> @@ -499,7 +499,7 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
>  	if (fill_res_name_pid(msg, res))
>  		goto err;
>  
> -	return dev->ops.fill_res_entry(msg, res);
> +	return dev->ops.fill_res_qp_entry(msg, qp);
>  
>  err:	return -EMSGSIZE;
>  }
> diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> index 031a4f94400e..33d7c0888753 100644
> +++ b/drivers/infiniband/core/restrack.c
> @@ -29,11 +29,13 @@ static int fill_res_dummy(struct sk_buff *msg,
>  
>  FILL_DUMMY(mr);
>  FILL_DUMMY(cq);
> +FILL_DUMMY(qp);

Lists of things should be sorted

>  static const struct ib_device_ops restrack_dummy_ops = {
>  	.fill_res_cq_entry = fill_res_cq,
>  	.fill_res_entry = fill_res_dummy,
>  	.fill_res_mr_entry = fill_res_mr,
> +	.fill_res_qp_entry = fill_res_qp,
>  	.fill_stat_mr_entry = fill_res_mr,
>  };

Here too

I'm also not sure the FILL_DUMMY obfuscation is worthwhile for 3
functions.

> @@ -2571,6 +2571,7 @@ struct ib_device_ops {
>  			      struct rdma_restrack_entry *entry);
>  	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
>  	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq);
> +	int (*fill_res_qp_entry)(struct sk_buff *msg, struct ib_qp *ibqp);

Sorted too

Jason

Re: [PATCH rdma-next 09/14] RDMA: Add a dedicated QP resource tracker function

[Leon Romanovsky]

On Mon, May 25, 2020 at 11:34:47AM -0300, Jason Gunthorpe wrote:
> On Wed, May 13, 2020 at 12:50:29PM +0300, Leon Romanovsky wrote:
> > From: Maor Gottlieb <maorg@mellanox.com>
> >
> > In order to avoid double multiplexing of the resource when it's QP,
> > add a dedicated callback function.
> >
> > Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> > Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
> >  drivers/infiniband/core/device.c       | 1 +
> >  drivers/infiniband/core/nldev.c        | 2 +-
> >  drivers/infiniband/core/restrack.c     | 2 ++
> >  drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 1 +
> >  drivers/infiniband/hw/cxgb4/restrack.c | 5 +----
> >  include/rdma/ib_verbs.h                | 1 +
> >  6 files changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
> > index 1f9f44e62e49..23af3cc27ee1 100644
> > +++ b/drivers/infiniband/core/device.c
> > @@ -2619,6 +2619,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
> >  	SET_DEVICE_OP(dev_ops, fill_res_cq_entry);
> >  	SET_DEVICE_OP(dev_ops, fill_res_entry);
> >  	SET_DEVICE_OP(dev_ops, fill_res_mr_entry);
> > +	SET_DEVICE_OP(dev_ops, fill_res_qp_entry);
> >  	SET_DEVICE_OP(dev_ops, fill_stat_mr_entry);
> >  	SET_DEVICE_OP(dev_ops, get_dev_fw_str);
> >  	SET_DEVICE_OP(dev_ops, get_dma_mr);
> > diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
> > index 6207b68453a1..8c748888bf28 100644
> > +++ b/drivers/infiniband/core/nldev.c
> > @@ -499,7 +499,7 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
> >  	if (fill_res_name_pid(msg, res))
> >  		goto err;
> >
> > -	return dev->ops.fill_res_entry(msg, res);
> > +	return dev->ops.fill_res_qp_entry(msg, qp);
> >
> >  err:	return -EMSGSIZE;
> >  }
> > diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
> > index 031a4f94400e..33d7c0888753 100644
> > +++ b/drivers/infiniband/core/restrack.c
> > @@ -29,11 +29,13 @@ static int fill_res_dummy(struct sk_buff *msg,
> >
> >  FILL_DUMMY(mr);
> >  FILL_DUMMY(cq);
> > +FILL_DUMMY(qp);
>
> Lists of things should be sorted

I will fix.

>
> >  static const struct ib_device_ops restrack_dummy_ops = {
> >  	.fill_res_cq_entry = fill_res_cq,
> >  	.fill_res_entry = fill_res_dummy,
> >  	.fill_res_mr_entry = fill_res_mr,
> > +	.fill_res_qp_entry = fill_res_qp,
> >  	.fill_stat_mr_entry = fill_res_mr,
> >  };
>
> Here too
>
> I'm also not sure the FILL_DUMMY obfuscation is worthwhile for 3
> functions.

At the end, we removed a lot of duplicated code, despite adding code the
diffstat shows that we added not so much code after all.

17 files changed, 258 insertions(+), 201 deletions(-)

>
> > @@ -2571,6 +2571,7 @@ struct ib_device_ops {
> >  			      struct rdma_restrack_entry *entry);
> >  	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
> >  	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq);
> > +	int (*fill_res_qp_entry)(struct sk_buff *msg, struct ib_qp *ibqp);
>
> Sorted too

I will fix.

>
> Jason

[PATCH rdma-next 10/14] RDMA: Add dedicated cm id resource tracker function

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

In order to avoid double multiplexing of the resource when it's
cm id, add a dedicated callback function.
In addition remove fill_res_entry which is not used anymore.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/device.c       | 2 +-
 drivers/infiniband/core/nldev.c        | 2 +-
 drivers/infiniband/core/restrack.c     | 5 ++---
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +---
 drivers/infiniband/hw/cxgb4/provider.c | 9 +--------
 drivers/infiniband/hw/cxgb4/restrack.c | 9 ++-------
 include/rdma/ib_verbs.h                | 4 ++--
 7 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 23af3cc27ee1..49637a3cd068 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -2616,8 +2616,8 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
 	SET_DEVICE_OP(dev_ops, drain_rq);
 	SET_DEVICE_OP(dev_ops, drain_sq);
 	SET_DEVICE_OP(dev_ops, enable_driver);
+	SET_DEVICE_OP(dev_ops, fill_res_cm_id_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_cq_entry);
-	SET_DEVICE_OP(dev_ops, fill_res_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_mr_entry);
 	SET_DEVICE_OP(dev_ops, fill_res_qp_entry);
 	SET_DEVICE_OP(dev_ops, fill_stat_mr_entry);
diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 8c748888bf28..02a1f50b23be 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -549,7 +549,7 @@ static int fill_res_cm_id_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	return dev->ops.fill_res_entry(msg, res);
+	return dev->ops.fill_res_cm_id_entry(msg, cm_id);
 
 err: return -EMSGSIZE;
 }
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index 33d7c0888753..bad96ab58fc9 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -21,8 +21,7 @@
 		return 0;					\
 	}
 
-static int fill_res_dummy(struct sk_buff *msg,
-			  struct rdma_restrack_entry *entry)
+static int fill_res_cm_id(struct sk_buff *msg, struct rdma_cm_id *id)
 {
 	return 0;
 }
@@ -32,8 +31,8 @@ FILL_DUMMY(cq);
 FILL_DUMMY(qp);
 
 static const struct ib_device_ops restrack_dummy_ops = {
+	.fill_res_cm_id_entry = fill_res_cm_id,
 	.fill_res_cq_entry = fill_res_cq,
-	.fill_res_entry = fill_res_dummy,
 	.fill_res_mr_entry = fill_res_mr,
 	.fill_res_qp_entry = fill_res_qp,
 	.fill_stat_mr_entry = fill_res_mr,
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index c84aa7c937f1..27da0705c88a 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -1053,11 +1053,9 @@ int c4iw_post_srq_recv(struct ib_srq *ibsrq, const struct ib_recv_wr *wr,
 		       const struct ib_recv_wr **bad_wr);
 struct c4iw_wr_wait *c4iw_alloc_wr_wait(gfp_t gfp);
 
-typedef int c4iw_restrack_func(struct sk_buff *msg,
-			       struct rdma_restrack_entry *res);
 int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr);
 int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq);
 int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp);
-extern c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX];
+int c4iw_fill_res_cm_id_entry(struct sk_buff *msg, struct rdma_cm_id *cm_id);
 
 #endif
diff --git a/drivers/infiniband/hw/cxgb4/provider.c b/drivers/infiniband/hw/cxgb4/provider.c
index d6b20aa314a0..1d3ff59e4060 100644
--- a/drivers/infiniband/hw/cxgb4/provider.c
+++ b/drivers/infiniband/hw/cxgb4/provider.c
@@ -458,13 +458,6 @@ static void get_dev_fw_str(struct ib_device *dev, char *str)
 		 FW_HDR_FW_VER_BUILD_G(c4iw_dev->rdev.lldi.fw_vers));
 }
 
-static int fill_res_entry(struct sk_buff *msg, struct rdma_restrack_entry *res)
-{
-	return (res->type < ARRAY_SIZE(c4iw_restrack_funcs) &&
-		c4iw_restrack_funcs[res->type]) ?
-		c4iw_restrack_funcs[res->type](msg, res) : 0;
-}
-
 static const struct ib_device_ops c4iw_dev_ops = {
 	.owner = THIS_MODULE,
 	.driver_id = RDMA_DRIVER_CXGB4,
@@ -486,7 +479,7 @@ static const struct ib_device_ops c4iw_dev_ops = {
 	.destroy_qp = c4iw_destroy_qp,
 	.destroy_srq = c4iw_destroy_srq,
 	.fill_res_cq_entry = c4iw_fill_res_cq_entry,
-	.fill_res_entry = fill_res_entry,
+	.fill_res_cm_id_entry = c4iw_fill_res_cm_id_entry,
 	.fill_res_mr_entry = c4iw_fill_res_mr_entry,
 	.get_dev_fw_str = get_dev_fw_str,
 	.get_dma_mr = c4iw_get_dma_mr,
diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
index 5144d3b67293..b32e6516d65f 100644
--- a/drivers/infiniband/hw/cxgb4/restrack.c
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -193,10 +193,9 @@ union union_ep {
 	struct c4iw_ep ep;
 };
 
-static int fill_res_ep_entry(struct sk_buff *msg,
-			     struct rdma_restrack_entry *res)
+int c4iw_fill_res_cm_id_entry(struct sk_buff *msg,
+			      struct rdma_cm_id *cm_id)
 {
-	struct rdma_cm_id *cm_id = rdma_res_to_id(res);
 	struct nlattr *table_attr;
 	struct c4iw_ep_common *epcp;
 	struct c4iw_listen_ep *listen_ep = NULL;
@@ -486,7 +485,3 @@ int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr)
 err:
 	return -EMSGSIZE;
 }
-
-c4iw_restrack_func *c4iw_restrack_funcs[RDMA_RESTRACK_MAX] = {
-	[RDMA_RESTRACK_CM_ID]	= fill_res_ep_entry,
-};
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 194a15b71498..4b7812340167 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -75,6 +75,7 @@ struct ib_umem_odp;
 struct ib_uqp_object;
 struct ib_usrq_object;
 struct ib_uwq_object;
+struct rdma_cm_id;
 
 extern struct workqueue_struct *ib_wq;
 extern struct workqueue_struct *ib_comp_wq;
@@ -2567,11 +2568,10 @@ struct ib_device_ops {
 	/**
 	 * Allows rdma drivers to add their own restrack attributes.
 	 */
-	int (*fill_res_entry)(struct sk_buff *msg,
-			      struct rdma_restrack_entry *entry);
 	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
 	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq);
 	int (*fill_res_qp_entry)(struct sk_buff *msg, struct ib_qp *ibqp);
+	int (*fill_res_cm_id_entry)(struct sk_buff *msg, struct rdma_cm_id *id);
 
 	/* Device lifecycle callbacks */
 	/*
-- 
2.26.2

[PATCH rdma-next 11/14] RDMA: Add support to dump resource tracker in RAW format

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Add support to get resource dump in raw format. It enable vendors
to return the entire QP/CQ/MR context without a need from the vendor
to set each field separately.
When user request to get the data in RAW, we return as key value
the generic fields which not require to query the vendor and in addition
we return the rest of the data as binary.

Example:

$rdma res show mr dev mlx5_1 mrn 2 -r -j
[{"ifindex":7,"ifname":"mlx5_1","mrn":2,"mrlen":4096,"pdn":5,
pid":24336, "comm":"ibv_rc_pingpong",
"data":[0,4,255,254,0,0,0,0,0,0,0,0,16,28,0,216,...]}]

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/core/nldev.c               | 84 ++++++++++++-------
 drivers/infiniband/core/restrack.c            | 18 ++--
 drivers/infiniband/hw/cxgb4/iw_cxgb4.h        |  6 +-
 drivers/infiniband/hw/cxgb4/restrack.c        | 15 +++-
 drivers/infiniband/hw/hns/hns_roce_device.h   |  2 +-
 drivers/infiniband/hw/hns/hns_roce_restrack.c |  5 +-
 drivers/infiniband/hw/mlx5/mlx5_ib.h          |  3 +-
 drivers/infiniband/hw/mlx5/restrack.c         |  5 +-
 include/rdma/ib_verbs.h                       |  9 +-
 include/uapi/rdma/rdma_netlink.h              |  2 +
 10 files changed, 102 insertions(+), 47 deletions(-)

diff --git a/drivers/infiniband/core/nldev.c b/drivers/infiniband/core/nldev.c
index 02a1f50b23be..7437b5c22479 100644
--- a/drivers/infiniband/core/nldev.c
+++ b/drivers/infiniband/core/nldev.c
@@ -44,7 +44,7 @@
 #include "uverbs.h"
 
 typedef int (*res_fill_func_t)(struct sk_buff*, bool,
-			       struct rdma_restrack_entry*, uint32_t);
+			       struct rdma_restrack_entry*, uint32_t, bool);
 
 /*
  * Sort array elements by the netlink attribute name
@@ -114,6 +114,7 @@ static const struct nla_policy nldev_policy[RDMA_NLDEV_ATTR_MAX] = {
 	[RDMA_NLDEV_ATTR_RES_PS]		= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_RES_QP]		= { .type = NLA_NESTED },
 	[RDMA_NLDEV_ATTR_RES_QP_ENTRY]		= { .type = NLA_NESTED },
+	[RDMA_NLDEV_ATTR_RES_RAW]		= { .type = NLA_BINARY },
 	[RDMA_NLDEV_ATTR_RES_RKEY]		= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_RES_RQPN]		= { .type = NLA_U32 },
 	[RDMA_NLDEV_ATTR_RES_RQ_PSN]		= { .type = NLA_U32 },
@@ -446,11 +447,11 @@ static int fill_res_name_pid(struct sk_buff *msg,
 	return err ? -EMSGSIZE : 0;
 }
 
-static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
-			     struct rdma_restrack_entry *res, uint32_t port)
+static int fill_res_qp_entry_query(struct sk_buff *msg,
+				   struct rdma_restrack_entry *res,
+				   struct ib_device *dev,
+				   struct ib_qp *qp)
 {
-	struct ib_qp *qp = container_of(res, struct ib_qp, res);
-	struct ib_device *dev = qp->device;
 	struct ib_qp_init_attr qp_init_attr;
 	struct ib_qp_attr qp_attr;
 	int ret;
@@ -459,16 +460,6 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (ret)
 		return ret;
 
-	if (port && port != qp_attr.port_num)
-		return -EAGAIN;
-
-	/* In create_qp() port is not set yet */
-	if (qp_attr.port_num &&
-	    nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, qp_attr.port_num))
-		goto err;
-
-	if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LQPN, qp->qp_num))
-		goto err;
 	if (qp->qp_type == IB_QPT_RC || qp->qp_type == IB_QPT_UC) {
 		if (nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_RQPN,
 				qp_attr.dest_qp_num))
@@ -492,20 +483,48 @@ static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (nla_put_u8(msg, RDMA_NLDEV_ATTR_RES_STATE, qp_attr.qp_state))
 		goto err;
 
+	return dev->ops.fill_res_qp_entry(msg, qp, false);
+
+err:	return -EMSGSIZE;
+}
+
+static int fill_res_qp_entry(struct sk_buff *msg, bool has_cap_net_admin,
+			     struct rdma_restrack_entry *res, uint32_t port,
+			     bool raw)
+{
+	struct ib_qp *qp = container_of(res, struct ib_qp, res);
+	struct ib_device *dev = qp->device;
+	int ret;
+
+	if (port && port != qp->port)
+		return -EAGAIN;
+
+	/* In create_qp() port is not set yet */
+	if (qp->port && nla_put_u32(msg, RDMA_NLDEV_ATTR_PORT_INDEX, qp->port))
+		return -EINVAL;
+
+	ret = nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_LQPN, qp->qp_num);
+	if (ret)
+		goto err;
+
 	if (!rdma_is_kernel_res(res) &&
 	    nla_put_u32(msg, RDMA_NLDEV_ATTR_RES_PDN, qp->pd->res.id))
 		goto err;
 
-	if (fill_res_name_pid(msg, res))
+	ret = fill_res_name_pid(msg, res);
+	if (ret)
 		goto err;
 
-	return dev->ops.fill_res_qp_entry(msg, qp);
+	if (raw)
+		return dev->ops.fill_res_qp_entry(msg, qp, raw);
+	return fill_res_qp_entry_query(msg, res, dev, qp);
 
 err:	return -EMSGSIZE;
 }
 
 static int fill_res_cm_id_entry(struct sk_buff *msg, bool has_cap_net_admin,
-				struct rdma_restrack_entry *res, uint32_t port)
+				struct rdma_restrack_entry *res, uint32_t port,
+				bool raw)
 {
 	struct rdma_id_private *id_priv =
 				container_of(res, struct rdma_id_private, res);
@@ -555,7 +574,8 @@ err: return -EMSGSIZE;
 }
 
 static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
-			     struct rdma_restrack_entry *res, uint32_t port)
+			     struct rdma_restrack_entry *res, uint32_t port,
+			     bool raw)
 {
 	struct ib_cq *cq = container_of(res, struct ib_cq, res);
 	struct ib_device *dev = cq->device;
@@ -584,13 +604,14 @@ static int fill_res_cq_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	return dev->ops.fill_res_cq_entry(msg, cq);
+	return dev->ops.fill_res_cq_entry(msg, cq, raw);
 
 err:	return -EMSGSIZE;
 }
 
 static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
-			     struct rdma_restrack_entry *res, uint32_t port)
+			     struct rdma_restrack_entry *res, uint32_t port,
+			     bool raw)
 {
 	struct ib_mr *mr = container_of(res, struct ib_mr, res);
 	struct ib_device *dev = mr->pd->device;
@@ -616,13 +637,14 @@ static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
 	if (fill_res_name_pid(msg, res))
 		goto err;
 
-	return dev->ops.fill_res_mr_entry(msg, mr);
+	return dev->ops.fill_res_mr_entry(msg, mr, raw);
 
 err:	return -EMSGSIZE;
 }
 
 static int fill_res_pd_entry(struct sk_buff *msg, bool has_cap_net_admin,
-			     struct rdma_restrack_entry *res, uint32_t port)
+			     struct rdma_restrack_entry *res, uint32_t port,
+			     bool raw)
 {
 	struct ib_pd *pd = container_of(res, struct ib_pd, res);
 
@@ -750,7 +772,8 @@ int rdma_nl_stat_hwcounter_entry(struct sk_buff *msg, const char *name,
 EXPORT_SYMBOL(rdma_nl_stat_hwcounter_entry);
 
 static int fill_stat_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
-			      struct rdma_restrack_entry *res, uint32_t port)
+			      struct rdma_restrack_entry *res, uint32_t port,
+			      bool raw)
 {
 	struct ib_mr *mr = container_of(res, struct ib_mr, res);
 	struct ib_device *dev = mr->pd->device;
@@ -786,7 +809,7 @@ static int fill_stat_counter_hwcounters(struct sk_buff *msg,
 
 static int fill_res_counter_entry(struct sk_buff *msg, bool has_cap_net_admin,
 				  struct rdma_restrack_entry *res,
-				  uint32_t port)
+				  uint32_t port, bool raw)
 {
 	struct rdma_counter *counter =
 		container_of(res, struct rdma_counter, res);
@@ -1200,6 +1223,7 @@ static int res_get_common_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	u32 index, id, port = 0;
 	bool has_cap_net_admin;
 	struct sk_buff *msg;
+	bool raw = false;
 	int ret;
 
 	ret = nlmsg_parse_deprecated(nlh, 0, tb, RDMA_NLDEV_ATTR_MAX - 1,
@@ -1207,6 +1231,11 @@ static int res_get_common_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (ret || !tb[RDMA_NLDEV_ATTR_DEV_INDEX] || !fe->id || !tb[fe->id])
 		return -EINVAL;
 
+	if (tb[RDMA_NLDEV_ATTR_RES_RAW])
+		raw = nla_get_u8(tb[RDMA_NLDEV_ATTR_RES_RAW]);
+	if (raw && res_type == RDMA_RESTRACK_CM_ID)
+		return -EOPNOTSUPP;
+
 	index = nla_get_u32(tb[RDMA_NLDEV_ATTR_DEV_INDEX]);
 	device = ib_device_get_by_index(sock_net(skb->sk), index);
 	if (!device)
@@ -1249,8 +1278,7 @@ static int res_get_common_doit(struct sk_buff *skb, struct nlmsghdr *nlh,
 	}
 
 	has_cap_net_admin = netlink_capable(skb, CAP_NET_ADMIN);
-
-	ret = fill_func(msg, has_cap_net_admin, res, port);
+	ret = fill_func(msg, has_cap_net_admin, res, port, raw);
 	if (ret)
 		goto err_free;
 
@@ -1356,7 +1384,7 @@ static int res_get_common_dumpit(struct sk_buff *skb,
 			goto msg_full;
 		}
 
-		ret = fill_func(skb, has_cap_net_admin, res, port);
+		ret = fill_func(skb, has_cap_net_admin, res, port, false);
 
 		rdma_restrack_put(res);
 
diff --git a/drivers/infiniband/core/restrack.c b/drivers/infiniband/core/restrack.c
index bad96ab58fc9..dfebe992fab6 100644
--- a/drivers/infiniband/core/restrack.c
+++ b/drivers/infiniband/core/restrack.c
@@ -15,17 +15,23 @@
 #include "restrack.h"
 
 #define FILL_DUMMY(type) \
-	static int fill_res_##type(struct sk_buff *msg,		\
-				   struct ib_##type *res)	\
-	{							\
-		return 0;					\
-	}
+	static int fill_res_##type(struct sk_buff *msg,			\
+				   struct ib_##type *res, bool raw)	\
+	{								\
+		return 0;						\
+	}								\
+									\
 
 static int fill_res_cm_id(struct sk_buff *msg, struct rdma_cm_id *id)
 {
 	return 0;
 }
 
+static int fill_stat_mr(struct sk_buff *msg, struct ib_mr *ibmr)
+{
+	return 0;
+}
+
 FILL_DUMMY(mr);
 FILL_DUMMY(cq);
 FILL_DUMMY(qp);
@@ -35,7 +41,7 @@ static const struct ib_device_ops restrack_dummy_ops = {
 	.fill_res_cq_entry = fill_res_cq,
 	.fill_res_mr_entry = fill_res_mr,
 	.fill_res_qp_entry = fill_res_qp,
-	.fill_stat_mr_entry = fill_res_mr,
+	.fill_stat_mr_entry = fill_stat_mr,
 };
 
 /**
diff --git a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
index 27da0705c88a..858c9ae9a5e3 100644
--- a/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
+++ b/drivers/infiniband/hw/cxgb4/iw_cxgb4.h
@@ -1053,9 +1053,9 @@ int c4iw_post_srq_recv(struct ib_srq *ibsrq, const struct ib_recv_wr *wr,
 		       const struct ib_recv_wr **bad_wr);
 struct c4iw_wr_wait *c4iw_alloc_wr_wait(gfp_t gfp);
 
-int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr);
-int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq);
-int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp);
+int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr, bool raw);
+int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq, bool raw);
+int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp, bool raw);
 int c4iw_fill_res_cm_id_entry(struct sk_buff *msg, struct rdma_cm_id *cm_id);
 
 #endif
diff --git a/drivers/infiniband/hw/cxgb4/restrack.c b/drivers/infiniband/hw/cxgb4/restrack.c
index b32e6516d65f..5ca11daae9d4 100644
--- a/drivers/infiniband/hw/cxgb4/restrack.c
+++ b/drivers/infiniband/hw/cxgb4/restrack.c
@@ -134,7 +134,7 @@ static int fill_swsqes(struct sk_buff *msg, struct t4_sq *sq,
 	return -EMSGSIZE;
 }
 
-int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp)
+int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp, bool raw)
 {
 	struct t4_swsqe *fsp = NULL, *lsp = NULL;
 	struct c4iw_qp *qhp = to_c4iw_qp(ibqp);
@@ -143,6 +143,9 @@ int c4iw_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp)
 	struct nlattr *table_attr;
 	struct t4_wq wq;
 
+	if (raw)
+		return -EOPNOTSUPP;
+
 	/* User qp state is not available, so don't dump user qps */
 	if (qhp->ucontext)
 		return 0;
@@ -369,7 +372,7 @@ static int fill_swcqes(struct sk_buff *msg, struct t4_cq *cq,
 	return -EMSGSIZE;
 }
 
-int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq)
+int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq, bool raw)
 {
 	struct c4iw_cq *chp = to_c4iw_cq(ibcq);
 	struct nlattr *table_attr;
@@ -378,6 +381,9 @@ int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq)
 	struct t4_cq cq;
 	u16 idx;
 
+	if (raw)
+		return -EOPNOTSUPP;
+
 	/* User cq state is not available, so don't dump user cqs */
 	if (ibcq->uobject)
 		return 0;
@@ -428,7 +434,7 @@ int c4iw_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq)
 	return -EMSGSIZE;
 }
 
-int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr)
+int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr, bool raw)
 {
 	struct c4iw_mr *mhp = to_c4iw_mr(ibmr);
 	struct c4iw_dev *dev = mhp->rhp;
@@ -437,6 +443,9 @@ int c4iw_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ibmr)
 	struct fw_ri_tpte tpte;
 	int ret;
 
+	if (raw)
+		return -EOPNOTSUPP;
+
 	if (!stag)
 		return 0;
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 556449a143ed..08375b5dfb27 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -1258,5 +1258,5 @@ int hns_roce_init(struct hns_roce_dev *hr_dev);
 void hns_roce_exit(struct hns_roce_dev *hr_dev);
 
 int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
-			       struct ib_cq *ib_cq);
+			       struct ib_cq *ib_cq, bool raw);
 #endif /* _HNS_ROCE_DEVICE_H */
diff --git a/drivers/infiniband/hw/hns/hns_roce_restrack.c b/drivers/infiniband/hw/hns/hns_roce_restrack.c
index 259444c0a630..c3e95ef8decd 100644
--- a/drivers/infiniband/hw/hns/hns_roce_restrack.c
+++ b/drivers/infiniband/hw/hns/hns_roce_restrack.c
@@ -77,7 +77,7 @@ static int hns_roce_fill_cq(struct sk_buff *msg,
 }
 
 int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
-			       struct ib_cq *ib_cq)
+			       struct ib_cq *ib_cq, bool raw)
 {
 	struct hns_roce_dev *hr_dev = to_hr_dev(ib_cq->device);
 	struct hns_roce_cq *hr_cq = to_hr_cq(ib_cq);
@@ -85,6 +85,9 @@ int hns_roce_fill_res_cq_entry(struct sk_buff *msg,
 	struct nlattr *table_attr;
 	int ret;
 
+	if (raw)
+		return -EOPNOTSUPP;
+
 	if (!hr_dev->dfx->query_cqc_info)
 		return -EINVAL;
 
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index d2b36f4ce508..270a9f1c92c4 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -1383,7 +1383,8 @@ struct mlx5_core_dev *mlx5_ib_get_native_port_mdev(struct mlx5_ib_dev *dev,
 						   u8 *native_port_num);
 void mlx5_ib_put_native_port_mdev(struct mlx5_ib_dev *dev,
 				  u8 port_num);
-int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr);
+int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr,
+			      bool raw);
 int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr);
 
 extern const struct uapi_definition mlx5_ib_devx_defs[];
diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
index 598a09796d09..32b1bdf5768b 100644
--- a/drivers/infiniband/hw/mlx5/restrack.c
+++ b/drivers/infiniband/hw/mlx5/restrack.c
@@ -41,11 +41,14 @@ int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg,
 }
 
 int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg,
-			      struct ib_mr *ibmr)
+			      struct ib_mr *ibmr, bool raw)
 {
 	struct mlx5_ib_mr *mr = to_mmr(ibmr);
 	struct nlattr *table_attr;
 
+	if (raw)
+		return -EOPNOTSUPP;
+
 	if (!(mr->access_flags & IB_ACCESS_ON_DEMAND))
 		return 0;
 
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 4b7812340167..4bf222e8195f 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2568,9 +2568,12 @@ struct ib_device_ops {
 	/**
 	 * Allows rdma drivers to add their own restrack attributes.
 	 */
-	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr);
-	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq);
-	int (*fill_res_qp_entry)(struct sk_buff *msg, struct ib_qp *ibqp);
+	int (*fill_res_mr_entry)(struct sk_buff *msg, struct ib_mr *ibmr,
+				 bool raw);
+	int (*fill_res_cq_entry)(struct sk_buff *msg, struct ib_cq *ibcq,
+				 bool raw);
+	int (*fill_res_qp_entry)(struct sk_buff *msg, struct ib_qp *ibqp,
+				 bool raw);
 	int (*fill_res_cm_id_entry)(struct sk_buff *msg, struct rdma_cm_id *id);
 
 	/* Device lifecycle callbacks */
diff --git a/include/uapi/rdma/rdma_netlink.h b/include/uapi/rdma/rdma_netlink.h
index 8e277783fa96..122658d1fae4 100644
--- a/include/uapi/rdma/rdma_netlink.h
+++ b/include/uapi/rdma/rdma_netlink.h
@@ -525,6 +525,8 @@ enum rdma_nldev_attr {
 	 */
 	RDMA_NLDEV_ATTR_DEV_DIM,                /* u8 */
 
+	RDMA_NLDEV_ATTR_RES_RAW,	/* binary */
+
 	/*
 	 * Always the end
 	 */
-- 
2.26.2

Re: [PATCH rdma-next 11/14] RDMA: Add support to dump resource tracker in RAW format

[Jason Gunthorpe]

On Wed, May 13, 2020 at 12:50:31PM +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@mellanox.com>
> 
> Add support to get resource dump in raw format. It enable vendors
> to return the entire QP/CQ/MR context without a need from the vendor
> to set each field separately.
> When user request to get the data in RAW, we return as key value
> the generic fields which not require to query the vendor and in addition
> we return the rest of the data as binary.
> 
> Example:
> 
> $rdma res show mr dev mlx5_1 mrn 2 -r -j
> [{"ifindex":7,"ifname":"mlx5_1","mrn":2,"mrlen":4096,"pdn":5,
> pid":24336, "comm":"ibv_rc_pingpong",
> "data":[0,4,255,254,0,0,0,0,0,0,0,0,16,28,0,216,...]}]

That is pretty gross, why not bas64 encode it or something?

>  static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
> -			     struct rdma_restrack_entry *res, uint32_t port)
> +			     struct rdma_restrack_entry *res, uint32_t port,
> +			     bool raw)
>  {

Should it be a flag not a bool?

Jason

Re: [PATCH rdma-next 11/14] RDMA: Add support to dump resource tracker in RAW format

[Leon Romanovsky]

On Mon, May 25, 2020 at 11:41:36AM -0300, Jason Gunthorpe wrote:
> On Wed, May 13, 2020 at 12:50:31PM +0300, Leon Romanovsky wrote:
> > From: Maor Gottlieb <maorg@mellanox.com>
> >
> > Add support to get resource dump in raw format. It enable vendors
> > to return the entire QP/CQ/MR context without a need from the vendor
> > to set each field separately.
> > When user request to get the data in RAW, we return as key value
> > the generic fields which not require to query the vendor and in addition
> > we return the rest of the data as binary.
> >
> > Example:
> >
> > $rdma res show mr dev mlx5_1 mrn 2 -r -j
> > [{"ifindex":7,"ifname":"mlx5_1","mrn":2,"mrlen":4096,"pdn":5,
> > pid":24336, "comm":"ibv_rc_pingpong",
> > "data":[0,4,255,254,0,0,0,0,0,0,0,0,16,28,0,216,...]}]
>
> That is pretty gross, why not bas64 encode it or something?

We are talking about rdmatool output, right? It can.

>
> >  static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
> > -			     struct rdma_restrack_entry *res, uint32_t port)
> > +			     struct rdma_restrack_entry *res, uint32_t port,
> > +			     bool raw)
> >  {
>
> Should it be a flag not a bool?

I assume that once this RAW series will be merged, the MR res dump will
be close to feature complete.

Thank

>
> Jason

Re: [PATCH rdma-next 11/14] RDMA: Add support to dump resource tracker in RAW format

[Maor Gottlieb]

On 5/25/2020 7:21 PM, Leon Romanovsky wrote:
> On Mon, May 25, 2020 at 11:41:36AM -0300, Jason Gunthorpe wrote:
>> On Wed, May 13, 2020 at 12:50:31PM +0300, Leon Romanovsky wrote:
>>> From: Maor Gottlieb <maorg@mellanox.com>
>>>
>>> Add support to get resource dump in raw format. It enable vendors
>>> to return the entire QP/CQ/MR context without a need from the vendor
>>> to set each field separately.
>>> When user request to get the data in RAW, we return as key value
>>> the generic fields which not require to query the vendor and in addition
>>> we return the rest of the data as binary.
>>>
>>> Example:
>>>
>>> $rdma res show mr dev mlx5_1 mrn 2 -r -j
>>> [{"ifindex":7,"ifname":"mlx5_1","mrn":2,"mrlen":4096,"pdn":5,
>>> pid":24336, "comm":"ibv_rc_pingpong",
>>> "data":[0,4,255,254,0,0,0,0,0,0,0,0,16,28,0,216,...]}]
>> That is pretty gross, why not bas64 encode it or something?
> We are talking about rdmatool output, right? It can.

Consider we are referring to the user space part, this kind of change 
requires to implement some encode and decode which looks redundant for 
me. Further more, devlink reporter do the same and I would like to use 
the same parser tool which used to parse the devlink output.
>
>>>   static int fill_res_mr_entry(struct sk_buff *msg, bool has_cap_net_admin,
>>> -			     struct rdma_restrack_entry *res, uint32_t port)
>>> +			     struct rdma_restrack_entry *res, uint32_t port,
>>> +			     bool raw)
>>>   {
>> Should it be a flag not a bool?
> I assume that once this RAW series will be merged, the MR res dump will
> be close to feature complete.
>
> Thank
>
>> Jason

[PATCH rdma-next 12/14] RDMA/mlx5: Add support to get QP resource in raw format

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Add a generic function that use the resource dump mechanism
to get the resource data.
This patch adds the support to QP.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c     |  1 +
 drivers/infiniband/hw/mlx5/mlx5_ib.h  |  2 +
 drivers/infiniband/hw/mlx5/restrack.c | 79 +++++++++++++++++++++++++++
 3 files changed, 82 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index fdc9ab990f6b..ba298a8e3ce1 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -6615,6 +6615,7 @@ static const struct ib_device_ops mlx5_ib_dev_ops = {
 	.drain_sq = mlx5_ib_drain_sq,
 	.enable_driver = mlx5_ib_enable_driver,
 	.fill_res_mr_entry = mlx5_ib_fill_res_mr_entry,
+	.fill_res_qp_entry = mlx5_ib_fill_res_qp_entry,
 	.fill_stat_mr_entry = mlx5_ib_fill_stat_mr_entry,
 	.get_dev_fw_str = get_dev_fw_str,
 	.get_dma_mr = mlx5_ib_get_dma_mr,
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 270a9f1c92c4..8959db266a35 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -1385,6 +1385,8 @@ void mlx5_ib_put_native_port_mdev(struct mlx5_ib_dev *dev,
 				  u8 port_num);
 int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr,
 			      bool raw);
+int mlx5_ib_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp,
+			      bool raw);
 int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr);
 
 extern const struct uapi_definition mlx5_ib_devx_defs[];
diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
index 32b1bdf5768b..cf2322109f88 100644
--- a/drivers/infiniband/hw/mlx5/restrack.c
+++ b/drivers/infiniband/hw/mlx5/restrack.c
@@ -4,10 +4,79 @@
  */
 
 #include <uapi/rdma/rdma_netlink.h>
+#include <linux/mlx5/rsc_dump.h>
 #include <rdma/ib_umem_odp.h>
 #include <rdma/restrack.h>
 #include "mlx5_ib.h"
 
+#define MAX_DUMP_SIZE 1024
+
+static int dump_rsc(struct mlx5_core_dev *dev, enum mlx5_sgmt_type type,
+		    int index, void *data, int *data_len)
+{
+	struct mlx5_core_dev *mdev = dev;
+	struct mlx5_rsc_dump_cmd *cmd;
+	struct mlx5_rsc_key key = {};
+	struct page *page;
+	int offset = 0;
+	int err = 0;
+	int cmd_err;
+	int size;
+
+	page = alloc_page(GFP_KERNEL);
+	if (!page)
+		return -ENOMEM;
+
+	key.size = PAGE_SIZE;
+	key.rsc = type;
+	key.index1 = index;
+	key.num_of_obj1 = 1;
+
+	cmd = mlx5_rsc_dump_cmd_create(mdev, &key);
+	if (IS_ERR(cmd)) {
+		err = PTR_ERR(cmd);
+		goto free_page;
+	}
+
+	do {
+		cmd_err = mlx5_rsc_dump_next(mdev, cmd, page, &size);
+		if (cmd_err < 0 || size + offset > MAX_DUMP_SIZE) {
+			err = cmd_err;
+			goto destroy_cmd;
+		}
+		memcpy(data + offset, page_address(page), size);
+		offset += size;
+	} while (cmd_err > 0);
+	*data_len = offset;
+
+destroy_cmd:
+	mlx5_rsc_dump_cmd_destroy(cmd);
+free_page:
+	__free_page(page);
+	return err;
+}
+
+static int fill_res_raw(struct sk_buff *msg, struct mlx5_ib_dev *dev,
+			enum mlx5_sgmt_type type, u32 key)
+{
+	int len = 0;
+	void *data;
+	int err;
+
+	data = kzalloc(MAX_DUMP_SIZE, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	err = dump_rsc(dev->mdev, type, key, data, &len);
+	if (err)
+		goto out;
+
+	err = nla_put(msg, RDMA_NLDEV_ATTR_RES_RAW, len, data);
+out:
+	kfree(data);
+	return err;
+}
+
 int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg,
 			       struct ib_mr *ibmr)
 {
@@ -71,3 +140,13 @@ int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg,
 	nla_nest_cancel(msg, table_attr);
 	return -EMSGSIZE;
 }
+
+int mlx5_ib_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp, bool raw)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ibqp->device);
+
+	if (!raw)
+		return 0;
+	return fill_res_raw(msg, dev, MLX5_SGMT_TYPE_PRM_QUERY_QP,
+			    ibqp->qp_num);
+}
-- 
2.26.2

[PATCH rdma-next 13/14] RDMA/mlx5: Add support to get CQ resource in RAW format

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Add support to get CQ resource dump in RAW format.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/main.c     |  1 +
 drivers/infiniband/hw/mlx5/mlx5_ib.h  |  2 ++
 drivers/infiniband/hw/mlx5/restrack.c | 10 ++++++++++
 3 files changed, 13 insertions(+)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index ba298a8e3ce1..a707576a8337 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -6616,6 +6616,7 @@ static const struct ib_device_ops mlx5_ib_dev_ops = {
 	.enable_driver = mlx5_ib_enable_driver,
 	.fill_res_mr_entry = mlx5_ib_fill_res_mr_entry,
 	.fill_res_qp_entry = mlx5_ib_fill_res_qp_entry,
+	.fill_res_cq_entry = mlx5_ib_fill_res_cq_entry,
 	.fill_stat_mr_entry = mlx5_ib_fill_stat_mr_entry,
 	.get_dev_fw_str = get_dev_fw_str,
 	.get_dma_mr = mlx5_ib_get_dma_mr,
diff --git a/drivers/infiniband/hw/mlx5/mlx5_ib.h b/drivers/infiniband/hw/mlx5/mlx5_ib.h
index 8959db266a35..b486139b08ce 100644
--- a/drivers/infiniband/hw/mlx5/mlx5_ib.h
+++ b/drivers/infiniband/hw/mlx5/mlx5_ib.h
@@ -1387,6 +1387,8 @@ int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr,
 			      bool raw);
 int mlx5_ib_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp,
 			      bool raw);
+int mlx5_ib_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq,
+			      bool raw);
 int mlx5_ib_fill_stat_mr_entry(struct sk_buff *msg, struct ib_mr *ib_mr);
 
 extern const struct uapi_definition mlx5_ib_devx_defs[];
diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
index cf2322109f88..9e1389b8dd9f 100644
--- a/drivers/infiniband/hw/mlx5/restrack.c
+++ b/drivers/infiniband/hw/mlx5/restrack.c
@@ -141,6 +141,16 @@ int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg,
 	return -EMSGSIZE;
 }
 
+int mlx5_ib_fill_res_cq_entry(struct sk_buff *msg, struct ib_cq *ibcq, bool raw)
+{
+	struct mlx5_ib_dev *dev = to_mdev(ibcq->device);
+	struct mlx5_ib_cq *cq = to_mcq(ibcq);
+
+	if (!raw)
+		return 0;
+	return fill_res_raw(msg, dev, MLX5_SGMT_TYPE_PRM_QUERY_CQ, cq->mcq.cqn);
+}
+
 int mlx5_ib_fill_res_qp_entry(struct sk_buff *msg, struct ib_qp *ibqp, bool raw)
 {
 	struct mlx5_ib_dev *dev = to_mdev(ibqp->device);
-- 
2.26.2

[PATCH rdma-next 14/14] RDMA/mlx5: Add support to get MR resource in RAW format

[Leon Romanovsky]

From: Maor Gottlieb <maorg@mellanox.com>

Add support to get MR (mkey) resource dump in RAW format.

Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
---
 drivers/infiniband/hw/mlx5/restrack.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
index 9e1389b8dd9f..834886536127 100644
--- a/drivers/infiniband/hw/mlx5/restrack.c
+++ b/drivers/infiniband/hw/mlx5/restrack.c
@@ -116,7 +116,8 @@ int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg,
 	struct nlattr *table_attr;
 
 	if (raw)
-		return -EOPNOTSUPP;
+		return fill_res_raw(msg, mr->dev, MLX5_SGMT_TYPE_PRM_QUERY_MKEY,
+				    mlx5_mkey_to_idx(mr->mmkey.key));
 
 	if (!(mr->access_flags & IB_ACCESS_ON_DEMAND))
 		return 0;
-- 
2.26.2

Re: [PATCH rdma-next 14/14] RDMA/mlx5: Add support to get MR resource in RAW format

[Jason Gunthorpe]

On Wed, May 13, 2020 at 12:50:34PM +0300, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@mellanox.com>
> 
> Add support to get MR (mkey) resource dump in RAW format.
> 
> Signed-off-by: Maor Gottlieb <maorg@mellanox.com>
> Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
>  drivers/infiniband/hw/mlx5/restrack.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/mlx5/restrack.c b/drivers/infiniband/hw/mlx5/restrack.c
> index 9e1389b8dd9f..834886536127 100644
> +++ b/drivers/infiniband/hw/mlx5/restrack.c
> @@ -116,7 +116,8 @@ int mlx5_ib_fill_res_mr_entry(struct sk_buff *msg,
>  	struct nlattr *table_attr;
>  
>  	if (raw)
> -		return -EOPNOTSUPP;

Does it make sense if raw is requested only raw is returned?

That whole thing doesn't sound very netlinky to me..

Jason

Re: [PATCH rdma-next 00/14] RAW format dumps through RDMAtool

[Leon Romanovsky]

On Wed, May 13, 2020 at 12:50:20PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> From Maor:
>
> Hi,
>
> The following series adds support to get the RDMA resource data in RAW
> format. The main motivation for doing this is to enable vendors to return
> the entire QP/CQ/MR data without a need from the vendor to set each field
> separately.
>
> Thanks
>
> Maor Gottlieb (14):
>   net/mlx5: Export resource dump interface
>   net/mlx5: Add support in query QP, CQ and MKEY segments
>   RDMA/core: Fix double put of resource

This specific patch was already sent to -rc and accepted.
Please ignore.

>   RDMA/core: Allow to override device op
>   RDMA/core: Don't call fill_res_entry for PD
>   RDMA/core: Add restrack dummy ops
>   RDMA: Add dedicated MR resource tracker function
>   RDMA: Add a dedicated CQ resource tracker function
>   RDMA: Add a dedicated QP resource tracker function
>   RDMA: Add dedicated cm id resource tracker function
>   RDMA: Add support to dump resource tracker in RAW format
>   RDMA/mlx5: Add support to get QP resource in raw format
>   RDMA/mlx5: Add support to get CQ resource in RAW format
>   RDMA/mlx5: Add support to get MR resource in RAW format
>
>  drivers/infiniband/core/device.c              |  16 ++-
>  drivers/infiniband/core/nldev.c               | 136 ++++++++----------
>  drivers/infiniband/core/restrack.c            |  32 +++++
>  drivers/infiniband/hw/cxgb4/iw_cxgb4.h        |   7 +-
>  drivers/infiniband/hw/cxgb4/provider.c        |  11 +-
>  drivers/infiniband/hw/cxgb4/restrack.c        |  33 ++---
>  drivers/infiniband/hw/hns/hns_roce_device.h   |   4 +-
>  drivers/infiniband/hw/hns/hns_roce_main.c     |   2 +-
>  drivers/infiniband/hw/hns/hns_roce_restrack.c |  17 +--
>  drivers/infiniband/hw/mlx5/main.c             |   6 +-
>  drivers/infiniband/hw/mlx5/mlx5_ib.h          |  11 +-
>  drivers/infiniband/hw/mlx5/restrack.c         | 105 +++++++++++---
>  .../mellanox/mlx5/core/diag/rsc_dump.c        |   6 +
>  .../mellanox/mlx5/core/diag/rsc_dump.h        |  33 +----
>  .../diag => include/linux/mlx5}/rsc_dump.h    |  25 ++--
>  include/rdma/ib_verbs.h                       |  13 +-
>  include/uapi/rdma/rdma_netlink.h              |   2 +
>  17 files changed, 258 insertions(+), 201 deletions(-)
>  copy {drivers/net/ethernet/mellanox/mlx5/core/diag => include/linux/mlx5}/rsc_dump.h (68%)
>
> --
> 2.26.2
>