Re: [PATCH] RDMA/efa: Add RDMA write HW statistics counters

"Margolin, Michael" <mrgolin@xxxxxxxxxx> · Tue, 4 Jul 2023 11:29:08 +0300

On 7/4/2023 10:20 AM, Gal Pressman wrote:
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.



On 03/07/2023 18:34, Michael Margolin wrote:
Update device API and request RDMA write counters if RDMA write is
supported by device. Expose newly added counters through ib core
counters mechanism.

Reviewed-by: Daniel Kranzdorf <dkkranzd@xxxxxxxxxx>
Reviewed-by: Yonatan Nachum <ynachum@xxxxxxxxxx>
Signed-off-by: Michael Margolin <mrgolin@xxxxxxxxxx>
---
  .../infiniband/hw/efa/efa_admin_cmds_defs.h    | 13 +++++++++++++
  drivers/infiniband/hw/efa/efa_com_cmd.c        |  8 +++++++-
  drivers/infiniband/hw/efa/efa_com_cmd.h        | 10 +++++++++-
  drivers/infiniband/hw/efa/efa_verbs.c          | 18 ++++++++++++++++++
  4 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h b/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h
index 4e93ef7f84ee..9c65bd27bae0 100644
--- a/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h
+++ b/drivers/infiniband/hw/efa/efa_admin_cmds_defs.h
@@ -66,6 +66,7 @@ enum efa_admin_get_stats_type {
       EFA_ADMIN_GET_STATS_TYPE_BASIC              = 0,
       EFA_ADMIN_GET_STATS_TYPE_MESSAGES           = 1,
       EFA_ADMIN_GET_STATS_TYPE_RDMA_READ          = 2,
+     EFA_ADMIN_GET_STATS_TYPE_RDMA_WRITE         = 3,
  };

  enum efa_admin_get_stats_scope {
@@ -570,6 +571,16 @@ struct efa_admin_rdma_read_stats {
       u64 read_resp_bytes;
  };

+struct efa_admin_rdma_write_stats {
+     u64 write_wrs;
+
+     u64 write_bytes;
+
+     u64 write_wr_err;
+
+     u64 write_recv_bytes;
+};
+
  struct efa_admin_acq_get_stats_resp {
       struct efa_admin_acq_common_desc acq_common_desc;

@@ -579,6 +590,8 @@ struct efa_admin_acq_get_stats_resp {
               struct efa_admin_messages_stats messages_stats;

               struct efa_admin_rdma_read_stats rdma_read_stats;
+
+             struct efa_admin_rdma_write_stats rdma_write_stats;
       } u;
  };

diff --git a/drivers/infiniband/hw/efa/efa_com_cmd.c b/drivers/infiniband/hw/efa/efa_com_cmd.c
index 8f8885e002ba..576811885d59 100644
--- a/drivers/infiniband/hw/efa/efa_com_cmd.c
+++ b/drivers/infiniband/hw/efa/efa_com_cmd.c
@@ -1,6 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
  /*
- * Copyright 2018-2021 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
   */

  #include "efa_com.h"
@@ -794,6 +794,12 @@ int efa_com_get_stats(struct efa_com_dev *edev,
               result->rdma_read_stats.read_wr_err = resp.u.rdma_read_stats.read_wr_err;
               result->rdma_read_stats.read_resp_bytes = resp.u.rdma_read_stats.read_resp_bytes;
               break;
+     case EFA_ADMIN_GET_STATS_TYPE_RDMA_WRITE:
+             result->rdma_write_stats.write_wrs = resp.u.rdma_write_stats.write_wrs;
+             result->rdma_write_stats.write_bytes = resp.u.rdma_write_stats.write_bytes;
+             result->rdma_write_stats.write_wr_err = resp.u.rdma_write_stats.write_wr_err;
+             result->rdma_write_stats.write_recv_bytes = resp.u.rdma_write_stats.write_recv_bytes;
+             break;
       }

       return 0;
diff --git a/drivers/infiniband/hw/efa/efa_com_cmd.h b/drivers/infiniband/hw/efa/efa_com_cmd.h
index 0898ad5bc340..fc97f37bb39b 100644
--- a/drivers/infiniband/hw/efa/efa_com_cmd.h
+++ b/drivers/infiniband/hw/efa/efa_com_cmd.h
@@ -1,6 +1,6 @@
  /* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */
  /*
- * Copyright 2018-2021 Amazon.com, Inc. or its affiliates. All rights reserved.
+ * Copyright 2018-2023 Amazon.com, Inc. or its affiliates. All rights reserved.
   */

  #ifndef _EFA_COM_CMD_H_
@@ -262,10 +262,18 @@ struct efa_com_rdma_read_stats {
       u64 read_resp_bytes;
  };

+struct efa_com_rdma_write_stats {
+     u64 write_wrs;
+     u64 write_bytes;
+     u64 write_wr_err;
+     u64 write_recv_bytes;
+};
+
  union efa_com_get_stats_result {
       struct efa_com_basic_stats basic_stats;
       struct efa_com_messages_stats messages_stats;
       struct efa_com_rdma_read_stats rdma_read_stats;
+     struct efa_com_rdma_write_stats rdma_write_stats;
  };

  int efa_com_create_qp(struct efa_com_dev *edev,
diff --git a/drivers/infiniband/hw/efa/efa_verbs.c b/drivers/infiniband/hw/efa/efa_verbs.c
index 2a195c4b0f17..7a27d79c0541 100644
--- a/drivers/infiniband/hw/efa/efa_verbs.c
+++ b/drivers/infiniband/hw/efa/efa_verbs.c
@@ -61,6 +61,10 @@ struct efa_user_mmap_entry {
       op(EFA_RDMA_READ_BYTES, "rdma_read_bytes") \
       op(EFA_RDMA_READ_WR_ERR, "rdma_read_wr_err") \
       op(EFA_RDMA_READ_RESP_BYTES, "rdma_read_resp_bytes") \
+     op(EFA_RDMA_WRITE_WRS, "rdma_write_wrs") \
+     op(EFA_RDMA_WRITE_BYTES, "rdma_write_bytes") \
+     op(EFA_RDMA_WRITE_WR_ERR, "rdma_write_wr_err") \
+     op(EFA_RDMA_WRITE_RECV_BYTES, "rdma_write_recv_bytes") \

  #define EFA_STATS_ENUM(ename, name) ename,
  #define EFA_STATS_STR(ename, nam) \
@@ -2080,6 +2084,7 @@ static int efa_fill_port_stats(struct efa_dev *dev, struct rdma_hw_stats *stats,
  {
       struct efa_com_get_stats_params params = {};
       union efa_com_get_stats_result result;
+     struct efa_com_rdma_write_stats *rws;
       struct efa_com_rdma_read_stats *rrs;
       struct efa_com_messages_stats *ms;
       struct efa_com_basic_stats *bs;
@@ -2121,6 +2126,19 @@ static int efa_fill_port_stats(struct efa_dev *dev, struct rdma_hw_stats *stats,
       stats->value[EFA_RDMA_READ_WR_ERR] = rrs->read_wr_err;
       stats->value[EFA_RDMA_READ_RESP_BYTES] = rrs->read_resp_bytes;

+     if (EFA_DEV_CAP(dev, RDMA_WRITE)) {
I wonder if the same check is missing for RDMA_READ? Or is it redundant
for RDMA_WRITE?

Patch looks good,
Reviewed-by: Gal Pressman <gal.pressman@xxxxxxxxx>

Thanks Gal, RDMA read is fully deployed so device will always support 
requests for related counters.


Michael







