LNST-developers September 2014

lnst-developers@lists.fedorahosted.org

64 participants
119 discussions

[lnst] ServerHandler: inherit from ConnectionHandler
by Jiří Pírko 30 Sep '14

30 Sep '14

commit af4c2e09b6f5f8740401cc5be615924f153c2b09 Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Tue Sep 30 13:25:33 2014 +0200 ServerHandler: inherit from ConnectionHandler This patch makes the class ServerHandler a class derived from the ConnectionHandler. In addition to the standard connection_mapping it adds a new _netns_con_mapping that maps network namespaces to Pipe objects. This solves the problems of the previous implementation where we would have two ConnectionHandlers controlled by the ServerHandler which caused either 100% CPU consumption or slowdowns because of select timeouts. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Slave/NetTestSlave.py | 59 +++++++++++++++++++++----------------------- 1 files changed, 28 insertions(+), 31 deletions(-) --- diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index bd023cb..c63c719 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -608,10 +608,10 @@ class SlaveMethods: device.set_netns(None) return True -class ServerHandler(object): +class ServerHandler(ConnectionHandler): def __init__(self, addr): - self._connection_handler = ConnectionHandler() - self._netns_con_handler = ConnectionHandler() + super(ServerHandler, self).__init__() + self._netns_con_mapping = {} try: self._s_socket = socket.socket() self._s_socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) @@ -621,16 +621,15 @@ class ServerHandler(object): logging.error(e[1]) exit(1) - self._c_socket = None self._netns = None + self._c_socket = None def accept_connection(self): self._c_socket, addr = self._s_socket.accept() self._c_socket = (self._c_socket, addr[0]) logging.info("Recieved connection from %s" % self._c_socket[1]) - self._connection_handler.add_connection(self._c_socket[1], - self._c_socket[0]) + self.add_connection(self._c_socket[1], self._c_socket[0]) return self._c_socket def get_ctl_sock(self): @@ -644,8 +643,7 @@ class ServerHandler(object): self._c_socket.close() self._c_socket = None self._c_socket = sock - self._connection_handler.add_connection(self._c_socket[1], - self._c_socket[0]) + self.add_connection(self._c_socket[1], self._c_socket[0]) def close_s_sock(self): self._s_socket.close() @@ -653,12 +651,11 @@ class ServerHandler(object): def close_c_sock(self): self._c_socket[0].close() - self._connection_handler.remove_connection(self._c_socket[0]) + self.remove_connection(self._c_socket[0]) self._c_socket = None def get_messages(self): - messages = self._connection_handler.check_connections() - messages += self._netns_con_handler.check_connections() + messages = self.check_connections() #push ctl messages to the end of message queue, this ensures that #update messages are handled first @@ -672,18 +669,19 @@ class ServerHandler(object): messages = non_ctl_msgs + ctl_msgs addr = self._c_socket[1] - if self._connection_handler.get_connection(addr) == None: + if self.get_connection(addr) == None: logging.info("Lost controller connection.") self._c_socket = None return messages def get_messages_from_con(self, con_id): - if self._connection_handler.get_connection(con_id) != None: - return self._connection_handler.check_connections_by_id([con_id]) - elif self._netns_con_handler.get_connection(con_id) != None: - return self._netns_con_handler.check_connections_by_id([con_id]) + if con_id in self._connection_mapping: + connection = self._connection_mapping[con_id] + elif con_id in self._netns_con_mapping: + connection = self._netns_con_mapping[con_id] else: raise Exception("Unknown connection id '%s'." % con_id) + return self._check_connections([connection]) def send_data_to_ctl(self, data): if self._c_socket != None: @@ -696,39 +694,38 @@ class ServerHandler(object): return False def send_data_to_netns(self, netns, data): - netns_con = self._netns_con_handler.get_connection(netns) - if netns_con == None: + if netns not in self._netns_con_mapping: raise Exception("No such namespace!") else: + netns_con = self._netns_con_mapping[netns] return send_data(netns_con, data) - def add_connection(self, id, connection): - self._connection_handler.add_connection(id, connection) - - def remove_connection(self, key): - connection = self._connection_handler.get_connection(key) - self._connection_handler.remove_connection(connection) - def clear_connections(self): - self._connection_handler.clear_connections() + super(ServerHandler, self).clear_connections() + self._netns_con_mapping = {} def update_connections(self, connections): for key, connection in connections.iteritems(): - self.remove_connection(key) + self.remove_connection_by_id(key) self.add_connection(key, connection) def set_netns(self, netns): self._netns = netns def add_netns(self, netns, connection): - self._netns_con_handler.add_connection(netns, connection) + self._connections.append(connection) + self._netns_con_mapping[netns] = connection def del_netns(self, netns): - connection = self._netns_con_handler.get_connection(netns) - self._netns_con_handler.remove_connection(connection) + if netns in self._netns_con_mapping: + connection = self._netns_con_mapping[netns] + self._connections.remove(connection) + del self._netns_con_mapping[netns] def clear_netns_connections(self): - self._netns_con_handler.clear_connections() + for netns, con in self._netns_con_mapping: + self._connections.remove(con) + self._netns_con_mapping = {} class NetTestSlave: def __init__(self, log_ctl, port = DefaultRPCPort):

