Skip to content

[Nexthop][fboss2-dev] Add config icmpv4-unavailable-src-addr CLI command#1268

Open
vybhav-nexthop wants to merge 3 commits into
facebook:mainfrom
nexthop-ai:add-config-icmpv4-unavailable-src-addr
Open

[Nexthop][fboss2-dev] Add config icmpv4-unavailable-src-addr CLI command#1268
vybhav-nexthop wants to merge 3 commits into
facebook:mainfrom
nexthop-ai:add-config-icmpv4-unavailable-src-addr

Conversation

@vybhav-nexthop

@vybhav-nexthop vybhav-nexthop commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Pre-submission checklist

  • I've ran the linters locally and fixed lint errors related to the files I modified in this PR. You can install the linters by running pip install -r requirements-dev.txt && pre-commit install
  • pre-commit run

Summary

Adds fboss2-dev config icmpv4-unavailable-src-addr <ipv4-address> to set icmpV4UnavailableSrcAddress in 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 in SwitchSettings::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.h refactor

icmpV4UnavailableSrcAddress is 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 existing getSwConfigField is refactored:

  • A private getSwConfigFieldOpt<T> helper does the actual field lookup and type conversion, returning std::optional<T>.
  • The original single-arg getSwConfigField delegates to it and throws when nullopt (required fields — existing callers unaffected).
  • A new two-arg overload delegates to it and returns the caller-supplied default when nullopt (optional fields).

The integration test reads the field as:

getSwConfigField<std::string>("icmpV4UnavailableSrcAddress", std::string(kDefaultIcmpV4SrcAddr))

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:CmdConfigIcmpV4UnavailableSrcAddrTest

  • Arg validation (valid IPs, invalid strings, out-of-range octets, IPv6 rejected)
  • setAddress: sets value, verifies in-memory config updated, confirms success path taken
  • alreadySet: no-op when value unchanged, verifies config untouched, confirms no-op path taken
  • setAddressWhenFieldAbsent: first-ever set when field not present in config

Integration test — NH-4010-F device

Built and tested on an NH-4010-F device running FBOSS main with all agents active:

$ ./fboss2_integration_test --gtest_filter="ConfigIcmpV4UnavailableSrcAddrTest.*"

[==========] Running 1 test from 1 test suite.
[ RUN      ] ConfigIcmpV4UnavailableSrcAddrTest.SetAndRestoreAddress
[Step 1] Reading current ICMPv4 unavailable source address...
  Current: 192.0.0.8
[Step 2] Setting address to 10.0.0.1...
[Step 3] Restoring original address 192.0.0.8...
TEST PASSED
[       OK ] ConfigIcmpV4UnavailableSrcAddrTest.SetAndRestoreAddress (153 ms)
[  PASSED  ] 1 test.
  • Read default 192.0.0.8 (field absent from config)
  • Set 10.0.0.1 via CLI + commit, verified in agent running config
  • Restored 192.0.0.8 + committed
  • Change applied HITLESS — no agent restart
Signed-off-by: vybhav-nexthop <vybhav@nexthop.ai>
@vybhav-nexthop vybhav-nexthop requested review from a team as code owners June 8, 2026 04:45
@meta-cla meta-cla Bot added the CLA Signed label Jun 8, 2026
@joseph5wu

Copy link
Copy Markdown
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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

2 participants