Adding AFT Base cases #3506

sudhinj · 2024-10-10T04:35:09Z

This includes the test cases for validating AFT Base path.

OpenConfigBot · 2024-10-10T04:35:24Z

Pull Request Functional Test Report for #3506 / `a08b48f`

No tests identified for validation.

Help

coveralls · 2024-10-10T04:38:35Z

Pull Request Test Coverage Report for Build 11360029682

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 55.268%

Totals
Change from base Build 11359443484:	0.0%
Covered Lines:	1983
Relevant Lines:	3588

💛 - Coveralls

dplore · 2024-10-10T21:27:27Z

@ElodinLaarz can you review please?

ElodinLaarz

Did a first pass on the READMEs. Thanks for writing these up :)

feature/aft/aft_base/otg_tests/aft_base/README.md

ElodinLaarz · 2024-10-14T21:36:45Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+*   IS-IS must be level 2 only with wide metric.
+*   IS-IS must be point to point.
+*   Send 1000 ipv4 and 1000 ipv6 IS-IS prefixes from ATE:port3 to DUT:port3.
+*   IS-IS must be up and running, prefixes must be in RIB and FIB.


What is the proposed method of confirming the values are in the RIB and FIB using the provided paths?

(Same question in the other README.)

The README should be precise in how the validation is performed. (if the go test code was included, we could use the code to review the most specific options. You are welcome to include the code and the readme in the same PR).

For example, we use gnmi subscribe with ON_CHANGE option. So the subscription should start before advertising prefixes.

Suggested change

* IS-IS must be up and running, prefixes must be in RIB and FIB.

* Use gnmi Subscribe with ON_CHANGE option to /network-instances/network-instance/afts

* Advertise 1,000 ipv4,ipv6 prefixes from ATE port1, port2

* Verify afts prefix entries using the folllowing paths within a timeout period of 10 seconds

/network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/prefix
/network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/origin-protocol
/network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/prefix
/network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/origin-protocol
/network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/counters

Hi, I don't see the recommended paths to be verified in this section as requested?

/network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/prefix /network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/origin-protocol /network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/prefix /network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/origin-protocol

ElodinLaarz · 2024-10-14T21:38:21Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+*   Advertise 1000 ipv4,ipv6 prefixes from ATE port1,port2 observe received prefixes at DUT.
+*   Validate total number of entries of AFT for IPv4 and IPv6.
+*   Each prefix must have 2 next hops pointing to ATE port1,port2.
+*   Advertise 100 ipv4,ipv6 from ATE port3 observe received prefixes at DUT.


What is the hope here? Are we testing gNMI updates here? Should we be concurrently handling a gRPC stream with a subscription (ON_CHANGE, for example) to make sure we get these 100 prefixes in addition to the previous 1,000?

Or should this be in the next section where we would like these to be the destination at the end of the tunnels?

Can you explicitly list out which of the options I suggested you are doing here?

ElodinLaarz · 2024-10-14T21:46:21Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+
+  /network-instances/network-instance/afts/ethernet/mac-entry/state/next-hop-group:
+  /network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/next-hop-group:
+  /network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/origin-protocol:


Maybe add a little comment to the verification where we check the origin-protocol for the advertised prefixes, if we're including it here?

added verifications pointing towards this path.

ElodinLaarz · 2024-10-14T21:46:50Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+  /network-instances/network-instance/afts/next-hops/next-hop/state/ip-address:
+  /network-instances/network-instance/afts/next-hops/next-hop/state/mac-address:
+  /network-instances/network-instance/afts/next-hops/next-hop/state/origin-protocol:
+  /network-instances/network-instance/afts/state-synced/state/ipv4-unicast:


Do we call out these state-synced leaves as being tested here?

Kindly clarify

I did not see which test above checked this path. Could you mention how it's being used?

ElodinLaarz · 2024-10-14T21:47:38Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+  /network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/entry-metadata:
+  /network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/next-hop-group-network-instance:
+  /network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/origin-network-instance:
+  /network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/prefix:


Again, these non-state paths are not needed, as the telemetry verification occurs on the already present state paths /network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/prefix, etc.

didnt follow please clarify