1 0

[lnst] MessageDispatcher: use connection_mapping
by Jiří Pírko 30 Sep '14

30 Sep '14

commit 85385ca00c3ae27569994650c0a96ecf253562af Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Tue Sep 30 13:25:32 2014 +0200 MessageDispatcher: use connection_mapping This patch updates both the MessageDispatcher and the MessageDispatcherLite classes to use the _connection_mapping attribute of the parent class. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Controller/MessageDispatcherLite.py | 4 ++-- lnst/Controller/NetTestController.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) --- diff --git a/lnst/Controller/MessageDispatcherLite.py b/lnst/Controller/MessageDispatcherLite.py index 70a006a..8e26d17 100644 --- a/lnst/Controller/MessageDispatcherLite.py +++ b/lnst/Controller/MessageDispatcherLite.py @@ -31,11 +31,11 @@ class MessageDispatcherLite(ConnectionHandler): def wait_for_result(self, machine_id): wait = True while wait: - connected_slaves = self._connections.keys() + connected_slaves = self._connection_mapping.keys() messages = self.check_connections() - remaining_slaves = self._connections.keys() + remaining_slaves = self._connection_mapping.keys() for msg in messages: if msg[1]["type"] == "result" and msg[0] == 1: diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py index da7df41..bdf1bd3 100644 --- a/lnst/Controller/NetTestController.py +++ b/lnst/Controller/NetTestController.py @@ -836,11 +836,11 @@ class MessageDispatcher(ConnectionHandler): def wait_for_result(self, machine_id): wait = True while wait: - connected_slaves = self._connections.keys() + connected_slaves = self._connection_mapping.keys() messages = self.check_connections() - remaining_slaves = self._connections.keys() + remaining_slaves = self._connection_mapping.keys() for msg in messages: if msg[1]["type"] == "result" and msg[0] == machine_id:

1 0

[lnst] ConnectionHandler: reimplementation
by Jiří Pírko 30 Sep '14

30 Sep '14

