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

[Gal Pressman <gal.pressman@linux.dev>]

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@amazon.com>
> Reviewed-by: Yonatan Nachum <ynachum@amazon.com>
> Signed-off-by: Michael Margolin <mrgolin@amazon.com>
> ---
>  .../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@linux.dev>

> +		params.type = EFA_ADMIN_GET_STATS_TYPE_RDMA_WRITE;
> +		err = efa_com_get_stats(&dev->edev, &params, &result);
> +		if (err)
> +			return err;
> +
> +		rws = &result.rdma_write_stats;
> +		stats->value[EFA_RDMA_WRITE_WRS] = rws->write_wrs;
> +		stats->value[EFA_RDMA_WRITE_BYTES] = rws->write_bytes;
> +		stats->value[EFA_RDMA_WRITE_WR_ERR] = rws->write_wr_err;
> +		stats->value[EFA_RDMA_WRITE_RECV_BYTES] = rws->write_recv_bytes;
> +	}
> +
>  	return ARRAY_SIZE(efa_port_stats_descs);
>  }
>