Skip to content

[202511] recover from stale-SAK after dirty macsec docker restart#4787

Draft
senthil-nexthop wants to merge 1 commit into
sonic-net:202511from
nexthop-ai:mabel.macsec-stale-sak-202511
Draft

[202511] recover from stale-SAK after dirty macsec docker restart#4787
senthil-nexthop wants to merge 1 commit into
sonic-net:202511from
nexthop-ai:mabel.macsec-stale-sak-202511

Conversation

@senthil-nexthop

Copy link
Copy Markdown
Contributor

Backport of #4583 to 202511

Cherry-pick of commit 5314f2d7f8ee48cfc501237dae296fb9395f162e from #4583 — clean apply, no conflicts.


Why I did it

After a macsec docker restart that doesn't give macsecmgrd enough time to call disableMACsec on every port (e.g. systemd TimeoutStopSec elapsed mid-loop, SIGKILL, or simply 16 ports exceeding the stop window), orchagent's MACsecOrch in-memory state survives with OIDs pointing at SAs whose SAK was distributed in the prior MKA cycle. The post-restart wpa_supplicant negotiates a fresh SAK and writes it to APPL_DB, but three code paths fail to propagate it to SAI. The result is asymmetric encryption: the ASIC keeps using the stale SAK while userspace believes the re-key happened, so ICV fails on every received frame and LACPDUs / LLDPDUs get silently dropped.

How I did it

Three matching re-key paths in orchagent/macsecorch.cpp, all detected by the presence of a sak field in the incoming SET:

  1. taskUpdateIngressSA: wpa_supplicant's macsec_sonic driver installs a new ingress SA in two stages: stage-1 writes active=false + full key material; stage-2 writes active=true only. After a dirty restart the SA pre-exists, so the legacy path deletes on active=false and the subsequent active=true createMACsecSA fails with no SAK to consume. New behaviour: when active=false arrives on an existing SA AND the SET carries a SAK, do deleteMACsecSA + createMACsecSA inline using THIS sa_attr's key material — atomically. When no SAK is in the SET, original delete-only path is kept (true deactivate).

  2. createMACsecSA: the function's "SA already exists → return task_success" early-exit silently discarded any SAK in the SET. SAI_MACSEC_SA_ATTR_SAK is create-only in SAI, so a re-key requires delete+create. New behaviour: when the SA exists and the SET carries a SAK, treat as re-key: deleteMACsecSA + recurse. When no SAK is present, keep the existing fast-path.

  3. taskUpdateEgressSA: the egress "SA already exists" branch only updated next_pn; the new SAK in the SET was silently dropped. Mirror the ingress re-key pattern: detect SAK presence, deleteMACsecSA + createMACsecSA with the new key material.

How to verify it

See #4583.


  • 202405
  • 202411
  • 202511

🤖 Generated with Claude Code

Co-authored-by: Senthil Krishnamurthy senthil@nexthop.ai

Signed-off-by: senthil-nexthop <senthil@nexthop.ai>
@mssonicbld

Copy link
Copy Markdown
Collaborator

/azp run

@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).

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.

2 participants