Skip to content

Commit 946bdbf

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 (14) + 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 82594cf commit 946bdbf

12 files changed

Lines changed: 1129 additions & 3 deletions

cmake/CliFboss2.cmake

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -902,6 +902,8 @@ add_library(fboss2_config_lib
902902
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/BgpRoutingPolicyCliUtils.h
903903
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.cpp
904904
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h
905+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.cpp
906+
fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.h
905907
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.cpp
906908
fboss/cli/fboss2/commands/config/ptp/CmdConfigPtp.h
907909
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
@@ -1109,6 +1109,7 @@ cpp_library(
11091109
"commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.cpp",
11101110
"commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.cpp",
11111111
"commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.cpp",
1112+
"commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.cpp",
11121113
"commands/config/protocol/static/CmdConfigProtocolStatic.cpp",
11131114
"commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.cpp",
11141115
"commands/config/ptp/CmdConfigPtp.cpp",
@@ -1235,6 +1236,7 @@ cpp_library(
12351236
"commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h",
12361237
"commands/config/protocol/bgp/policy/routing-policy/BgpRoutingPolicyCliUtils.h",
12371238
"commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h",
1239+
"commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.h",
12381240
"commands/config/protocol/static/CmdConfigProtocolStatic.h",
12391241
"commands/config/protocol/static/route/StaticRouteUtils.h",
12401242
"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
@@ -49,6 +49,7 @@
4949
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/prefix-list/entry/CmdConfigProtocolBgpPolicyPrefixListEntry.h"
5050
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/CmdConfigProtocolBgpPolicyRoutingPolicy.h"
5151
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/CmdConfigProtocolBgpPolicyRoutingPolicyTerm.h"
52+
#include "fboss/cli/fboss2/commands/config/protocol/bgp/policy/routing-policy/term/action/CmdConfigProtocolBgpPolicyRoutingPolicyTermAction.h"
5253
#include "fboss/cli/fboss2/commands/config/protocol/static/CmdConfigProtocolStatic.h"
5354
#include "fboss/cli/fboss2/commands/config/protocol/static/route/add/CmdConfigProtocolStaticRouteAdd.h"
5455
#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
@@ -62,9 +62,11 @@ struct CmdConfigProtocolBgpPolicyRoutingPolicyTermTraits
6262
: public WriteCommandTraits {
6363
using ParentCmd = CmdConfigProtocolBgpPolicyRoutingPolicy;
6464
static void addCliArg(CLI::App& cmd, std::vector<std::string>& args) {
65-
// Term has no nested subcommands; stop CLI11's parent-chain fallthrough
66-
// from stealing a value token that spells `term` (e.g. in a description).
67-
cmd.positionals_at_end();
65+
// No positionals_at_end() here: the term's action (and later match)
66+
// level is a real CLI11 subcommand, and positionals_at_end() would stop
67+
// CLI11 from classifying it after the seq-num. The trade-off is that a
68+
// term-level attribute value spelling a child subcommand name is stolen
69+
// by subcommand matching (same exposure as the routing-policy parent).
6870
cmd.add_option("args", args, "<seq-num> [<attribute> <value> ...]");
6971
}
7072
using ObjectArgType = BgpRoutingPolicyTermConfig;

0 commit comments

Comments
 (0)