[dpdk-dev,v2,4/7] rte_sched: don't clear statistics when read
Commit Message
From: Stephen Hemminger <shemming@brocade.com>
Make rte_sched statistics API work like the ethernet statistics API.
Don't auto-clear statistics.
Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
lib/librte_sched/rte_sched.c | 30 ++++++++++++++++++++++++++++++
lib/librte_sched/rte_sched.h | 29 +++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
Comments
On Wed, Feb 04, 2015 at 10:13:58PM -0800, Stephen Hemminger wrote:
> From: Stephen Hemminger <shemming@brocade.com>
>
> Make rte_sched statistics API work like the ethernet statistics API.
> Don't auto-clear statistics.
>
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> ---
> lib/librte_sched/rte_sched.c | 30 ++++++++++++++++++++++++++++++
> lib/librte_sched/rte_sched.h | 29 +++++++++++++++++++++++++++++
> 2 files changed, 59 insertions(+)
>
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index 8cb8bf1..d891e50 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -935,6 +935,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
> }
>
> int
> +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> + uint32_t subport_id)
> +{
> + struct rte_sched_subport *s;
> +
> + /* Check user parameters */
> + if (port == NULL || subport_id >= port->n_subports_per_port)
> + return -1;
> +
> + s = port->subport + subport_id;
> + memset(&s->stats, 0, sizeof(struct rte_sched_subport_stats));
> + return 0;
> +}
> +
> +int
> rte_sched_queue_read_stats(struct rte_sched_port *port,
> uint32_t queue_id,
> struct rte_sched_queue_stats *stats,
> @@ -963,6 +978,21 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
> return 0;
> }
>
> +int
> +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> + uint32_t queue_id)
> +{
> + struct rte_sched_queue_extra *qe;
> +
> + /* Check user parameters */
> + if (port == NULL || queue_id >= rte_sched_port_queues_per_port(port))
> + return -1;
> +
> + qe = port->queue_extra + queue_id;
> + memset(&qe->stats, 0, sizeof(struct rte_sched_queue_stats));
> + return 0;
> +}
> +
> static inline uint32_t
> rte_sched_port_qindex(struct rte_sched_port *port, uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue)
> {
> diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h
> index e9bf18a..3d007e4 100644
> --- a/lib/librte_sched/rte_sched.h
> +++ b/lib/librte_sched/rte_sched.h
> @@ -317,6 +317,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
> struct rte_sched_subport_stats *stats,
> uint32_t *tc_ov);
>
> +
> +/**
> + * Hierarchical scheduler subport statistics reset
> + *
> + * @param port
> + * Handle to port scheduler instance
> + * @param subport_id
> + * Subport ID
> + * @return
> + * 0 upon success, error code otherwise
> + */
> +int
> +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> + uint32_t subport_id);
> +
> /**
> * Hierarchical scheduler queue statistics read
> *
> @@ -338,6 +353,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
> struct rte_sched_queue_stats *stats,
> uint16_t *qlen);
>
> +/**
> + * Hierarchical scheduler queue statistics reset
> + *
> + * @param port
> + * Handle to port scheduler instance
> + * @param queue_id
> + * Queue ID within port scheduler
> + * @return
> + * 0 upon success, error code otherwise
> + */
> +int
> +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> + uint32_t queue_id);
> +
Both need to be added to the version map to expose them properly.
Neil
2015-02-05 07:43, Neil Horman:
> On Wed, Feb 04, 2015 at 10:13:58PM -0800, Stephen Hemminger wrote:
> > +
> > +/**
> > + * Hierarchical scheduler subport statistics reset
> > + *
> > + * @param port
> > + * Handle to port scheduler instance
> > + * @param subport_id
> > + * Subport ID
> > + * @return
> > + * 0 upon success, error code otherwise
> > + */
> > +int
> > +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> > + uint32_t subport_id);
> > +
> > /**
> > * Hierarchical scheduler queue statistics read
> > *
> > @@ -338,6 +353,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
> > struct rte_sched_queue_stats *stats,
> > uint16_t *qlen);
> >
> > +/**
> > + * Hierarchical scheduler queue statistics reset
> > + *
> > + * @param port
> > + * Handle to port scheduler instance
> > + * @param queue_id
> > + * Queue ID within port scheduler
> > + * @return
> > + * 0 upon success, error code otherwise
> > + */
> > +int
> > +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> > + uint32_t queue_id);
> > +
> Both need to be added to the version map to expose them properly.
> Neil
Stephen, this patchset is partially acked and could enter in 2.0.0-rc1.
May you send a v3 addressing comments? Or should I break the serie by
applying only some of them? Or postpone the serie to 2.1?
On Mon, 23 Feb 2015 23:51:31 +0000
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 2015-02-05 07:43, Neil Horman:
> > On Wed, Feb 04, 2015 at 10:13:58PM -0800, Stephen Hemminger wrote:
> > > +
> > > +/**
> > > + * Hierarchical scheduler subport statistics reset
> > > + *
> > > + * @param port
> > > + * Handle to port scheduler instance
> > > + * @param subport_id
> > > + * Subport ID
> > > + * @return
> > > + * 0 upon success, error code otherwise
> > > + */
> > > +int
> > > +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> > > + uint32_t subport_id);
> > > +
> > > /**
> > > * Hierarchical scheduler queue statistics read
> > > *
> > > @@ -338,6 +353,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
> > > struct rte_sched_queue_stats *stats,
> > > uint16_t *qlen);
> > >
> > > +/**
> > > + * Hierarchical scheduler queue statistics reset
> > > + *
> > > + * @param port
> > > + * Handle to port scheduler instance
> > > + * @param queue_id
> > > + * Queue ID within port scheduler
> > > + * @return
> > > + * 0 upon success, error code otherwise
> > > + */
> > > +int
> > > +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> > > + uint32_t queue_id);
> > > +
> > Both need to be added to the version map to expose them properly.
> > Neil
>
> Stephen, this patchset is partially acked and could enter in 2.0.0-rc1.
> May you send a v3 addressing comments? Or should I break the serie by
> applying only some of them? Or postpone the serie to 2.1?
I can resend v3. Wasn't clear that a conclusion was reached.
IMHO read should not clear.
2015-02-24 11:18, Stephen Hemminger:
> On Mon, 23 Feb 2015 23:51:31 +0000
> Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
>
> > 2015-02-05 07:43, Neil Horman:
> > > On Wed, Feb 04, 2015 at 10:13:58PM -0800, Stephen Hemminger wrote:
> > > > +
> > > > +/**
> > > > + * Hierarchical scheduler subport statistics reset
> > > > + *
> > > > + * @param port
> > > > + * Handle to port scheduler instance
> > > > + * @param subport_id
> > > > + * Subport ID
> > > > + * @return
> > > > + * 0 upon success, error code otherwise
> > > > + */
> > > > +int
> > > > +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> > > > + uint32_t subport_id);
> > > > +
> > > > /**
> > > > * Hierarchical scheduler queue statistics read
> > > > *
> > > > @@ -338,6 +353,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
> > > > struct rte_sched_queue_stats *stats,
> > > > uint16_t *qlen);
> > > >
> > > > +/**
> > > > + * Hierarchical scheduler queue statistics reset
> > > > + *
> > > > + * @param port
> > > > + * Handle to port scheduler instance
> > > > + * @param queue_id
> > > > + * Queue ID within port scheduler
> > > > + * @return
> > > > + * 0 upon success, error code otherwise
> > > > + */
> > > > +int
> > > > +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> > > > + uint32_t queue_id);
> > > > +
> > > Both need to be added to the version map to expose them properly.
> > > Neil
> >
> > Stephen, this patchset is partially acked and could enter in 2.0.0-rc1.
> > May you send a v3 addressing comments? Or should I break the serie by
> > applying only some of them? Or postpone the serie to 2.1?
>
> I can resend v3. Wasn't clear that a conclusion was reached.
> IMHO read should not clear.
Me too. I'm just saying that I cannot apply anything.
So you have to decide the strategy to adopt for your patches.
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Tuesday, February 24, 2015 8:07 PM
> To: Stephen Hemminger
> Cc: dev@dpdk.org; Stephen Hemminger
> Subject: Re: [dpdk-dev] [PATCH v2 4/7] rte_sched: don't clear statistics when
> read
>
> 2015-02-24 11:18, Stephen Hemminger:
> > On Mon, 23 Feb 2015 23:51:31 +0000
> > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> >
> > > 2015-02-05 07:43, Neil Horman:
> > > > On Wed, Feb 04, 2015 at 10:13:58PM -0800, Stephen Hemminger wrote:
> > > > > +
> > > > > +/**
> > > > > + * Hierarchical scheduler subport statistics reset
> > > > > + *
> > > > > + * @param port
> > > > > + * Handle to port scheduler instance
> > > > > + * @param subport_id
> > > > > + * Subport ID
> > > > > + * @return
> > > > > + * 0 upon success, error code otherwise
> > > > > + */
> > > > > +int
> > > > > +rte_sched_subport_stats_reset(struct rte_sched_port *port,
> > > > > + uint32_t subport_id);
> > > > > +
> > > > > /**
> > > > > * Hierarchical scheduler queue statistics read
> > > > > *
> > > > > @@ -338,6 +353,20 @@ rte_sched_queue_read_stats(struct
> rte_sched_port *port,
> > > > > struct rte_sched_queue_stats *stats,
> > > > > uint16_t *qlen);
> > > > >
> > > > > +/**
> > > > > + * Hierarchical scheduler queue statistics reset
> > > > > + *
> > > > > + * @param port
> > > > > + * Handle to port scheduler instance
> > > > > + * @param queue_id
> > > > > + * Queue ID within port scheduler
> > > > > + * @return
> > > > > + * 0 upon success, error code otherwise
> > > > > + */
> > > > > +int
> > > > > +rte_sched_queue_stats_reset(struct rte_sched_port *port,
> > > > > + uint32_t queue_id);
> > > > > +
> > > > Both need to be added to the version map to expose them properly.
> > > > Neil
> > >
> > > Stephen, this patchset is partially acked and could enter in 2.0.0-rc1.
> > > May you send a v3 addressing comments? Or should I break the serie by
> > > applying only some of them? Or postpone the serie to 2.1?
> >
> > I can resend v3. Wasn't clear that a conclusion was reached.
> > IMHO read should not clear.
>
> Me too. I'm just saying that I cannot apply anything.
> So you have to decide the strategy to adopt for your patches.
How about my latest proposal to have the stats read functions either reset the counters or not, based on init-time user configuration? I did not see any reply on this.
Maybe you guys missed my reply, I am pasting it below:
"Personally, I think we should avoid proliferating the number of stats functions, I would keep a single set of stats read functions, which can clear the stats or not, depending on behaviour configured per rte_sched object at creation time. Basically, based on the value of configuration parameter struct rte_sched_params::clear_stats_on_reset, the stats read functions do clear the counters or not. In my opinion, this allows a clean init-time selection of the required behaviour, and it also provides backward compatibility. Any issues with this approach?"
2015-02-24 21:06, Thomas Monjalon:
> 2015-02-24 11:18, Stephen Hemminger:
> > On Mon, 23 Feb 2015 23:51:31 +0000
> > Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> > > Stephen, this patchset is partially acked and could enter in 2.0.0-rc1.
> > > May you send a v3 addressing comments? Or should I break the serie by
> > > applying only some of them? Or postpone the serie to 2.1?
> >
> > I can resend v3. Wasn't clear that a conclusion was reached.
> > IMHO read should not clear.
>
> Me too. I'm just saying that I cannot apply anything.
> So you have to decide the strategy to adopt for your patches.
Please re-send accepted patches with a correct threading.
Decision will be taken in 2.1 for discussed patches.
@@ -935,6 +935,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
}
int
+rte_sched_subport_stats_reset(struct rte_sched_port *port,
+ uint32_t subport_id)
+{
+ struct rte_sched_subport *s;
+
+ /* Check user parameters */
+ if (port == NULL || subport_id >= port->n_subports_per_port)
+ return -1;
+
+ s = port->subport + subport_id;
+ memset(&s->stats, 0, sizeof(struct rte_sched_subport_stats));
+ return 0;
+}
+
+int
rte_sched_queue_read_stats(struct rte_sched_port *port,
uint32_t queue_id,
struct rte_sched_queue_stats *stats,
@@ -963,6 +978,21 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
return 0;
}
+int
+rte_sched_queue_stats_reset(struct rte_sched_port *port,
+ uint32_t queue_id)
+{
+ struct rte_sched_queue_extra *qe;
+
+ /* Check user parameters */
+ if (port == NULL || queue_id >= rte_sched_port_queues_per_port(port))
+ return -1;
+
+ qe = port->queue_extra + queue_id;
+ memset(&qe->stats, 0, sizeof(struct rte_sched_queue_stats));
+ return 0;
+}
+
static inline uint32_t
rte_sched_port_qindex(struct rte_sched_port *port, uint32_t subport, uint32_t pipe, uint32_t traffic_class, uint32_t queue)
{
@@ -317,6 +317,21 @@ rte_sched_subport_read_stats(struct rte_sched_port *port,
struct rte_sched_subport_stats *stats,
uint32_t *tc_ov);
+
+/**
+ * Hierarchical scheduler subport statistics reset
+ *
+ * @param port
+ * Handle to port scheduler instance
+ * @param subport_id
+ * Subport ID
+ * @return
+ * 0 upon success, error code otherwise
+ */
+int
+rte_sched_subport_stats_reset(struct rte_sched_port *port,
+ uint32_t subport_id);
+
/**
* Hierarchical scheduler queue statistics read
*
@@ -338,6 +353,20 @@ rte_sched_queue_read_stats(struct rte_sched_port *port,
struct rte_sched_queue_stats *stats,
uint16_t *qlen);
+/**
+ * Hierarchical scheduler queue statistics reset
+ *
+ * @param port
+ * Handle to port scheduler instance
+ * @param queue_id
+ * Queue ID within port scheduler
+ * @return
+ * 0 upon success, error code otherwise
+ */
+int
+rte_sched_queue_stats_reset(struct rte_sched_port *port,
+ uint32_t queue_id);
+
/*
* Run-time
*