Skip to content

Conversation

@rjarry
Copy link
Collaborator

@rjarry rjarry commented Nov 7, 2025

The previous test architecture was overly complex with two separate network namespaces simulating end hosts connected through routers. This complexity made the test harder to understand and maintain while not providing additional validation of BGP functionality.

Simplify the test to focus on the core requirement: verify that grout can learn BGP routes from an FRR peer and use them for forwarding. The test now uses an iBGP session between grout and a single FRR instance running in a network namespace. The FRR peer advertises a route to its loopback interface which grout must learn and use to successfully ping the remote address.

This reduces the number of interfaces from three to one and eliminates the need for two additional network namespaces and veth pairs.

Summary by CodeRabbit

  • Tests
    • Streamlined BGP routing test: replaced multi-host topology with a single contained BGP peer namespace, moved interface and route setup into that namespace, and removed prior preconfigured host/loopback routing.
    • Simplified readiness and validation: replaced multi-step route-learning checks with a repeated route-presence loop and switched end-to-end validation to an in-namespace ping, improving reliability and speed.

@coderabbitai
Copy link

coderabbitai bot commented Nov 7, 2025

Caution

Review failed

The pull request is closed.

📝 Walkthrough

Walkthrough

The test replaces host-based BGP checks with an isolated FRR+Grout peer in the bgp-peer namespace. FRR is configured via vtysh with router-id 172.16.0.2, neighbor 172.16.0.1 remote-as 64512, and advertises 172.16.0.0/24 and 16.0.0.0/24. Interface x-p0 is assigned 172.16.0.2/24 and FRR loopback 16.0.0.1/24. Grout loopback preconfiguration and host routing are removed. Readiness uses a retry loop checking Grout for the learned 16.0.0.0/24 route (40 attempts, 0.5s sleep). Validation uses Grout grcli ping to 16.0.0.1.

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'smoke: simplify bgp test' directly and clearly summarizes the main change—simplifying the BGP smoke test from a complex multi-namespace setup to a streamlined single-namespace FRR peer configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 15acc17 and 115a792.

📒 Files selected for processing (1)
  • smoke/bgp_frr_test.sh (1 hunks)

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@rjarry rjarry force-pushed the bgp-smoke-fix branch 2 times, most recently from 500ab1c to 19a2a30 Compare November 7, 2025 16:03
@rjarry rjarry force-pushed the bgp-smoke-fix branch 4 times, most recently from bd9bdfd to 8e320b3 Compare November 7, 2025 23:25
@maxime-leroy
Copy link
Collaborator

For vpn srv6 with bgp, it could be useful to have 2 netns to simulate 2 hosts.

The previous test architecture was overly complex with two separate
network namespaces simulating end hosts connected through routers. This
complexity made the test harder to understand and maintain while not
providing additional validation of BGP functionality.

Simplify the test to focus on the core requirement: verify that grout
can learn BGP routes from an FRR peer and use them for forwarding. The
test now uses an iBGP session between grout and a single FRR instance
running in a network namespace. The FRR peer advertises a route to its
loopback interface which grout must learn and use to successfully ping
the remote address.

This reduces the number of interfaces from three to one and eliminates
the need for two additional network namespaces and veth pairs.

Signed-off-by: Robin Jarry <[email protected]>
Reviewed-by: Christophe Fontaine <[email protected]>
@christophefontaine christophefontaine merged commit 4e3e474 into DPDK:main Nov 8, 2025
6 of 7 checks passed
@rjarry rjarry deleted the bgp-smoke-fix branch November 8, 2025 17:01
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.

3 participants