Skip to content

[bgp/test]: Add config_reload after suppress-fib-pending changes#22916

Open
mike-dubrovsky wants to merge 4 commits into
sonic-net:masterfrom
mike-dubrovsky:fib-suppression
Open

[bgp/test]: Add config_reload after suppress-fib-pending changes#22916
mike-dubrovsky wants to merge 4 commits into
sonic-net:masterfrom
mike-dubrovsky:fib-suppression

Conversation

@mike-dubrovsky
Copy link
Copy Markdown
Contributor

@mike-dubrovsky mike-dubrovsky commented Mar 12, 2026

The suppress-fib-pending configuration change now requires a config save followed by config reload to take effect.

This change needs to go together with
sonic-net/sonic-swss#4333
sonic-net/sonic-buildimage#26151
sonic-net/sonic-utilities#4361
#22916

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • New Test case
    • Skipped for non-supported platforms
  • Test case improvement

Back port request

  • 202205
  • 202305
  • 202311
  • 202405
  • 202411
  • 202505
  • 202511

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

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

yxieca
yxieca previously approved these changes Mar 31, 2026
Copy link
Copy Markdown
Collaborator

@yxieca yxieca left a comment

Choose a reason for hiding this comment

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

AI agent on behalf of Ying. Reviewed; no issues found.

@deepak-singhal0408
Copy link
Copy Markdown
Contributor

@mike-dubrovsky please help resolve the conflicts..

@rawal01 please help take a look as well.. thanks,

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

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.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

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.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

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.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

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.

@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

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.

config suppress-fib-pending now requires a config reload to take effect.

- conftest.py: Add config reload (with wait_for_bgp) after CLI change in
  config_bgp_suppress_fib fixture so that changes become operational.
- test_bgp_peer_shutdown.py, test_bgp_update_timer.py: Remove redundant
  delete_running_config teardown that is no longer needed.
Update test plans to reflect that config suppress-fib-pending requires a
config reload to become operational.

- Add a general note that changing the suppress-fib-pending configuration
  requires a reload to take effect.
- Remove Test case sonic-net#4 (BGP route suppress in multiple VRF scenario), which
  was not implemented in the test script.
- Renumber subsequent test cases accordingly.
- Redesign Test case sonic-net#6 (stress): enable suppress once, flap routes to
  stress the pipeline, suspend orchagent, verify queued/blackholing, then
  restore orchagent and verify offloaded state and traffic forwarding.
- Update T2-Chassis testplan to match T1 testplan changes.
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

… block

When orch_northbond_route_zmq_enabled is true, fpmsyncd uses ZMQ instead
of TCP to communicate with orchagent. Pausing orchagent (SIGSTOP) blocks
ZMQ send, which causes fpmsyncd to time out and prevents route updates
from being received - breaking tests that intentionally stop orchagent to
simulate route install delays.

Add a module-scoped autouse fixture disable_zmq_for_fib_suppress that:
- Reads the current orch_northbond_route_zmq_enabled value from CONFIG_DB
- If enabled, sets it to false and reloads the config
- Restores the original value after the test module completes

Also add the resulting ZMQ error to the loganalyzer ignore list so that
tests do not fail due to spurious ZMQ errors on devices where ZMQ is not
active.
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

ZhaohuiS
ZhaohuiS previously approved these changes May 19, 2026
Copy link
Copy Markdown
Contributor

@ZhaohuiS ZhaohuiS left a comment

Choose a reason for hiding this comment

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

Solid rework -- the state-check-before-reload optimization in config_bgp_suppress_fib avoids unnecessary reloads, the ZMQ disable fixture correctly targets the fpmsyncd<->orchagent path that breaks when orchagent is paused, and the module-scoped restore iterating all DUTs handles the multi-DUT T2 topology properly. The LLDP wait_until(90s) after reload is a good safeguard.

…ib_stress

config_bgp_suppress_fib:
- Add idempotency: skip CLI change and reload if CONFIG_DB state already
  matches the desired state.
- Add config save and config reload (with wait_for_bgp) after CLI change
  so the setting becomes operational.
- Restore multi-ASIC validation loop in the validate_result block.

restore_bgp_suppress_fib fixture:
- Change to scope="module", autouse=True to run once per module and avoid
  repeated expensive reloads.
- Iterate over all DUTs (duthosts) so that both upstream and downstream
  hosts are covered in multi-DUT topologies.
- Assert on rc != 0 instead of silently assuming enabled state.

test_suppress_fib_stress:
- Remove ptfhost and tcpdump_helper dependencies.
- Enable suppress-fib-pending once at the start (module fixture restores).
- Flow: route flap stress -> suspend orchagent -> announce routes ->
  verify QUEUED state and blackholing -> restore orchagent ->
  verify OFFLOADED state and traffic forwarded to T0.
- Use validate_route_states and validate_traffic helpers consistent with
  other tests in the module.

test_credit_loop:
- Remove stale "Restore orchagent process" step; config_bgp_suppress_fib
  now performs a config_reload which implicitly restarts orchagent.

test_bgp_route_with_suppress:
- Remove redundant config save after config_bgp_suppress_fib calls.
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

1 similar comment
@mssonicbld
Copy link
Copy Markdown
Collaborator

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 1 pipeline(s).

@mike-dubrovsky
Copy link
Copy Markdown
Contributor Author

@ZhaohuiS @yxieca
Hi folks, could you please re-approve again ? I added small nits to harden the script.

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

Labels

None yet

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants