maglev: fix handling of unspecified locality weights.

api/envoy/api/v2/endpoint/endpoint.proto

@@ -111,8 +111,8 @@ message LocalityLbEndpoints {
   // Locality weights are only considered when :ref:`locality weighted load
   // balancing <arch_overview_load_balancing_locality_weighted_lb>` is
   // configured. These weights are ignored otherwise. If no weights are
-  // specified when locality weighted load balancing is enabled, the cluster is
-  // assumed to have a weight of 1.
+  // specified when locality weighted load balancing is enabled, the locality is
+  // assigned no load.
   //
   // .. attention::
   //

source/common/upstream/maglev_lb.cc

@@ -26,7 +26,7 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality,
       return host_weight;
     } else {
       auto locality_weight = (*locality_weights)[locality_index];
-      ASSERT(locality_weight != 0);
+      // This might be zero, since locality weight might not be specified.
       return host_weight * locality_weight;
     }
   };
@@ -52,12 +52,22 @@ MaglevTable::MaglevTable(const HostsPerLocality& hosts_per_locality,
   for (uint32_t i = 0; i < hosts_per_locality.get().size(); ++i) {
     for (const auto& host : hosts_per_locality.get()[i]) {
       const std::string& address = host->address()->asString();
-      table_build_entries.emplace_back(host, HashUtil::xxHash64(address) % table_size_,
-                                       (HashUtil::xxHash64(address, 1) % (table_size_ - 1)) + 1,
-                                       effective_weight(host->weight(), i));
+      const uint32_t weight = effective_weight(host->weight(), i);
+      // If weight is zero, it should be totally excluded from table building
+      // below.
+      if (weight > 0) {
+        table_build_entries.emplace_back(host, HashUtil::xxHash64(address) % table_size_,
+                                         (HashUtil::xxHash64(address, 1) % (table_size_ - 1)) + 1,
+                                         weight);
+      }
     }
   }
 
+  // We can't do anything sensible with no table entries.
+  if (table_build_entries.empty()) {
+    return;
+  }
+
   table_.resize(table_size_);
   uint64_t table_index = 0;
   uint32_t iteration = 1;

test/common/upstream/maglev_lb_test.cc

@@ -151,12 +151,13 @@ TEST_F(MaglevLoadBalancerTest, LocalityWeightedSameLocalityWeights) {
 // invert the Weighted effective weights).
 TEST_F(MaglevLoadBalancerTest, LocalityWeightedDifferentLocalityWeights) {
   host_set_.hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:90", 1),
-                      makeTestHost(info_, "tcp://127.0.0.1:91", 2)};
+                      makeTestHost(info_, "tcp://127.0.0.1:91", 2),
+                      makeTestHost(info_, "tcp://127.0.0.1:92", 3)};
   host_set_.healthy_hosts_ = host_set_.hosts_;
   host_set_.hosts_per_locality_ =
-      makeHostsPerLocality({{host_set_.hosts_[0]}, {host_set_.hosts_[1]}});
+      makeHostsPerLocality({{host_set_.hosts_[0]}, {host_set_.hosts_[2]}, {host_set_.hosts_[1]}});
   host_set_.healthy_hosts_per_locality_ = host_set_.hosts_per_locality_;
-  LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{8, 2}};
+  LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{8, 0, 2}};
   host_set_.locality_weights_ = locality_weights;
   host_set_.runCallbacks({}, {});
   init(17);
@@ -187,6 +188,21 @@ TEST_F(MaglevLoadBalancerTest, LocalityWeightedDifferentLocalityWeights) {
   }
 }
 
+// Locality weighted with all localities zero weighted.
+TEST_F(MaglevLoadBalancerTest, LocalityWeightedAllZeroLocalityWeights) {
+  host_set_.hosts_ = {makeTestHost(info_, "tcp://127.0.0.1:90", 1)};
+  host_set_.healthy_hosts_ = host_set_.hosts_;
+  host_set_.hosts_per_locality_ = makeHostsPerLocality({{host_set_.hosts_[0]}});
+  host_set_.healthy_hosts_per_locality_ = host_set_.hosts_per_locality_;
+  LocalityWeightsConstSharedPtr locality_weights{new LocalityWeights{0}};
+  host_set_.locality_weights_ = locality_weights;
+  host_set_.runCallbacks({}, {});
+  init(17);
+  LoadBalancerPtr lb = lb_->factory()->create();
+  TestLoadBalancerContext context(0);
+  EXPECT_EQ(nullptr, lb->chooseHost(&context));
+}
+
 // Validate that when we are in global panic and have localities, we get sane
 // results (fall back to non-healthy hosts).
 TEST_F(MaglevLoadBalancerTest, LocalityWeightedGlobalPanic) {

test/server/server_corpus/clusterfuzz-testcase-config_fuzz_test-5067970991095808

@@ -0,0 +1,58 @@
+static_resources {
+  clusters {
+    name: "6"
+    connect_timeout {
+      seconds: 2321
+    }
+    lb_policy: MAGLEV
+    hosts {
+      pipe {
+        path: "="
+      }
+    }
+    hosts {
+      pipe {
+        path: "="
+      }
+    }
+    hosts {
+      pipe {
+        path: "t"
+      }
+    }
+    max_requests_per_connection {
+      value: 67108864
+    }
+    dns_lookup_family: V4_ONLY
+    outlier_detection {
+      success_rate_minimum_hosts {
+        value: 4096
+      }
+    }
+    common_lb_config {
+      locality_weighted_lb_config {
+      }
+    }
+  }
+}
+stats_sinks {
+  typed_config {
+    type_url: "type.googleapis.com/envoy.api.v2.route.Route"
+    value: "\022*J :2222222222222222222222222\022"
+  }
+}
+stats_sinks {
+  typed_config {
+    type_url: "type.googleapis.com/envoy.api.v2.route.Route"
+    value: "\022*J.:*static\'_resourc\022es {(\n  cluster`s"
+  }
+}
+stats_sinks {
+  typed_config {
+    type_url: "type.googleapis.com/envoy.api.v2.route.Route"
+    value: "\022*J :2222222222222222222222221\022"
+  }
+}
+stats_flush_interval {
+  nanos: 2883584
+}