Skip to content

Fix show macsec crash after deleting macsec ports#28585

Draft
darius-nexthop wants to merge 2 commits into
sonic-net:masterfrom
nexthop-ai:fix-show-macsec-crash-after-deleting
Draft

Fix show macsec crash after deleting macsec ports#28585
darius-nexthop wants to merge 2 commits into
sonic-net:masterfrom
nexthop-ai:fix-show-macsec-crash-after-deleting

Conversation

@darius-nexthop

@darius-nexthop darius-nexthop commented Jul 22, 2026

Copy link
Copy Markdown

Why I did it

config macsec port del <port> removes the macsec field from the CONFIG_DB PORT entry immediately, while the APPL_DB MACSEC_PORT_TABLE entry is cleaned up asynchronously by macsecmgrd. If show macsec runs in that window (for example, while deleting several ports at once), MACsecPort.dump_str finds the APPL_DB entry but no macsec field in CONFIG_DB and crashes:

File ".../show/plugins/macsec.py", line 158, in dump_str
    self.meta["profile"] = self.cfgMeta["macsec"]
KeyError: 'macsec'

The error is transient and re-running show macsec after teardown completes succeeds.

fixes #28584

Work item tracking
  • Microsoft ADO (number only):

How I did it

Switch to self.cfgMeta.get("macsec", "") in MACsecPort.dump_str

If a port is mid-teardown, it will be shown with an empty profile instead of crashing, e.g.:

MACsec port(Ethernet5)
cipher_suite           GCM-AES-256
enable                 true
enable_encrypt         true
enable_protect         true
enable_replay_protect  false
profile
replay_window          0
send_sci               true

Reasoning is that the port still exists in APPL_DB, so we should still show the port in show macsec while it is still programmed. This output is transient and the port will simply disappear once macsecmgrd has unprogrammed it.

Also adds a guard for the port append in create_macsec_objs against None result from create_macsec_obj.

How to verify it

New regression test test_show_port_deleted_from_config: Ethernet5 in the mock DBs has an APPL_DB MACSEC_PORT_TABLE entry but no macsec field in CONFIG_DB (the mid-teardown state). Without the fix, test_show_all and the new test both crash with the KeyError above; with the fix the port shows with an empty profile and all macsec cli-plugin-tests pass.

Which release branch to backport (provide reason below if selected)

  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608

Tracking issue/work item for backport/cherry-pick request (GitHub issue or Microsoft ADO): #28584
Failure type: day-one issue

Tested branch

  • master
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511
  • 202512
  • 202605
  • 202608
  • N/A

Test result

(with fix)
202511:

  MACsec port(Ethernet16)
  ------------  -----------
  cipher_suite  GCM-AES-256
  enable        true
  profile
  ------------  -----------

Description for the changelog

Fix show macsec KeyError crash when run while MACsec port deletion is still being processed.

Link to config_db schema for YANG module changes

N/A

A picture of a cute animal (not mandatory but encouraged)

Signed-off-by: Darius Grassi <darius@nexthop.ai>
@darius-nexthop
darius-nexthop requested a review from lguohan as a code owner July 22, 2026 19:22
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@darius-nexthop
darius-nexthop marked this pull request as draft July 22, 2026 19:22
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request for branch(es): 202511.
Added label(s) for branch(es) 202511.

---Powered by SONiC BuildBot

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request label(s) for branch(es): 202511, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202511: <test result>) in the Test result section as well in your PR description.

---Powered by SONiC BuildBot

@darius-nexthop

Copy link
Copy Markdown
Author

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Commenter does not have sufficient privileges for PR 28585 in repo sonic-net/sonic-buildimage

Signed-off-by: Darius Grassi <darius@nexthop.ai>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run Azure.sonic-buildimage

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request for branch(es): 202511,202605.

Request label(s) for branch(es) 202511 already exists.
Added label(s) for branch(es) 202605.

---Powered by SONiC BuildBot

@mssonicbld mssonicbld added the Tested for 202511 branch Verified/tested against the 202511 release branch label Jul 24, 2026
@mssonicbld

Copy link
Copy Markdown
Collaborator

The Tested branch section has been ticked and Test result is provided for branch(es): 202511. Added label(s): Tested for 202511 Branch.

---Powered by SONiC BuildBot

@mssonicbld

Copy link
Copy Markdown
Collaborator

This PR has backport request label(s) for branch(es): 202605, but is missing required test information. Please make sure you tick the tested branch(es) in the Tested branch section and provide test evidence (e.g., 202605: <test result>) in the Test result section as well in your PR description.

---Powered by SONiC BuildBot

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: show macsec crashes with KeyError: 'macsec' when run during config macsec port del

2 participants