diff mbox

[rdma-next,07/16] IB/core: Introduce counter set query verb

Message ID 1508424118-27205-8-git-send-email-yishaih@mellanox.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Yishai Hadas Oct. 19, 2017, 2:41 p.m. UTC
From: Raed Salem <raeds@mellanox.com>

This patch adds the query counter set verb, enabling the user to
read the hardware counters which are associated with the counter
set.

The user supplies counter set instance and an output address.
The hardware queries the counter set and writes statistics to the
output address as an array of uint64_t, each entry in the uint64_t
array represents a single counter.

Note:
A counter set must be first attached to an IB object in order to
be queried for its underlay counters, downstream patches will
present bind and query methods for flow counters.
The user has an option as part of the query verb to force reading
the up-to-date hardware values instead of reading some cached
values by using the IB_COUNTER_SET_FORCE_UPDATE flag.

Signed-off-by: Raed Salem <raeds@mellanox.com>
Reviewed-by: Yishai Hadas <yishaih@mellanox.com>
---
 drivers/infiniband/core/verbs.c | 21 +++++++++++++++++++++
 include/rdma/ib_verbs.h         | 17 +++++++++++++++++
 2 files changed, 38 insertions(+)

Comments

Christoph Lameter (Ampere) Oct. 20, 2017, 10:48 a.m. UTC | #1
On Thu, 19 Oct 2017, Yishai Hadas wrote:

> A counter set must be first attached to an IB object in order to
> be queried for its underlay counters, downstream patches will
> present bind and query methods for flow counters.
> The user has an option as part of the query verb to force reading
> the up-to-date hardware values instead of reading some cached
> values by using the IB_COUNTER_SET_FORCE_UPDATE flag.

What does "cached" mean in this context? Accurate to the last second? Or
just return what was returned earlier? Semantics look a bit unclear to me.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Guy Shattah Oct. 20, 2017, 3:40 p.m. UTC | #2
On Thu, 19 Oct 2017, Christopher Lameter wrote:
On Thu, 19 Oct 2017, Yishai Hadas wrote:

>> A counter set must be first attached to an IB object in order to
>> be queried for its underlay counters, downstream patches will
>> present bind and query methods for flow counters.
>> The user has an option as part of the query verb to force reading
>> the up-to-date hardware values instead of reading some cached
>> values by using the IB_COUNTER_SET_FORCE_UPDATE flag.

>What does "cached" mean in this context? Accurate to the last second? Or
>just return what was returned earlier? Semantics look a bit unclear to me.

When this flag is used - querying the counter does not guarantee accurate value from the hardware.
The driver might return a value that was true several nonsecs. or even several seconds ago.


Explanation:

Querying the hardware is expensive,  when user queries the hardware
the driver could decide to use the same query to query multiple counter-set
rather than a single one ( the one the user asked for)

The counter-sets the user did not ask for - are stored in the driver cache.
Driver may decide to return these value rather than query the hardware again.


Nevertheless,  when using the the query API user can force the driver to 
execute direct hardware query , rather than return cached value.


Guy


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

Patch

diff --git a/drivers/infiniband/core/verbs.c b/drivers/infiniband/core/verbs.c
index ea71393..ffe51fd 100644
--- a/drivers/infiniband/core/verbs.c
+++ b/drivers/infiniband/core/verbs.c
@@ -2352,3 +2352,24 @@  int ib_destroy_counter_set(struct ib_counter_set *cs)
 	return cs->device->destroy_counter_set(cs);
 }
 EXPORT_SYMBOL(ib_destroy_counter_set);
+
+/**
+ * ib_query_counter_set - Queries a Counter Set
+ * @cs: The counter set to query.
+ * @cs_query_attr: A list of query attributes
+ * required to get the outcome.
+ */
+int ib_query_counter_set(struct ib_counter_set *cs,
+			 struct ib_counter_set_query_attr *cs_query_attr)
+{
+	if (!cs->device->query_counter_set)
+		return -ENOSYS;
+
+	if (!atomic_read(&cs->usecnt))
+		return -EINVAL;
+
+	return cs->device->query_counter_set(cs,
+				cs_query_attr,
+				NULL);
+}
+EXPORT_SYMBOL(ib_query_counter_set);
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index 1c78db7..bc33bc2 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2085,6 +2085,18 @@  struct ib_counter_set {
 	atomic_t	usecnt;
 };
 
+enum ib_query_counter_set_flags {
+	/* force hardware query instead of cached value */
+	IB_COUNTER_SET_FORCE_UPDATE = 1 << 0,
+};
+
+struct ib_counter_set_query_attr {
+	u32	query_flags; /* Use enum ib_query_counter_set_flags */
+	u64	*out_buff;
+	u32	buff_len;
+	u32	outlen;
+};
+
 struct ib_device {
 	/* Do not access @dma_device directly from ULP nor from HW drivers. */
 	struct device                *dma_device;
@@ -2348,6 +2360,9 @@  struct ib_device {
 						      u16 cs_id,
 						      struct ib_udata *udata);
 	int	(*destroy_counter_set)(struct ib_counter_set *cs);
+	int	(*query_counter_set)(struct ib_counter_set *cs,
+				     struct ib_counter_set_query_attr *cs_query_attr,
+				     struct ib_udata *udata);
 
 	/**
 	 * rdma netdev operation
@@ -3662,6 +3677,8 @@  int ib_describe_counter_set(struct ib_device *device,
 struct ib_counter_set *ib_create_counter_set(struct ib_device *device,
 					     u16 cs_id);
 int ib_destroy_counter_set(struct ib_counter_set *cs);
+int ib_query_counter_set(struct ib_counter_set *cs,
+			 struct ib_counter_set_query_attr *cs_query_attr);
 
 int ib_map_mr_sg(struct ib_mr *mr, struct scatterlist *sg, int sg_nents,
 		 unsigned int *sg_offset, unsigned int page_size);