netdev - Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour for namespaces in tdc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMDBHYJdeh_AO-FEunTuTNVFAEtixuniq1b6vRqa_oS_Ru5wjg@mail.gmail.com>
Date:   Sun, 16 Jun 2019 22:04:50 -0400
From:   Lucas Bates <lucasb@...atatu.com>
To:     Nicolas Dichtel <nicolas.dichtel@...nd.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...atatu.com>, kernel@...atatu.com,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Marcelo Ricardo Leitner <mleitner@...hat.com>,
        Vlad Buslov <vladbu@...lanox.com>,
        Davide Caratti <dcaratti@...hat.com>
Subject: Re: [RFC PATCH net-next 1/1] tc-testing: Restore original behaviour
 for namespaces in tdc

On Fri, Jun 14, 2019 at 5:37 AM Nicolas Dichtel
<nicolas.dichtel@...nd.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