[Nexthop][fboss2-dev] fboss2 config/delete protocol bgp policy prefix-list commands

cmake/CliFboss2.cmake

@@ -978,6 +978,9 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h
   fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp
   fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h
+  fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/BgpPrefixListCliUtils.h
+  fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.cpp
+  fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h
   fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
   fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
   fboss/cli/fboss2/commands/config/ptp/transparent_clock/CmdConfigPtpTransparentClock.cpp
@@ -1120,6 +1123,8 @@ add_library(fboss2_config_lib
   fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h
   fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp
   fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h
+  fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.cpp
+  fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h
   fboss/cli/fboss2/commands/delete/protocol/CmdDeleteProtocol.h
   fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp
   fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h

cmake/CliFboss2TestConfig.cmake

@@ -16,6 +16,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdConfigBgpPeerGroupTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigBgpPolicyAsPathListTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigBgpPolicyCommunityListTest.cpp
+  fboss/cli/fboss2/test/config/CmdConfigBgpPolicyPrefixListTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigDataPlaneTrafficPolicyTest.cpp
   fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
@@ -55,6 +56,7 @@ add_executable(fboss2_cmd_config_test
   fboss/cli/fboss2/test/config/CmdDeleteBgpPeerGroupTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyAsPathListTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyCommunityListTest.cpp
+  fboss/cli/fboss2/test/config/CmdDeleteBgpPolicyPrefixListTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteConfigInterfaceTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteCoppQueueTest.cpp
   fboss/cli/fboss2/test/config/CmdDeleteCoppReasonTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

@@ -20,6 +20,7 @@ add_executable(fboss2_integration_test
   fboss/cli/fboss2/test/integration_test/ConfigBgpPeerGroupTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyAsPathListTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyCommunityListTest.cpp
+  fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyPrefixListTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
   fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp

fboss/cli/fboss2/BUCK

@@ -1212,6 +1212,7 @@ cpp_library(
         "commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.cpp",
         "commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.cpp",
         "commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp",
+        "commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.cpp",
         "commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
         "commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
         "commands/config/ptp/CmdConfigPtp.cpp",
@@ -1282,6 +1283,7 @@ cpp_library(
         "commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.cpp",
         "commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.cpp",
         "commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.cpp",
+        "commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.cpp",
         "commands/delete/protocol/static/CmdDeleteProtocolStatic.cpp",
         "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.cpp",
         "commands/delete/qos/CmdDeleteQos.cpp",
@@ -1361,6 +1363,8 @@ cpp_library(
         "commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h",
         "commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h",
         "commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h",
+        "commands/config/protocol/bgp/policy/prefix-list/BgpPrefixListCliUtils.h",
+        "commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h",
         "commands/config/protocol/static/CmdConfigProtocolStatic.h",
         "commands/config/protocol/static/route/StaticRouteUtils.h",
         "commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h",
@@ -1432,6 +1436,7 @@ cpp_library(
         "commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h",
         "commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h",
         "commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h",
+        "commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h",
         "commands/delete/protocol/static/CmdDeleteProtocolStatic.h",
         "commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h",
         "commands/delete/qos/CmdDeleteQos.h",

fboss/cli/fboss2/CmdListConfig.cpp

@@ -45,6 +45,7 @@
 #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h"
 #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/as-path-list/CmdConfigProtocolBgpPolicyAsPathList.h"
 #include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.h"
+#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/CmdConfigProtocolBgpPolicyPrefixList.h"
 #include "fboss/cli/fboss2/commands/config/protocol/static/CmdConfigProtocolStatic.h"
 #include "fboss/cli/fboss2/commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h"
 #include "fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h"
@@ -112,6 +113,7 @@
 #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/CmdDeleteProtocolBgpPolicy.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/as-path-list/CmdDeleteProtocolBgpPolicyAsPathList.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/community-list/CmdDeleteProtocolBgpPolicyCommunityList.h"
+#include "fboss/cli/fboss2/commands/delete/protocol/bgp/policy/prefix-list/CmdDeleteProtocolBgpPolicyPrefixList.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/static/CmdDeleteProtocolStatic.h"
 #include "fboss/cli/fboss2/commands/delete/protocol/static/route/CmdDeleteProtocolStaticRoute.h"
 #include "fboss/cli/fboss2/commands/delete/qos/CmdDeleteQos.h"
@@ -444,6 +446,17 @@ const CommandTree& kConfigCommandTree() {
                                    CmdConfigProtocolBgpPolicyCommunityList>,
                                argRegistrar<
                                    CmdConfigProtocolBgpPolicyCommunityListTraits>,
+                           },
+                           {
+                               "prefix-list",
+                               "Configure BGP prefix-list: <name> "
+                               "[<attribute> <value> ...] "
+                               "(boolean-operator, description, "
+                               "ip-version)",
+                               commandHandler<
+                                   CmdConfigProtocolBgpPolicyPrefixList>,
+                               argRegistrar<
+                                   CmdConfigProtocolBgpPolicyPrefixListTraits>,
                            }},
                       },
                   },
@@ -808,6 +821,13 @@ const CommandTree& kConfigCommandTree() {
                              CmdDeleteProtocolBgpPolicyCommunityList>,
                          argRegistrar<
                              CmdDeleteProtocolBgpPolicyCommunityListTraits>,
+                     },
+                     {
+                         "prefix-list",
+                         "Delete a BGP prefix-list: <name>",
+                         commandHandler<CmdDeleteProtocolBgpPolicyPrefixList>,
+                         argRegistrar<
+                             CmdDeleteProtocolBgpPolicyPrefixListTraits>,
                      }},
                 }},
            },

