openconfig / featureprofiles

Feature Profiles are groups of OpenConfig paths and tests which verify their behavior
Apache License 2.0
53 stars 150 forks source link

Issues with RT-5.1 test (Singleton Interface) #383

Closed mojiiba closed 1 year ago

mojiiba commented 2 years ago

Hi, There are two issue with RT-5.1. Can u please help to resolve the issues.

Screen Shot 2022-08-04 at 11 58 03 AM

To fix this issue, either we should have a threshold (+3) to account for the control packets or change the test logic to read the number of packets in a way to account for the control packets. I think ATE statistics at the port level account for control packets.

earies commented 2 years ago

@dplore @bstoll - I would suggest if any IP (or upper layer) MTU is being altered that a prerequisite would be to alter the interface MTU otherwise IP MTUs cannot be set. We should not introduce any internal implicit behaviors of treatment to the interface MTU if only an IP MTU is set but rather this should be explicit intent.

It is likely the YANG node descriptions for the mtu leafs should be updated to reflect this behavioral expectation as well

liulk commented 2 years ago

We did have a deviation for omitting/setting the L2 MTU some point in the past. I agree that devices that don't allow setting the L2 MTU is not fully compliant.

liulk commented 2 years ago

While #412 is intended to rectify a more precise language for the deviation, I want to explain how I interpret the OpenConfig MTU spec here.

So if we have these scenarios, this is how I interpret the OpenConfig compliance:

Is this something we can agree on?

dplore commented 2 years ago

IMO, the test should be modified to set both the L2 and L3 MTU to appropriate, compatible values. All compliant devices should allow setting of both these MTUs.

Agreed that scenario/device B scenario is compliant to the test requirements as they appear now. It is implementing behavior that is not prohibited and not specified by OpenConfig.

liulk commented 2 years ago

Hi Darren, if you could approve #412, then I'll send a follow up PR to add setting of L2 MTU as a deviation.

dplore commented 2 years ago

Done. IMO, L2 MTU should be the standard and if useful, we could add a deviation to disable of setting the L2 MTU.

If this doesn't sound right to you, we can discuss offline and see if/why it makes sense.

liulk commented 2 years ago

Sorry, I mean add the setting of L2 MTU, but add a deviation to omit the setting of L2 MTU.

liulk commented 2 years ago

Re: the second issue above "The test assumes that the number of output unicast packets from ATE that is captured at the flow level should be equal or larger than the number of inbound unicast packets"

Upon looking at this more closely, I agree that the direction of the less-than is an error. Fortunately, we've always had the ATE and DUT packet counters be equal at least for the unicast packets because I think the control packets tend to be broadcast, so that's why we didn't observe any test failures.

agvarghe-cisco commented 2 years ago

Still seeing the issue , ipv6 control packets is still not accounted for

singleton_test.go:356: ATE received too few destination packets: got 260906, want >= 260909
liulk commented 2 years ago

Are you sure this is not packet loss?

mojiiba commented 2 years ago

Yes, we are sure. If you notice the difference is only three packets (max six packets depending on the execution time of the test). We did the packet capture and the differences matches with the control packets (ipv6 neighborliness advertisement generated by ixia). We also do not see this issue when we do not configure ipv6 (we could pass the test for ipv4 cases).