When one want to add a new function to the slave, one needs to add it also to the slave communicator (NetTestSlave), and to the controller side that control it (Macine) and to Task, that controls it. While some of this functions have logic, most of them are just calling a function of an object that they holds, usually, with the same name.
Those function can, and I think they should, be manufactured in run time for the following reasons: * it shorten the code considerably (new code is 25% of the old one). * it makes adding new function much shorter. * it singles out the function that do have logic in them.
There are however some down sides to it: * code is a bit harder to read at first. * Inheriting function is a bit more tricky.
This is just a start and it can be expanded for: * more classes with functions with this problem * setter-getters without inner logic * function with arguments with default values
I know that LNST is now in freeze and I don't know how much of it will be relevant after it. But in case it is, I will be glad to hear your opinions on it in the meantime.
Nogah Frankel (4): Machine: Interface: create RPC calls functions in run time Slave: NetTestSlave: SlaveMethods: create dev functions calls in run time Slave: NetTestSlave: SlaveMethods: create bridge function calls in run time Task: InterfaceAPI: create interface calls functions in run time
lnst/Controller/Machine.py | 106 ++++----------- lnst/Controller/Task.py | 93 ++++--------- lnst/Slave/NetTestSlave.py | 326 +++++++++------------------------------------ 3 files changed, 113 insertions(+), 412 deletions(-)
In the class Interface there are many functions of the following prototype: def <function name> (self, <additional arguments>): return self._machine._rpc_call_x(self._netns, <str name>, \ self._id, <additional arguments>) where the <function name> and <str name> are often the same and the additional arguments (if exist) sent without change.
All this function holds no logic of their own and can be generated in run time. This commit replaces 23 functions of this type with lambdas created in the init.
Signed-off-by: Nogah Frankel nogahf@mellanox.com --- lnst/Controller/Machine.py | 106 ++++++++++++--------------------------------- 1 file changed, 28 insertions(+), 78 deletions(-)
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 74bd64e..3c8790a 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -632,6 +632,34 @@ class Interface(object): self._driver = None self._devlink = None self._routes = [] + self._init_manufacture_funcs() + + _MACHINE_RPC_CALL = ["link_stats", "set_link_up", "set_link_down", + "add_br_vlan", "del_br_vlan", "get_br_vlans", + "add_br_fdb", "del_br_fdb", "get_br_fdbs", + "set_br_learning", "set_br_learning_sync", + "set_br_flooding", "set_br_state", "set_speed", + "set_autoneg", "get_ethtool_stats", "enable_lldp", + "set_pause_on", "set_pause_off"] + + _MACHINE_RPC_CALL_DIFF_NAME = [("up", "set_device_up"), + ("down", "set_device_down"), + ("address_setup", "device_address_setup"), + ("address_cleanup","device_address_cleanup") + ] + + def _create_machine_rpc_call_func(self, name, func_name): + func = lambda *args: self._machine._rpc_call_x(self._netns, func_name, + self.get_id(), *args) + setattr(self, name, func) + + def _init_manufacture_funcs(self): + for name, func_name in self._MACHINE_RPC_CALL_DIFF_NAME: + if not hasattr(self, name): + self._create_machine_rpc_call_func(name, func_name) + for name in self._MACHINE_RPC_CALL: + if not hasattr(self, name): + self._create_machine_rpc_call_func(name, name)
def get_id(self): return self._id @@ -774,10 +802,6 @@ class Interface(object): self._mtu = mtu return self._mtu
- def link_stats(self): - stats = self._machine._rpc_call_x(self._netns, "link_stats", - self._id) - return stats
def set_addresses(self, ips): self._addresses = ips @@ -850,24 +874,6 @@ class Interface(object):
return config
- def up(self): - self._machine._rpc_call_x(self._netns, "set_device_up", self._id) - - def down(self): - self._machine._rpc_call_x(self._netns, "set_device_down", self._id) - - def address_setup(self): - self._machine._rpc_call_x(self._netns, "device_address_setup", self._id) - - def address_cleanup(self): - self._machine._rpc_call_x(self._netns, "device_address_cleanup", self._id) - - def set_link_up(self): - self._machine._rpc_call_x(self._netns, "set_link_up", self._id) - - def set_link_down(self): - 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", self._id, self._hwaddr) @@ -924,50 +930,6 @@ 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) - - def get_br_vlans(self): - return self._machine._rpc_call_x(self._netns, "get_br_vlans", self._id) - - 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) - - def get_br_fdbs(self): - return self._machine._rpc_call_x(self._netns, "get_br_fdbs", self._id) - - def set_br_learning(self, br_learning_info): - self._machine._rpc_call_x(self._netns, "set_br_learning", self._id, - br_learning_info) - - def set_br_learning_sync(self, br_learning_sync_info): - self._machine._rpc_call_x(self._netns, "set_br_learning_sync", self._id, - br_learning_sync_info) - - def set_br_flooding(self, br_flooding_info): - self._machine._rpc_call_x(self._netns, "set_br_flooding", self._id, - br_flooding_info) - - def set_br_state(self, br_state_info): - self._machine._rpc_call_x(self._netns, "set_br_state", self._id, - br_state_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) - def slave_add(self, if_id): self._machine._rpc_call_x(self._netns, "slave_add", self._id, if_id) self.add_slave(self._machine.get_interface(if_id)) @@ -988,18 +950,6 @@ class Interface(object): self._devlink["port_index"]) return None
- def get_ethtool_stats(self): - return self._machine._rpc_call_x(self._netns, "get_ethtool_stats", - self._id) - - def enable_lldp(self): - return self._machine._rpc_call_x(self._netns, "enable_lldp", self._id) - - def set_pause_on(self): - return self._machine._rpc_call_x(self._netns, "set_pause_on", self._id) - - def set_pause_off(self): - return self._machine._rpc_call_x(self._netns, "set_pause_off", self._id)
class StaticInterface(Interface): """ Static interface
In the class SlaveMethods there are many functions of the following prototype:
def <func_name>(self, if_id, <additional arguments>): dev = self._if_manager.get_mapped_device(if_id) if dev is None: logging.error("Device with id '%s' not found." % if_id) return False dev.<func_name> return True
with small exceptions: - different error return value - return the dev.<func_name> result - different <func_name> and dev.<func_name> - no logging on error
Overall this functions have no logic of their own, and they can be replaced with one generic function (_call_dev_func). This commit replaces 10 no-exceptions functions and 12 exceptions functions with lambdas created on run-time.
Signed-off-by: Nogah Frankel nogahf@mellanox.com --- lnst/Slave/NetTestSlave.py | 225 ++++++++++----------------------------------- 1 file changed, 49 insertions(+), 176 deletions(-)
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 8e26b4b..4e69955 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -65,6 +65,55 @@ class SlaveMethods: self._resource_table = {'module': {}, 'tools': {}}
self._bkp_nm_opt_val = lnst_config.get_option("environment", "use_nm") + self._init_manufacture_funcs() + + def _call_dev_func(self, inputs, func_name, print_err=True, + error_value=False, return_res=False): + inputs = list(inputs) + if_id = inputs.pop(0) + dev = self._if_manager.get_mapped_device(if_id) + if dev is None: + if print_err: + logging.error("Device with id '%s' not found." % if_id) + return error_value + func = getattr(dev, func_name) + res = func(*inputs) + if return_res: + return res + return True + + def _create_dev_call(self, func_name, options={}): + return lambda *args: self._call_dev_func(args, func_name, **options) + + _DEV_FUNCS = ["set_addresses", "set_addresses", "del_route", + "add_nhs_route", "del_nhs_route", "enable_lldp", + "set_pause_on", "set_pause_off", "set_speed", "set_autoneg", + "slave_add", "slave_del"] + + _DEV_FUNC_OPTIONS = \ + [("get_if_data", {"print_err": False, "error_value": {}, + "return_res": True}), + ("link_stats", {"error_value": {}, "return_res": True}), + ("get_ethtool_stats", {"return_res": True}), + ("set_device_up", {"func_name": "up"}), + ("set_device_down", {"func_name": "down"}), + ("device_address_setup", {"func_name": "address_setup"}), + ("device_address_cleanup", {"func_name": "address_cleanup"}), + ("set_link_up", {"func_name": "link_up"}), + ("set_link_down", {"func_name": "link_down"}), + ("deconfigure_interface", {"func_name": "clear_configuration", + "error_value": True}), + ] + + def _init_manufacture_funcs(self): + for name, options in self._DEV_FUNC_OPTIONS: + if options.has_key("func_name"): + func_name = options.pop("func_name") + else: + func_name = name + setattr(self, name, self._create_dev_call(func_name, options)) + for name in self._DEV_FUNCS: + setattr(self, name, self._create_dev_call(name))
def hello(self, recipe_path): self.machine_cleanup() @@ -187,105 +236,6 @@ class SlaveMethods:
return matched
- def get_if_data(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - return {} - return dev.get_if_data() - - def link_stats(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - logging.error("Device with id '%s' not found." % if_id) - return {} - return dev.link_stats() - - def set_addresses(self, if_id, ips): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - logging.error("Device with id '%s' not found." % if_id) - return False - dev.set_addresses(ips) - return True - - def add_route(self, if_id, dest, ipv6): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - logging.error("Device with id '%s' not found." % if_id) - return False - dev.add_route(dest, ipv6) - return True - - def del_route(self, if_id, dest, ipv6): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - logging.error("Device with id '%s' not found." % if_id) - return False - dev.del_route(dest, ipv6) - return True - - def add_nhs_route(self, if_id, dest, nhs, ipv6): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - logging.error("Device with id '%s' not found." % if_id) - return False - dev.add_nhs_route(dest, nhs, ipv6) - return True - - def del_nhs_route(self, if_id, dest, nhs, ipv6): - dev = self._if_manager.get_mapped_device(if_id) - if dev is None: - logging.error("Device with id '%s' not found." % if_id) - return False - dev.del_nhs_route(dest, nhs, ipv6) - return True - - def set_device_up(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - dev.up() - return True - - def set_device_down(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.down() - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - - def device_address_setup(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - dev.address_setup() - return True - - def device_address_cleanup(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.address_cleanup() - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - - def set_link_up(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.link_up() - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - - def set_link_down(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.link_down() - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - def set_unmapped_device_down(self, hwaddr): dev = self._if_manager.get_device_by_hwaddr(hwaddr) if dev is not None: @@ -381,14 +331,6 @@ class SlaveMethods: dev2.del_configuration() return True
- def deconfigure_interface(self, if_id): - device = self._if_manager.get_mapped_device(if_id) - if device is not None: - device.clear_configuration() - else: - logging.error("No device with id '%s' to deconfigure." % if_id) - return True - def start_packet_capture(self, filt): if not is_installed("tcpdump"): raise Exception("Can't start packet capture, tcpdump not available") @@ -878,46 +820,10 @@ class SlaveMethods: brt.set_state(br_state_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: - logging.error("Device with id '%s' not found." % if_id) - 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: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - def wait_interface_init(self): self._if_manager.wait_interface_init() return True
- def slave_add(self, if_id, slave_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.slave_add(slave_id) - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - - def slave_del(self, if_id, slave_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.slave_del(slave_id) - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - def _is_systemd(self): stdout, _ = exec_cmd("pidof systemd", die_on_err=False) return len(stdout) != 0 @@ -939,39 +845,6 @@ class SlaveMethods: def get_num_cpus(self): return int(os.sysconf('SC_NPROCESSORS_ONLN'))
- def get_ethtool_stats(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if not dev: - logging.error("Device with id '%s' not found." % if_id) - return False - return dev.get_ethtool_stats() - - def enable_lldp(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if not dev: - logging.error("Device with id '%s' not found." % if_id) - return False - dev.enable_lldp() - return True - - def set_pause_on(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.set_pause_on() - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - - def set_pause_off(self, if_id): - dev = self._if_manager.get_mapped_device(if_id) - if dev is not None: - dev.set_pause_off() - else: - logging.error("Device with id '%s' not found." % if_id) - return False - return True - class ServerHandler(ConnectionHandler): def __init__(self, addr): super(ServerHandler, self).__init__()
Like the function that were changed in the last commit (44cbcd44), there are also many bridge function that have no logic of their own and all they do is to call an inner function. They all do: - Get a device from dev_id - Log error and return False is this device not exist. - Create a bridge from the device name. - Call a bridge function with the same name of their own, but without the "br_" prefix. - Return True. (beside the get_fdb function that returns the inner function results).
This commit adds the option of bridge function to _call_dev_func and replaces 10 bridge functions with lambdas.
Signed-off-by: Nogah Frankel nogahf@mellanox.com --- lnst/Slave/NetTestSlave.py | 103 ++++++--------------------------------------- 1 file changed, 14 insertions(+), 89 deletions(-)
diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 4e69955..eb1ec6a 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -68,7 +68,7 @@ class SlaveMethods: self._init_manufacture_funcs()
def _call_dev_func(self, inputs, func_name, print_err=True, - error_value=False, return_res=False): + error_value=False, return_res=False, is_bridge=False): inputs = list(inputs) if_id = inputs.pop(0) dev = self._if_manager.get_mapped_device(if_id) @@ -76,6 +76,8 @@ class SlaveMethods: if print_err: logging.error("Device with id '%s' not found." % if_id) return error_value + if is_bridge: + dev = BridgeTool(dev.get_name()) func = getattr(dev, func_name) res = func(*inputs) if return_res: @@ -103,8 +105,15 @@ class SlaveMethods: ("set_link_down", {"func_name": "link_down"}), ("deconfigure_interface", {"func_name": "clear_configuration", "error_value": True}), + ("get_br_fdbs", {"func_name": "get_fdbs", "is_bridge": True, + "return_res": True}), ]
+ _BRIDGE_FUNCS = ["add_br_vlan", "del_br_vlan", "get_br_vlans", + "add_br_fdb", "del_br_fdb", "set_br_learning", + "set_br_learning_sync", "set_br_flooding", + "set_br_state"] + def _init_manufacture_funcs(self): for name, options in self._DEV_FUNC_OPTIONS: if options.has_key("func_name"): @@ -114,6 +123,10 @@ class SlaveMethods: setattr(self, name, self._create_dev_call(func_name, options)) for name in self._DEV_FUNCS: setattr(self, name, self._create_dev_call(name)) + for name in self._BRIDGE_FUNCS: + func_name = name.replace("br_", "") + setattr(self, name, self._create_dev_call(func_name, + {"is_bridge": True}))
def hello(self, recipe_path): self.machine_cleanup() @@ -732,94 +745,6 @@ 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 - - def get_br_vlans(self, if_id): - 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()) - return brt.get_vlans() - - 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 - - def get_br_fdbs(self, if_id): - 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()) - return brt.get_fdbs() - - def set_br_learning(self, if_id, br_learning_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.set_learning(br_learning_info) - return True - - def set_br_learning_sync(self, if_id, br_learning_sync_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.set_learning_sync(br_learning_sync_info) - return True - - def set_br_flooding(self, if_id, br_flooding_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.set_flooding(br_flooding_info) - return True - - def set_br_state(self, if_id, br_state_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.set_state(br_state_info) - return True - def wait_interface_init(self): self._if_manager.wait_interface_init() return True
In the class InterfaceAPI there are many functions of the following prototype:
def <func_name>(self, <args>): return self._if.<func_name>(<args>)
or sometimes: def <func_name>(self, <args>): return VolatileValue(self._if.<func_name>, <args>)
All this function holds no logic of their own and can be generated in run time. This commit replaces 25 functions of this type with lambdas created in the init.
Signed-off-by: Nogah Frankel nogahf@mellanox.com --- lnst/Controller/Task.py | 93 ++++++++++++------------------------------------- 1 file changed, 23 insertions(+), 70 deletions(-)
diff --git a/lnst/Controller/Task.py b/lnst/Controller/Task.py index 6fd1946..3c56025 100644 --- a/lnst/Controller/Task.py +++ b/lnst/Controller/Task.py @@ -557,24 +557,34 @@ class InterfaceAPI(object): def __init__(self, interface, host): self._if = interface self._host = host + self._init_manufacture_funcs()
- def get_id(self): - return self._if.get_id() - - def get_type(self): - return self._if.get_type() + def _if_func_call(self, name, args=[], dict_args={}, is_volatile=False): + func = getattr(self._if, name) + if is_volatile: + return VolatileValue(func, *args, **dict_args) + else: + return func(*args, **dict_args)
- def get_network(self): - return self._if.get_network() + def _add_if_func(self, name, is_volatile=False): + func = lambda *args, **dict_args: \ + self._if_func_call(name, args, dict_args, is_volatile) + setattr(self, name, func)
- def get_driver(self): - return VolatileValue(self._if.get_driver) + _IF_FUNCS = ["get_id", "get_type", "get_network", "set_mtu", + "link_stats", "set_link_up", "set_link_down", "get_netns", + "set_addresses", "get_br_vlans", "get_br_fdbs", "set_speed", + "set_autoneg", "slave_add", "slave_del", "get_devlink_name", + "get_devlink_port_name", "get_ethtool_stats", "enable_lldp", + "set_pause_on", "set_pause_off"] + _IF_VOLATILES = ["get_driver", "get_devname", "get_hwaddr", "get_mtu"]
- def get_devname(self): - return VolatileValue(self._if.get_devname) + def _init_manufacture_funcs(self): + for i in self._IF_FUNCS: + self._add_if_func(i) + for i in self._IF_VOLATILES: + self._add_if_func(i, True)
- def get_hwaddr(self): - return VolatileValue(self._if.get_hwaddr)
def get_ip(self, ip_index=0, selector={}): return VolatileValue(self._if.get_address, ip_index) @@ -597,27 +607,9 @@ class InterfaceAPI(object): def get_ip_prefix(self, ip_index=0): return self.get_prefix(ip_index)
- def get_mtu(self): - return VolatileValue(self._if.get_mtu) - - def set_mtu(self, mtu): - return self._if.set_mtu(mtu) - - def link_stats(self): - return self._if.link_stats() - - def set_link_up(self): - return self._if.set_link_up() - - def set_link_down(self): - return self._if.set_link_down() - def get_host(self): return self._host
- def get_netns(self): - return self._if.get_netns() - def reset(self, ip=None, netns=None): self._if.address_cleanup() self._if.down() @@ -633,9 +625,6 @@ class InterfaceAPI(object): self._if.up() self._if.address_setup()
- def set_addresses(self, ips): - self._if.set_addresses(ips) - def add_route(self, dest, ipv6 = False): self._if.add_route(dest, ipv6)
@@ -669,9 +658,6 @@ class InterfaceAPI(object): "untagged": untagged, "self": self, "master": master})
- def get_br_vlans(self): - return self._if.get_br_vlans() - 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}) @@ -680,9 +666,6 @@ class InterfaceAPI(object): _self._if.del_br_fdb({"hwaddr": hwaddr, "self": self, "master": master, "vlan_id": vlan_tci})
- def get_br_fdbs(self): - return self._if.get_br_fdbs() - def set_br_learning(_self, on=True, self=False, master=False): _self._if.set_br_learning({"on": on, "self": self, "master": master})
@@ -696,36 +679,6 @@ class InterfaceAPI(object): def set_br_state(_self, state, self=False, master=False): _self._if.set_br_state({"state": state, "self": self, "master": master})
- def set_speed(self, speed): - return self._if.set_speed(speed) - - def set_autoneg(self): - return self._if.set_autoneg() - - def slave_add(self, slave_id): - return self._if.slave_add(slave_id) - - def slave_del(self, slave_id): - return self._if.slave_del(slave_id) - - def get_devlink_name(self): - return self._if.get_devlink_name() - - def get_devlink_port_name(self): - return self._if.get_devlink_port_name() - - def get_ethtool_stats(self): - return self._if.get_ethtool_stats() - - def enable_lldp(self): - return self._if.enable_lldp() - - def set_pause_on(self): - return self._if.set_pause_on() - - def set_pause_off(self): - return self._if.set_pause_off() - class ModuleAPI(object): """ An API class representing a module. """
On Mon, Oct 31, 2016 at 12:34:17PM +0200, Nogah Frankel wrote:
When one want to add a new function to the slave, one needs to add it also to the slave communicator (NetTestSlave), and to the controller side that control it (Macine) and to Task, that controls it. While some of this functions have logic, most of them are just calling a function of an object that they holds, usually, with the same name.
Those function can, and I think they should, be manufactured in run time for the following reasons:
- it shorten the code considerably (new code is 25% of the old one).
- it makes adding new function much shorter.
- it singles out the function that do have logic in them.
There are however some down sides to it:
- code is a bit harder to read at first.
- Inheriting function is a bit more tricky.
This is just a start and it can be expanded for:
- more classes with functions with this problem
- setter-getters without inner logic
- function with arguments with default values
I know that LNST is now in freeze and I don't know how much of it will be relevant after it. But in case it is, I will be glad to hear your opinions on it in the meantime.
Nogah Frankel (4): Machine: Interface: create RPC calls functions in run time Slave: NetTestSlave: SlaveMethods: create dev functions calls in run time Slave: NetTestSlave: SlaveMethods: create bridge function calls in run time Task: InterfaceAPI: create interface calls functions in run time
lnst/Controller/Machine.py | 106 ++++----------- lnst/Controller/Task.py | 93 ++++--------- lnst/Slave/NetTestSlave.py | 326 +++++++++------------------------------------ 3 files changed, 113 insertions(+), 412 deletions(-)
-- 2.4.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org
Hi,
I further discussed this with jpirko after last weeks meeting, and came to the conclusion that we only want to have 2 branches - master and next. master will stay frozen for now, accepting only fixes or new recipes and next is where we can put new experiments.
So if you want to apply this patchset to upstream, it would have to be rebased on top of the current next branch. Is that ok?
Meanwhile, I've finished updating the wiki and I'm going to work on the next branch from this week on. As discussed, I'll start by creating specifications for the new API, based on our previous discussions. From now on I should be fully focused on getting Python Recipes ready.
-Ondrej
-----Original Message----- From: Ondrej Lichtner [mailto:olichtne@redhat.com] Sent: Monday, November 07, 2016 1:13 PM To: Nogah Frankel nogahf@mellanox.com Cc: Jiri Pirko jiri@mellanox.com; Ido Schimmel idosch@mellanox.com; Elad Raz eladr@mellanox.com; Yotam Gigi yotamg@mellanox.com; lnst- developers@lists.fedorahosted.org Subject: Re: [RFC LNST 0/4] create functions without logic in run time
On Mon, Oct 31, 2016 at 12:34:17PM +0200, Nogah Frankel wrote:
When one want to add a new function to the slave, one needs to add it also to the slave communicator (NetTestSlave), and to the controller side that control it (Macine) and to Task, that controls it. While some of this functions have logic, most of them are just calling a function of an object that they holds, usually, with the same name.
Those function can, and I think they should, be manufactured in run time for the following reasons:
- it shorten the code considerably (new code is 25% of the old one).
- it makes adding new function much shorter.
- it singles out the function that do have logic in them.
There are however some down sides to it:
- code is a bit harder to read at first.
- Inheriting function is a bit more tricky.
This is just a start and it can be expanded for:
- more classes with functions with this problem
- setter-getters without inner logic
- function with arguments with default values
I know that LNST is now in freeze and I don't know how much of it will be relevant after it. But in case it is, I will be glad to hear your opinions on it in the meantime.
Nogah Frankel (4): Machine: Interface: create RPC calls functions in run time Slave: NetTestSlave: SlaveMethods: create dev functions calls in run time Slave: NetTestSlave: SlaveMethods: create bridge function calls in run time Task: InterfaceAPI: create interface calls functions in run time
lnst/Controller/Machine.py | 106 ++++----------- lnst/Controller/Task.py | 93 ++++--------- lnst/Slave/NetTestSlave.py | 326 +++++++++------------------------------------ 3 files changed, 113 insertions(+), 412 deletions(-)
-- 2.4.3 _______________________________________________ LNST-developers mailing list -- lnst-developers@lists.fedorahosted.org To unsubscribe send an email to lnst-developers-leave@lists.fedorahosted.org
Hi,
I further discussed this with jpirko after last weeks meeting, and came to the conclusion that we only want to have 2 branches - master and next. master will stay frozen for now, accepting only fixes or new recipes and next is where we can put new experiments.
So if you want to apply this patchset to upstream, it would have to be rebased on top of the current next branch. Is that ok?
Sorry about the late answer. It sounds great. I'll rebase, check and re-send.
Meanwhile, I've finished updating the wiki and I'm going to work on the next branch from this week on. As discussed, I'll start by creating specifications for the new API, based on our previous discussions. From now on I should be fully focused on getting Python Recipes ready.
-Ondrej
lnst-developers@lists.fedorahosted.org
-
Nogah Frankel -
Ondrej Lichtner