[macsec] Add fault-handling tests for MKA timeout on LAG member by karthik-nexthop · Pull Request #26062 · sonic-net/sonic-mgmt

karthik-nexthop · 2026-07-10T11:39:17Z

Description of PR

Adds MACsec fault-handling tests for LAG members:

test_macsec_down_disables_lag_member: blocks inbound EAPOL (ethertype 0x888e, which
carries MKA PDUs) on the DUT ingress while the link stays physically up. Verifies the MKA
session expires within ~MKA_TIMEOUT (6s) and that the LAG member is brought down quickly
(single-member PortChannel goes Dw, multi-member LAG deselects the member) instead of
blackholing traffic until the 90s LACP timeout. Also checks that a teamsyncd
LAG_MEMBER_TABLE refresh does not re-enable the member while MACsec is down, and that
MACsec/LAG/BGP recover after the block is removed. Multi-member PortChannel/BGP staying
up is gated on remaining members still satisfying CONFIG_DB min_links.
test_macsec_rekey_keeps_lag_member_up: verifies a periodic MACsec rekey does not
spuriously disable or flap the LAG member while the session stays up (primary gate is
ASIC_DB disable state).

Summary:
Adds regression coverage in tests/macsec/test_fault_handling.py for MACsec session loss
on LAG members: prompt LAG-member teardown on MKA timeout, no spurious member flap during
rekey, and recovery after the fault is cleared.

Depends on orchagent fix: sonic-net/sonic-swss#4744 (covers sonic-buildimage#19254).

Fixes # (issue)

Type of change

Back port request

Tracking issue/work item for backport/cherry-pick request:
Failure type:

Approach

What is the motivation for this PR?

When a MACsec session on a LAG member dies (MKA PDUs stop arriving), the member should be
brought down promptly rather than staying selected in the LAG until the LACP timeout, which
would blackhole traffic over a link with no working MACsec. A healthy MACsec rekey should
not spuriously disable or flap the LAG member while the session remains up.

How did you do it?

Extended TestFaultHandling in tests/macsec/test_fault_handling.py with helper functions
to observe LAG-member state via teamd, ASIC_DB, and APPL_DB, then added:

