ofproto-dpif-xlate: Do not use zero-weight buckets in select groups.

The OpenFlow specification says that buckets in select groups with a weight of zero should not be selected, but the ofproto-dpif implementation could select them in corner cases. This fixes the problem. Reported-by: ychen <[email protected]> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-May/359349.html Signed-off-by: Ben Pfaff <[email protected]> Signed-off-by: Ilya Maximets <[email protected]>
LorenzoBianconi · Oct 18, 2022 · 6f53538 · 6f53538
1 parent edeefe7
commit 6f53538
Showing 1 changed file with 11 additions and 5 deletions.
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
@@ -1924,15 +1924,21 @@ group_is_alive(const struct xlate_ctx *ctx, uint32_t group_id, int depth)
 #define MAX_LIVENESS_RECURSION 128 /* Arbitrary limit */
 
 static bool
-bucket_is_alive(const struct xlate_ctx *ctx,
-                struct ofputil_bucket *bucket, int depth)
+bucket_is_alive(const struct xlate_ctx *ctx, const struct group_dpif *group,
+                const struct ofputil_bucket *bucket, int depth)
 {
     if (depth >= MAX_LIVENESS_RECURSION) {
         xlate_report_error(ctx, "bucket chaining exceeded %d links",
                            MAX_LIVENESS_RECURSION);
         return false;
     }
 
+    /* In "select" groups, buckets with weight 0 are not used.
+     * In other kinds of groups, weight does not matter. */
+    if (group->up.type == OFPGT11_SELECT && bucket->weight == 0) {
+        return false;
+    }
+
     return (!ofputil_bucket_has_liveness(bucket)
             || (bucket->watch_port != OFPP_ANY
                && bucket->watch_port != OFPP_CONTROLLER
@@ -1973,7 +1979,7 @@ group_first_live_bucket(const struct xlate_ctx *ctx,
 {
     struct ofputil_bucket *bucket;
     LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
-        if (bucket_is_alive(ctx, bucket, depth)) {
+        if (bucket_is_alive(ctx, group, bucket, depth)) {
             return bucket;
         }
         xlate_report_bucket_not_live(ctx, bucket);
@@ -1992,7 +1998,7 @@ group_best_live_bucket(const struct xlate_ctx *ctx,
 
     struct ofputil_bucket *bucket;
     LIST_FOR_EACH (bucket, list_node, &group->up.buckets) {
-        if (bucket_is_alive(ctx, bucket, 0)) {
+        if (bucket_is_alive(ctx, group, bucket, 0)) {
             uint32_t score =
                 (hash_int(bucket->bucket_id, basis) & 0xffff) * bucket->weight;
             if (score >= best_score) {
@@ -4755,7 +4761,7 @@ pick_dp_hash_select_group(struct xlate_ctx *ctx, struct group_dpif *group)
         for (int i = 0; i <= hash_mask; i++) {
             struct ofputil_bucket *b =
                     group->hash_map[(dp_hash + i) & hash_mask];
-            if (bucket_is_alive(ctx, b, 0)) {
+            if (bucket_is_alive(ctx, group, b, 0)) {
                 return b;
             }
         }