From: Jiri Pirko jiri@mellanox.com
On lot of places, there is if-else construction due to netns and non-netns rpc call. Wrap this up by _rpc_call_x method and make the code more simple.
Signed-off-by: Jiri Pirko jiri@mellanox.com --- lnst/Controller/Machine.py | 164 +++++++++++++-------------------------------- 1 file changed, 45 insertions(+), 119 deletions(-)
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index ce78866..8cfe5bf 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -191,6 +191,11 @@ class Machine(object):
return result
+ def _rpc_call_x(self, netns, method_name, *args): + if not netns: + return self._rpc_call(method_name, *args) + return self._rpc_call_to_netns(netns, method_name, *args) + def configure(self, recipe_name): """ Prepare the machine
@@ -289,16 +294,12 @@ class Machine(object): if bg_cmd["netns"] != None: command["netns"] = bg_cmd["netns"]
+ netns = command["netns"] if command["type"] == "wait": logging.debug("Get remaining time of bg process with bg_id == %s" % command["proc_id"]) - if command["netns"] != None: - remaining_time = self._rpc_call_to_netns(command["netns"], - "get_remaining_time", - command["proc_id"]) - else: - remaining_time = self._rpc_call("get_remaining_time", - command["proc_id"]) + remaining_time = self._rpc_call_x(netns, "get_remaining_time", + command["proc_id"]) logging.debug("Setting timeout to %d", remaining_time) if remaining_time > 0: signal.alarm(remaining_time) @@ -316,25 +317,14 @@ class Machine(object): signal.alarm(DEFAULT_TIMEOUT)
try: - if 'netns' in command and command['netns'] != None: - netns = command["netns"] - cmd_res = self._rpc_call_to_netns(netns, "run_command", command) - else: - cmd_res = self._rpc_call("run_command", command) + cmd_res = self._rpc_call_x(netns, "run_command", command) except MachineError as exc: - if command["netns"] is not None: - netns = command["netns"] - if "proc_id" in command: - cmd_res = self._rpc_call_to_netns(netns, "kill_command", - command["proc_id"]) - else: - cmd_res = self._rpc_call_to_netns(netns, "kill_command", - None) + if "proc_id" in command: + cmd_res = self._rpc_call_x(netns, "kill_command", + command["proc_id"]) else: - if "proc_id" in command: - cmd_res = self._rpc_call("kill_command", command["proc_id"]) - else: - cmd_res = self._rpc_call("kill_command", None) + cmd_res = self._rpc_call_x(netns, "kill_command", + None)
if "killed" in cmd_res and cmd_res["killed"]: cmd_res["passed"] = False @@ -400,10 +390,7 @@ class Machine(object):
tmp = {} for netns in namespaces: - if netns == None: - tmp.update(self._rpc_call("start_packet_capture", "")) - else: - tmp.update(self._rpc_call_to_netns(netns, "start_packet_capture", "")) + tmp.update(self._rpc_call_x(netns, "start_packet_capture", "")) return tmp
def stop_packet_capture(self): @@ -412,17 +399,10 @@ class Machine(object): namespaces.add(iface.get_netns())
for netns in namespaces: - if netns == None: - self._rpc_call("stop_packet_capture") - else: - self._rpc_call_to_netns(netns, "stop_packet_capture") + self._rpc_call_x(netns, "stop_packet_capture")
def copy_file_to_machine(self, local_path, remote_path=None, netns=None): - if netns: - remote_path = self._rpc_call_to_netns(netns, "start_copy_to", - remote_path) - else: - remote_path = self._rpc_call("start_copy_to", remote_path) + remote_path = self._rpc_call_x(netns, "start_copy_to", remote_path)
f = open(local_path, "rb")
@@ -431,16 +411,9 @@ class Machine(object): if len(data) == 0: break
- if netns: - self._rpc_call_to_netns(netns, "copy_part_to", remote_path, - Binary(data)) - else: - self._rpc_call("copy_part_to", remote_path, Binary(data)) + self._rpc_call_x(netns, "copy_part_to", remote_path, Binary(data))
- if netns: - self._rpc_call_to_netns(netns, "finish_copy_to", remote_path) - else: - self._rpc_call("finish_copy_to", remote_path) + self._rpc_call_x(netns, "finish_copy_to", remote_path)
return remote_path
@@ -689,12 +662,8 @@ class Interface(object): return self._mtu
def update_from_slave(self): - if self._netns != None: - if_data = self._machine._rpc_call_to_netns(self._netns, - "get_if_data", - self._id) - else: - if_data = self._machine._rpc_call("get_if_data", self._id) + if_data = self._machine._rpc_call_x(self._netns, "get_if_data", + self._id)
if if_data is not None: self.update(if_data) @@ -734,32 +703,16 @@ class Interface(object): return config
def up(self): - netns = self._netns - if netns != None: - self._machine._rpc_call_to_netns(netns, "set_device_up", self._id) - else: - self._machine._rpc_call("set_device_up", self._id) + self._machine._rpc_call_x(self._netns, "set_device_up", self._id)
def down(self): - netns = self._netns - if netns != None: - self._machine._rpc_call_to_netns(netns, "set_device_down", self._id) - else: - self._machine._rpc_call("set_device_down", self._id) + self._machine._rpc_call_x(self._netns, "set_device_down", self._id)
def set_link_up(self): - netns = self._netns - if netns != None: - self._machine._rpc_call_to_netns(netns, "set_link_up", self._id) - else: - self._machine._rpc_call("set_link_up", self._id) + self._machine._rpc_call_x(self._netns, "set_link_up", self._id)
def set_link_down(self): - netns = self._netns - if netns != None: - self._machine._rpc_call_to_netns(netns, "set_link_down", self._id) - else: - self._machine._rpc_call("set_link_down", self._id) + self._machine._rpc_call_x(self._netns, "set_link_down", self._id)
def initialize(self): phys_devs = self._machine._rpc_call("map_if_by_hwaddr", @@ -795,24 +748,20 @@ class Interface(object):
if self._netns != None: self._machine._rpc_call("set_if_netns", self.get_id(), self._netns) - self._machine._rpc_call_to_netns(self._netns, "configure_interface", - self.get_id(), self.get_config()) - else: - self._machine._rpc_call("configure_interface", self.get_id(), - self.get_config()) + self._machine._rpc_call_x(self._netns, "configure_interface", + self.get_id(), self.get_config()) + self.update_from_slave()
def deconfigure(self): if not self._configured: return
+ self._machine._rpc_call_x(self._netns, "deconfigure_interface", + self.get_id()) if self._netns != None: self._machine._rpc_call_to_netns(self._netns, - "deconfigure_interface", self.get_id()) - self._machine._rpc_call_to_netns(self._netns, "return_if_netns", self.get_id()) - else: - self._machine._rpc_call("deconfigure_interface", self.get_id()) self._configured = False
class StaticInterface(Interface): @@ -849,16 +798,10 @@ class LoopbackInterface(Interface): self._hwaddr = '00:00:00:00:00:00' self._driver = 'loopback'
- if self._netns: - phys_devs = self._machine._rpc_call_to_netns(self._netns, - "map_if_by_params", self._id, - { 'hwaddr': self._hwaddr, - 'driver': self._driver }) - else: - phys_devs = self._machine._rpc_call("map_if_by_params", - self._id, - { 'hwaddr': self._hwaddr, - 'driver': self._driver }) + phys_devs = self._machine._rpc_call_x(self._netns, + "map_if_by_params", self._id, + { 'hwaddr': self._hwaddr, + 'driver': self._driver })
if len(phys_devs) == 1: self.set_devname(phys_devs[0]["name"]) @@ -880,12 +823,8 @@ class LoopbackInterface(Interface): logging.info("Configuring interface %s on machine %s", self.get_id(), self._machine.get_id())
- if self._netns != None: - self._machine._rpc_call_to_netns(self._netns, "configure_interface", - self.get_id(), self.get_config()) - else: - self._machine._rpc_call("configure_interface", self.get_id(), - self.get_config()) + self._machine._rpc_call_x(self._netns, "configure_interface", + self.get_id(), self.get_config()) self._configured = True self.update_from_slave()
@@ -893,14 +832,9 @@ class LoopbackInterface(Interface): if not self._configured: return
- if self._netns != None: - self._machine._rpc_call_to_netns(self._netns, - "deconfigure_interface", self.get_id()) - self._machine._rpc_call_to_netns(self._netns, - "unmap_if", self.get_id()) - else: - self._machine._rpc_call("deconfigure_interface", self.get_id()) - self._machine._rpc_call("unmap_if", self.get_id()) + self._machine._rpc_call_x(self._netns, "deconfigure_interface", + self.get_id()) + self._machine._rpc_call_x(self._netns, "unmap_if", self.get_id()) self._configured = False
class VirtualInterface(Interface): @@ -1024,12 +958,9 @@ class SoftInterface(Interface): peer_if._configured = True return
- if self._netns != None: - dev_name = self._machine._rpc_call_to_netns(self._netns, - "create_soft_interface", self._id, self.get_config()) - else: - dev_name = self._machine._rpc_call("create_soft_interface", - self._id, self.get_config()) + dev_name = self._machine._rpc_call_x(self._netns, + "create_soft_interface", + self._id, self.get_config()) self.set_devname(dev_name) self.update_from_slave()
@@ -1048,14 +979,9 @@ class SoftInterface(Interface): peer_if._configured = False return
- if self._netns != None: - self._machine._rpc_call_to_netns(self._netns, - "deconfigure_interface", self.get_id()) - self._machine._rpc_call_to_netns(self._netns, - "unmap_if", self.get_id()) - else: - self._machine._rpc_call("deconfigure_interface", self.get_id()) - self._machine._rpc_call("unmap_if", self.get_id()) + self._machine._rpc_call_x(self._netns, "deconfigure_interface", + self.get_id()) + self._machine._rpc_call_x(self._netns, "unmap_if", self.get_id()) self._configured = False
class UnusedInterface(Interface):
From: Jiri Pirko jiri@mellanox.com
Kernel exposes an interface primary used by bridge to work with internal VLAN implementation. However, other devices (for example SR-IOV drivers) implement this as well making this not bridge-specific.
Allow task to work with the bridge VLAN settings. This is implemented simply by calling "bridge" utility. This may be later on changed to pyroute2 implementation, alas it does not implement PF_BRIDGE Netlink at the moment.
Signed-off-by: Jiri Pirko jiri@mellanox.com --- lnst/Controller/Machine.py | 8 ++++++++ lnst/Controller/Task.py | 12 ++++++++++++ lnst/Slave/BridgeTool.py | 36 ++++++++++++++++++++++++++++++++++++ lnst/Slave/NetTestSlave.py | 19 +++++++++++++++++++ 4 files changed, 75 insertions(+) create mode 100644 lnst/Slave/BridgeTool.py
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 8cfe5bf..b457c3c 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -764,6 +764,14 @@ class Interface(object): "return_if_netns", self.get_id()) self._configured = False
+ def add_br_vlan(self, br_vlan_info): + self._machine._rpc_call_x(self._netns, "add_br_vlan", + self._id, br_vlan_info) + + def del_br_vlan(self, br_vlan_info): + self._machine._rpc_call_x(self._netns, "del_br_vlan", + self._id, br_vlan_info) + class StaticInterface(Interface): """ Static interface
diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py index 4aeed17..3c5b71d 100644 --- a/lnst/Controller/Task.py +++ b/lnst/Controller/Task.py @@ -508,6 +508,18 @@ class InterfaceAPI(object): def destroy(self): self._host._remove_iface(self)
+ def add_br_vlan(_self, vlan_tci, pvid=False, untagged=False, + self=False, master=False): + _self._if.add_br_vlan({"vlan_id": vlan_tci, "pvid": pvid, + "untagged": untagged, + "self": self, "master": master}) + + def del_br_vlan(_self, vlan_tci, pvid=False, untagged=False, + self=False, master=False): + _self._if.del_br_vlan({"vlan_id": vlan_tci, "pvid": pvid, + "untagged": untagged, + "self": self, "master": master}) + class ModuleAPI(object): """ An API class representing a module. """
diff --git a/lnst/Slave/BridgeTool.py b/lnst/Slave/BridgeTool.py new file mode 100644 index 0000000..017ed1e --- /dev/null +++ b/lnst/Slave/BridgeTool.py @@ -0,0 +1,36 @@ +""" +This module defines a class useful to work with "bridge" tool. + +Copyright 2015 Mellanox Technologies. All rights reserved. +Licensed under the GNU General Public License, version 2 as +published by the Free Software Foundation; see COPYING for details. +""" + +__author__ = """ +jiri@mellanox.com (Jiri Pirko) +""" + +from lnst.Common.ExecCmd import exec_cmd + +class BridgeTool: + def __init__(self, dev_name): + self._dev_name = dev_name + + def _add_del_vlan(self, op, br_vlan_info): + cmd = "bridge vlan %s dev %s vid %d" % (op, self._dev_name, + br_vlan_info["vlan_id"]) + if br_vlan_info["pvid"]: + cmd += " pvid" + if br_vlan_info["untagged"]: + cmd += " untagged" + if br_vlan_info["self"]: + cmd += " self" + if br_vlan_info["master"]: + cmd += " master" + exec_cmd(cmd) + + def add_vlan(self, br_vlan_info): + return self._add_del_vlan("add", br_vlan_info) + + def del_vlan(self, br_vlan_info): + return self._add_del_vlan("del", br_vlan_info) diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 9104e31..df28a6a 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -36,6 +36,7 @@ from lnst.Common.ConnectionHandler import ConnectionHandler from lnst.Common.Config import lnst_config from lnst.Common.Config import DefaultRPCPort from lnst.Slave.InterfaceManager import InterfaceManager +from lnst.Slave.BridgeTool import BridgeTool
class SlaveMethods: ''' @@ -692,6 +693,24 @@ class SlaveMethods: device.set_netns(None) return True
+ def add_br_vlan(self, if_id, br_vlan_info): + dev = self._if_manager.get_mapped_device(if_id) + if not dev: + logging.error("Device with id '%s' not found." % if_id) + return False + brt = BridgeTool(dev.get_name()) + brt.add_vlan(br_vlan_info) + return True + + def del_br_vlan(self, if_id, br_vlan_info): + dev = self._if_manager.get_mapped_device(if_id) + if not dev: + logging.error("Device with id '%s' not found." % if_id) + return False + brt = BridgeTool(dev.get_name()) + brt.del_vlan(br_vlan_info) + return True + class ServerHandler(ConnectionHandler): def __init__(self, addr): super(ServerHandler, self).__init__()
From: Jiri Pirko jiri@mellanox.com
Kernel exposes an interface primary used by bridge to work with FDB entries. However, other devices (for example SR-IOV drivers) implement this as well making this not bridge-specific.
Allow task to work with the bridge FDB entries. This is implemented simply by calling "bridge" utility. This may be later on changed to pyroute2 implementation, alas it does not implement PF_BRIDGE Netlink at the moment.
Signed-off-by: Jiri Pirko jiri@mellanox.com --- lnst/Controller/Machine.py | 8 ++++++++ lnst/Controller/Task.py | 8 ++++++++ lnst/Slave/BridgeTool.py | 17 +++++++++++++++++ lnst/Slave/NetTestSlave.py | 18 ++++++++++++++++++ 4 files changed, 51 insertions(+)
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index b457c3c..12f8757 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -772,6 +772,14 @@ class Interface(object): self._machine._rpc_call_x(self._netns, "del_br_vlan", self._id, br_vlan_info)
+ def add_br_fdb(self, br_fdb_info): + self._machine._rpc_call_x(self._netns, "add_br_fdb", + self._id, br_fdb_info) + + def del_br_fdb(self, br_fdb_info): + self._machine._rpc_call_x(self._netns, "del_br_fdb", + self._id, br_fdb_info) + class StaticInterface(Interface): """ Static interface
diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py index 3c5b71d..e47b025 100644 --- a/lnst/Controller/Task.py +++ b/lnst/Controller/Task.py @@ -520,6 +520,14 @@ class InterfaceAPI(object): "untagged": untagged, "self": self, "master": master})
+ def add_br_fdb(_self, hwaddr, self=False, master=False, vlan_tci=None): + _self._if.add_br_fdb({"hwaddr": hwaddr, "self": self, "master": master, + "vlan_id": vlan_tci}) + + def del_br_fdb(_self, hwaddr, self=False, master=False, vlan_tci=None): + _self._if.del_br_fdb({"hwaddr": hwaddr, "self": self, "master": master, + "vlan_id": vlan_tci}) + class ModuleAPI(object): """ An API class representing a module. """
diff --git a/lnst/Slave/BridgeTool.py b/lnst/Slave/BridgeTool.py index 017ed1e..1d87cd9 100644 --- a/lnst/Slave/BridgeTool.py +++ b/lnst/Slave/BridgeTool.py @@ -34,3 +34,20 @@ class BridgeTool:
def del_vlan(self, br_vlan_info): return self._add_del_vlan("del", br_vlan_info) + + def _add_del_fdb(self, op, br_fdb_info): + cmd = "bridge fdb %s %s dev %s" % (op, br_fdb_info["hwaddr"], + self._dev_name) + if br_fdb_info["self"]: + cmd += " self" + if br_fdb_info["master"]: + cmd += " master" + if br_fdb_info["vlan_id"]: + cmd += " vlan %s" % br_fdb_info["vlan_id"] + exec_cmd(cmd) + + def add_fdb(self, br_fdb_info): + return self._add_del_fdb("add", br_fdb_info) + + def del_fdb(self, br_fdb_info): + return self._add_del_fdb("del", br_fdb_info) diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index df28a6a..dc58e1c 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -711,6 +711,24 @@ class SlaveMethods: brt.del_vlan(br_vlan_info) return True
+ def add_br_fdb(self, if_id, br_fdb_info): + dev = self._if_manager.get_mapped_device(if_id) + if not dev: + logging.error("Device with id '%s' not found." % if_id) + return False + brt = BridgeTool(dev.get_name()) + brt.add_fdb(br_fdb_info) + return True + + def del_br_fdb(self, if_id, br_fdb_info): + dev = self._if_manager.get_mapped_device(if_id) + if not dev: + logging.error("Device with id '%s' not found." % if_id) + return False + brt = BridgeTool(dev.get_name()) + brt.del_fdb(br_fdb_info) + return True + class ServerHandler(ConnectionHandler): def __init__(self, addr): super(ServerHandler, self).__init__()
Tue, Dec 29, 2015 at 07:02:35PM CET, jiri@resnulli.us wrote:
From: Jiri Pirko jiri@mellanox.com
On lot of places, there is if-else construction due to netns and non-netns rpc call. Wrap this up by _rpc_call_x method and make the code more simple.
Signed-off-by: Jiri Pirko jiri@mellanox.com
Oh, By mistake I sent this patch twice :/ Please ignore.
lnst-developers@lists.fedorahosted.org