commit 01feb9bb03c8cd91bf2709a9871e0dab0a4f8c4d Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Tue Sep 30 13:25:31 2014 +0200 ConnectionHandler: reimplementation This commit reimplements the ConnectionHandler class while kkeping the same interface. The main point was to store file handles in a list instead of a dictionary which can have name conflicts. The old dictionary is still used but only to map a connection name to the connection. This allows us to introduce more mappings in derived classes and keep the connection checking code from the main class. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Common/ConnectionHandler.py | 43 +++++++++++++++++++++++-------------- 1 files changed, 27 insertions(+), 16 deletions(-) --- diff --git a/lnst/Common/ConnectionHandler.py b/lnst/Common/ConnectionHandler.py index e8a6976..36349dc 100644 --- a/lnst/Common/ConnectionHandler.py +++ b/lnst/Common/ConnectionHandler.py @@ -67,18 +67,22 @@ def recv_data(s): class ConnectionHandler(object): def __init__(self): - self._connections = {} + self._connections = [] + self._connection_mapping = {} def check_connections(self): - return self.check_connections_by_id(self._connections.keys()) + return self._check_connections(self._connections) def check_connections_by_id(self, connection_ids): connections = [] for con_id in connection_ids: - connections.append(self._connections[con_id]) + connections.append(self._connection_mapping[con_id]) + return self._check_connections(connections) + + def _check_connections(self, connections): requests = [] try: - rl, wl, xl = select.select(connections, [], [], 0) + rl, wl, xl = select.select(connections, [], []) except select.error: return [] for f in rl: @@ -112,27 +116,34 @@ class ConnectionHandler(object): return requests def get_connection(self, id): - if id in self._connections: - return self._connections[id] + if id in self._connection_mapping: + return self._connection_mapping[id] else: return None def get_connection_id(self, connection): - for id in self._connections: - if self._connections[id] == connection: + for id in self._connection_mapping: + if self._connection_mapping[id] == connection: return id return None def add_connection(self, id, connection): - if id not in self._connections: - self._connections[id] = connection + if id not in self._connection_mapping: + self._connections.append(connection) + self._connection_mapping[id] = connection def remove_connection(self, connection): - d = {} - for key, value in self._connections.iteritems(): - if value != connection: - d[key] = value - self._connections = d + if connection in self._connections: + id = self.get_connection_id(connection) + self._connections.remove(connection) + del self._connection_mapping[id] + + def remove_connection_by_id(self, id): + if id in self._connection_mapping: + connection = self._connection_mapping[id] + self._connections.remove(connection) + del self._connection_mapping[id] def clear_connections(self): - self._connections = {} + self._connections = [] + self._connection_mapping = {}

1 0

[lnst] Machine: unmap veth devices during deconfiguration
by Jiří Pírko 30 Sep '14

30 Sep '14

commit 44b54a4444f3bb64d25f79f6b07cbed7bdee39d2 Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Tue Sep 30 13:25:00 2014 +0200 Machine: unmap veth devices during deconfiguration This patch adds two unmap_if calls that were forgotten in the recent reimplementation of interface (un)mapping. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Controller/Machine.py | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) --- diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index b1b4ecf..cfc4d77 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -882,6 +882,8 @@ class SoftInterface(Interface): peer_if = self._machine.get_interface(self._peer) self._machine._rpc_call("deconfigure_if_pair", self._id, self._peer) + self._machine._rpc_call("unmap_if", self._id) + self._machine._rpc_call("unmap_if", self._peer) self._configured = False peer_if._configured = False

1 0

[PATCH 1/3] ConnectionHandler: reimplementation
by Ondrej Lichtner 30 Sep '14

30 Sep '14

From: Ondrej Lichtner <olichtne(a)redhat.com> This commit reimplements the ConnectionHandler class while kkeping the same interface. The main point was to store file handles in a list instead of a dictionary which can have name conflicts. The old dictionary is still used but only to map a connection name to the connection. This allows us to introduce more mappings in derived classes and keep the connection checking code from the main class. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/Common/ConnectionHandler.py | 43 +++++++++++++++++++++++++--------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/lnst/Common/ConnectionHandler.py b/lnst/Common/ConnectionHandler.py index e8a6976..36349dc 100644 --- a/lnst/Common/ConnectionHandler.py +++ b/lnst/Common/ConnectionHandler.py @@ -67,18 +67,22 @@ def recv_data(s): class ConnectionHandler(object): def __init__(self): - self._connections = {} + self._connections = [] + self._connection_mapping = {} def check_connections(self): - return self.check_connections_by_id(self._connections.keys()) + return self._check_connections(self._connections) def check_connections_by_id(self, connection_ids): connections = [] for con_id in connection_ids: - connections.append(self._connections[con_id]) + connections.append(self._connection_mapping[con_id]) + return self._check_connections(connections) + + def _check_connections(self, connections): requests = [] try: - rl, wl, xl = select.select(connections, [], [], 0) + rl, wl, xl = select.select(connections, [], []) except select.error: return [] for f in rl: @@ -112,27 +116,34 @@ class ConnectionHandler(object): return requests def get_connection(self, id): - if id in self._connections: - return self._connections[id] + if id in self._connection_mapping: + return self._connection_mapping[id] else: return None def get_connection_id(self, connection): - for id in self._connections: - if self._connections[id] == connection: + for id in self._connection_mapping: + if self._connection_mapping[id] == connection: return id return None def add_connection(self, id, connection): - if id not in self._connections: - self._connections[id] = connection + if id not in self._connection_mapping: + self._connections.append(connection) + self._connection_mapping[id] = connection def remove_connection(self, connection): - d = {} - for key, value in self._connections.iteritems(): - if value != connection: - d[key] = value - self._connections = d + if connection in self._connections: + id = self.get_connection_id(connection) + self._connections.remove(connection) + del self._connection_mapping[id] + + def remove_connection_by_id(self, id): + if id in self._connection_mapping: + connection = self._connection_mapping[id] + self._connections.remove(connection) + del self._connection_mapping[id] def clear_connections(self): - self._connections = {} + self._connections = [] + self._connection_mapping = {} -- 1.9.3

