Support test_drop_counters.py in t0-vpp - #279
Conversation
Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Signed-off-by: AkeelAli <701916+AkeelAli@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
| return 0; | ||
| } | ||
|
|
||
| VLIB_MAIN_LOOP_ENTER_FUNCTION (sonic_ext_drop_member_stats_loop_enter); |
There was a problem hiding this comment.
I think this only enables the feature in the interfaces that are created when the main loop calls this function, which should include all the physical interfaces. We don't need to enable the feature on dynamically created interfaces, such as bvi or lag, right?
There is no way to disable this feature unless we remove sonic-ext plugin, correct? Can you please look at the option to enable/disable a feature using startup.conf? For applications don't care sonic compliant, they can opt out this feature using the config? That removes the code level of dependency to enable/disable through API. If this works, other features in sonic-ext should follow.
There was a problem hiding this comment.
VNET_SW_INTERFACE_ADD_DEL_FUNCTION will enable the feature on dynamically created interfaces including BVI and LAG. It needs to be enabled on all interfaces because of how interface_drop_punt() dispatches the error-drop arc for the whole frame using a single interface's feature config (likely a performance decision). See https://github.com/FDio/vpp/blob/0663bc8dcbd08c671131d61c775cf6e9b790418c/src/vnet/interface_output.c#L1050
A single error-drop frame can batch drops from multiple RX interfaces, but only one of them decides whether the whole frame is sent to our feature node. So if the feature isn't enabled everywhere, LAG/BVI drops batched with other interfaces could be missed.
Regarding the enabling/disabling of the sonic-ext features, I think it's a great idea to do it via config as you suggested and I plan to follow-up on this in a subsequent PR. The performance penalty from visiting our node should be limited for now as it is only incurred in the drop path and the node short-circuits on non-LAG/BVI drops.
3 participants
Summary
Changes to support passing test_drop_counters.py in t0-vpp.
Redesigns the previous solution from #219 by replacing the VPP patch with a plugin-only node to credit drops to the original LAG/VLAN member interface.
Test Results
t1-lag-vpp
sonic-net/sonic-buildimage#29463: sonic-buildimage with current platform/vpp changes to ensure test_drop_counters.py continues to pass in t1-lag-vpp and no regressions are introduced in t1-lag-vpp and t0-vpp.
test_drop_counters.py still passing in t1-lag-vpp:
https://elastictest.org/scheduler/publictestplan/6aa5baafc6c96c8b4c45714b?searchTestCase=test_drop&testcase=drop_packets%2Ftest_drop_counters.py&type=console
t0-vpp
sonic-net/sonic-mgmt#27901: sonic-mgmt with image from above sonic-buildimage and test_drop_counters.py enabled in t0-vpp.
test_drop_counters.py passing in t0-vpp with current change:
https://elastictest.org/scheduler/publictestplan/6aa80a162963ddabcd34ba40?searchTestCase=test_drop_counters.py&testcase=drop_packets%2Ftest_drop_counters.py&type=console