[ovs-dev,v2,1/2] coverage: Add command for reading counter value

Message ID 20180531104111.15790-2-jkbs@redhat.com
State Superseded
Headers show
Series
  • ovn: Check for effects of incremental processing
Related show

Commit Message

Jakub Sitnicki May 31, 2018, 10:41 a.m.
Facilitate checking coverage counters from scripts and tests with a new
"coverage/read-counter" command that gets the total count for a counter.

Same could be achieved by scraping the output of "coverage/show" command
but the difficulties there are that output is in human readable format
and zero-value counters are not listed.

Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
---
 lib/coverage-unixctl.man |  2 ++
 lib/coverage.c           | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+)

Comments

Han Zhou May 31, 2018, 4:39 p.m. | #1
On Thu, May 31, 2018 at 3:41 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
>
> Facilitate checking coverage counters from scripts and tests with a new
> "coverage/read-counter" command that gets the total count for a counter.
>
> Same could be achieved by scraping the output of "coverage/show" command
> but the difficulties there are that output is in human readable format
> and zero-value counters are not listed.
>
> Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
> ---
>  lib/coverage-unixctl.man |  2 ++
>  lib/coverage.c           | 43 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 insertions(+)
>
> diff --git a/lib/coverage-unixctl.man b/lib/coverage-unixctl.man
> index 8e5df818e..81eb93902 100644
> --- a/lib/coverage-unixctl.man
> +++ b/lib/coverage-unixctl.man
> @@ -11,3 +11,5 @@ debugging.
>  Displays the averaged per-second rates for the last few seconds, the
>  last minute and the last hour, and the total counts of all of the
>  coverage counters.
> +.IP "\fBcoverage/read-counter\fR \fIcounter\fR"
> +Displays the total count for the given coverage \fIcounter\fR.
> diff --git a/lib/coverage.c b/lib/coverage.c
> index 6cef82614..2cf2d38cc 100644
> --- a/lib/coverage.c
> +++ b/lib/coverage.c
> @@ -44,6 +44,8 @@ static unsigned int idx_count = 0;
>  static void coverage_read(struct svec *);
>  static unsigned int coverage_array_sum(const unsigned int *arr,
>                                         const unsigned int len);
> +static bool coverage_read_counter(const char *name,
> +                                  unsigned long long int *count);
>
>  /* Registers a coverage counter with the coverage core */
>  void
> @@ -72,11 +74,33 @@ coverage_unixctl_show(struct unixctl_conn *conn, int
argc OVS_UNUSED,
>      svec_destroy(&lines);
>  }
>
> +static void
> +coverage_unixctl_read_counter(struct unixctl_conn *conn, int argc
OVS_UNUSED,
> +                              const char *argv[], void *aux OVS_UNUSED)
> +{
> +    unsigned long long count;
> +    char *reply;
> +    bool ok;
> +
> +    coverage_clear();

Why calling coverage_clear() here? It is supposed to be called by timer,
and it doesn't do anything if time is not reached, so calling it here seems
useless. Please correct me if I am wrong.

> +    ok = coverage_read_counter(argv[1], &count);
> +    if (!ok) {
> +        unixctl_command_reply_error(conn, "No such counter");
> +        return;
> +    }
> +
> +    reply = xasprintf("%llu\n", count);
> +    unixctl_command_reply(conn, reply);
> +    free(reply);
> +}
> +
>  void
>  coverage_init(void)
>  {
>      unixctl_command_register("coverage/show", "", 0, 0,
>                               coverage_unixctl_show, NULL);
> +    unixctl_command_register("coverage/read-counter", "COUNTER", 1, 1,
> +                             coverage_unixctl_read_counter, NULL);
>  }
>
>  /* Sorts coverage counters in descending order by total, within equal
> @@ -372,3 +396,22 @@ coverage_array_sum(const unsigned int *arr, const
unsigned int len)
>      ovs_mutex_unlock(&coverage_mutex);
>      return sum;
>  }
> +
> +static bool
> +coverage_read_counter(const char *name, unsigned long long int *count)
> +{
> +    for (size_t i = 0; i < n_coverage_counters; i++) {
> +        struct coverage_counter *c = coverage_counters[i];
> +
> +        if (!strcmp(c->name, name)) {
> +            ovs_mutex_lock(&coverage_mutex);
> +            c->total += c->count();
> +            *count = c->total;
> +            ovs_mutex_unlock(&coverage_mutex);
> +
> +            return true;
> +        }
> +    }
> +
> +    return false;
> +}
> --
> 2.14.3
>
Jakub Sitnicki June 1, 2018, 1:45 p.m. | #2
On Thu, 31 May 2018 09:39:44 -0700
Han Zhou <zhouhan@gmail.com> wrote:

> On Thu, May 31, 2018 at 3:41 AM, Jakub Sitnicki <jkbs@redhat.com> wrote:
> >
> > Facilitate checking coverage counters from scripts and tests with a new
> > "coverage/read-counter" command that gets the total count for a counter.
> >
> > Same could be achieved by scraping the output of "coverage/show" command
> > but the difficulties there are that output is in human readable format
> > and zero-value counters are not listed.
> >
> > Signed-off-by: Jakub Sitnicki <jkbs@redhat.com>
> > ---
> >  lib/coverage-unixctl.man |  2 ++
> >  lib/coverage.c           | 43 +++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 45 insertions(+)
> >
> > diff --git a/lib/coverage-unixctl.man b/lib/coverage-unixctl.man
> > index 8e5df818e..81eb93902 100644
> > --- a/lib/coverage-unixctl.man
> > +++ b/lib/coverage-unixctl.man
> > @@ -11,3 +11,5 @@ debugging.
> >  Displays the averaged per-second rates for the last few seconds, the
> >  last minute and the last hour, and the total counts of all of the
> >  coverage counters.
> > +.IP "\fBcoverage/read-counter\fR \fIcounter\fR"
> > +Displays the total count for the given coverage \fIcounter\fR.
> > diff --git a/lib/coverage.c b/lib/coverage.c
> > index 6cef82614..2cf2d38cc 100644
> > --- a/lib/coverage.c
> > +++ b/lib/coverage.c
> > @@ -44,6 +44,8 @@ static unsigned int idx_count = 0;
> >  static void coverage_read(struct svec *);
> >  static unsigned int coverage_array_sum(const unsigned int *arr,
> >                                         const unsigned int len);
> > +static bool coverage_read_counter(const char *name,
> > +                                  unsigned long long int *count);
> >
> >  /* Registers a coverage counter with the coverage core */
> >  void
> > @@ -72,11 +74,33 @@ coverage_unixctl_show(struct unixctl_conn *conn, int  
> argc OVS_UNUSED,
> >      svec_destroy(&lines);
> >  }
> >
> > +static void
> > +coverage_unixctl_read_counter(struct unixctl_conn *conn, int argc  
> OVS_UNUSED,
> > +                              const char *argv[], void *aux OVS_UNUSED)
> > +{
> > +    unsigned long long count;
> > +    char *reply;
> > +    bool ok;
> > +
> > +    coverage_clear();  
> 
> Why calling coverage_clear() here? It is supposed to be called by timer,
> and it doesn't do anything if time is not reached, so calling it here seems
> useless. Please correct me if I am wrong.

This is a mistake, and a left-over from v1 which I didn't notice.
Thanks for pointing it out.

I'm syncing the per-thread counts with the total count in
coverage_read_counter() so calling here the coverage_clear() that will
sync counts based on the wall-clock time has no purpose.

I will remove this call in v3.

> 
> > +    ok = coverage_read_counter(argv[1], &count);
> > +    if (!ok) {
> > +        unixctl_command_reply_error(conn, "No such counter");
> > +        return;
> > +    }
> > +
> > +    reply = xasprintf("%llu\n", count);
> > +    unixctl_command_reply(conn, reply);
> > +    free(reply);
> > +}
> > +
> >  void
> >  coverage_init(void)
> >  {
> >      unixctl_command_register("coverage/show", "", 0, 0,
> >                               coverage_unixctl_show, NULL);
> > +    unixctl_command_register("coverage/read-counter", "COUNTER", 1, 1,
> > +                             coverage_unixctl_read_counter, NULL);
> >  }
> >
> >  /* Sorts coverage counters in descending order by total, within equal
> > @@ -372,3 +396,22 @@ coverage_array_sum(const unsigned int *arr, const  
> unsigned int len)
> >      ovs_mutex_unlock(&coverage_mutex);
> >      return sum;
> >  }
> > +
> > +static bool
> > +coverage_read_counter(const char *name, unsigned long long int *count)
> > +{
> > +    for (size_t i = 0; i < n_coverage_counters; i++) {
> > +        struct coverage_counter *c = coverage_counters[i];
> > +
> > +        if (!strcmp(c->name, name)) {
> > +            ovs_mutex_lock(&coverage_mutex);
> > +            c->total += c->count();
> > +            *count = c->total;
> > +            ovs_mutex_unlock(&coverage_mutex);
> > +
> > +            return true;
> > +        }
> > +    }
> > +
> > +    return false;
> > +}
> > --
> > 2.14.3
> >

Patch

diff --git a/lib/coverage-unixctl.man b/lib/coverage-unixctl.man
index 8e5df818e..81eb93902 100644
--- a/lib/coverage-unixctl.man
+++ b/lib/coverage-unixctl.man
@@ -11,3 +11,5 @@  debugging.
 Displays the averaged per-second rates for the last few seconds, the
 last minute and the last hour, and the total counts of all of the
 coverage counters.
+.IP "\fBcoverage/read-counter\fR \fIcounter\fR"
+Displays the total count for the given coverage \fIcounter\fR.
diff --git a/lib/coverage.c b/lib/coverage.c
index 6cef82614..2cf2d38cc 100644
--- a/lib/coverage.c
+++ b/lib/coverage.c
@@ -44,6 +44,8 @@  static unsigned int idx_count = 0;
 static void coverage_read(struct svec *);
 static unsigned int coverage_array_sum(const unsigned int *arr,
                                        const unsigned int len);
+static bool coverage_read_counter(const char *name,
+                                  unsigned long long int *count);
 
 /* Registers a coverage counter with the coverage core */
 void