2 3

[PATCH] Machine: unmap veth devices during deconfiguration
by Ondrej Lichtner 30 Sep '14

30 Sep '14

From: Ondrej Lichtner <olichtne(a)redhat.com> This patch adds two unmap_if calls that were forgotten in the recent reimplementation of interface (un)mapping. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> --- lnst/Controller/Machine.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index b1b4ecf..cfc4d77 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -882,6 +882,8 @@ class SoftInterface(Interface): peer_if = self._machine.get_interface(self._peer) self._machine._rpc_call("deconfigure_if_pair", self._id, self._peer) + self._machine._rpc_call("unmap_if", self._id) + self._machine._rpc_call("unmap_if", self._peer) self._configured = False peer_if._configured = False -- 1.9.3

2 1

千事吉祥
by 裘吟龙 30 Sep '14

30 Sep '14

千事吉祥

1 0

[lnst] InterfaceManager: keep information about peered interfaces
by Jiří Pírko 29 Sep '14

29 Sep '14

commit cc8de6a5b26158dcc1d99cbc5c23f64ddaf533f1 Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Mon Sep 29 16:08:28 2014 +0200 InterfaceManager: keep information about peered interfaces Device object now know when they're paired with another interface (veth) this is needed to properly deconfigure them when deconfiguration is not coordinated by the controller. This patch also adds the get_id_by_if_index method to the InterfaceManager which is a helper function that implements the reverse mapping of if_index to if_id. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Slave/InterfaceManager.py | 16 ++++++++++++++++ lnst/Slave/NetTestSlave.py | 11 +++++++++++ 2 files changed, 27 insertions(+), 0 deletions(-) --- diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index fb191b0..81347a6 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -53,6 +53,12 @@ class InterfaceManager(object): def clear_if_mapping(self): self._id_mapping = {} + def get_id_by_if_index(self, if_index): + for if_id, index in self._id_mapping.iteritems(): + if if_index == index: + return if_id + return None + def reconnect_netlink(self): if self._nl_socket != None: self._nl_socket.close() @@ -173,6 +179,9 @@ class InterfaceManager(object): device2.set_configuration(config2) device1.create() + device1.set_peer(device2) + device2.set_peer(device1) + self._tmp_mapping[if_id1] = device1 self._tmp_mapping[if_id2] = device2 return name1, name2 @@ -248,6 +257,7 @@ class Device(object): self._master = {"primary": None, "other": []} self._slaves = [] self._netns = None + self._peer = None self._if_manager = if_manager @@ -332,6 +342,12 @@ class Device(object): def get_conf_dict(self): return self._conf_dict + def set_peer(self, dev): + self._peer = dev + + def get_peer(self): + return self._peer + def set_configuration(self, conf): self.clear_configuration() if "name" not in conf or conf["name"] == None: diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index 14d1d48..bd023cb 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -371,6 +371,17 @@ class SlaveMethods: def machine_cleanup(self): logging.info("Performing machine cleanup.") self._command_context.cleanup() + + devs = self._if_manager.get_mapped_devices() + for if_id, dev in devs.iteritems(): + peer = dev.get_peer() + if peer == None: + dev.clear_configuration() + else: + peer_if_index = peer.get_if_index() + peer_id = self._if_manager.get_id_by_if_index(peer_if_index) + self.deconfigure_if_pair(if_id, peer_id) + self._if_manager.deconfigure_all() for netns in self._net_namespaces.keys():

