dlb: optimize auto detect logic

contrib/network/connection_balance/dlb/source/connection_balancer_impl.cc

@@ -7,8 +7,6 @@
 #include <cstdlib>
 #include <memory>
 
-#include "source/common/api/os_sys_calls_impl.h"
-
 #ifndef DLB_DISABLED
 #include "dlb.h"
 #endif
@@ -37,35 +35,17 @@ DlbConnectionBalanceFactory::createConnectionBalancerFromProto(
         "please decrease the number of threads by `--concurrency`");
   }
 
+  const int& config_id = dlb_config.id();
+  const int& device_id = detectDlbDevice(config_id, "/dev");
+  if (device_id == -1) {
+    ExceptionUtil::throwEnvoyException("no available dlb hardware");
+  } else if (device_id != config_id) {
+    ENVOY_LOG(warn, "dlb device {} is not found, use dlb device {} instead", config_id, device_id);
+  }
+
 #ifdef DLB_DISABLED
   throw EnvoyException("X86_64 architecture is required for Dlb.");
 #else
-  int device_id = 0;
-  Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get();
-  struct stat buffer;
-
-  if (dlb_config.id()) {
-    device_id = dlb_config.id();
-    const std::string& device_name = fmt::format("/dev/dlb{}", device_id);
-    if (os_sys_calls.stat(device_name.c_str(), &buffer).return_value_ != 0) {
-      ExceptionUtil::throwEnvoyException(fmt::format("dlb hardware {} not found", device_name));
-    }
-  } else {
-    std::string device_name;
-    int i = 0;
-    // auto detect available dlb devices, now the max number of dlb device id is 63.
-    const int max_id = 64;
-    for (; i < max_id; i++) {
-      device_name = fmt::format("/dev/dlb{}", i);
-      if (os_sys_calls.stat(device_name.c_str(), &buffer).return_value_ == 0) {
-        device_id = i;
-        break;
-      }
-    }
-    if (i == 64) {
-      ExceptionUtil::throwEnvoyException("no available dlb hardware");
-    }
-  }
 
   dlb_resources_t rsrcs;
   if (dlb_open(device_id, &dlb) == -1) {
@@ -239,6 +219,8 @@ DlbConnectionBalanceFactory::~DlbConnectionBalanceFactory() {
   }
 }
 
+REGISTER_FACTORY(DlbConnectionBalanceFactory, Envoy::Network::ConnectionBalanceFactory);
+
 void DlbBalancedConnectionHandlerImpl::setDlbEvent() {
   auto listener = dynamic_cast<Envoy::Server::ActiveTcpListener*>(&handler_);
 

contrib/network/connection_balance/dlb/source/connection_balancer_impl.h

@@ -1,11 +1,13 @@
 #pragma once
 
 #include <memory>
+#include <string>
 
 #include "envoy/event/dispatcher.h"
 #include "envoy/registry/registry.h"
 #include "envoy/server/filter_config.h"
 
+#include "source/common/api/os_sys_calls_impl.h"
 #include "source/common/network/connection_balancer_impl.h"
 #include "source/common/protobuf/protobuf.h"
 #include "source/server/active_tcp_listener.h"
@@ -49,6 +51,33 @@ class DlbBalancedConnectionHandlerImpl : public Envoy::Network::BalancedConnecti
   Envoy::Event::FileEventPtr dlb_event_;
 };
 
+// The dir should always be "/dev" in production.
+// For test it is a temporary directory.
+// Return Dlb device id, -1 means error.
+static int detectDlbDevice(const int& config_id, const std::string& dir) {
+  int device_id = config_id;
+  Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get();
+  struct stat buffer;
+
+  std::string device_path = fmt::format("{}/dlb{}", dir, device_id);
+  if (os_sys_calls.stat(device_path.c_str(), &buffer).return_value_ != 0) {
+    int i = 0;
+    // auto detect available dlb devices, now the max number of dlb device id is 63.
+    const int max_id = 64;
+    for (; i < max_id; i++) {
+      device_path = fmt::format("{}/dlb{}", dir, i);
+      if (os_sys_calls.stat(device_path.c_str(), &buffer).return_value_ == 0) {
+        device_id = i;
+        break;
+      }
+    }
+    if (i == 64) {
+      return -1;
+    }
+  }
+  return device_id;
+}
+
 class DlbConnectionBalanceFactory : public Envoy::Network::ConnectionBalanceFactory,
                                     public Logger::Loggable<Logger::Id::config> {
 public:
@@ -127,7 +156,6 @@ class DlbConnectionBalanceFactory : public Envoy::Network::ConnectionBalanceFact
 #endif
 };
 
