Creating Readme Documentation for AIGP test#5216
Creating Readme Documentation for AIGP test#5216toadeniy-g wants to merge 2 commits intoopenconfig:mainfrom
Conversation
Pull Request Functional Test Report for #5216 / da516a3Virtual Devices
Hardware Devices
|
|
Warning Gemini is experiencing higher than usual traffic and was unable to create the summary. Please try again in a few hours by commenting |
|
/gemini summary |
Summary of ChangesThis pull request introduces a new, thoroughly documented test for AIGP (Accumulated Interior Gateway Protocol) feature support. The test is designed to validate the granular control over AIGP attribute transmission and reception, its modification via policies within both default and non-default network instances, and its behavior in various scenarios including next-hop self and explicit next-hop IP settings. The changes ensure that the new test is properly registered and its purpose and procedures are clearly outlined for future reference and execution. Highlights
Changelog
Activity
|
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive README file for the AIGP feature test (RT-1.36), along with the necessary metadata and test registry entries. The documentation is well-structured and detailed. My review focuses on improving the clarity and correctness of the new README file. I've pointed out several minor typos, grammatical errors, and inconsistencies that should be addressed. Additionally, there is a placeholder for the DUT platform that needs to be filled in. Overall, this is a great addition once these minor issues are resolved.
e2f208d to
42b6028
Compare
| /network-instances/network-instance/table-connections/table-connection/config/address-family: | ||
| /network-instances/network-instance/table-connections/table-connection/config/src-protocol: | ||
| /network-instances/network-instance/table-connections/table-connection/config/dst-protocol: | ||
| # TODO: /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/config/enable-aigp |
There was a problem hiding this comment.
@toadeniy-g - Any reason why we mark this as TODO? this OC coverage seem to be critical
| /network-instances/network-instance/table-connections/table-connection/config/src-protocol: | ||
| /network-instances/network-instance/table-connections/table-connection/config/dst-protocol: | ||
| # TODO: /network-instances/network-instance/protocols/protocol/bgp/neighbors/neighbor/afi-safis/afi-safi/config/enable-aigp | ||
| # TODO: /routing-policy/policy-definitions/policy-definition/statements/statement/actions/bgp-actions/config/set-aigp |
There was a problem hiding this comment.
@toadeniy-g - Any reason why we mark this as TODO? this OC coverage seem to be critical
|
|
||
| ### Test Pass Criteria | ||
|
|
||
| #### Pass criteria on DUT 1 |
There was a problem hiding this comment.
@toadeniy-g - This test is only doing a control plane check of the metric's, but how do we know if the preferred paths are installed for the data plane. We should be validating this using a traffic flow.
There was a problem hiding this comment.
the OTG does not understand the AIGP metric, reference on https://github.com/open-traffic-generator/snappi/blob/main/gosnappi/bgp_route_advanced.go
|
i would suggest dd actual traffic flow verification using the OTG too then intentionally change the AIGP metric on the fly so the "best path" flips to a different physical interface. verify that traffic moves to the new interface with zero loss. this prove hardware is actually listening to the AIGP attribute |
|
would add we xplicitly test the BGP decision process hierarchy too. as Ram correctly pointed out that if both paths have the same AIGP value we arent testing the featur we re testing luck how: scenario a: path 1 (AS-Path 3, AIGP 100) vs path 2 (AS-Path 2, AIGP 200) scenario b: both paths have identical AIGP values. ensure it falls back to the next tie-breaker |
a new test-case (RT-1.36.2) has been added based on this review |
Creating Readme Documentation for AIGP test