fboss/cli/fboss2/commands/config/protocol/bgp/policy/CmdConfigProtocolBgpPolicy.h

@@ -20,7 +20,7 @@ namespace facebook::fboss {
 
 // Grouping node for `config protocol bgp policy <object-type> ...`. Holds no
 // state of its own; the per-object-type dispatchers (as-path-list,
-// community-list, and later prefix-list/routing-policy) are its subcommands.
+// community-list, prefix-list, and later routing-policy) are its subcommands.
 // Mirrors CmdConfigProtocolBgp.
 struct CmdConfigProtocolBgpPolicyTraits : public WriteCommandTraits {
   using ParentCmd = CmdConfigProtocolBgp;

fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/BgpCommunityListCliUtils.h

@@ -10,8 +10,10 @@
 
 #pragma once
 
+#include <fmt/core.h>
 #include <algorithm>
 #include <string>
+#include <vector>
 
 #ifndef IS_OSS
 #include "configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/bgp_policy_types.h"
@@ -60,4 +62,67 @@ inline bgp::bgp_policy::CommunityList& findOrCreateCommunityList(
   return list;
 }
 
+// A routing-policy term that names a community-list. A match reference
+// (`filter` set) is a COMMUNITY match whose
+// communities_filter.community_list_names carries the name; an action
+// reference (`filter` null, `isAction`) is a community action whose
+// community_action_list_names carries it. `label` identifies the term the
+// way the CLI addresses it (`policy <P> term <seq>`).
+struct CommunityListReference {
+  bgp::bgp_policy::CommunityList* filter;
+  std::string label;
+  bool isAction;
+};
+
+// Every reference to `listName`. bgpd resolves both kinds by name at config
+// load (CommunityMatch / CommunityAction::PopulateReferences) and fails the
+// load on a dangling one, so the delete command refuses while this is
+// non-empty. A match's inline copy must also carry the same boolean_operator
+// as the list ("Conflicting boolean_operator from the reference") and bgpd
+// reads exact_match only from that inline copy, so the config command writes
+// both through to every match reference.
+inline std::vector<CommunityListReference> findTermsReferencingCommunityList(
+    bgp::thrift::BgpConfig& cfg,
+    const std::string& listName) {
+  std::vector<CommunityListReference> refs;
+  if (!cfg.policies().has_value()) {
+    return refs;
+  }
+  auto names = [&](const auto& list) {
+    return std::find(list.begin(), list.end(), listName) != list.end();
+  };
+  for (auto& policy : *cfg.policies()->bgp_policy_statements()) {
+    for (auto& term : *policy.policy_entries()) {
+      auto label = term.sequence_number().has_value()
+          ? fmt::format(
+                "policy {} term {}", *policy.name(), *term.sequence_number())
+          : fmt::format("policy {} term {}", *policy.name(), *term.name());
+      if (term.policy_match_entries().has_value()) {
+        for (auto& match : *term.policy_match_entries()->match_entries()) {
+          if (!match.communities_filter().has_value() ||
+              !match.communities_filter()->community_list_names().has_value()) {
+            continue;
+          }
+          auto& filter = *match.communities_filter();
+          if (names(*filter.community_list_names())) {
+            refs.push_back({&filter, label, false});
+          }
+        }
+      }
+      for (auto& action : *term.policy_action_entries()) {
+        if (!action.community_action().has_value() ||
+            !action.community_action()
+                 ->community_action_list_names()
+                 .has_value()) {
+          continue;
+        }
+        if (names(*action.community_action()->community_action_list_names())) {
+          refs.push_back({nullptr, label, true});
+        }
+      }
+    }
+  }
+  return refs;
+}
+
 } // namespace facebook::fboss::bgpcli

fboss/cli/fboss2/commands/config/protocol/bgp/policy/community-list/CmdConfigProtocolBgpPolicyCommunityList.cpp

@@ -237,6 +237,22 @@ CmdConfigProtocolBgpPolicyCommunityList::queryClient(
     if (!args.attr().empty()) {
       result.message += fmt::format(" for community-list {}", args.listName());
     }
+    if (args.attr() == kBooleanOperator || args.attr() == kExactMatch) {
+      // bgpd requires a term's inline copy of the list to carry the same
+      // boolean_operator ("Conflicting boolean_operator from the reference")
+      // and reads exact_match only from that copy, so keep every match
+      // reference in step with the named list.
+      for (auto& ref :
+           bgpcli::findTermsReferencingCommunityList(cfg, args.listName())) {
+        if (ref.filter == nullptr) {
+          continue;
+        }
+        ref.filter->boolean_operator() = *list.boolean_operator();
+        if (list.exact_match().has_value()) {
+          ref.filter->exact_match() = *list.exact_match();
+        }
+      }
+    }
     session.saveBgpConfig();
     result.message +=
         fmt::format("\nConfig saved to: {}", session.getBgpSessionConfigPath());

fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/BgpPrefixListCliUtils.h

@@ -0,0 +1,63 @@
+/*
+ *  Copyright (c) 2004-present, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+
+#pragma once
+
+#include <algorithm>
+#include <string>
+
+#ifndef IS_OSS
+#include <configerator/structs/neteng/bgp_policy/thrift/gen-cpp2/routing_policy_types.h>
+#include <configerator/structs/neteng/fboss/bgp/gen-cpp2/bgp_config_types.h>
+#else
+#include <neteng/fboss/bgp/public_tld/configerator/structs/neteng/fboss/bgp/gen-cpp2/bgp_config_types.h>
+#endif
+
+/**
+ * Lookup/create helpers for the prefix-list CLI family, shared between the
+ * list-level dispatcher (CmdConfigProtocolBgpPolicyPrefixList) and its delete
+ * counterpart. A PrefixList is keyed by name. Entry helpers arrive with the
+ * entry subcommand, which decides the entry's identity (bgpd rejects a
+ * PrefixListEntry carrying seq_num).
+ */
+namespace facebook::fboss::bgpcli {
+
+inline bool prefixListExists(
+    const bgp::thrift::BgpConfig& cfg,
+    const std::string& name) {
+  if (!cfg.policies().has_value()) {
+    return false;
+  }
+  const auto& lists = *cfg.policies()->prefix_lists();
+  return std::any_of(lists.begin(), lists.end(), [&](const auto& list) {
+    return *list.name() == name;
+  });
+}
+
+// Find the prefix-list keyed by name, creating it if absent. Setting an
+// attribute on a not-yet-created list implicitly creates it, so command
+// ordering stays forgiving; a bare `prefix-list <name>` creates one
+// explicitly. PrefixList's only key field is `name`.
+inline bgp::routing_policy::PrefixList& findOrCreatePrefixList(
+    bgp::thrift::BgpConfig& cfg,
+    const std::string& name) {
+  auto& lists = *cfg.policies().ensure().prefix_lists();
+  for (auto& list : lists) {
+    if (*list.name() == name) {
+      return list;
+    }
+  }
+  lists.emplace_back();
+  auto& list = lists.back();
+  list.name() = name;
+  return list;
+}
+
+} // namespace facebook::fboss::bgpcli