1 0

[lnst] NetTestSlave: don't remove netns in the bye method
by Jiří Pírko 29 Sep '14

29 Sep '14

commit 90a8efda33a17f30a2c579ac2bcc01142e1cba5b Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Mon Sep 29 16:08:27 2014 +0200 NetTestSlave: don't remove netns in the bye method Removing network namespaces in the "bye" method removes part of the configuration which shouldn't happen on config_only runs. This patch fixes that. Instead we remove network namespaces in machine_cleanup which is called by the "hello" method. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Slave/NetTestSlave.py | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-) --- diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index eeb0337..14d1d48 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -99,10 +99,6 @@ class SlaveMethods: self._cache.del_old_entries() self.reset_file_transfers() self._remove_capture_files() - - for netns in self._net_namespaces.keys(): - self.del_namespace(netns) - self._net_namespaces = {} return "bye" def kill_cmds(self):

1 0

[lnst] Machine: deconfiguration fixes
by Jiří Pírko 29 Sep '14

29 Sep '14

commit f0579e1a04a420b1294dd6dc8cd3bc84722d12e8 Author: Ondrej Lichtner <olichtne(a)redhat.com> Date: Mon Sep 29 16:08:26 2014 +0200 Machine: deconfiguration fixes Interface configuration starts with mapping the interface id to a kernel interface index. This mapping was previously only globally cleared by the slave when new configuration started. This patch makes the controller unmap interfaces one by one in the correct order. One other minor change - restoring the use_nm option now happens before calling the "bye" method and not is not called in the "finally" block since the connection might not be functional anymore. Signed-off-by: Ondrej Lichtner <olichtne(a)redhat.com> Signed-off-by: Jiri Pirko <jiri(a)resnulli.us> lnst/Controller/Machine.py | 20 +++++++++++++++++--- 1 files changed, 17 insertions(+), 3 deletions(-) --- diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 33d1795..b1b4ecf 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -223,6 +223,7 @@ class Machine(object): self.del_namespaces() + self.restore_nm_option() self._rpc_call("bye") except: #cleanup is only meaningful on dynamic interfaces, and should @@ -234,7 +235,6 @@ class Machine(object): iface.cleanup() raise finally: - self.restore_nm_option() self._msg_dispatcher.disconnect_slave(self.get_id()) self._configured = False @@ -629,7 +629,7 @@ class Interface(object): self.down() def cleanup(self): - pass + self._machine._rpc_call("unmap_if", self._id) def configure(self): if self._configured: @@ -657,7 +657,8 @@ class Interface(object): if self._netns != None: self._machine._rpc_call_to_netns(self._netns, "deconfigure_interface", self.get_id()) - self._machine._rpc_call_to("return_if_netns", 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 @@ -689,6 +690,9 @@ class LoopbackInterface(Interface): def initialize(self): pass + def cleanup(self): + pass + def configure(self): self._hwaddr = '00:00:00:00:00:00' if self._netns: @@ -735,8 +739,11 @@ class LoopbackInterface(Interface): 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._configured = False class VirtualInterface(Interface): @@ -815,6 +822,7 @@ class VirtualInterface(Interface): super(VirtualInterface, self).initialize() def cleanup(self): + self._machine._rpc_call("unmap_if", self._id) domain_ctl = self._machine.get_domain_ctl() domain_ctl.detach_interface(self._orig_hwaddr) @@ -835,6 +843,9 @@ class SoftInterface(Interface): def initialize(self): pass + def cleanup(self): + pass + def configure(self): if self._configured: return @@ -879,8 +890,11 @@ class SoftInterface(Interface): 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._configured = False class UnusedInterface(Interface):

1 0

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

LNST-developers September 2014