coverage: Make the coverage counters catalog program-specific.

Until now, the collection of coverage counters supported by a given OVS
program was not specific to that program.  That means that, for example,
even though ovs-dpctl does not have anything to do with mac_learning, it
still has a coverage counter for it.  This is confusing, at best.

This commit fixes the problem on some systems, in particular on ones that
use GCC and the GNU linker.  It uses the feature of the GNU linker
described in its manual as:

    If an orphaned section's name is representable as a C identifier then
    the linker will automatically see PROVIDE two symbols: __start_SECNAME
    and __end_SECNAME, where SECNAME is the name of the section.  These
    indicate the start address and end address of the orphaned section
    respectively.

Systems that don't support these features retain the earlier behavior.

This commit also fixes the annoyance that files that include coverage
counters must be listed on COVERAGE_FILES in lib/automake.mk.

This commit also fixes the annoyance that modifying any source file that
includes a coverage counter caused all programs that link against
libopenvswitch.a to relink, even programs that the source file was not
linked into.  For example, modifying ofproto/ofproto.c (which includes
coverage counters) caused tests/test-aes128 to relink, even though
test-aes128 does not link again ofproto.o.

Author: blp

lib/automake.mk

@@ -26,7 +26,6 @@ lib_libopenvswitch_a_SOURCES = \
 	lib/compiler.h \
 	lib/coverage.c \
 	lib/coverage.h \
-	lib/coverage-counters.h \
 	lib/csum.c \
 	lib/csum.h \
 	lib/daemon.c \
@@ -159,7 +158,6 @@ lib_libopenvswitch_a_SOURCES = \
 	lib/vlog.c \
 	lib/vlog.h
 nodist_lib_libopenvswitch_a_SOURCES = \
-	lib/coverage-counters.c \
 	lib/dirs.c
 CLEANFILES += $(nodist_lib_libopenvswitch_a_SOURCES)
 
@@ -246,6 +244,7 @@ lib-install-data-local:
 	$(MKDIR_P) $(DESTDIR)$(PKIDIR)
 	$(MKDIR_P) $(DESTDIR)$(LOGDIR)
 
+if !USE_LINKER_SECTIONS
 # All distributed sources, with names adjust properly for referencing
 # from $(builddir).
 all_sources = \