@@ -72,11 +74,33 @@  coverage_unixctl_show(struct unixctl_conn *conn, int argc OVS_UNUSED,
     svec_destroy(&lines);
 }
 
+static void
+coverage_unixctl_read_counter(struct unixctl_conn *conn, int argc OVS_UNUSED,
+                              const char *argv[], void *aux OVS_UNUSED)
+{
+    unsigned long long count;
+    char *reply;
+    bool ok;
+
+    coverage_clear();
+    ok = coverage_read_counter(argv[1], &count);
+    if (!ok) {
+        unixctl_command_reply_error(conn, "No such counter");
+        return;
+    }
+
+    reply = xasprintf("%llu\n", count);
+    unixctl_command_reply(conn, reply);
+    free(reply);
+}
+
 void
 coverage_init(void)
 {
     unixctl_command_register("coverage/show", "", 0, 0,
                              coverage_unixctl_show, NULL);
+    unixctl_command_register("coverage/read-counter", "COUNTER", 1, 1,
+                             coverage_unixctl_read_counter, NULL);
 }
 
 /* Sorts coverage counters in descending order by total, within equal
@@ -372,3 +396,22 @@  coverage_array_sum(const unsigned int *arr, const unsigned int len)
     ovs_mutex_unlock(&coverage_mutex);
     return sum;
 }
+
+static bool
+coverage_read_counter(const char *name, unsigned long long int *count)
+{
+    for (size_t i = 0; i < n_coverage_counters; i++) {
+        struct coverage_counter *c = coverage_counters[i];
+
+        if (!strcmp(c->name, name)) {
+            ovs_mutex_lock(&coverage_mutex);
+            c->total += c->count();
+            *count = c->total;
+            ovs_mutex_unlock(&coverage_mutex);
+
+            return true;
+        }
+    }
+
+    return false;
+}