netdev - Re: [RFC net-next 1/1] tdc.py: Introduce required plugins

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: <10a8c95a-7285-9918-c607-67ed827a9959@6wind.com>
Date:   Fri, 12 Apr 2019 10:31:20 +0200
From:   Nicolas Dichtel <nicolas.dichtel@...nd.com>
To:     Lucas Bates <lucasb@...atatu.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        David Miller <davem@...emloft.net>,
        Jamal Hadi Salim <jhs@...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>, kernel@...atatu.com
Subject: Re: [RFC net-next 1/1] tdc.py: Introduce required plugins

Le 11/04/2019 à 22:54, Lucas Bates a écrit :
> On Wed, Apr 10, 2019 at 10:33 AM Nicolas Dichtel
> <nicolas.dichtel@...nd.com> wrote:
>>
>> Le 09/04/2019 à 23:44, Lucas Bates a écrit :
>>> Some of the testcases (for example, all of the fw tests) in tdc
>>> require activating the nsplugin. This RFC introduces a feature which
>>> tags one such test with the keyword "requires". Anyone running a test
>>> that requires nsplugin will now get a warning if they are missing
>>> the plugin.
>>>
>>> After compiling the list of test cases to execute, tdc will
>>> gather all of the required plugins for that run, and validate
>>> that they have been enabled with a symlink in the plugins/
>>> directory. If required plugins are missing, tdc will create the
>>> symlink for the user and then terminate. (This is because plugin-
>>> specific options may exist and need to be parsed at startup)
>> I still don't understand the goal of this plugin. Why not simply include it in
>> the core code (like it was some times ago)?
> 
> Because the use of namespaces and the veth pair are specific to some
> test cases. We're also making it possible to create complex test cases
> which will require more than one namespace.  The starting point was to
> isolate the namespace into its own plugin.  We had a discussion today
Isolating a part of the code in a plugin is not the problem. The problem is that
this code is not available by default.

> in our tri-weekly tc test meeting and the general consensus to address
> what you brought up is leaning towards the following:
> 
> - adding a symlink to nsPlugin
I don't understand why a symlink is needed. Just load it by default and use it
when needed. A property can be added to each test to tell which plugin (in fact,
which topology) is needed to run it.
Thus, if a new complex test is added, it can define another topology.

> - Changing default behaviour so that unless an option is explicitly
> specified, all the tests will be run under a namespace with automatic
> creation of the ports
Yes.

> - If the user chooses /not/ to use namespaces, it will still create
> the veth pair to use.
In fact, I would say an option so that the user can choose another topology.

> 
> I'll send the patch tomorrow as another RFC.
> 
> What do you think of this?
> 
>>> Please provide feedback.  If this is amenable to all, I will proceed
>>> to submit for the rest.
>> After your patch, I got the following error:
>> $ ./tdc.py
>> Traceback (most recent call last):
>>   File "./tdc.py", line 740, in <module>
>>     main()
>>   File "./tdc.py", line 734, in main
>>     set_operation_mode(pm, args)
>>   File "./tdc.py", line 692, in set_operation_mode
>>     check_required_plugins(pm, alltests)
>>   File "./tdc.py", line 583, in check_required_plugins
>>     os.chown('plugins/{}'.format(fname), uid=int(os.getenv('SUDO_UID')),
>> TypeError: int() argument must be a string or a number, not 'NoneType'
> 
> That would be expected if you aren't running tdc with sudo or as root
> or as a user with network admin capability.
I was root for the test.


Regards,
Nicolas