-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
README PF-1.3: Policy-based Static GUE Encapsulation to IPv4 tunnel #3482
base: main
Are you sure you want to change the base?
Conversation
danameme
commented
Sep 30, 2024
- Add test details for Static GUE Encapsulation.
- Added a new topology for running the tests.
Pull Request Test Coverage Report for Build 11134399863Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Use right nomenclature, use ToS instead of DSCP.
Fix failing check in pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the README @danameme -- PTAL at the comments in-line.
feature/policy_forwarding/encapsulation/otg_tests/static_encap_gue_ipv4/README.md
Outdated
Show resolved
Hide resolved
ATE action: | ||
|
||
* Generate **IPv4 traffic** from ATE:Port1 to random IP addresses in | ||
IPv4-DST-NET using a random combination source addresses at linerate. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a required number of source addresses? (A random combination could mean that the test just picks one source address.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to specify fixed packet count and number of distinct flows.
|
||
Verify: | ||
|
||
* Policy forwarding packet counters matches the packet count of traffic |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we clarify which counter this is expected to be? I assume that it is the matched-packets
path that is defined below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's matched-packets, updated the test cases to reflect this.
generated from ATE:Port1. | ||
* The packet count of traffic sent from ATE:Port1 should be equal to the sum | ||
of all packets egressing DUT ports 2 to 5. | ||
* All packets egressing DUT ports 2 to 5 are GUE encapsulated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be harder to verify if we have DUM
in the topology -- in the scenario that we are using ATE here I think we can do this with ingress accounting or pcap, but otherwise don't we need to pcap on DUM
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the topology to use one device only and we can do ingress accounting on ATE.
of all packets egressing DUT ports 2 to 5. | ||
* All packets egressing DUT ports 2 to 5 are GUE encapsulated. | ||
* ECMP hashing works (equal traffic) over the two LAG interfaces. | ||
* LAG hashing works (equal traffic) over the two Singleton ports. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps just clarify that the 'two Singleton ports' here are the individual ports within LAG1
and LAG2
-- you are not saying that we need to check the ATE-DUT and ATE-DUM interfaces AIUI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, done.
DUT and ATE actions: | ||
|
||
* Re-configure the IPv4 GUE tunnel on the DUT with ToS value *0x60*. | ||
* Generate **IPv4 traffic** from ATE:Port1 to random IP addresses in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, I think it's worth clarifying the minimum entropy that you expect here. Something like "ensuring that there are at least X unique source-destination pairs".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would specify the number of 5 tuple flows and a fixed number of packets to send from ATE port 1. Then the validation should have some number of packets received on each ATE port 2 interface +/- some tolerance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated to specify fixed packet count and number of distinct flows.
* Set ToS value of *0x80* for all packets. | ||
* Set TTL of the packets to *10*. | ||
|
||
Verify: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ditto re: clarifications from the IPv4 case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are the un-encapsulated packets from ATE:Port1, did you mean to state again in the bullets?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just meant to ensure that we make the same clarifications that were expressed above in this section. It looks like you did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok cool.
feature/policy_forwarding/encapsulation/otg_tests/static_encap_gue_ipv4/README.md
Show resolved
Hide resolved
feature/policy_forwarding/encapsulation/otg_tests/static_encap_gue_ipv4/README.md
Outdated
Show resolved
Hide resolved
feature/policy_forwarding/encapsulation/otg_tests/static_encap_gue_ipv4/README.md
Show resolved
Hide resolved
|
||
```yaml | ||
paths: | ||
# TODO propose new OC paths for GUE encap based on the protocol next hop of a route |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is true, new OC paths will be needed. FYI, they should follow the pattern being established in the /afts tree, but using /policy-forwarding instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, thanks for the pointer. I'll follow this to send proposal for the OC paths.
|
||
* Routes are advertised from ATE:Port2. | ||
* Traffic is generated from ATE:Port1. | ||
* ATE:Port2 is used as the destination port for GUE encapsulated traffic. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
instead, consider ATE port 2 is just the next hop for the DUT. The encapsulated packet can be captured at ATE port 2 and decoded to ensure it has the expected format.
Make decap testing a separate test.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, updated the test verifications. Decap will being done in a separate test.
feature/policy_forwarding/encapsulation/otg_tests/static_encap_gue_ipv4/README.md
Show resolved
Hide resolved
DUT and ATE actions: | ||
|
||
* Re-configure the IPv4 GUE tunnel on the DUT with ToS value *0x60*. | ||
* Generate **IPv4 traffic** from ATE:Port1 to random IP addresses in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would specify the number of 5 tuple flows and a fixed number of packets to send from ATE port 1. Then the validation should have some number of packets received on each ATE port 2 interface +/- some tolerance.
* GUE header TTL is **20**. | ||
* Inner header TTL is **9**. | ||
|
||
### PF-1.3.6: IPv6 traffic GUE encapsulation with explicit TTL configuration on tunnel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, it is ok to say, modify the flows in PF-1.3.5 to using IPv6 and repeat the traffic generation and validation. This is likely how one would write the code for this anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, updated the IPv6 versions to be this way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all of the parameters that we want to specify the same between IPv4 and IPv6? It seems OK to do this as long as we need no modifications and (of course) we assume that IPv4 field names are translated to v6 ones. (I didn't mind the repetition for clarity.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, all the parameters are the same. IPv4 ToS/TTL fields will be translated to IPv6 Traffic Class/Hop Limit respectively. Hope this works, otherwise let me know and I'll repeat with just these two field names being different.
Update README.md
Update and rename ate2_dut5_dum5.testbed to atedut_5.testbed
topologies/ate2_dut5_dum5.testbed
Outdated
@@ -0,0 +1,84 @@ | |||
# proto-file: github.com/openconfig/ondatra/blob/main/proto/testbed.proto |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you need this proto any more, right?
|
||
### Test environment setup | ||
|
||
* DUT and DUM have 5 ports each. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this diagram needs to be updated with the change to have no DUM.
GUE encapsulated. | ||
* ECMP hashing works over the two LAG interfaces with a tolerance of 6%. | ||
* LAG hashing works over the two Singleton ports within LAG1 and LAG2 with a | ||
tolerance of 6%. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How is the 6% arrived at here as a reasonable threshold?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is based on prior testings. However, if there's a different threshold we need to use, please let me know and I'll update accordingly. Thanks.
* Set ToS value of *0x80* for all packets. | ||
* Set TTL of the packets to *10*. | ||
|
||
Verify: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just meant to ensure that we make the same clarifications that were expressed above in this section. It looks like you did.
* GUE header TTL is **20**. | ||
* Inner header TTL is **9**. | ||
|
||
### PF-1.3.6: IPv6 traffic GUE encapsulation with explicit TTL configuration on tunnel |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all of the parameters that we want to specify the same between IPv4 and IPv6? It seems OK to do this as long as we need no modifications and (of course) we assume that IPv4 field names are translated to v6 ones. (I didn't mind the repetition for clarity.)