Skip to content

Commit cf0eb36

Browse files
NOS-6672..NOS-6679: fboss2 bgp policy routing-policy term action commands
Adds `config protocol bgp policy routing-policy <name> term <seq-num> action result <ACCEPT|REJECT|CONTINUE>` and `... action set <attribute> <value> ...` — the action level of a routing-policy term. `action` is a pure grouping node; `result` and `set` are its subcommands, each with its own handler (the policy and term args arrive through the ancestor-args tuple). The term traits drop positionals_at_end() so CLI11 can classify `action` (and later `match`) after the seq-num; ancestor attributes mixed with an action command are rejected since only the leaf runs. `set` MUST be a real subcommand rather than a parsed token: CLI11's _valid_subcommand walks the parent chain unconditionally, so a bare `set` arg token inside `action` was classified as the top-level `set` VERB and stolen ("The following arguments were not expected"). A local subcommand named `set` shadows the verb because _find_subcommand checks the local scope first. - `result` (NOS-6672) maps ACCEPT/REJECT/CONTINUE onto FlowControlAction ACCEPT/DENY/NEXT_TERM in term_miss_action. GOTO-TERM stays deferred: no FlowControlAction arm, and the per-action-entry next_term_id target has no slot in the documented grammar. - `set <attr>` writes one bgp_policy.BgpPolicyAction entry per action kind in policy_entries[].policy_action_entries[] (keyed by which payload field is set; re-issuing a kind updates its entry): - as-path prepend <asn> [<asn> ...] (NOS-6673) -> SetAsPathPrepend{asn, repeat_times}; the documented <asn-list> must be uniform since the thrift models one ASN repeated N times - community <community-string> [additive] (NOS-6674) -> inline single-member CommunityList + route_action COMMUNITY_LIST_ADD/SET - local-pref <0-4294967295> (NOS-6675) -> LocalPreference.local_pref - med <0-4294967295> (NOS-6676) -> MedAction{med_value, SET} - next-hop <ip-address> (NOS-6677) -> SetNextHop; the sheet's `self` (bgpd rejects set_self) and `peer-address` (no thrift arm) are deferred - origin <IGP|EGP|INCOMPLETE> (NOS-6678) -> set_origin - weight <0-65535> (NOS-6679) -> WeightAction{weight_value, SET} - unit tests (12) + a commit-path integration test staging every action kind and verifying each entry in bgpd's running config. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 98a0313 commit cf0eb36

12 files changed

Lines changed: 1134 additions & 3 deletions

cmake/CliFboss2.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,6 +904,8 @@ add_library(fboss2_config_lib
904904
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/BgpRoutingPolicyCliUtils.h
905905
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.cpp
906906
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h
907+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.cpp
908+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.h
907909
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
908910
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
909911
fboss/cli/fboss2/commands/config/ptp/transparent_clock/CmdConfigPtpTransparentClock.cpp

cmake/CliFboss2TestConfig.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ add_executable(fboss2_cmd_config_test
1616
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyPrefixListTest.cpp
1717
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyRoutingPolicyTest.cpp
1818
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyRoutingPolicyTermTest.cpp
19+
fboss/cli/fboss2/test/config/CmdConfigBgpPolicyRoutingPolicyTermActionTest.cpp
1920
fboss/cli/fboss2/test/config/CmdConfigCoppTest.cpp
2021
fboss/cli/fboss2/test/config/CmdConfigDhcpTest.cpp
2122
fboss/cli/fboss2/test/config/CmdConfigHostnameTest.cpp

cmake/CliFboss2TestIntegrationTest.cmake

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ add_executable(fboss2_integration_test
1818
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyPrefixListTest.cpp
1919
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyRoutingPolicyTest.cpp
2020
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyRoutingPolicyTermTest.cpp
21+
fboss/cli/fboss2/test/integration_test/ConfigBgpPolicyRoutingPolicyTermActionTest.cpp
2122
fboss/cli/fboss2/test/integration_test/ConfigBgpSessionTest.cpp
2223
fboss/cli/fboss2/test/integration_test/ConfigConcurrentSessionsTest.cpp
2324
fboss/cli/fboss2/test/integration_test/ConfigHostnameTest.cpp

fboss/cli/fboss2/BUCK

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1110,6 +1110,7 @@ cpp_library(
11101110
"commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.cpp",
11111111
"commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp",
11121112
"commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.cpp",
1113+
"commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.cpp",
11131114
"commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
11141115
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
11151116
"commands/config/ptp/CmdConfigPtp.cpp",
@@ -1229,6 +1230,7 @@ cpp_library(
12291230
"commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h",
12301231
"commands/config/protocol/bgp/policy/routing-policy/BgpRoutingPolicyCliUtils.h",
12311232
"commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h",
1233+
"commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.h",
12321234
"commands/config/protocol/static/CmdConfigProtocolStatic.h",
12331235
"commands/config/protocol/static/route/StaticRouteUtils.h",
12341236
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h",

fboss/cli/fboss2/CmdListConfig.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h"
5151
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h"
5252
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h"
53+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.h"
5354
#include "fboss/cli/fboss2/commands/config/protocol/static/CmdConfigProtocolStatic.h"
5455
#include "fboss/cli/fboss2/commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h"
5556
#include "fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h"
@@ -478,6 +479,30 @@ const CommandTree& kConfigCommandTree() {
478479
CmdConfigProtocolBgpPolicyRoutingPolicyTerm>,
479480
argRegistrar<
480481
CmdConfigProtocolBgpPolicyRoutingPolicyTermTraits>,
482+
{{
483+
"action",
484+
"Configure a term action",
485+
{{
486+
"result",
487+
"Set the term result: "
488+
"<ACCEPT|REJECT|CONTINUE>",
489+
commandHandler<
490+
CmdConfigProtocolBgpPolicyRoutingPolicyTermActionResult>,
491+
argRegistrar<
492+
CmdConfigProtocolBgpPolicyRoutingPolicyTermActionResultTraits>,
493+
},
494+
{
495+
"set",
496+
"Set a route attribute: "
497+
"as-path prepend|community|"
498+
"local-pref|med|next-hop|"
499+
"origin|weight <value> ...",
500+
commandHandler<
501+
CmdConfigProtocolBgpPolicyRoutingPolicyTermActionSet>,
502+
argRegistrar<
503+
CmdConfigProtocolBgpPolicyRoutingPolicyTermActionSetTraits>,
504+
}},
505+
}},
481506
}},
482507
}},
483508
},

fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,11 @@ struct CmdConfigProtocolBgpPolicyRoutingPolicyTermTraits
6161
: public WriteCommandTraits {
6262
using ParentCmd = CmdConfigProtocolBgpPolicyRoutingPolicy;
6363
static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
64-
// Term has no nested subcommands; stop CLI11's parent-chain fallthrough
65-
// from stealing a value token that spells `term` (e.g. in a description).
66-
cmd.positionals_at_end();
64+
// No positionals_at_end() here: the term's action (and later match)
65+
// level is a real CLI11 subcommand, and positionals_at_end() would stop
66+
// CLI11 from classifying it after the seq-num. The trade-off is that a
67+
// term-level attribute value spelling a child subcommand name is stolen
68+
// by subcommand matching (same exposure as the routing-policy parent).
6769
cmd.add_option("args", args, "<seq-num> [<attribute> <value> ...]");
6870
}
6971
using ObjectArgType = BgpRoutingPolicyTermConfig;

0 commit comments

Comments
 (0)