Skip to content

feat(agent): add per-port FEC configuration knobs#1394

Draft
edipascale wants to merge 1 commit into
masterfrom
ema/interface-fec
Draft

feat(agent): add per-port FEC configuration knobs#1394
edipascale wants to merge 1 commit into
masterfrom
ema/interface-fec

Conversation

@edipascale

Copy link
Copy Markdown
Contributor

Add PortFECs map[string]PortFECMode to SwitchSpec, mirroring the PortAutoNegs pattern. Supported modes: rs, fc, auto, disabled.

FEC defaults follow Broadcom SONiC rules: FEC RS is applied by default on 1x400G, 2x400G, 1x200G, 2x200G, and 4x100G breakout modes; all other ports default to FEC disabled. Defaults are encoded per breakout mode in SwitchProfilePortProfileBreakoutMode.FECDefault, with a profile-level FECDefault as fallback for speed-type profiles.

planPortFECs always sets an explicit FEC value (never UNSET/DEFAULT), consistent with observed Broadcom SONiC behaviour. FEC is applied via the existing specInterfaceEthernetBaseEnforcer using the PortFec field already present in the OpenConfig bindings.

Fix https://github.com/githedgehog/internal/issues/350

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 32 out of 32 changed files in this pull request and generated no comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Apr 3, 2026

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-143b240b5 🚀

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/agent/dozer/bcm/spec_interface.go
Comment thread pkg/agent/dozer/bcm/util.go
@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-e6cdcb8c1 🚀

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Apr 7, 2026

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-fef6a19e5 🚀

@qmonnet

qmonnet commented May 19, 2026

Copy link
Copy Markdown
Member

From sprint plan: We need to confirm whether we actually want this

@edipascale edipascale force-pushed the ema/interface-fec branch 2 times, most recently from ebb77a9 to 534f6e8 Compare May 25, 2026 08:00
@github-actions

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-534f6e8d8 🚀

@edipascale edipascale self-assigned this May 26, 2026
@edipascale edipascale force-pushed the ema/interface-fec branch from 534f6e8 to d0b0170 Compare June 4, 2026 14:19
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-d0b0170dc 🚀

Add PortFECs map[string]PortFECMode to SwitchSpec, mirroring the
PortAutoNegs pattern. Supported modes: rs, fc, auto, disabled.

Only ports that are explicitly configured by the user are
managed by the agent. Removing the relevant config from the
switch spec does NOT reset the FEC value on the port; the ideal
action would be to set it to DEFAULT, but that value is then
translated to something else depending on a number of factors
(i.e. port speed/ breakout mode, auto neg setting), and the
config diff would never be resolved.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Emanuele Di Pascale <emanuele@githedgehog.com>
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 126ce602-e1d8-4583-bb6a-8c7834f0681a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ema/interface-fec

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-8a9e711d8 🚀

1 similar comment
@github-actions

Copy link
Copy Markdown

🚀 Temp artifacts published: v0-8a9e711d8 🚀

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants