-
Notifications
You must be signed in to change notification settings - Fork 197
Update Static GUE decap README.md with IPv6 transport
#4958
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
base: main
Are you sure you want to change the base?
Conversation
Replaces v4 transport with v6 transport.
Summary of ChangesHello @sachendras, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request updates the documentation and configuration examples for static GUEv1 decapsulation tests to transition from using IPv4 transport to IPv6 transport. The changes ensure that the test descriptions, network configurations, and policy forwarding rules accurately reflect an IPv6-based GUE tunnel setup, providing clear guidance for testing GUE decapsulation with IPv6 outer headers. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request updates the README.md for the static GUE decapsulation test to use IPv6 for the outer transport layer, replacing the previous IPv4 transport. While the changes in the documentation are mostly consistent with this goal, there is a major discrepancy I've identified. The associated test file, ip_guev1_static_decap_subnet_range_test.go, appears to be hardcoded to use IPv4 for the outer transport header in all test cases. This means the updated documentation will not match the actual test implementation. This is a critical issue that should be addressed, either by updating the test code in this PR or clarifying the intent. I have also pointed out a minor inconsistency within the README itself.
| ## Summary | ||
|
|
||
| This is to test the functionality of decapsulation of static GUEv1 to IPv4 or IPv6 payload ([GUE Variant 1](https://datatracker.ietf.org/doc/html/draft-ietf-intarea-gue-09#section-4)). These tests verify the use case of IPv4 and IPv6 encapsulated traffic in IPv4 GUE tunnel. | ||
| This is to test the functionality of decapsulation of static GUEv1 to IPv4 or IPv6 payload ([GUE Variant 1](https://datatracker.ietf.org/doc/html/draft-ietf-intarea-gue-09#section-4)). These tests verify the use case of IPv4 and IPv6 encapsulated traffic in an IPv6 GUE 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.
This README is being updated to describe a test with an IPv6 outer transport for GUE. However, the corresponding test implementation in ip_guev1_static_decap_subnet_range_test.go seems to be hardcoded to use an IPv4 outer header for all traffic flows (see configureIPv4Traffic and configureIPv6Traffic functions). This creates a significant divergence between the documentation (which serves as the test plan) and the implementation. Please either update the test code to match this new documentation or revert the documentation changes.
| - The outer header destination IP of the traffic falls within the DECAP-DST-SUBNET-V6/64 range | ||
| - The outer header destination UDP port (6085) of the traffic is not configured for decapsulation, therefore it does not match the decapsulation criteria. | ||
| - The DUT should not decapsulate these packets. Packets with `DECAP-DST_3/32` will be sent to the device controller/CPU while the packets with DECAP-DST_1/32, DECAP-DST_2/32 and DECAP-DST_4/32 will be dropped due to specific no-route to the destination present in local FIB | ||
| - The DUT should not decapsulate these packets. Packets with `DECAP-DST_3-V6/128` will be sent to the device controller/CPU while the packets with DECAP-DST_1/32, DECAP-DST_2/32 and DECAP-DST_4/32 will be dropped due to specific no-route to the destination present in local FIB |
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.
There's an inconsistency in this line. While DECAP-DST_3/32 was correctly updated to its IPv6 equivalent DECAP-DST_3-V6/128, the other prefixes (DECAP-DST_1/32, DECAP-DST_2/32, and DECAP-DST_4/32) were not. For consistency with the rest of the document's update to IPv6, they should also be changed to their IPv6 versions.
| - The DUT should not decapsulate these packets. Packets with `DECAP-DST_3-V6/128` will be sent to the device controller/CPU while the packets with DECAP-DST_1/32, DECAP-DST_2/32 and DECAP-DST_4/32 will be dropped due to specific no-route to the destination present in local FIB | |
| - The DUT should not decapsulate these packets. Packets with `DECAP-DST_3-V6/128` will be sent to the device controller/CPU while the packets with `DECAP-DST_1-V6/128`, `DECAP-DST_2-V6/128` and `DECAP-DST_4-V6/128` will be dropped due to specific no-route to the destination present in local FIB |
Pull Request Test Coverage Report for Build 20481237671Details
💛 - Coveralls |
Replaces v4 transport with v6 transport.