Mentioned in the other comment, but since this is not a state path it can be removed.

ElodinLaarz · 2024-10-14T21:49:18Z

feature/aft/aft_base/otg_tests/aft_prefixcounters/README.md

+*   IS-IS must be level 2 only with wide metric.
+*   IS-IS must be point to point.
+*   Send 1000 ipv4 and 1000 ipv6 IS-IS prefixes from ATE:port1 to DUT:port1.
+*   IS-IS must be up and running, prefixes must be in RIB and FIB.


Same question-- what is the intended means to verify this you were thinking?

It still says the same as before?
* IS-IS must be up and running, prefixes must be in RIB and FIB. (which is not what we want to say here.)
But I do see you updated the other README. Can we update this one as well?

ElodinLaarz · 2024-10-14T21:51:39Z

feature/aft/aft_base/otg_tests/aft_prefixcounters/README.md

+
+## AFT-2.1.1: AFT Prefix Counters ipv4 packets forwarded, IS-IS route.
+
+From ATE:port2 send 10000 pps to one of the ipv4 prefix advertise by IS-IS, after 1 minute stop the traffic.


Potentially, we can replace this "1 minute of traffic" with a certain number of packets sent (e.g. send 100k, etc.)? This can save time in the test and make it a bit more consistent?

ElodinLaarz · 2024-10-14T21:53:34Z

feature/aft/aft_base/otg_tests/aft_prefixcounters/README.md

+*   IS-IS routes advertised from ATE:port1 must have one next hop.
+
+
+## AFT-2.1.1: AFT Prefix Counters ipv4 packets forwarded, IS-IS route.


Minor, optional comment-- since you're sending packets and then waiting a bunch for the packets to be sent, to minimize runtime of the test, you can group the checks (check packets and octets in the same test), and send a fixed number of packets, instead of waiting for a fixed amount of time.

Then, you can subscribe to the data and make sure it updates within a prescribed amount of time after the packets have been sent.

for debugging purpose and avoid complication. I want to keep it as 2 different cases.

Sure, that is okay if we replace the sleep/wait with sending a fixed number of packets, I think.

dplore · 2024-10-14T22:07:22Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+
+IPv4/IPv6 unicast routes next hop group and next hop.
+
+## Procedure


Name this heading ## Test setup`` and add a sub-heading for ### Generate DUT configuration`. Place the description of the DUT and ATE configurations you want generated there.

The test procedure should have a gnmi.Set with REPLACE option to push the config to the DUT.

(The ATE config push can be in the test setup or in the test procedure, whatever works best for your use case)

dplore · 2024-10-16T02:15:32Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+*   IS-IS must be level 2 only with wide metric.
+*   IS-IS must be point to point.
+*   Send 1000 ipv4 and 1000 ipv6 IS-IS prefixes from ATE:port3 to DUT:port3.
+*   IS-IS must be up and running, prefixes must be in RIB and FIB.


The README should be precise in how the validation is performed. (if the go test code was included, we could use the code to review the most specific options. You are welcome to include the code and the readme in the same PR).

For example, we use gnmi subscribe with ON_CHANGE option. So the subscription should start before advertising prefixes.

Suggested change

* IS-IS must be up and running, prefixes must be in RIB and FIB.

* Use gnmi Subscribe with ON_CHANGE option to /network-instances/network-instance/afts

* Advertise 1,000 ipv4,ipv6 prefixes from ATE port1, port2

* Verify afts prefix entries using the folllowing paths within a timeout period of 10 seconds

/network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/prefix
/network-instances/network-instance/afts/ipv4-unicast/ipv4-entry/state/origin-protocol
/network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/prefix
/network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/origin-protocol
/network-instances/network-instance/afts/ipv6-unicast/ipv6-entry/state/counters

dplore · 2024-10-16T02:20:48Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+IPv4/IPv6 unicast routes next hop group and next hop.
+
+## Procedure
+


Since you have tests here, declare a subtest heading like:

## AFT-1.1.1: AFT ipv4/6 unicast entries

dplore · 2024-10-16T02:20:49Z

feature/aft/aft_base/otg_tests/aft_base/README.md

