> -----Original Message----- > From: Stephen Hemminger [mailto:stephen at networkplumber.org] > Sent: Tuesday, March 10, 2015 4:14 PM > To: Dumitrescu, Cristian > Cc: dev at dpdk.org; Stephen Hemminger; Stephen Hemminger > Subject: [PATCH v2 5/6] rte_sched: allow reading statistics without clearing > > From: Stephen Hemminger <shemming at brocade.com> > > The rte_sched statistics API should allow reading statistics without > clearing. Make auto-clear optional. > > Signed-off-by: Stephen Hemminger <stephen at networkplumber.org> > --- > v2 -- change to flag for clear from separate read/clear > > app/test/test_sched.c | 4 ++-- > examples/qos_sched/stats.c | 16 +++++++++++----- > lib/librte_sched/rte_sched.c | 44 ++++++++++++++++++++++----------------- > ----- > lib/librte_sched/rte_sched.h | 18 ++++++++++-------- > 4 files changed, 45 insertions(+), 37 deletions(-) > > diff --git a/app/test/test_sched.c b/app/test/test_sched.c > index 60c62de..3c9c9e3 100644 > --- a/app/test/test_sched.c > +++ b/app/test/test_sched.c > @@ -208,13 +208,13 @@ test_sched(void) > > struct rte_sched_subport_stats subport_stats; > uint32_t tc_ov; > - rte_sched_subport_read_stats(port, SUBPORT, &subport_stats, > &tc_ov); > + rte_sched_subport_read_stats(port, SUBPORT, &subport_stats, > &tc_ov, 1); > #if 0 > TEST_ASSERT_EQUAL(subport_stats.n_pkts_tc[TC-1], 10, "Wrong > subport stats\n"); > #endif > struct rte_sched_queue_stats queue_stats; > uint16_t qlen; > - rte_sched_queue_read_stats(port, QUEUE, &queue_stats, &qlen); > + rte_sched_queue_read_stats(port, QUEUE, &queue_stats, &qlen, > 1); > #if 0 > TEST_ASSERT_EQUAL(queue_stats.n_pkts, 10, "Wrong queue > stats\n"); > #endif > diff --git a/examples/qos_sched/stats.c b/examples/qos_sched/stats.c > index b4db7b5..88caa54 100644 > --- a/examples/qos_sched/stats.c > +++ b/examples/qos_sched/stats.c > @@ -61,7 +61,7 @@ qavg_q(uint8_t port_id, uint32_t subport_id, uint32_t > pipe_id, uint8_t tc, uint8 > average = 0; > > for (count = 0; count < qavg_ntimes; count++) { > - rte_sched_queue_read_stats(port, queue_id, &stats, &qlen); > + rte_sched_queue_read_stats(port, queue_id, &stats, &qlen, 1); > average += qlen; > usleep(qavg_period); > } > @@ -99,7 +99,9 @@ qavg_tcpipe(uint8_t port_id, uint32_t subport_id, > uint32_t pipe_id, uint8_t tc) > for (count = 0; count < qavg_ntimes; count++) { > part_average = 0; > for (i = 0; i < RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS; i++) { > - rte_sched_queue_read_stats(port, queue_id + (tc * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS + i), &stats, &qlen); > + rte_sched_queue_read_stats(port, > + queue_id + (tc * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS + i), > + &stats, &qlen, 1); > part_average += qlen; > } > average += part_average / > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS; > @@ -138,7 +140,8 @@ qavg_pipe(uint8_t port_id, uint32_t subport_id, > uint32_t pipe_id) > for (count = 0; count < qavg_ntimes; count++) { > part_average = 0; > for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS; i++) { > - rte_sched_queue_read_stats(port, queue_id + i, > &stats, > &qlen); > + rte_sched_queue_read_stats(port, queue_id + i, > + &stats, &qlen, 1); > part_average += qlen; > } > average += part_average / > (RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS); > @@ -178,7 +181,9 @@ qavg_tcsubport(uint8_t port_id, uint32_t subport_id, > uint8_t tc) > queue_id = RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS * (subport_id * > port_params.n_pipes_per_subport + i); > > for (j = 0; j < RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS; > j++) { > - rte_sched_queue_read_stats(port, queue_id + > (tc * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS + j), &stats, &qlen); > + rte_sched_queue_read_stats(port, > + queue_id + (tc * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS + j), > + &stats, &qlen, 1); > part_average += qlen; > } > } > @@ -220,7 +225,8 @@ qavg_subport(uint8_t port_id, uint32_t subport_id) > queue_id = RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS * (subport_id * > port_params.n_pipes_per_subport + i); > > for (j = 0; j < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE * > RTE_SCHED_QUEUES_PER_TRAFFIC_CLASS; j++) { > - rte_sched_queue_read_stats(port, queue_id + > j, &stats, > &qlen); > + rte_sched_queue_read_stats(port, queue_id + > j, > + &stats, &qlen, 1); > part_average += qlen; > } > } > diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c > index 2071414..74d0e0a 100644 > --- a/lib/librte_sched/rte_sched.c > +++ b/lib/librte_sched/rte_sched.c > @@ -909,56 +909,56 @@ rte_sched_pipe_config(struct rte_sched_port > *port, > > int > rte_sched_subport_read_stats(struct rte_sched_port *port, > - uint32_t subport_id, > - struct rte_sched_subport_stats *stats, > - uint32_t *tc_ov) > + uint32_t subport_id, > + struct rte_sched_subport_stats *stats, > + uint32_t *tc_ov, int clear) > { > struct rte_sched_subport *s; > > /* Check user parameters */ > - if ((port == NULL) || > - (subport_id >= port->n_subports_per_port) || > - (stats == NULL) || > - (tc_ov == NULL)) { > + if (port == NULL || subport_id >= port->n_subports_per_port) > return -1; > - } > + > s = port->subport + subport_id; > > /* Copy subport stats and clear */ > - memcpy(stats, &s->stats, sizeof(struct rte_sched_subport_stats)); > - memset(&s->stats, 0, sizeof(struct rte_sched_subport_stats)); > + if (stats) > + *stats = s->stats; > + if (clear) > + memset(&s->stats, 0, sizeof(struct > rte_sched_subport_stats)); > > /* Subport TC ovesubscription status */ > - *tc_ov = s->tc_ov; > + if (tc_ov) > + *tc_ov = s->tc_ov; > > return 0; > } > > int > rte_sched_queue_read_stats(struct rte_sched_port *port, > - uint32_t queue_id, > - struct rte_sched_queue_stats *stats, > - uint16_t *qlen) > + uint32_t queue_id, > + struct rte_sched_queue_stats *stats, > + uint16_t *qlen, int clear) > { > struct rte_sched_queue *q; > struct rte_sched_queue_extra *qe; > > /* Check user parameters */ > - if ((port == NULL) || > - (queue_id >= rte_sched_port_queues_per_port(port)) || > - (stats == NULL) || > - (qlen == NULL)) { > + if (port == NULL || queue_id >= > rte_sched_port_queues_per_port(port)) > return -1; > - } > + > q = port->queue + queue_id; > qe = port->queue_extra + queue_id; > > /* Copy queue stats and clear */ > - memcpy(stats, &qe->stats, sizeof(struct rte_sched_queue_stats)); > - memset(&qe->stats, 0, sizeof(struct rte_sched_queue_stats)); > + if (stats) > + *stats = qe->stats; > + if (clear) > + memset(&qe->stats, 0, sizeof(struct > rte_sched_queue_stats)); > > /* Queue length */ > - *qlen = q->qw - q->qr; > + if (qlen) > + *qlen = q->qw - q->qr; > > return 0; > } > diff --git a/lib/librte_sched/rte_sched.h b/lib/librte_sched/rte_sched.h > index e9bf18a..f0bf088 100644 > --- a/lib/librte_sched/rte_sched.h > +++ b/lib/librte_sched/rte_sched.h > @@ -308,14 +308,15 @@ rte_sched_port_get_memory_footprint(struct > rte_sched_port_params *params); > * @param tc_ov > * Pointer to pre-allocated 4-entry array where the oversubscription status > for > * each of the 4 subport traffic classes should be stored. > + * @parm clear > + * Reset statistics after read > * @return > * 0 upon success, error code otherwise > */ > int > -rte_sched_subport_read_stats(struct rte_sched_port *port, > - uint32_t subport_id, > - struct rte_sched_subport_stats *stats, > - uint32_t *tc_ov); > +rte_sched_subport_read_stats(struct rte_sched_port *port, uint32_t > subport_id, > + struct rte_sched_subport_stats *stats, > + uint32_t *tc_ov, int clear); > > /** > * Hierarchical scheduler queue statistics read > @@ -329,14 +330,15 @@ rte_sched_subport_read_stats(struct > rte_sched_port *port, > * counters should be stored > * @param qlen > * Pointer to pre-allocated variable where the current queue length should > be stored. > + * @parm clear > + * Reset statistics after read > * @return > * 0 upon success, error code otherwise > */ > int > -rte_sched_queue_read_stats(struct rte_sched_port *port, > - uint32_t queue_id, > - struct rte_sched_queue_stats *stats, > - uint16_t *qlen); > +rte_sched_queue_read_stats(struct rte_sched_port *port, uint32_t > queue_id, > + struct rte_sched_queue_stats *stats, > + uint16_t *qlen, int clear); > > /* > * Run-time > -- > 2.1.4
Hi Stephen, Thank you for adding flexibility over clearing the stats or not. I have one concern though: why change the stats read API to add a clear parameter rather than keep prototype for the stats functions unchanged and add the flag as part of the port creation parameters in struct rte_sched_port_params? This parameter could be saved into the internal struct rte_sched_port, which is passed (as a pointer) to the stats read functions. In my opinion, this approach is slightly more elegant and it keeps the changes to a minimum. int rte_sched_queue_read_stats(struct rte_sched_port *port, uint32_t queue_id, struct rte_sched_queue_stats *stats, uint16_t *qlen) { ... if (port->clear_stats_on_read) memset(...); } I made this suggestion during the previous round, but I did not get any opinion from you on it yet. Regards, Cristian