[Link Event Damping] Add config/show/clear CLI commands - #4367
[Link Event Damping] Add config/show/clear CLI commands#4367DendroLabs wants to merge 5 commits into
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Implements CLI support for RFC 2439/7196 link event dampening: - config interface dampening enable/disable: configure AIED dampening with validation (IntRange per review feedback), monitor-only mode, and configurable flap-penalty - show interfaces dampening: display config and operational state from CONFIG_DB and STATE_DB with per-interface counters - sonic-clear interfaces dampening: reset penalty via STATE_DB flag Supersedes sonic-net#3001 with improvements: - click.IntRange validation (per @Junchao-Mellanox review) - Monitor-only mode per RFC 7196 "Calculate But Do Not Damp" - Configurable flap-penalty (vs hardcoded 1000) - Show command with operational state and counters - Clear command per RFC 2439 Section 4.8.6 Signed-off-by: DendroLabs <info@dendrolabs.com>
- Guard against exponential overflow in ceiling calculation - Add interface alias support to clear command - Improve error handling consistency Signed-off-by: DendroLabs <info@dendrolabs.com>
cf10a20 to
f484733
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/easycla |
- Add missing blank line before dampening subgroup (E302) - Fix continuation line indentation in warning messages (E127) Signed-off-by: DendroLabs <info@dendrolabs.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Tests cover config enable/disable, show, and clear commands: - Config: defaults, custom params, monitor mode, validation (invalid interface, reuse>=suppress, half-life>max-suppress, IntRange bounds, overflow warning) - Show: no config, disabled, configured, monitor mode display - Clear: specific interface, all interfaces, no config, invalid Signed-off-by: DendroLabs <info@dendrolabs.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Signed-off-by: DendroLabs <info@dendrolabs.com>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Hi @prsunny @Junchao-Mellanox — friendly ping on this PR. The companion sairedis PRs are progressing (sonic-net/sonic-sairedis#1798 merged, sonic-net/sonic-sairedis#1935 open for review), and this CLI PR addresses all the feedback from the original #3001. CI was green when submitted. Would appreciate a review when you get a chance. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end CLI support in sonic-utilities for the Link Event Dampening feature by introducing new config, show, and sonic-clear commands, along with unit tests.
Changes:
- Added
config interface dampening <interface> enable|disableto configure AIED dampening parameters (including monitor-only mode) in CONFIG_DB. - Added
show interfaces dampening [interface]to display dampening configuration plus operational state/counters. - Added
sonic-clear interfaces dampening [interface]to request clearing (reset/unsuppress) of dampening state via STATE_DB, and added unit tests for the new CLIs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
config/main.py |
Adds dampening subgroup under config interface with enable/disable commands and parameter validation. |
show/interfaces/__init__.py |
Adds show interfaces dampening plus helper to print per-interface dampening counters. |
clear/main.py |
Adds sonic-clear interfaces dampening command that writes clear requests to STATE_DB. |
tests/link_event_damping_test.py |
Adds unit tests covering config/show/clear CLI behavior and validation scenarios. |
| if clicommon.get_interface_naming_mode() == "alias": | ||
| interface_name = interface_alias_to_name(config_db, interface_name) | ||
| if interface_name is None: | ||
| ctx.fail("'interface_name' is None!") | ||
|
|
| if clicommon.get_interface_naming_mode() == "alias": | ||
| interface_name = interface_alias_to_name(config_db, interface_name) | ||
| if interface_name is None: | ||
| ctx.fail("'interface_name' is None!") | ||
|
|
| click.echo("Error: invalid interface alias {}".format(alias)) | ||
| sys.exit(1) | ||
| if interface_name not in port_table: | ||
| click.echo("Error: Interface {} does not exist".format(interface_name)) | ||
| sys.exit(1) |
| state_db = SonicV2Connector(host="127.0.0.1") | ||
| state_db.connect(state_db.STATE_DB) | ||
|
|
| config_db = db.cfgdb | ||
| state_db = db.db | ||
|
|
|
@DendroLabs , could you check copilot comments and address them. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| Without an interface name, clears dampening on all interfaces. | ||
| The interface is immediately unsuppressed if currently damped. | ||
| """ | ||
| config_db = ConfigDBConnector() |
There was a problem hiding this comment.
host CONFIG_DB only — on multi-ASIC, PORT lives in per-namespace CONFIG_DB and that's where orchagent reads. Same issue with ctx.obj['config_db'] in the enable/disable commands. See how config interface ip add resolves the namespace via multi_asic.get_port_namespace(port) / db.cfgdb_clients[ns].
|
@DendroLabs , could you address comment? |
|
Thanks @prsunny. Before I push the namespace fixes, I want to surface something I ran into while wiring the CLI against the now merged syncd implementation (#1906), because I think it is a design question worth settling together rather than me papering over it in the CLI. What I am trying to do: make Where the CLI and the merged syncd schema do not line up today:
Why it matters: without these, the CLI either reports That said, I am very happy to defer to you and @sivat6 on how to land this. If you would prefer I align the CLI to the existing VID keyed schema for now and treat the clear and monitor pieces as later follow ups, I am glad to do that and keep moving on the namespace fixes in parallel. If you would rather settle the schema first, I am happy to go whichever way fits best with how you see this feature evolving. Just let me know the direction you would like and I will follow it. |
|
@DendroLabs , my ask is to address copilot comments and from @qiluo-msft . not sure which one specifically you are referring to. |
Summary
Supersedes #3001 by @Ashish1805. Adds complete CLI support for the Link Event Damping feature (HLD: sonic-net/SONiC#1071).
All review feedback from #3001 has been addressed:
click.IntRangefor parameter validation (@Junchao-Mellanox)showcommand that was flagged as missing (@Junchao-Mellanox)Commands added:
config interface dampening <interface> enable [options]--monitorflag enables monitor-only mode (RFC 7196 "Calculate But Do Not Damp")config interface dampening <interface> disableshow interfaces dampening [interface_name]sonic-clear interfaces dampening [interface_name]New features not in any other NOS vendor:
--monitor): Calculates penalties and emits syslog but does NOT suppress events. Allows safe parameter tuning in production. Recommended by RFC 7196; no vendor has implemented this.Test plan
config interface dampening enablewrites correct fields to CONFIG_DB PORT tableconfig interface dampening disableresets all fieldsconfig interface dampening enable --monitorsets algorithm to "aied-monitor"show interfaces dampeningdisplays configured interfacesshow interfaces dampening <port>shows counterssonic-clear interfaces dampeningwrites clear flag to STATE_DB🤖 Generated with Claude Code
Co-Authored-By: Ashish Singh ashish.singh@google.com