+IPv4/IPv6 unicast routes next hop group and next hop.
+
+## Procedure
+


Since you have tests here, declare a subtest heading like:

## AFT-1.1.1: AFT ipv4/6 unicast entries

didnt follow, it is separated isnt it?

sudhinj added 7 commits October 7, 2024 06:15

adding a new id by sudhin

cff9734

adding aft

e755346

removing files

e66be74

adding readme

f1c90b7

adding test cases

d6f52a7

modify

be46d35

Merge branch 'openconfig:main' into main

d7d7481

sudhinj requested a review from a team as a code owner October 10, 2024 04:35

sudhinj added 3 commits October 10, 2024 04:43

removing faulty oc

6f1c043

adding test cases for prefix counters

ce1275b

adding test case numbers

910c66b

dplore assigned ElodinLaarz Oct 10, 2024

ElodinLaarz reviewed Oct 14, 2024

View reviewed changes

dplore reviewed Oct 16, 2024

View reviewed changes

sudhinj added 3 commits October 16, 2024 05:27

updating the changes

5f47ede

adding comments for pull request

4408124

Merge branch 'main' into main

a08b48f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding AFT Base cases #3506

Adding AFT Base cases #3506

sudhinj commented Oct 10, 2024

OpenConfigBot commented Oct 10, 2024 •

edited

Loading

coveralls commented Oct 10, 2024 •

edited

Loading

dplore commented Oct 10, 2024

ElodinLaarz left a comment

ElodinLaarz Oct 14, 2024

dplore Oct 16, 2024

sudhinj Oct 16, 2024

dplore Oct 17, 2024

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024 •

edited

Loading

ElodinLaarz Oct 17, 2024

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024

ElodinLaarz Oct 17, 2024

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024 •

edited

Loading

ElodinLaarz Oct 17, 2024

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024

ElodinLaarz Oct 17, 2024

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024 •

edited

Loading

ElodinLaarz Oct 14, 2024

sudhinj Oct 16, 2024 •

edited

Loading

ElodinLaarz Oct 17, 2024

dplore Oct 14, 2024

sudhinj Oct 16, 2024

dplore Oct 16, 2024

dplore Oct 16, 2024

sudhinj Oct 16, 2024

dplore Oct 16, 2024

sudhinj Oct 16, 2024

-*   IS-IS must be up and running, prefixes must be in RIB and FIB.
+* Use gnmi Subscribe with ON_CHANGE option to /network-instances/network-instance/afts
+* Advertise 1,000 ipv4,ipv6 prefixes from ATE port1, port2
+* Verify afts prefix entries using the folllowing paths within a timeout period of 10 seconds


		## AFT-2.1.1: AFT Prefix Counters ipv4 packets forwarded, IS-IS route.

		From ATE:port2 send 10000 pps to one of the ipv4 prefix advertise by IS-IS, after 1 minute stop the traffic.

		* IS-IS routes advertised from ATE:port1 must have one next hop.


		## AFT-2.1.1: AFT Prefix Counters ipv4 packets forwarded, IS-IS route.


		IPv4/IPv6 unicast routes next hop group and next hop.

		## Procedure

Adding AFT Base cases #3506

Are you sure you want to change the base?

Adding AFT Base cases #3506

Conversation

sudhinj commented Oct 10, 2024

OpenConfigBot commented Oct 10, 2024 • edited Loading

Pull Request Functional Test Report for #3506 / a08b48f

coveralls commented Oct 10, 2024 • edited Loading

Pull Request Test Coverage Report for Build 11360029682

Details

💛 - Coveralls

dplore commented Oct 10, 2024

ElodinLaarz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudhinj Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudhinj Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudhinj Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sudhinj Oct 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OpenConfigBot commented Oct 10, 2024 •

edited

Loading

Pull Request Functional Test Report for #3506 / `a08b48f`

coveralls commented Oct 10, 2024 •

edited

Loading

sudhinj Oct 16, 2024 •

edited

Loading

sudhinj Oct 16, 2024 •

edited

Loading

sudhinj Oct 16, 2024 •

edited

Loading

sudhinj Oct 16, 2024 •

edited

Loading