Re: [PATCH 1/2] InterfaceManager: Add speed setting methods

[Ido Schimmel]

Tue, Jan 05, 2016 at 09:21:11AM IST, jprochaz(a)redhat.com wrote:
> 2016-01-04 22:31 GMT+01:00 Ido Schimmel <idosch(a)mellanox.com&gt;:
>
> > In order to test speed negotiation it is useful to have methods that
> > can change the speed of an interface.
> >
> > The first method, 'set_speed', allows one to specify a speed (in Mb/s,
> > as in ethtool) to be set on an interface. At its lowest level, it's
> > implemented by calling 'ethtool' with desired 'speed' parameter and
> > 'autoneg' set to 'off'. This is needed in order to prevent the
> driver
> > from advertising any other speeds.
> >
> > On the other end of the link, the 'set_autoneg' method should be used
> > (again, implemented via 'ethtool'). When set it basically tells the
> > driver to advertise all the supported speeds.
> >
> > To make sure no interface is left with auto negotiation off, set auto
> > negotation on when the Ethernet device is deconfigured.
> >
> > Note that it's possible to tell the driver which speed and duplex to
> > advertise in autonegotiation, but for now we omit that and simply
> > advertise all of them. If needed, this can be added in the future.
> >
> > Reviewed-by: Jiri Pirko <jiri(a)mellanox.com&gt;
> > Signed-off-by: Ido Schimmel <idosch(a)mellanox.com&gt;
> > ---
> >  lnst/Controller/Machine.py     |  6 ++++++
> >  lnst/Controller/Task.py        |  6 ++++++
> >  lnst/Slave/InterfaceManager.py |  6 ++++++
> >  lnst/Slave/NetConfigDevice.py  |  1 +
> >  lnst/Slave/NetTestSlave.py     | 18 ++++++++++++++++++
> >  5 files changed, 37 insertions(+)
> >
> > diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py
> > index 7281203..89ea05a 100644
> > --- a/lnst/Controller/Machine.py
> > +++ b/lnst/Controller/Machine.py
> > @@ -779,6 +779,12 @@ class Interface(object):
> >          self._machine._rpc_call_x(self._netns, "del_br_fdb",
> >                                    self._id, br_fdb_info)
> >
> > +    def set_speed(self, speed):
> > +        self._machine._rpc_call_x(self._netns, "set_speed", self._id,
> > speed)
> > +
> > +    def set_autoneg(self):
> > +        self._machine._rpc_call_x(self._netns, "set_autoneg", self._id)
> > +
> >  class StaticInterface(Interface):
> >      """ Static interface
> >
> > diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py
> > index e47b025..0ae1b25 100644
> > --- a/lnst/Controller/Task.py
> > +++ b/lnst/Controller/Task.py
> > @@ -528,6 +528,12 @@ class InterfaceAPI(object):
> >          _self._if.del_br_fdb({"hwaddr": hwaddr, "self": self,
> "master":
> > master,
> >                                "vlan_id": vlan_tci})
> >
> > +    def set_speed(self, speed):
> > +        return self._if.set_speed(speed)
> > +
> > +    def set_autoneg(self):
> > +        return self._if.set_autoneg()
> > +
> >  class ModuleAPI(object):
> >      """ An API class representing a module. """
> >
> > diff --git a/lnst/Slave/InterfaceManager.py
> > b/lnst/Slave/InterfaceManager.py
> > index a99c2ae..be45153 100644
> > --- a/lnst/Slave/InterfaceManager.py
> > +++ b/lnst/Slave/InterfaceManager.py
> > @@ -530,3 +530,9 @@ class Device(object):
> >                     "mtu": self._mtu,
> >                     "driver": self._driver}
> >          return if_data
> > +
> > +    def set_speed(self, speed):
> > +        exec_cmd("ethtool -s %s speed %s autoneg off" % (self._name,
> > speed))
> > +
> > +    def set_autoneg(self):
> > +        exec_cmd("ethtool -s %s autoneg on" % self._name)
> > diff --git a/lnst/Slave/NetConfigDevice.py b/lnst/Slave/NetConfigDevice.py
> > index 809c0c5..7d41873 100644
> > --- a/lnst/Slave/NetConfigDevice.py
> > +++ b/lnst/Slave/NetConfigDevice.py
> > @@ -90,6 +90,7 @@ class NetConfigDeviceEth(NetConfigDeviceGeneric):
> >
> >      def deconfigure(self):
> >          config = self._dev_config
> > +        exec_cmd("ethtool -s %s autoneg on" % config["name"])
> >          if "netem_cmd" in config:
> >              exec_cmd(config["netem_cmd"].replace("add",
> "del"))
> >
> > diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py
> > index d65db5a..7a6ac88 100644
> > --- a/lnst/Slave/NetTestSlave.py
> > +++ b/lnst/Slave/NetTestSlave.py
> > @@ -729,6 +729,24 @@ class SlaveMethods:
> >          brt.del_fdb(br_fdb_info)
> >          return True
> >
> > +    def set_speed(self, if_id, speed):
> > +        dev = self._if_manager.get_mapped_device(if_id)
> > +        if dev is not None:
> > +            dev.set_speed(speed)
> > +        else:
> > +            dev.error("Device with id '%s' not found." % if_id)
> >
> ​This should be logging.error()​
>
>
> > +            return False
> > +        return True
> > +
> > +    def set_autoneg(self, if_id):
> > +        dev = self._if_manager.get_mapped_device(if_id)
> > +        if dev is not None:
> > +            dev.set_autoneg()
> > +        else:
> > +            dev.error("Device with id '%s' not found." % if_id)
> >
> ​This should be logging.error() too​ 
Yep. Thanks for catching these! Will fix in v2.

> > +            return False
> > +        return True
> > +
> >  class ServerHandler(ConnectionHandler):
> >      def __init__(self, addr):
> >          super(ServerHandler, self).__init__()
> > --
> > 2.4.10
> > _______________________________________________
> > LNST-developers mailing list
> > lnst-developers(a)lists.fedorahosted.org
> >
> > https://lists.fedorahosted.org/admin/lists/[e-mail]...
> >
>
>
> Apart from those typos, it looks fine to me, good work :)​ 
Thanks!

>
>-- 
>​Best regards,​
>
>Jiri Prochazka
>LNST Developer
>​| www.lnst-project.org​
>
>[phone] | jprochaz(a)redhat.com
>Red Hat Czech | Purkyňova 71/99, 612 00 Brno