fix for 498147479 PF-1.4 ip_guev1_static_decap_subnet_range#5437
fix for 498147479 PF-1.4 ip_guev1_static_decap_subnet_range#5437mihaitomaro wants to merge 3 commits into
Conversation
…psulation/otg_tests/ip_guev1_static_decap_subnet_range)
Summary of ChangesHello, 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 addresses a test failure in the GUE decapsulation suite by introducing logic to distinguish between expected decapsulation and pass-through behavior. It enhances the test coverage for PF-1.4 scenarios by adding a dedicated verification helper for pass-through packets and correcting an issue in how DSCP values were parsed during packet inspection. Highlights
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 the 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 counterproductive. 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. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces GUE pass-through verification logic to the OTG tests, allowing the test suite to validate that outer packets remain unmodified (except for TTL decrement) when decapsulation is not performed. The review feedback identifies a regression in IPv6 DSCP verification where a necessary bit shift was removed, and suggests refactoring the new verification helper to include UDP port validation and improved parsing efficiency by moving IP parsing outside the packet loop.
| verifyCaptureDscpTtlValue(t, ate, "port2", int(innerDscpValue), int(innerTTL-1)) | ||
| } else { | ||
| // PF-1.4.5 / PF-1.4.6: pass-through — verify outer packet unmodified (no decap) | ||
| verifyPassThroughGuePacket(t, ate, "port2") |
There was a problem hiding this comment.
Pass the expected UDP port to verifyPassThroughGuePacket to ensure more robust verification of the GUE packet. This aligns with the practice of using parameterized functions to improve code maintainability and reuse.
| verifyPassThroughGuePacket(t, ate, "port2") | |
| verifyPassThroughGuePacket(t, ate, "port2", ateUdpPort) |
References
- Refactor functions to use parameterized versions to reduce code duplication.
| verifyCaptureDscpTtlValue(t, ate, "port2", int(innerDscpValue), int(innerTTL-1)) | ||
| } else { | ||
| // PF-1.4.5 / PF-1.4.6: pass-through — verify outer packet unmodified (no decap) | ||
| verifyPassThroughGuePacket(t, ate, "port2") |
There was a problem hiding this comment.
Pass the expected UDP port to verifyPassThroughGuePacket to ensure more robust verification of the GUE packet. This aligns with the practice of using parameterized functions to improve code maintainability and reuse.
| verifyPassThroughGuePacket(t, ate, "port2") | |
| verifyPassThroughGuePacket(t, ate, "port2", ateUdpPort) |
References
- Refactor functions to use parameterized versions to reduce code duplication.
| ip6, _ := ip6Layer.(*layers.IPv6) | ||
| if ip6.SrcIP.Equal(net.ParseIP(ipv6Src)) { | ||
| dscpValue := ip6.TrafficClass >> 2 | ||
| dscpValue := ip6.TrafficClass |
There was a problem hiding this comment.
The removal of the bit shift (>> 2) for the IPv6 Traffic Class creates an inconsistency with the IPv4 verification logic (line 615) and deviates from the standard definition of DSCP (the 6 most significant bits). This change likely masks an issue in configureIPv6Traffic (line 593) where the raw Traffic Class byte is set instead of the DSCP value. It is recommended to update configureIPv6Traffic to use the DSCP setter (e.g., .Dscp().Phb().SetValue(innerDscpValue)) and revert this change to maintain consistent and correct DSCP verification.
| func verifyPassThroughGuePacket(t *testing.T, ate *ondatra.ATEDevice, port string) { | ||
| pcapfilename := processCapture(t, ate, port) | ||
| handle, err := pcap.OpenOffline(pcapfilename) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| defer handle.Close() | ||
| packetSource := gopacket.NewPacketSource(handle, handle.LinkType()) | ||
| for packet := range packetSource.Packets() { | ||
| if ip6Layer := packet.Layer(layers.LayerTypeIPv6); ip6Layer != nil { | ||
| ip6, _ := ip6Layer.(*layers.IPv6) | ||
| if ip6.SrcIP.Equal(net.ParseIP(atePort1.IPv6)) { | ||
| expectedTTL := uint8(outerTTL - 1) | ||
| if ip6.HopLimit == expectedTTL { | ||
| t.Logf("PASS: GUE pass-through packet verified: outer src=%s, TTL=%d (decremented by 1)", atePort1.IPv6, ip6.HopLimit) | ||
| return | ||
| } | ||
| t.Fatalf("ERROR: Outer TTL mismatch in pass-through. Expected: %d, Got: %d", expectedTTL, ip6.HopLimit) | ||
| } | ||
| } | ||
| } | ||
| t.Fatalf("ERROR: Could not find GUE pass-through packet with outer src IP (%s) in capture", atePort1.IPv6) | ||
| } |
There was a problem hiding this comment.
Refactor verifyPassThroughGuePacket to include UDP port verification and improve efficiency by parsing the IP address outside the loop. Using parameters for values like the expected port improves the function's maintainability.
func verifyPassThroughGuePacket(t *testing.T, ate *ondatra.ATEDevice, port string, expectedPort int) {
pcapfilename := processCapture(t, ate, port)
handle, err := pcap.OpenOffline(pcapfilename)
if err != nil {
t.Fatal(err)
}
defer handle.Close()
packetSource := gopacket.NewPacketSource(handle, handle.LinkType())
srcIP := net.ParseIP(atePort1.IPv6)
for packet := range packetSource.Packets() {
if ip6Layer := packet.Layer(layers.LayerTypeIPv6); ip6Layer != nil {
ip6, _ := ip6Layer.(*layers.IPv6)
if ip6.SrcIP.Equal(srcIP) {
if udpLayer := packet.Layer(layers.LayerTypeUDP); udpLayer != nil {
udp, _ := udpLayer.(*layers.UDP)
if int(udp.DstPort) == expectedPort {
expectedTTL := uint8(outerTTL - 1)
if ip6.HopLimit == expectedTTL {
t.Logf("PASS: GUE pass-through packet verified: outer src=%s, port=%d, TTL=%d (decremented by 1)", atePort1.IPv6, expectedPort, ip6.HopLimit)
return
}
t.Fatalf("ERROR: Outer TTL mismatch in pass-through. Expected: %d, Got: %d", expectedTTL, ip6.HopLimit)
}
}
}
}
}
t.Fatalf("ERROR: Could not find GUE pass-through packet with outer src IP (%s) and UDP port (%d) in capture", atePort1.IPv6, expectedPort)
}References
- Refactor functions to use parameterized versions to reduce code duplication.
2 participants
fix for 498147479 (Arista Failing FNT: feature/policy_forwarding/decapsulation/otg_tests/ip_guev1_static_decap_subnet_range)
PF-1.4
https://github.com/open-traffic-generator/featureprofiles/blob/fix498147479/feature/policy_forwarding/decapsulation/otg_tests/ip_guev1_static_decap_subnet_range/README.md