Better kmod extraction from lsmod

Summary:
Parse the lsmod content to list of kmods. So we can full string comparison instead of sub-string that can be faulty some times.
E.g kmod_x matches to kmod_x_y_z.

Only dealing with kmod names.

Reviewed By: somasun

Differential Revision: D68463973

fbshipit-source-id: 36ab35ffecbc0b9035bdc5c61407a2da4b111812

Author: Justin Kim

fboss/platform/platform_manager/BUCK

@@ -129,6 +129,7 @@ cpp_library(
         "PkgManager.cpp",
     ],
     exported_deps = [
+        "fbsource//third-party/range-v3:range-v3",
         ":platform_manager_config-cpp2-types",
         "//fb303:service_data",
         "//fboss/platform/helpers:platform_fs_utils",

fboss/platform/platform_manager/PkgManager.cpp

@@ -6,12 +6,14 @@
 #include <folly/FileUtil.h>
 #include <folly/String.h>
 #include <folly/logging/xlog.h>
+#include <range/v3/range/conversion.hpp>
+#include <range/v3/view/drop.hpp>
+#include <range/v3/view/filter.hpp>
+#include <range/v3/view/split.hpp>
 #include <re2/re2.h>
 #include <sys/utsname.h>
 #include <thrift/lib/cpp2/protocol/Serializer.h>
 
-#include "fboss/platform/helpers/PlatformUtils.h"
-
 DEFINE_bool(
     enable_pkg_mgmnt,
     true,
@@ -37,14 +39,16 @@ const re2::RE2 kBspRpmNameRe = "(?P<KEYWORD>[a-z]+)_bsp_kmods";
 } // namespace
 
 namespace package_manager {
-SystemInterface::SystemInterface() {}
+SystemInterface::SystemInterface(
+    const std::shared_ptr<PlatformUtils>& platformUtils)
+    : platformUtils_(platformUtils) {}
 
 bool SystemInterface::loadKmod(const std::string& moduleName) const {
   int exitStatus{0};
   std::string standardOut{};
   auto unloadCmd = fmt::format("modprobe {}", moduleName);
   VLOG(1) << fmt::format("Running command ({})", unloadCmd);
-  std::tie(exitStatus, standardOut) = PlatformUtils().execCommand(unloadCmd);
+  std::tie(exitStatus, standardOut) = platformUtils_->execCommand(unloadCmd);
   return exitStatus == 0;
 }
 
@@ -53,7 +57,7 @@ bool SystemInterface::unloadKmod(const std::string& moduleName) const {
   std::string standardOut{};
   auto unloadCmd = fmt::format("rmmod {}", moduleName);
   VLOG(1) << fmt::format("Running command ({})", unloadCmd);
-  std::tie(exitStatus, standardOut) = PlatformUtils().execCommand(unloadCmd);
+  std::tie(exitStatus, standardOut) = platformUtils_->execCommand(unloadCmd);
   return exitStatus == 0;
 }
 
@@ -62,7 +66,7 @@ int SystemInterface::installRpm(const std::string& rpmFullName) const {
   std::string standardOut{};
   auto cmd = fmt::format("dnf install {} --assumeyes", rpmFullName);
   VLOG(1) << fmt::format("Running command ({})", cmd);
-  std::tie(exitStatus, standardOut) = PlatformUtils().execCommand(cmd);
+  std::tie(exitStatus, standardOut) = platformUtils_->execCommand(cmd);
   return exitStatus;
 }
 
@@ -71,7 +75,7 @@ int SystemInterface::installLocalRpm() const {
   std::string standardOut{};
   auto cmd = fmt::format("rpm -i --force {}", FLAGS_local_rpm_path);
   VLOG(1) << fmt::format("Running command ({})", cmd);
-  std::tie(exitStatus, standardOut) = PlatformUtils().execCommand(cmd);
+  std::tie(exitStatus, standardOut) = platformUtils_->execCommand(cmd);
   return exitStatus;
 }
 
@@ -80,7 +84,7 @@ int SystemInterface::depmod() const {
   std::string standardOut{};
   auto depmodCmd = "depmod -a";
   VLOG(1) << fmt::format("Running command ({})", depmodCmd);
-  std::tie(exitStatus, standardOut) = PlatformUtils().execCommand(depmodCmd);
+  std::tie(exitStatus, standardOut) = platformUtils_->execCommand(depmodCmd);
   return exitStatus;
 }
 
@@ -90,7 +94,7 @@ std::vector<std::string> SystemInterface::getInstalledRpms(
   int exitStatus{0};
   auto cmd = fmt::format("rpm -qa | grep ^{}", rpmBaseName);
   VLOG(1) << fmt::format("Running command ({})", cmd);
-  std::tie(exitStatus, standardOut) = PlatformUtils().execCommand(cmd);
+  std::tie(exitStatus, standardOut) = platformUtils_->execCommand(cmd);
   if (exitStatus) {
     return {};
   }
@@ -107,14 +111,29 @@ int SystemInterface::removeRpms(
       fmt::format("rpm -e --allmatches {}", folly::join(" ", installedRpms));
   VLOG(1) << fmt::format("Running command ({})", removeOldRpmsCmd);
   std::tie(exitStatus, standardOut) =
-      PlatformUtils().execCommand(removeOldRpmsCmd);
+      platformUtils_->execCommand(removeOldRpmsCmd);
   return exitStatus;
 }
 
-std::string SystemInterface::lsmod() const {
+std::set<std::string> SystemInterface::lsmod() const {
   VLOG(1) << "Running command (lsmod)";
-  auto result = PlatformUtils().execCommand("lsmod");
-  return result.second;
+  auto result = platformUtils_->execCommand("lsmod");
+  auto rows = result.second | ranges::views::split('\n') |
+      ranges::views::drop(1) | ranges::to<std::vector<std::string>>;
+  std::set<std::string> kmods;
+  // row -> kmod | size | used by | dependent kmods
+  for (const auto& row : rows) {
+    auto tokens = row | ranges::views::split(' ') |
+        ranges::views::filter(
+                      [](const auto& token) { return !token.empty(); }) |
+        ranges::to<std::vector<std::string>>;
+    if (tokens.empty()) {
+      XLOG(WARNING) << fmt::format("Failed to scan lsmod row -- {}", row);
+      continue;
+    }
+    kmods.emplace(tokens.front());
+  }
+  return kmods;
 }
 
 std::string SystemInterface::getHostKernelVersion() const {
@@ -299,7 +318,7 @@ void PkgManager::unloadBspKmods() const {
   XLOG(INFO) << "Unloading kernel modules based on kmods.json";
   const auto loadedKmods = systemInterface_->lsmod();
   for (const auto& kmod : *bspKmodsFile.bspKmods()) {
-    if (loadedKmods.find(kmod) != std::string::npos) {
+    if (loadedKmods.contains(kmod)) {
       XLOG(INFO) << fmt::format("Unloading {}", kmod);
       if (!systemInterface_->unloadKmod(kmod)) {
         throw std::runtime_error(fmt::format("Failed to unload ({})", kmod));
@@ -311,7 +330,7 @@ void PkgManager::unloadBspKmods() const {
   }
   XLOG(INFO) << "Unloading shared kernel modules";
   for (const auto& kmod : *bspKmodsFile.sharedKmods()) {
-    if (loadedKmods.find(kmod) != std::string::npos) {
+    if (loadedKmods.contains(kmod)) {
       XLOG(INFO) << fmt::format("Unloading {}", kmod);
       if (!systemInterface_->unloadKmod(kmod)) {
         throw std::runtime_error(fmt::format("Failed to unload ({})", kmod));

fboss/platform/platform_manager/PkgManager.h

@@ -3,6 +3,7 @@
 #pragma once
 
 #include "fboss/platform/helpers/PlatformFsUtils.h"
+#include "fboss/platform/helpers/PlatformUtils.h"
 #include "fboss/platform/platform_manager/gen-cpp2/platform_manager_config_types.h"
 
 DECLARE_bool(enable_pkg_mgmnt);
@@ -13,7 +14,9 @@ namespace facebook::fboss::platform::platform_manager {
 namespace package_manager {
 class SystemInterface {
  public:
-  explicit SystemInterface();
+  explicit SystemInterface(
+      const std::shared_ptr<PlatformUtils>& platformUtils =
+          std::make_shared<PlatformUtils>());
   virtual ~SystemInterface() = default;
   virtual bool loadKmod(const std::string& moduleName) const;
   virtual bool unloadKmod(const std::string& moduleName) const;
@@ -22,10 +25,13 @@ class SystemInterface {
   virtual std::vector<std::string> getInstalledRpms(
       const std::string& rpmBaseName) const;
   virtual int removeRpms(const std::vector<std::string>& installedRpms) const;
-  virtual std::string lsmod() const;
+  virtual std::set<std::string> lsmod() const;
   virtual bool isRpmInstalled(const std::string& rpmFullName) const;
   virtual std::string getHostKernelVersion() const;
   int installLocalRpm() const;
+
+ private:
+  std::shared_ptr<PlatformUtils> platformUtils_;
 };
 } // namespace package_manager
 

fboss/platform/platform_manager/tests/BUCK

@@ -99,9 +99,12 @@ cpp_unittest(
     name = "pkg_manager_test",
     srcs = [
         "PkgManagerTest.cpp",
+        "SystemInterfaceTest.cpp",
     ],
     deps = [
+        "fbsource//third-party/fmt:fmt",
         "fbsource//third-party/googletest:gmock",
+        "fbsource//third-party/range-v3:range-v3",
         "//fb303:service_data",
         "//fboss/platform/platform_manager:pkg_manager",
         "//thrift/lib/cpp2/protocol:protocol",

fboss/platform/platform_manager/tests/PkgManagerTest.cpp

@@ -3,6 +3,8 @@
 #include <fb303/ServiceData.h>
 #include <gmock/gmock.h>
 #include <gtest/gtest.h>
+#include <range/v3/range/conversion.hpp>
+#include <range/v3/view/concat.hpp>
 #include <thrift/lib/cpp2/protocol/Serializer.h>
 
 #include "fboss/platform/platform_manager/PkgManager.h"
@@ -21,7 +23,7 @@ class MockSystemInterface : public package_manager::SystemInterface {
   MOCK_METHOD(int, removeRpms, (const std::vector<std::string>&), (const));
   MOCK_METHOD(int, installRpm, (const std::string&), (const));
   MOCK_METHOD(int, depmod, (), (const));
-  MOCK_METHOD(std::string, lsmod, (), (const));
+  MOCK_METHOD(std::set<std::string>, lsmod, (), (const));
   MOCK_METHOD(bool, unloadKmod, (const std::string&), (const));
   MOCK_METHOD(bool, loadKmod, (const std::string&), (const));
   MOCK_METHOD(std::string, getHostKernelVersion, (), (const));
@@ -303,8 +305,10 @@ TEST_F(PkgManagerTest, unloadBspKmods) {
   EXPECT_CALL(*mockPlatformFsUtils_, getStringFileContent(_))
       .WillOnce(Return(jsonBspKmodsFile_));
   EXPECT_CALL(*mockSystemInterface_, lsmod())
-      .WillOnce(
-          Return(jsonBspKmodsFile_ /*any string containing kmods works*/));
+      .WillOnce(Return(
+          ranges::views::concat(
+              *bspKmodsFile_.bspKmods(), *bspKmodsFile_.sharedKmods()) |
+          ranges::to<std::set<std::string>>));
   EXPECT_CALL(*mockSystemInterface_, unloadKmod(_))
       .Times(
           bspKmodsFile_.sharedKmods()->size() +
@@ -317,15 +321,19 @@ TEST_F(PkgManagerTest, unloadBspKmods) {
   EXPECT_CALL(*mockPlatformFsUtils_, getStringFileContent(_))
       .WillOnce(Return(jsonBspKmodsFile_));
   EXPECT_CALL(*mockSystemInterface_, lsmod())
-      .WillOnce(Return(jsonBspKmodsFile_));
+      .WillOnce(Return(
+          ranges::views::concat(
+              *bspKmodsFile_.bspKmods(), *bspKmodsFile_.sharedKmods()) |
+          ranges::to<std::set<std::string>>));
   EXPECT_CALL(*mockSystemInterface_, unloadKmod(_)).WillOnce(Return(false));
   EXPECT_THROW(pkgManager_.unloadBspKmods(), std::runtime_error);
   EXPECT_EQ(
       facebook::fb303::fbData->getCounter(PkgManager::kUnloadKmodsFailure), 1);
   // kmods.json exist and all kmods aren't loaded
   EXPECT_CALL(*mockPlatformFsUtils_, getStringFileContent(_))
       .WillOnce(Return(jsonBspKmodsFile_));
-  EXPECT_CALL(*mockSystemInterface_, lsmod()).WillOnce(Return(""));
+  EXPECT_CALL(*mockSystemInterface_, lsmod())
+      .WillOnce(Return(std::set<std::string>{}));
   EXPECT_CALL(*mockSystemInterface_, unloadKmod(_)).Times(0);
   EXPECT_NO_THROW(pkgManager_.unloadBspKmods());
   EXPECT_EQ(

fboss/platform/platform_manager/tests/SystemInterfaceTest.cpp

@@ -0,0 +1,48 @@
+// (c) Meta Platforms, Inc. and affiliates. Confidential and proprietary.
+
+#include <fmt/args.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "fboss/platform/platform_manager/PkgManager.h"
+
+using namespace ::testing;
+
+namespace facebook::fboss::platform::platform_manager {
+class MockPlatformUtils : public PlatformUtils {
+ public:
+  MOCK_METHOD(
+      (std::pair<int, std::string>),
+      execCommand,
+      (const std::string&),
+      (const));
+};
+
+TEST(SystemInterfaceTest, lsmod) {
+  auto lsmod =
+      "Module                  Size  Used by\n"
+      "mp3_smbcpld            12288  0\n"
+      "mp3_scmcpld            28672  0\n"
+      "fboss_iob_xcvr         12288  0\n"
+      "fboss_iob_led          12288  0\n"
+      "led_class              20480  3 fboss_iob_led,mp3_scmcpld,mp3_fancpld\n"
+      "autofs4                49152  2\n";
+  auto expectedKmods = std::array<std::string, 6>{
+      "mp3_smbcpld",
+      "mp3_scmcpld",
+      "fboss_iob_xcvr",
+      "fboss_iob_led",
+      "led_class",
+      "autofs4"};
+  auto platformUtils = std::make_shared<MockPlatformUtils>();
+  EXPECT_CALL(*platformUtils, execCommand(_))
+      .WillOnce(Return(std::pair{0, lsmod}));
+  package_manager::SystemInterface interface(platformUtils);
+  auto kmods = interface.lsmod();
+  EXPECT_EQ(kmods.size(), expectedKmods.size());
+  for (const auto& expectedKmod : expectedKmods) {
+    EXPECT_TRUE(kmods.contains(expectedKmod));
+  }
+}
+
+}; // namespace facebook::fboss::platform::platform_manager