[Nexthop][fboss2-dev] Add config icmpv4-unavailable-src-addr CLI command#1268
Open
vybhav-nexthop wants to merge 3 commits into
Open
[Nexthop][fboss2-dev] Add config icmpv4-unavailable-src-addr CLI command#1268vybhav-nexthop wants to merge 3 commits into
vybhav-nexthop wants to merge 3 commits into
Conversation
Signed-off-by: vybhav-nexthop <vybhav@nexthop.ai>
Contributor
|
@vybhav-nexthop : Please consider to follow the review comments as #1274 to introduce the CmdConfigSwitch.h/cpp to handle the global SwitchSetting or SwitchConfig attributes |
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runSummary
Adds
fboss2-dev config icmpv4-unavailable-src-addr <ipv4-address>to seticmpV4UnavailableSrcAddressin the agent's SW config.FBOSS uses this address as the source IP in ICMP error replies (TTL exceeded, destination unreachable) when no interface address is available. When the field is absent from config, the agent defaults to
192.0.0.8(the RFC 7600 IANA dummy address — computed on-the-fly inSwitchSettings::getIcmpV4UnavailableSrcAddress, never written to config). This command lets operators override that with a routable address for better network diagnosability.The config change is applied HITLESS — no agent restart required. Input is validated via
folly::IPAddressV4::tryFromString. The command is a no-op if the value already matches.Fboss2IntegrationTest.hrefactoricmpV4UnavailableSrcAddressis optional in the agent config — devices that have never configured it have no entry in the JSON. The integration test needs to read the field with a fallback to the agent's implicit default (192.0.0.8) when absent.To support this without duplicating the type-dispatch logic (
bool/int/string/double), the existinggetSwConfigFieldis refactored:getSwConfigFieldOpt<T>helper does the actual field lookup and type conversion, returningstd::optional<T>.getSwConfigFielddelegates to it and throws whennullopt(required fields — existing callers unaffected).nullopt(optional fields).The integration test reads the field as:
which returns
"192.0.0.8"when absent — matching what the agent actually uses at runtime.Test Plan
Unit tests
fboss/cli/fboss2/test/config:CmdConfigIcmpV4UnavailableSrcAddrTestsetAddress: sets value, verifies in-memory config updated, confirms success path takenalreadySet: no-op when value unchanged, verifies config untouched, confirms no-op path takensetAddressWhenFieldAbsent: first-ever set when field not present in configIntegration test — NH-4010-F device
Built and tested on an NH-4010-F device running FBOSS main with all agents active:
192.0.0.8(field absent from config)10.0.0.1via CLI + commit, verified in agent running config192.0.0.8+ committed