On Fri, Jun 14, 2019 at 5:37 AM Nicolas Dichtel <nicolas.dich...@6wind.com> wrote: > > Le 05/06/2019 à 23:08, Lucas Bates a écrit : > > Apologies for the delay in getting this out. I've been busy > > with other things and this change was a little trickier than > > I expected. > > > > This patch restores the original behaviour for tdc prior to the > > introduction of the plugin system, where the network namespace > > functionality was split from the main script. > > > > It introduces the concept of required plugins for testcases, > > and will automatically load any plugin that isn't already > > enabled when said plugin is required by even one testcase. > > > > Additionally, the -n option for the nsPlugin is deprecated > > so the default action is to make use of the namespaces. > > Instead, we introduce -N to not use them, but still create > > the veth pair. > > > > Comments welcome! > Thanks for the follow up. I successfully tested your patch, it fixes the > netns case. Good, thank you for checking it out. I have to add a few more changes to the patch for the BPF-related tests, but once that's done I'll submit the finished version.
> Note that there is still a bunch of test that fails or are skipped after your > patch, for example: > ok 431 e41d - Add 1M flower filters with 10 parallel tc instances # skipped - > Not executed because DEV2 is not defined > The message is not really explicit, you have to dig into the code to > understand > that '-d <dev>' is needed. > Is it not possible to use a dummy interface by default? The tests that make use of DEV2 are intended to be run with a physical NIC. This feature was originally submitted by Chris Mi from Mellanox back in 2017 (commit 31c2611b) to reproduce a kernel panic, with d052 being the first test case submitted. Originally they were silently skipped, but once I added TdcResults.py this changed so they would be tracked and reported as skipped. > From my point of view, if all tests are not successful by default, it scares > users and prevent them to use those tests suite to validate their patches. For me, explicitly telling the user that a test was skipped, and /why/ it was skipped, is far better than excluding the test from the results: I don't want to waste someone's time with troubleshooting the script if they're expecting to see results for those tests when running tdc and nothing appears, or worse yet, stop using it because they think it doesn't work properly. I do believe the skip message should be improved so it better indicates why those tests are being skipped. And the '-d' feature should be documented. How do these changes sound? Thanks, Lucas