-REGISTER_FACTORY(DlbConnectionBalanceFactory, Envoy::Network::ConnectionBalanceFactory);
 using DlbConnectionBalanceFactorySingleton = InjectableSingleton<DlbConnectionBalanceFactory>;
 
 /**

contrib/network/connection_balance/dlb/test/BUILD

@@ -12,9 +12,11 @@ envoy_cc_test(
     name = "config_test",
     srcs = ["config_test.cc"],
     deps = [
+        "//contrib/network/connection_balance/dlb/source:connection_balancer",
         "//source/common/protobuf:utility_lib",
+        "//test/mocks/server:factory_context_mocks",
+        "//test/test_common:environment_lib",
         "//test/test_common:status_utility_lib",
-        "@envoy_api//contrib/envoy/extensions/network/connection_balance/dlb/v3alpha:pkg_cc_proto",
         "@envoy_api//envoy/config/core/v3:pkg_cc_proto",
     ],
 )

contrib/network/connection_balance/dlb/test/config_test.cc

@@ -2,9 +2,11 @@
 
 #include "source/common/protobuf/utility.h"
 
+#include "test/mocks/server/factory_context.h"
+#include "test/test_common/environment.h"
 #include "test/test_common/status_utility.h"
 
-#include "contrib/envoy/extensions/network/connection_balance/dlb/v3alpha/dlb.pb.h"
+#include "contrib/network/connection_balance/dlb/source/connection_balancer_impl.h"
 #include "gtest/gtest.h"
 
 namespace Envoy {
@@ -52,6 +54,56 @@ TEST_F(DlbConnectionBalanceFactoryTest, MakeCustomConfig) {
   EXPECT_EQ(10, dlb.id());
 }
 
+TEST_F(DlbConnectionBalanceFactoryTest, EmptyProto) {
+  DlbConnectionBalanceFactory factory;
+  EXPECT_NE(nullptr,
+            dynamic_cast<envoy::extensions::network::connection_balance::dlb::v3alpha::Dlb*>(
+                factory.createEmptyConfigProto().get()));
+}
+
+TEST_F(DlbConnectionBalanceFactoryTest, MockDetectDlbDevice) {
+  envoy::extensions::network::connection_balance::dlb::v3alpha::Dlb dlb;
+  dlb.set_id(1);
+
+  const std::string& dlb_path = TestEnvironment::temporaryDirectory();
+  TestEnvironment::createPath(dlb_path);
+  const std::ofstream file(dlb_path + "/" + "dlb6");
+
+  EXPECT_EQ(6, detectDlbDevice(dlb.id(), dlb_path));
+  TestEnvironment::removePath(dlb_path);
+}
+
+#ifndef DLB_DISABLED
+
+using testing::HasSubstr;
+
+TEST_F(DlbConnectionBalanceFactoryTest, MakeFromDefaultProto) {
+  envoy::config::core::v3::TypedExtensionConfig typed_config;
+  DlbConnectionBalanceFactory factory;
+  NiceMock<Server::Configuration::MockFactoryContext> context;
+
+  envoy::extensions::network::connection_balance::dlb::v3alpha::Dlb dlb;
+  makeDlbConnectionBalanceConfig(typed_config, dlb);
+
+  EXPECT_THAT_THROWS_MESSAGE(factory.createConnectionBalancerFromProto(typed_config, context),
+                             EnvoyException, HasSubstr("no available dlb hardware"));
+}
+
+TEST_F(DlbConnectionBalanceFactoryTest, TooManyThreads) {
+  envoy::config::core::v3::TypedExtensionConfig typed_config;
+  DlbConnectionBalanceFactory factory;
+  NiceMock<Server::Configuration::MockFactoryContext> context;
+  context.options_.concurrency_ = 33;
+
+  envoy::extensions::network::connection_balance::dlb::v3alpha::Dlb dlb;
+  makeDlbConnectionBalanceConfig(typed_config, dlb);
+
+  EXPECT_THAT_THROWS_MESSAGE(
+      factory.createConnectionBalancerFromProto(typed_config, context), EnvoyException,
+      HasSubstr("Dlb connection balanncer only supports up to 32 worker threads"));
+}
+#endif
+
 } // namespace Dlb
 } // namespace Extensions
 } // namespace Envoy