test_macsec_down_disables_lag_member — installs a DUT-side tc clsact ingress
filter dropping protocol 0x888e (in the port's ASIC netns on multi-ASIC), then asserts:
- MACsec crypto plane goes down within ~2x MKA_TIMEOUT
- teamd deselects the member well under the 90s LACP timeout (per-ASIC teamd/teamdN)
- orchagent disables the member at the ASIC (EGRESS/INGRESS_DISABLE)
- a teamsyncd LAG_MEMBER_TABLE refresh with status=enabled does not re-enable the
  member while MACsec is down (swssconfig rc checked and APPL_DB status read back
  before the ASIC race assertion)
- single-member LAG (or multi-member when remaining members would fall below
  min_links): PortChannel and BGP withdraw; multi-member LAG with remaining members
  still meeting min_links: BGP stays up
- MACsec, LAG member, and BGP recover after the filter is removed
Parameterized over single_member and multi_member LAGs.
test_macsec_rekey_keeps_lag_member_up — observes across ~2x rekey_period, sampling
ASIC_DB LAG-member disable state as the primary gate, and asserts no spurious member
disable while SAs rotate and the session stays up. Orchagent disable-log greps are
secondary only (wording can differ by image).

Reuses existing ctrl_links, upstream_links, rekey_period, and wait_mka_establish
fixtures. Existing test_mismatch_macsec_configuration is preserved with upstream
port_profiles compatibility.

How did you verify/test it?

Ran on a t0 MACsec testbed with --enable_macsec and SONiC neighbors
(--neighbor_type sonic).
Verified EAPOL block, LAG member down, teamsyncd race guard, and recovery paths pass for
single-member LAGs.
Verified multi-member LAG variant skips when no MACsec control link sits on a multi-member
PortChannel, and that BGP stays Established when one member goes down when remaining
members still satisfy min_links.
Verified rekey test passes without spurious LAG-member flaps at the ASIC.

Any platform specific information?

The tc-based EAPOL block runs on the DUT (ASIC netns on multi-ASIC) and is neighbor-OS
independent. ASIC_DB LAG-member disable checks require platforms where orchagent programs
SAI_LAG_MEMBER_ATTR_EGRESS_DISABLE / INGRESS_DISABLE. The multi-member LAG variant
skips automatically when the testbed has no suitable multi-member MACsec control link.

Supported testbed topology if it's a new test case?

t0, t2, lrh, urh, t0-sonic (module pytestmark). Requires --enable_macsec and SONiC
neighbors (--neighbor_type sonic).

Documentation

N/A — test-only change; no documentation update required.

Signed-off-by: Karthik Siruvalam karthik@nexthop.ai

Adds MACsec fault-handling tests for LAG members: - test_eapol_block_brings_lag_member_down: blocks inbound EAPOL on the DUT ingress while the link stays physically up. Verifies the MKA session expires within ~MKA_TIMEOUT and that the LAG member is brought down quickly instead of blackholing traffic until the 90s LACP timeout. - test_macsec_rekey_keeps_lag_member_up: verifies a periodic MACsec rekey does not spuriously disable or flap the LAG member while the session stays up. Signed-off-by: karthik-nexthop <karthik@nexthop.ai>

linux-foundation-easycla · 2026-07-10T11:39:23Z

The committers listed above are authorized under a signed CLA.

✅ login: karthik-nexthop / name: karthik-nexthop (8af9953)

mssonicbld · 2026-07-10T11:39:25Z

/azp run

azure-pipelines · 2026-07-10T11:39:53Z

Azure Pipelines successfully started running 1 pipeline(s).

yxieca · 2026-07-14T17:42:41Z

/azp run

azure-pipelines · 2026-07-14T17:42:47Z

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

yxieca · 2026-07-14T18:02:42Z

Hi, the Pre_test Static Analysis pre-checker is failing on this PR due to a flake8 lint error in your change:

tests/macsec/test_fault_handling.py:297:1: E999 IndentationError: unexpected unindent

/azp run won't clear this since it's a real lint error in the code — please fix it (run python3 -m flake8 <file> --max-line-length=120 locally to confirm clean), then push an update. Thanks!

Resolve leftover conflict markers so flake8 can parse the file, keeping the new LAG MKA tests and port_profiles on the mismatch test. Signed-off-by: Karthik Siruvalam <karthik@nexthop.ai>

mssonicbld · 2026-07-15T05:35:38Z

/azp run

azure-pipelines · 2026-07-15T05:35:53Z

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

mssonicbld · 2026-07-15T11:21:05Z

Hi, there are workflow run(s) waiting for approval, you may be first-time contributor. I will notify maintainers to help approve once PR is approved. Thanks!

_{---Powered by SONiC BuildBot}

saksarav-nokia · 2026-07-15T17:28:43Z


+    @pytest.mark.disable_loganalyzer
+    @pytest.mark.parametrize("lag_kind", ["single_member", "multi_member"])
+    def test_macsec_down_disables_lag_member(self, duthost, ctrl_links,


The test case name is correct here, please update your PR Description with this name.

saksarav-nokia · 2026-07-15T18:32:11Z

+    cfg = [{"LAG_MEMBER_TABLE:{}:{}".format(pc_name, port_name): {"status": status}, "OP": "SET"}]
+    tmp = duthost.shell("mktemp")["stdout"].strip()
+    duthost.copy(content=json.dumps(cfg), dest=tmp, verbose=False)
+    duthost.docker_exec_swssconfig("/dev/stdin < {}".format(tmp), "swss", asic_idx)


can you check for result of docker_exec_swssconfig?

saksarav-nokia · 2026-07-15T18:40:53Z

+        90s LACP timeout -- and must keep it down (a teamsyncd APP_LAG_MEMBER_TABLE refresh
+        must not silently re-enable it while MACsec is down). For a single-member LAG the
+        whole PortChannel goes down, which withdraws the BGP session over it; for a
+        multi-member LAG the PortChannel stays up on the other members, so BGP must NOT be


Portchannel/BGP not going down on multi-member LAG depends on min_links settings in PortChannel right?

saksarav-nokia · 2026-07-15T18:55:44Z

+            # Block inbound EAPOL on the DUT so the DUT cannot receive peer MKA hellos and
+            # its session times out. del-before-add is idempotent.
+            ns_prefix = get_ipnetns_prefix(duthost, port_name)
+            duthost.shell("sudo tc qdisc del dev {} clsact".format(dut_eth_port),


few tc commands are executed in the asic namespace, and these are in the host namespace. Don't you need to use asic name space for all?

saksarav-nokia · 2026-07-15T18:58:54Z

+def teamd_member_selected(duthost, pc_name, port_name):
+    """teamd's view: is `port_name` currently a selected member of `pc_name`?"""
+    try:
+        state = duthost.get_port_channel_status(pc_name)


get_port_channel_status has "self.command("docker exec -i teamd teamdctl {} state dump".format(port_channel_name))" which won't work for multi-asic platform since there is teamd docker per asic

YatishSVC · 2026-07-27T18:17:22Z

+    return (egress == "true", ingress == "true")
+
+
+def push_lag_member_status(duthost, pc_name, port_name, status):


push_lag_member_status discards the docker_exec_swssconfig result. If the injection silently fails, the member simply stays disabled and check (5) (assert not member_reenabled_in_asic) passes without ever driving the re-enable race — which is the exact behavior this test exists to verify. Please confirm the injection actually took effect before asserting the ASIC stayed disabled (e.g. read back LAG_MEMBER_TABLE shows status=enabled), not just check the shell return.

YatishSVC · 2026-07-27T18:17:54Z

+    return duthost.shell(cmd)["stdout"].strip() == "Established"
+
+
+def macsec_lag_disable_log_count(duthost, port_name):


macsec_lag_disable_log_count greps exact orchagent strings ('MACsec disabled LAG member ...', 'Flapping host interface ... MACsec down'). If the log wording differs on the image/branch under test, the count is always 0 and disable_logs_after == disable_logs_before (0==0) passes even if a spurious flap happened. Consider anchoring the primary assertion on ASIC_DB disable state and treating the log grep as secondary.

Harden multi-ASIC tc/teamd paths, verify swssconfig injection via APPL_DB readback, honor PortChannel min_links for multi-member BGP expectations, and treat rekey disable-log greps as secondary to ASIC_DB checks. Signed-off-by: Karthik Siruvalam <karthik@nexthop.ai>

mssonicbld · 2026-07-28T06:37:53Z

/azp run

azure-pipelines · 2026-07-28T06:38:09Z

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

karthik-nexthop · 2026-07-29T10:05:11Z

Fix is in review stage. Hence the test script is failing . Will trigger tests again post fix merge.

github-actions Bot requested review from Pterosaur, judyjoseph and yejianquan July 10, 2026 11:39

saravanan-nexthop requested a review from tjchadaga July 10, 2026 14:45

[macsec] Fix merge conflict in test_fault_handling.py

35b5d3e

Resolve leftover conflict markers so flake8 can parse the file, keeping the new LAG MKA tests and port_profiles on the mismatch test. Signed-off-by: Karthik Siruvalam <karthik@nexthop.ai>

abdosi requested a review from saksarav-nokia July 15, 2026 17:08

abdosi added this to SONiC Chassis Jul 15, 2026

saksarav-nokia reviewed Jul 15, 2026

View reviewed changes

saravanan-nexthop requested a review from YatishSVC July 17, 2026 16:38

YatishSVC reviewed Jul 27, 2026

View reviewed changes

karthik-nexthop requested review from YatishSVC and saksarav-nokia July 28, 2026 06:47

saksarav-nokia approved these changes Jul 29, 2026

View reviewed changes

rlhui assigned karthik-nexthop Aug 10, 2026

		return (egress == "true", ingress == "true")


		def push_lag_member_status(duthost, pc_name, port_name, status):

		return duthost.shell(cmd)["stdout"].strip() == "Established"


		def macsec_lag_disable_log_count(duthost, port_name):

Uh oh!

Conversation

karthik-nexthop commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of PR

Type of change

Back port request

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Uh oh!

linux-foundation-easycla Bot commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mssonicbld commented Jul 10, 2026

Uh oh!

azure-pipelines Bot commented Jul 10, 2026

Uh oh!

yxieca commented Jul 14, 2026

Uh oh!

azure-pipelines Bot commented Jul 14, 2026

Uh oh!

yxieca commented Jul 14, 2026

Uh oh!

mssonicbld commented Jul 15, 2026

Uh oh!

azure-pipelines Bot commented Jul 15, 2026

Uh oh!

mssonicbld commented Jul 15, 2026

Uh oh!

Uh oh!

saksarav-nokia Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

saksarav-nokia Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

saksarav-nokia Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

saksarav-nokia Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

saksarav-nokia Jul 15, 2026

Choose a reason for hiding this comment

Uh oh!

YatishSVC Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

YatishSVC Jul 27, 2026

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Jul 28, 2026

Uh oh!

azure-pipelines Bot commented Jul 28, 2026

Uh oh!

karthik-nexthop commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

karthik-nexthop commented Jul 10, 2026 •

edited

Loading

linux-foundation-easycla Bot commented Jul 10, 2026 •

edited

Loading