[Nexthop][fboss2-dev] profile-change validation + subsumed-port handling#1240
Open
hillol-nexthop wants to merge 1 commit into
Open
[Nexthop][fboss2-dev] profile-change validation + subsumed-port handling#1240hillol-nexthop wants to merge 1 commit into
hillol-nexthop wants to merge 1 commit into
Conversation
hillol-nexthop
force-pushed
the
fboss2-profile-validation
branch
from
4277d5f to
14a233e
Compare
hillol-nexthop
force-pushed
the
fboss2-profile-validation
branch
from
14a233e to
b38212d
Compare
Add `fboss2-dev config interface <port> profile <P>` with validation and a safe apply path. Validation (ProfileValidator): - Per-port supported-profile membership check (QSFP optics-aware, falling back to PlatformMapping hardware capabilities). - Parent-subsumption check: a port whose parent (controllingPort) holds a profile that subsumes it cannot be configured. The parent's effective profile is its target profile when the parent is also in the same batch, otherwise its current live profile (batch overlay). Apply (applyProfile): - Validate-before-mutate so a batch is rejected atomically with no partial config changes. - Strip subsumed ports (and their vlanPorts/interfaces) from the agent config; leaving them present aborts the agent on apply. - Commit via AGENT_COLDBOOT — the BCM SAI flexport recreate path only applies a lane-count change reliably on a cold boot. - Fix: the profile attribute branch never set `changed`, so a profile-only command skipped saveConfig() entirely — the session config was never persisted and the coldboot action never recorded, so commit silently no-op'd and the change never took effect on the switch. Tests: - Unit tests for membership, parent-subsumption (including the same-batch parent-downshift overlay), and subsumed-port population. - End-to-end round-trip integration test that restores the original profile unconditionally via a cleanup guard, so a mid-test failure cannot leave the switch on the changed profile. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
hillol-nexthop
force-pushed
the
fboss2-profile-validation
branch
from
7a57c20 to
b6a7060
Compare
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
1 participant
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.
Summary
Brings
fboss2-dev config interface <intfs> profile <profile>to parity withpatcher.py:change_speed's validation but makes it more reliant on platform mapping and avoiding per platform logic. Profile changes apply only to the explicitly-listed target port(s); subsumed siblings are stripped on upshift; freed sibling lanes on downshift are not auto-created.Validation: per-port profile-membership + ASIC-family adjacent-port rules (12T/25T/51T) + 51T 800G-only-on-lane-1 + subsumed-port warnings + batch atomicity + aggregated errors.
Upshift (subsumed-port handling): when a larger-lane profile claims neighboring lanes, the sibling ports listed in
PlatformPortConfig.subsumedPortsare stripped fromcfg::ports,vlanPorts, andcfg::Interfaceentries that reference them — otherwise the agent aborts on apply.Downshift: only the user-listed port(s) change profile. Freed sibling lanes stay unconfigured; the operator brings them up explicitly.
Apply path: profile changes go through coldboot instead of
reloadConfig(rationale below).Test Plan
Unit
Integration