Re: [PATCH V2 1/2] Iperf: add option to use iperf3

[Jiri Pirko]

Mon, Aug 07, 2017 at 10:52:02AM CEST, roid(a)mellanox.com wrote:
> On 07/08/2017 11:45, Roi Dayan wrote:
> > 
> > 
> > On 07/08/2017 08:19, Jiri Pirko wrote:
> > > Mon, Aug 07, 2017 at 06:17:45AM CEST, roid(a)mellanox.com wrote:
> > > > 
> > > > 
> > > > On 06/08/2017 11:17, Roi Dayan wrote:
> > > > > iperf3 is based the original iperf and support similiar command line
> > > > > and have a similiar output.
> > > > > 
> > > > > This commit allows to pass option to Iperf test module to use iperf3
> > > > > binary instead of iperf and adds additional error catcher from
> iperf3.
> > > > > In iperf probelmatic input is just ignore with a print.
> > > > > 
> > > > > Signed-off-by: Roi Dayan <roid(a)mellanox.com&gt;
> > > > > ---
> > > > >  test_modules/Iperf.py | 17 ++++++++++++++---
> > > > >  1 file changed, 14 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/test_modules/Iperf.py b/test_modules/Iperf.py
> > > > > index 77bd1a0..c0b6ba5 100644
> > > > > --- a/test_modules/Iperf.py
> > > > > +++ b/test_modules/Iperf.py
> > > > > @@ -17,6 +17,11 @@ from lnst.Common.Utils import is_installed
> > > > > 
> > > > >  class Iperf(TestGeneric):
> > > > >      def _compose_iperf_cmd(self, role):
> > > > > +        if self.get_opt("iperf3"):
> > > > > +            iperf_bin = "iperf3"
> > > > > +        else:
> > > > > +            iperf_bin = "iperf"
> > > > > +
> > > > >          iperf_options = self.get_opt("iperf_opts")
> > > > >          if iperf_options is None:
> > > > >              iperf_options = ""
> > > > > @@ -24,13 +29,13 @@ class Iperf(TestGeneric):
> > > > >          cmd = ""
> > > > >          if role == "client":
> > > > >              iperf_server = self.get_mopt("iperf_server",
> > > > > opt_type="addr")
> > > > > -            cmd = "iperf --%s %s -t %s %s" % (role,
> iperf_server,
> > > > > self.duration, iperf_options)
> > > > > +            cmd = "%s --%s %s -t %s %s" % (iperf_bin,
> role,
> > > > > iperf_server, self.duration, iperf_options)
> > > > >          elif role == "server":
> > > > >              bind = self.get_opt("bind",
> opt_type="addr")
> > > > >              if bind != None:
> > > > > -                cmd = "iperf --%s -B %s %s" % (role, bind,
> > > > > iperf_options)
> > > > > +                cmd = "%s --%s -B %s %s" % (iperf_bin,
> role, bind,
> > > > > iperf_options)
> > > > >              else:
> > > > > -                cmd = "iperf --%s %s" % (role,
> iperf_options)
> > > > > +                cmd = "%s --%s %s" % (iperf_bin, role,
> iperf_options)
> > > > > 
> > > > >          return cmd
> > > > > 
> > > > > @@ -91,6 +96,12 @@ class Iperf(TestGeneric):
> > > > >              logging.info("Iperf connection failed!")
> > > > >              return (False, "Iperf connection failed!")
> > > > > 
> > > > > +        m = re.search("iperf3: error - (.*)", output)
> > > > > +        if m:
> > > > > +            err = m.groups()[0]
> > > > > +            logging.info("Iperf error: %s" % err)
> > > > > +            return (False, "Iperf error: %s" % err)
> > > > > +
> > > > >          m =
> > > > >
> re.search("\[[^0-9]*[0-9]*\]\s*0.0+-\s*\d*\.\d+\s*sec\s*\d*(\.\d*){0,1}\s*[
> > > > > kGMT]Bytes\s*(\d*(\.\d*){0,1}\s*[ kGMT]bits\/sec)", output,
> > > > > re.IGNORECASE)
> > > > >          if m is None:
> > > > >              logging.info("Could not get performance
> throughput!")
> > > > > 
> > > > 
> > > > 
> > > > another method I was thinking is to make Iperf3 test module to inherit
> > > > from Iperf and set the binary in an attribute for the compose cmd.
> > > 
> > > I like that approach better.
> > > 
> > 
> > was just thinking. I should not really inherit from Iperf as it's
> > another test module that might not exists if i didn't sync it with
> > sync_resources() ?
> > 
>
> so tested and I cannot do Iperf3 to inherit from Iperf as
> Iperf might not exists if not synced. 
Can't they be both in the same file?

Ondrej?