On Fri, Apr 12, 2019 at 11:42 AM Nicolas Dichtel
<nicolas.dich...@6wind.com> wrote:
>
> Le 12/04/2019 à 17:21, Lucas Bates a écrit :
> > On Fri, Apr 12, 2019 at 4:31 AM Nicolas Dichtel
> > <nicolas.dich...@6wind.com> wrote:
> >>> 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.
> >
> > I'm loathe to hard-code it into the script, but we can avoid the
> > symlink if we specify some default plugins to load in the
> > tdc_config.py file.
> Why is it a problem to specify the plugin in the test description? The fw
> tests
> depend on a specific topology, thus it's better explicitly state that.
So something like this? Note the usage of the keyword "requires",
{
"id": "901f",
"name": "Add fw filter with prio at 32-bit maxixum",
"category": [
"filter",
"fw"
],
"requires": ["nsPlugin"],
"setup": [
"$TC qdisc add dev $DEV1 ingress"
],
"cmdUnderTest": "$TC filter add dev $DEV1 parent ffff: handle
1 prio 65535 fw action ok",
"expExitCode": "0",
"verifyCmd": "$TC filter get dev $DEV1 parent ffff: handle 1
prio 65535 protocol all fw",
"matchPattern": "pref 65535 fw.*handle 0x1.*gact action pass",
"matchCount": "1",
"teardown": [
"$TC qdisc del dev $DEV1 ingress"
]
},
> >
> >>> - 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.
> >
> > Quite possibly, but for traffic generation (upcoming using scapy)
> > we're just reusing the simple topology right now.
> I was thinking you were arguing for this option. If nobody need it, let's just
> remove it.
>
> >
> >>>> 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.
> >
> > Ah. That environment variable is probably not present on your system.
> > Out of curiosity, what shell/distribution are you using?
> >
> It was on a debian 8.11 / bash.
>
Thanks. I'll look into it later.
