-
Notifications
You must be signed in to change notification settings - Fork 157
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
Adding TE-18.1 sub test cases code #3649
base: main
Are you sure you want to change the base?
Conversation
Pull Request Test Coverage Report for Build 12943332668Warning: 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 |
Added proto-file comment.
Added gRIBI encap header unsupported deviation.
Added gRIBI encap header unsupported.
Pull Request Functional Test Report for #3649 / 9ed4836Virtual Devices
Hardware Devices
|
Added missing colon at description.
Updated with description reflecting README file
// programEntries pushes RIB entries on the DUT required for Encap functionality | ||
func programEntries(t *testing.T, dut *ondatra.DUTDevice, c *gribi.Client) { | ||
t.Log("Programming RIB entries") | ||
// TODO: vvardhanreddy revisit when functionality is added. |
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.
why can't we leave the code here instead of commenting it out?
a failure test case basically indicate that the DUT does not support the expected behavior, right?
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.
Sorry code does not compile due to lack of fluent gribi support for encap, I had predicted fluent gRIBI API's in the commented out code for reference only and the code will be reworked when the functionality is available.
// TODO: b/364961777 upstream GUE decoder to gopacket addition is pending. | ||
// err := validatePacketCapture(t, tcArgs, tc.capturePorts) | ||
clearCapture(t, otg.OTG(), topo) | ||
// if err != nil { |
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.
why we comment these code?
@self-maurya, (putting the business logic aside), can you help quickly check if the code style aligns with our expectation? Thanks. |
@vishnureddybadveli can you put some description in the code about why it's a good idea to put |
} else { | ||
programEntries(t, dut, &c) | ||
} | ||
configDefaultRoute(t, dut, cidr(ipv6EntryPrefix, ipv6EntryPrefixLen), otgPort1.IPv6) |
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.
static route, not a default route, right? (also, nit: add a quick description about why we need such static route would be nice. It's not mentioned in the README.)
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.
what we mean by static route is anything configured explicitly(even default route is a configured one) when compared to routes learnt by protocols like BGP etc
} | ||
} | ||
|
||
func TestMPLSOUDPEncap(t *testing.T) { |
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.
which part of the code implement the PBR of TE-18.1.2?
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.
L226 tries to implement PBR, but action with encap v6 headers is missing which is been worked by our partner team. We will revisit this when functionality is available.
Add TE-18.1.1 MPLS in UDP OTG test for ARISTA.
Add TE-18.1.2 Validate prefix match rule for MPLS in GRE encap using default route.
Vendor/gRIBIGO client support is missing so some parts of code are commented out.