Mon, Aug 17, 2015 at 01:07:52PM IDT, olichtne(a)redhat.com wrote:
Hi,
some comments inline, let me know what you think.
Hi,
Thanks for reviewing!
-Ondrej
On Mon, Aug 17, 2015 at 09:23:38AM +0300, Ido Schimmel wrote:
> The CarrierWait test module allows one to test the link negotiation of
> two interfaces. By changing the administrative state of one interface
> (using the new 'set_carrier_on' / 'set_carrier_off' interface
methods) a
> change in the operational state of the corresponding interface should be
> observed. By performing this test multiple times hardware or firmware
> bugs may be detected.
>
> Signed-off-by: Ido Schimmel <idosch(a)mellanox.com>
> Signed-off-by: Jiri Pirko <jiri(a)mellanox.com>
> ---
> setup.py | 3 ++-
> test_modules/CarrierWait.py | 40 ++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 42 insertions(+), 1 deletion(-)
> create mode 100644 test_modules/CarrierWait.py
>
> diff --git a/setup.py b/setup.py
> index 4033d4a..f5982d1 100755
> --- a/setup.py
> +++ b/setup.py
> @@ -121,7 +121,8 @@ TEST_MODULES = [
> "test_modules/Netperf.py",
> "test_modules/PacketAssert.py",
> "test_modules/PktCounter.py",
> - "test_modules/PktgenTx.py"]
> + "test_modules/PktgenTx.py",
> + "test_modules/CarrierWait.py"]
> )
> ]
>
> diff --git a/test_modules/CarrierWait.py b/test_modules/CarrierWait.py
> new file mode 100644
> index 0000000..1719a44
> --- /dev/null
> +++ b/test_modules/CarrierWait.py
> @@ -0,0 +1,40 @@
> +"""
> +This module defines the carrier wait test
> +"""
> +
> +__author__ = """
> +idosch(a)mellanox.com (Ido Schimmel)
> +"""
> +
> +import logging
> +from lnst.Common.TestsCommon import TestGeneric
> +from lnst.Common.Utils import bool_it
> +from pyroute2 import IPDB
> +
> +
> +class CarrierWait(TestGeneric):
> + def _cb(self, ipdb, msg, action):
> + if action == 'RTM_NEWLINK':
> + self.oper_state = msg.get_attr('IFLA_OPERSTATE', '')
> +
> + def run(self):
> + logging.info('Started CarrierWait...')
> +
> + ip = IPDB()
> + iface = self.get_mopt('iface')
> + self.oper_state = ip.interfaces[iface]['operstate']
> + wd = ip.watchdog(ifname=iface)
> + cuid = ip.register_callback(self._cb)
> +
> + wd.wait(timeout=10)
^^^^^^^^^^
This looks like something that could be optionally configured through a
module option.
I can add that, no problem.
> + ip.unregister_callback(cuid)
> + ip.release()
> +
> + admin_state = 'UP' if bool_it(self.get_mopt('state')) else
'DOWN'
Parsing of mandatory options should be at the start of the test. If a
mandatory option is not defined get_mopt raises an exception
automatically failing the test, so doing anything before that is
useless.
Ha, I wasn't aware of that, but I do see that this is how other test
modules are constructed. Will fix that in V2.
> + oper_state = self.oper_state
> + res_data = {'admin_state': admin_state, 'oper_state':
oper_state}
> +
> + if admin_state == oper_state:
> + self.set_pass(res_data)
> + else:
> + self.set_fail(res_data)
> --
> 2.4.6
>
> _______________________________________________
> LNST-developers mailing list
> LNST-developers(a)lists.fedorahosted.org
>
https://lists.fedorahosted.org/mailman/listinfo/lnst-developers