Skip to content

[sai_test] Add opt-in port and BFD notification test cases - #2336

Open
nicholasching wants to merge 10 commits into
opencomputeproject:masterfrom
nicholasching:sai_vpp_ut_notification_tests
Open

[sai_test] Add opt-in port and BFD notification test cases#2336
nicholasching wants to merge 10 commits into
opencomputeproject:masterfrom
nicholasching:sai_vpp_ut_notification_tests

Conversation

@nicholasching

@nicholasching nicholasching commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Context / motivation
Part of the SAIVPP unit-test framework. sai_test currently has no coverage for asynchronous SAI notifications: nothing exercises SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY or SAI_SWITCH_ATTR_BFD_SESSION_STATE_CHANGE_NOTIFY. This adds the first tests for that path, using the saithrift notification RPCs added separately.

What this change does

  • test/sai_test/sai_notification_test.py. New module with five classes: PortStateChangeTest, PortStateRecoveryTest, BfdSessionUpTest, BfdSessionDownTest and BfdMultihopTest.
  • Opt-in. Every class skips unless the harness passes the vpp_notification_test PTF parameter; the BFD classes additionally require SIMULATE_SONIC=1. With neither set the module is inert.
  • Port cases. Flap the SAI host-interface netdev and wait for the matching port OID and oper status, polling every 0.5s within a 5s budget. The wire-side veth peer is deliberately not used: VPP binds it with an AF_PACKET socket whose driver does not watch the underlying netdev carrier, so Linux reports NO-CARRIER while vppctl show hardware-interfaces still reports the link up and no interface event is ever raised.
  • BFD cases. Anchor the session on a LAG router interface that already has a connected local address, create the peer neighbor so the dataplane can build the L2 header, and drive the far end with a small scapy responder. The responder follows the RFC 5880 state machine (a peer in Down only leaves that state on receiving Down or Init), targets UDP 3784/4784 in both directions with an ephemeral source port, and listens on every LAG member because the flow is hashed onto one of them.
  • Independent corroboration. Assertions are checked against vppctl output rather than only re-reading the SAI attribute that the notification path itself writes. All SAI objects and link state are restored in finally/tearDown.

Scope / risk

  • One new file. No existing test, helper, SAI header, or backend code is modified.
  • Default-off by construction, so other consumers (real ASICs, other harnesses) are unaffected.
  • BFD waits use a longer timeout than the port cases: RFC 5880 holds the control-packet interval at one second until a session is up, and measured bring-up against the responder is around 5.5s.
  • Validated behavior: on a VPP bench all five classes passed three consecutive module runs, plus a full five-module 90-selector matrix run at PORT_COUNT=32 with per-test isolation, which reported 90 observed selectors and zero regressions against the existing stable baseline.

Dependencies
Requires the saithrift notification bridge PR, #2335; must be merged before this PR can be merged.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
@azure-pipelines

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

@nicholasching

Copy link
Copy Markdown
Contributor Author

Hi @kcudnik @tjchadaga, when you have a chance, could you please invoke azure pipelines to run checks. Thanks!

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Comment thread test/sai_test/sai_notification_test.py Outdated
from sai_utils import sai_ipaddress, sai_ipprefix


PORT_NOTIFICATION_TYPE = 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should come from the enum defined in PR 2335

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, fixed in 2c4ea58.

Comment thread test/sai_test/sai_notification_test.py Outdated
self.start_session()
self.bfd_event(SAI_BFD_SESSION_STATE_UP)
self.assert_bfd_state(SAI_BFD_SESSION_STATE_UP)
self.assert_vpp_bfd_state("up")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check on asic type before calling vpp specific check.

@nicholasching nicholasching Aug 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch; thanks Fred. All done in f7c395d, now check self.platform == VPP_PLATFORM before call VPP checks.

def _run(self, interface_name):
from scapy.all import sniff

while not self.stop_event.is_set():

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI points out a gap between sniff calls that may cause loss of bfd message because between sniff calls, there is no BPF socket to receive the bfd packet. It suggests to use AsyncSniffer.
def start(self):
import functools

    from scapy.all import AsyncSniffer

    for name in self.interface_names:
        ready = threading.Event()
        sniffer = AsyncSniffer(
            iface=name,
            # Narrow the match in BPF rather than in Python. The filter is
            # now compiled once instead of twice a second, and the responder
            # never sees its own replies looped back by AF_PACKET.
            filter="udp dst port {} and src host {}".format(
                self.udp_port, self.local_ip
            ),
            store=False,
            # functools.partial, not a lambda: a lambda closing over the
            # loop variable would bind every sniffer to the last interface.
            prn=functools.partial(self._respond, name),
            started_callback=ready.set,
        )
        sniffer.start()
        self.sniffers.append(sniffer)

        # start() returns before the capture socket is bound. Without this
        # the first control packets are missed exactly as before.
        if not ready.wait(timeout=SNIFFER_START_TIMEOUT):
            self.stop()
            raise AssertionError(
                "BFD responder failed to start capture on {}".format(name)
            )

def stop(self):
    for sniffer in self.sniffers:
        try:
            sniffer.stop()
        except Exception:
            # stop() raises if the sniffer never reached the run loop.
            pass
    self.sniffers = []

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that is very true! Implemented in 2a0aa35, BFD responder now uses one continuously running AsyncSniffer per peer interface and waits for capture startup before creating the session, avoiding packet-loss windows between polling sniff calls.

Comment thread test/sai_test/sai_notification_test.py Outdated

def setUp(self):
params = test_params_get() or {}
if params.get(PLATFORM_PARAM) != VPP_PLATFORM:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this check? what is required for another platform to enable notification test? There is also notification test opt-in flag. If it is not supported in certain flag, they can opt it out.

Our goal is making this test suitable for other platforms. They can add platform specific check. But without it, it still works.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need this check? what is required for another platform to enable notification test?

Hi Fred, yes, unfortunately, unlike the port tests, the current BFD test implementation is VPP-specific. Since it constructs the complete working BFD environment inside the VPP test container, it uses the VPP LAG topology, VPP-specific addressing and routing, Linux peer interfaces, a Scapy BFD responder, and VPP state inspection. Other platforms would need equivalent platform-specific BFD fixtures, which have not been implemented yet.

Our goal is making this test suitable for other platforms. They can add platform specific check. But without it, it still works.

I've done some more research; it is possible to move the VPP-specific topology and observability from the shared SAI test suite into the SAI VPP UT harness. For a final structure that looks something like this:

SAI PR #2336:
    Generic notification tests and BFD fixture interface

sonic-sairedis Phase 4 PR:
    VPP BFD fixture
    VPP Docker image injection
    VPP test parameters
    VPP-specific CI invocation

I'm making this change now. This should make the test fully generic and require the platform under test to provide their platform-specific fixture implementation instead.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Fred, I’ve pushed a couple more commits to generalize the notification tests. The harness is now responsible for supplying the platform-specific BFD fixture to SAI notification tests, moving the VPP-specific code from SAI into sonic-sairedis. Other platforms will be responsible for providing their own fixture when enabling the BFD cases.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
Comment thread test/sai_test/sai_notification_test.py Outdated
self.start_session()
self.bfd_event(SAI_BFD_SESSION_STATE_UP)
self.assert_bfd_state(SAI_BFD_SESSION_STATE_UP)
self.assert_vpp_bfd_state("up")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move the platform check to the caller. If other platforms are added to this test, we don't want this function calls assert_vpp_bfd_state, assert_cisco_8000_bfd_state etc. It should call the function corresponding to the platform.

Signed-off-by: Nicholas Ching <nicholaslching@gmail.com>
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