[ovs-dev] [PATCH v2 2/3] ofproto-dpif: Improve dp_hash selection method for select groups

Ben Pfaff blp at ovn.org
Tue Apr 17 20:42:54 UTC 2018


On Tue, Apr 17, 2018 at 01:03:17PM -0700, Ben Pfaff wrote:
> On Mon, Apr 16, 2018 at 04:26:27PM +0200, Jan Scheurich wrote:
> > The current implementation of the "dp_hash" selection method suffers
> > from two deficiences: 1. The hash mask and hence the number of dp_hash
> > values is just large enough to cover the number of group buckets, but
> > does not consider the case that buckets have different weights. 2. The
> > xlate-time selection of best bucket from the masked dp_hash value often
> > results in bucket load distributions that are quite different from the
> > bucket weights because the number of available masked dp_hash values
> > is too small (2-6 bits compared to 32 bits of a full hash in the default
> > hash selection method).
> 
> Clang gives me the following errors:
> 
>     ../ofproto/ofproto-dpif.c:4792:32: error: unknown warning group '-Wmaybe-uninitialized', ignored [-Werror,-Wunknown-pragmas]
>     ../ofproto/ofproto-dpif.c:4814:33: error: initializing 'struct ofputil_group_props *' with an expression of type 'const struct ofputil_group_props *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]

How about this approach, which should cleanly eliminate the warning?

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index e1a5c097f3aa..362339a4abb4 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -4780,22 +4780,17 @@ group_setup_dp_hash_table(struct group_dpif *group, size_t max_hash)
 
     /* Use Webster method to distribute hash values over buckets. */
     for (int hash = 0; hash < n_hash; hash++) {
-        double max_val = 0.0;
-        struct webster *winner;
-        for (i = 0; i < n_buckets; i++) {
-            if (webster[i].value > max_val) {
-                max_val = webster[i].value;
+        struct webster *winner = &webster[0];
+        for (i = 1; i < n_buckets; i++) {
+            if (webster[i].value > winner->value) {
                 winner = &webster[i];
             }
         }
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
         /* winner is a reference to a webster[] element initialized above. */
         winner->divisor += 2;
         winner->value = (double) winner->bucket->weight / winner->divisor;
         group->hash_map[hash] = winner->bucket;
         winner->bucket->aux++;
-#pragma GCC diagnostic pop
     }
 
     LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {


More information about the dev mailing list