@@ -257,37 +256,11 @@ all_sources = \
 		fi; \
 	 done`
 
-# All the source files that have coverage counters.
-COVERAGE_FILES = \
-	lib/dpif.c \
-	lib/flow.c \
-	lib/lockfile.c \
-	lib/hmap.c \
-	lib/mac-learning.c \
-	lib/netdev.c \
-	lib/netdev-linux.c \
-	lib/netlink.c \
-	lib/odp-util.c \
-	lib/poll-loop.c \
-	lib/process.c \
-	lib/rconn.c \
-	lib/rtnetlink.c \
-	lib/stream.c \
-	lib/stream-ssl.c \
-	lib/timeval.c \
-	lib/unixctl.c \
-	lib/util.c \
-	lib/vconn.c \
-	ofproto/ofproto.c \
-	ofproto/pktbuf.c \
-	vswitchd/bridge.c \
-	vswitchd/ovs-brcompatd.c
-lib/coverage-counters.c: $(COVERAGE_FILES) lib/coverage-scan.pl
-	(cd $(srcdir) && $(PERL) lib/coverage-scan.pl $(COVERAGE_FILES)) > $@.tmp
-	mv $@.tmp $@
-EXTRA_DIST += lib/coverage-scan.pl
+lib/coverage.$(OBJEXT): lib/coverage.def
+lib/coverage.def: $(DIST_SOURCES)
+	sed -n 's|^COVERAGE_DEFINE(\([_a-zA-Z0-9]\{1,\}\)).*$$|COVERAGE_COUNTER(\1)|p' $(all_sources) | LC_ALL=C sort -u > $@
+CLEANFILES += lib/coverage.def
 
-if !USE_LINKER_SECTIONS
 lib/vlog.$(OBJEXT): lib/vlog-modules.def
 lib/vlog-modules.def: $(DIST_SOURCES)
 	sed -n 's|^VLOG_DEFINE_\(THIS_\)\{0,1\}MODULE(\([_a-zA-Z0-9]\{1,\}\)).*$$|VLOG_MODULE(\2)|p' $(all_sources) | LC_ALL=C sort -u > $@

lib/coverage-counters.h

@@ -18,7 +18,6 @@
 #include "coverage.h"
 #include <inttypes.h>
 #include <stdlib.h>
-#include "coverage-counters.h"
 #include "dynamic-string.h"
 #include "hash.h"
 #include "unixctl.h"
@@ -27,6 +26,25 @@
 
 VLOG_DEFINE_THIS_MODULE(coverage);
 
+/* The coverage counters. */
+#if USE_LINKER_SECTIONS
+extern struct coverage_counter *__start_coverage[];
+extern struct coverage_counter *__stop_coverage[];
+#define coverage_counters __start_coverage
+#define n_coverage_counters  (__stop_coverage - __start_coverage)
+#else  /* !USE_LINKER_SECTIONS */
+#define COVERAGE_COUNTER(NAME) COVERAGE_DEFINE__(NAME);
+#include "coverage.def"
+#undef COVERAGE_COUNTER
+
+struct coverage_counter *coverage_counters[] = {
+#define COVERAGE_COUNTER(NAME) &counter_##NAME,
+#include "coverage.def"
+#undef COVERAGE_COUNTER
+};
+#define n_coverage_counters ARRAY_SIZE(coverage_counters)
+#endif  /* !USE_LINKER_SECTIONS */
+
 static unsigned int epoch;
 
 static void
@@ -67,23 +85,23 @@ coverage_hash(void)
     int n_groups, i;
 
     /* Sort coverage counters into groups with equal counts. */
-    c = xmalloc(coverage_n_counters * sizeof *c);
-    for (i = 0; i < coverage_n_counters; i++) {
+    c = xmalloc(n_coverage_counters * sizeof *c);
+    for (i = 0; i < n_coverage_counters; i++) {
         c[i] = coverage_counters[i];
     }
-    qsort(c, coverage_n_counters, sizeof *c, compare_coverage_counters);
+    qsort(c, n_coverage_counters, sizeof *c, compare_coverage_counters);
 
     /* Hash the names in each group along with the rank. */
     n_groups = 0;
-    for (i = 0; i < coverage_n_counters; ) {
+    for (i = 0; i < n_coverage_counters; ) {
         int j;
 
         if (!c[i]->count) {
             break;
         }
         n_groups++;
         hash = hash_int(i, hash);
-        for (j = i; j < coverage_n_counters; j++) {
+        for (j = i; j < n_coverage_counters; j++) {
             if (c[j]->count != c[i]->count) {
                 break;
             }
@@ -150,13 +168,13 @@ coverage_log(enum vlog_level level, bool suppress_dups)
     n_never_hit = 0;
     VLOG(level, "Event coverage (epoch %u/entire run), hash=%08"PRIx32":",
          epoch, hash);
-    for (i = 0; i < coverage_n_counters; i++) {
+    for (i = 0; i < n_coverage_counters; i++) {
         struct coverage_counter *c = coverage_counters[i];
         if (c->count) {
             coverage_log_counter(level, c);
         }
     }
-    for (i = 0; i < coverage_n_counters; i++) {
+    for (i = 0; i < n_coverage_counters; i++) {
         struct coverage_counter *c = coverage_counters[i];
         if (!c->count) {
             if (c->total) {
@@ -176,7 +194,7 @@ coverage_clear(void)
     size_t i;
 
     epoch++;
-    for (i = 0; i < coverage_n_counters; i++) {
+    for (i = 0; i < n_coverage_counters; i++) {
         struct coverage_counter *c = coverage_counters[i];
         c->total += c->count;
         c->count = 0;

lib/coverage-scan.pl

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009 Nicira Networks.
+ * Copyright (c) 2009, 2010 Nicira Networks.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,24 +36,30 @@ struct coverage_counter {
     unsigned long long int total; /* Total count over all epochs. */
 };
 
-/* Increments the counter with the given NAME.  Coverage counters need not be
- * declared explicitly, but when you add the first coverage counter to a given
- * file, you must also add that file to COVERAGE_FILES in lib/automake.mk. */
-#define COVERAGE_INC(NAME)                              \
-    do {                                                \
-        extern struct coverage_counter NAME##_count;    \
-        NAME##_count.count++;                           \
-    } while (0)
-
-/* Adds AMOUNT to the coverage counter with the given NAME. */
-#define COVERAGE_ADD(NAME, AMOUNT)                      \
-    do {                                                \
-        extern struct coverage_counter NAME##_count;    \
-        NAME##_count.count += AMOUNT;                   \
-    } while (0)
+/* Defines COUNTER.  There must be exactly one such definition at file scope
+ * within a program. */
+#if USE_LINKER_SECTIONS
+#define COVERAGE_DEFINE(COUNTER)                                        \
+        COVERAGE_DEFINE__(COUNTER);                                     \
+        struct coverage_counter *counter_ptr_##COUNTER                  \
+            __attribute__((section("coverage"))) = &counter_##COUNTER
+#else
+#define COVERAGE_DEFINE(MODULE) \
+        extern struct coverage_counter counter_##MODULE
+#endif
+
+/* Adds 1 to COUNTER. */
+#define COVERAGE_INC(COUNTER) counter_##COUNTER.count++;
+
+/* Adds AMOUNT to COUNTER. */
+#define COVERAGE_ADD(COUNTER, AMOUNT) counter_##COUNTER.count += (AMOUNT);
 
 void coverage_init(void);
 void coverage_log(enum vlog_level, bool suppress_dups);
 void coverage_clear(void);
 
+/* Implementation detail. */
+#define COVERAGE_DEFINE__(COUNTER)                              \
+        struct coverage_counter counter_##COUNTER = { #COUNTER, 0, 0 }
+
 #endif /* coverage.h */

lib/coverage.c

@@ -42,6 +42,18 @@
 
 VLOG_DEFINE_THIS_MODULE(dpif);
 
+COVERAGE_DEFINE(dpif_destroy);
+COVERAGE_DEFINE(dpif_port_add);
+COVERAGE_DEFINE(dpif_port_del);
+COVERAGE_DEFINE(dpif_flow_flush);
+COVERAGE_DEFINE(dpif_flow_get);
+COVERAGE_DEFINE(dpif_flow_put);
+COVERAGE_DEFINE(dpif_flow_del);
+COVERAGE_DEFINE(dpif_flow_query_list);
+COVERAGE_DEFINE(dpif_flow_query_list_n);
+COVERAGE_DEFINE(dpif_execute);
+COVERAGE_DEFINE(dpif_purge);
+
 static const struct dpif_class *base_dpif_classes[] = {
 #ifdef HAVE_NETLINK
     &dpif_linux_class,
@@ -375,6 +387,7 @@ dpif_get_all_names(const struct dpif *dpif, struct svec *all_names)
     }
 }
 
+
 /* Destroys the datapath that 'dpif' is connected to, first removing all of its
  * ports.  After calling this function, it does not make sense to pass 'dpif'
  * to any functions other than dpif_name() or dpif_close(). */

lib/coverage.h

@@ -34,6 +34,8 @@
 
 VLOG_DEFINE_THIS_MODULE(flow);
 
+COVERAGE_DEFINE(flow_extract);
+
 static struct arp_eth_header *
 pull_arp(struct ofpbuf *packet)
 {

lib/dpif.c

@@ -23,6 +23,11 @@
 #include "random.h"
 #include "util.h"
 
+COVERAGE_DEFINE(hmap_pathological);
+COVERAGE_DEFINE(hmap_expand);
+COVERAGE_DEFINE(hmap_shrink);
+COVERAGE_DEFINE(hmap_reserve);
+
 /* Initializes 'hmap' as an empty hash table. */
 void
 hmap_init(struct hmap *hmap)

lib/flow.c

@@ -33,6 +33,11 @@
 
 VLOG_DEFINE_THIS_MODULE(lockfile);
 
+COVERAGE_DEFINE(lockfile_lock);
+COVERAGE_DEFINE(lockfile_timeout);
+COVERAGE_DEFINE(lockfile_error);
+COVERAGE_DEFINE(lockfile_unlock);
+
 struct lockfile {
     struct hmap_node hmap_node;
     char *name;

lib/hmap.c

@@ -33,6 +33,9 @@
 
 VLOG_DEFINE_THIS_MODULE(mac_learning);
 
+COVERAGE_DEFINE(mac_learning_learned);
+COVERAGE_DEFINE(mac_learning_expired);
+
 /* Returns the number of seconds since 'e' was last learned. */
 int
 mac_entry_age(const struct mac_entry *e)