Skip to content

Added a new testcase for macsec_gearbox#4182

Open
shreyansh-nexthop wants to merge 3 commits intosonic-net:masterfrom
nexthop-ai:shreyansh.add_macsec_testcase
Open

Added a new testcase for macsec_gearbox#4182
shreyansh-nexthop wants to merge 3 commits intosonic-net:masterfrom
nexthop-ai:shreyansh.add_macsec_testcase

Conversation

@shreyansh-nexthop
Copy link

@shreyansh-nexthop shreyansh-nexthop commented Feb 2, 2026

Why I did it

  • Add a test case for mixed MACsec support across multiple gearbox PHYs, where some PHYs have macsec_supported=true and others have macsec_supported=false. This validates the scenario where a platform owner enables MACsec support only for specific gearbox PHYs. Got this as part of review comment for PR#3926.
  • Made the helper functions more robust by checking if the macsec is configured for the right interface or not.

How I did it

Added test_macsec_mixed_phy_support test case that:

  • Dynamically creates a second PHY (PHY 2) by reassigning an existing interface
  • Configures PHY 1 with macsec_supported=true (uses PHY backend → GB_ASIC_DB)
  • Configures PHY 2 with macsec_supported=false (uses NPU backend → ASIC_DB)
  • Enables MACsec on ports from both PHYs
  • Verifies MACsec keys are created in the correct database based on the PHY's macsec_supported setting

Added/Modified helper functions in gearbox.py:

  • reassign_interface_to_phy(): Reassigns an existing interface to a new PHY (creates the PHY if needed)
  • get_gearbox_port_by_phy(): Gets a port connected to a specific PHY
  • verify_macsec_for_port_in_asic_db() and verify_macsec_for_port_in_gb_asic_db: Modified the old helper functionsThis is more robust as it verifies if the macsec is configured for the right interface or not.

How to verify it

It can be verified by running the testcase TestMacsecGearbox::test_macsec_mixed_phy_support

Which release branch to backport

  • 202505

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 2, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: shreyansh-nexthop / name: shreyansh-nexthop (7a965a0)

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Signed-off-by: Shreyansh Jain <shreyansh@nexthop.ai>
@shreyansh-nexthop shreyansh-nexthop force-pushed the shreyansh.add_macsec_testcase branch from 243c4af to 8324132 Compare February 2, 2026 08:23
@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@shreyansh-nexthop
Copy link
Author

Hi, @judyjoseph, can you please help with the review? This is based on the review comment given on PR#3926.

I went ahead with the dynamically generating phy approach, as adding a second PHY to brcm_gearbox_vs/gearbox_config.json in sonic-buildimage won't work. It leads to failure of gearbox testcases, since multiple phys is not supported.

File: src/sonic-sairedis/vslib/VirtualSwitchSaiInterface.cpp

if (m_switchStateMap.size() == 1) {
    switch_id = m_switchStateMap.begin()->first;
} else {
    SWSS_LOG_THROW("multiple switches not supported, FIXME");
}

@rajshekhar-nexthop
Copy link
Contributor

Hi @prsunny , Please help to get this PR reviewed. Thanks

@prsunny
Copy link
Collaborator

prsunny commented Mar 4, 2026

@judyjoseph , please review and signoff. Lgtm

@prsunny
Copy link
Collaborator

prsunny commented Mar 4, 2026

@shreyansh-nexthop , could you rebase?

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

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.

4 participants