From: Ondrej Lichtner olichtne@redhat.com

The up() methods of NetConfigDevice and NmConfigDevice derived classes handled both setting the device up and applying ip addresses (activating connections, with ip addresses). Unfortunately this approach causes problems when using NM to configure interfaces in master-slave relationships (e.g. bond with ipv6 address and simple ethernet with no configuration).

The solution to our NM related issues was to activate all connections without ip address configuration, then add ip addresses to the connections and call Reapply().

For this to work properly I also split the corresponding up() method in NetConfigDevice classes so that the interfaces are compatible.

I split the inverse down() method into address_cleanup+down for consistency.

This commit also propagates these changes to the rest of the LNST code that needs to now use these new methods.

Signed-off-by: Ondrej Lichtner olichtne@redhat.com

---

v2: override address_{setup, cleanup} methods of the UnusedInterface class to fix a bug that left the Slave in an unusable state

v3: remove the workaround for L2 ethernet connections - this now works, but having the workaround there breaks the other changes introduced in this commit

Signed-off-by: Ondrej Lichtner olichtne@redhat.com --- lnst/Controller/Machine.py | 12 +++++ lnst/Controller/NetTestController.py | 3 ++ lnst/Slave/InterfaceManager.py | 9 ++++ lnst/Slave/NetConfigDevice.py | 36 ++++++++++++--- lnst/Slave/NetTestSlave.py | 14 ++++++ lnst/Slave/NmConfigDevice.py | 85 +++++++++++++++++++++++++----------- 6 files changed, 127 insertions(+), 32 deletions(-)

diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 70295ba..204a1a8 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -835,6 +835,12 @@ class Interface(object): 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)

@@ -1214,6 +1220,12 @@ class UnusedInterface(Interface): def down(self): pass

+ def address_setup(self): + pass + + def address_cleanup(self): + pass + def cleanup(self): pass

diff --git a/lnst/Controller/NetTestController.py b/lnst/Controller/NetTestController.py index a040933..860f727 100644 --- a/lnst/Controller/NetTestController.py +++ b/lnst/Controller/NetTestController.py @@ -225,6 +225,9 @@ class NetTestController: for iface in ifaces: iface.up()

+ for iface in ifaces: + iface.address_setup() + m.wait_interface_init()

def provision_machines(self): diff --git a/lnst/Slave/InterfaceManager.py b/lnst/Slave/InterfaceManager.py index 3dc3314..949817c 100644 --- a/lnst/Slave/InterfaceManager.py +++ b/lnst/Slave/InterfaceManager.py @@ -566,6 +566,7 @@ class Device(object): m_dev.clear_configuration()

if self._conf != None: + self.address_cleanup() self.down() self.deconfigure() self._clear_tc_qdisc() @@ -652,6 +653,14 @@ class Device(object): else: exec_cmd("ip link set %s down" % self._name)

+ def address_setup(self): + if self._conf != None: + self._conf.address_setup() + + def address_cleanup(self): + if self._conf != None: + self._conf.address_cleanup() + def link_up(self): exec_cmd("ip link set %s up" % self._name)

diff --git a/lnst/Slave/NetConfigDevice.py b/lnst/Slave/NetConfigDevice.py index 960ff07..152764b 100644 --- a/lnst/Slave/NetConfigDevice.py +++ b/lnst/Slave/NetConfigDevice.py @@ -57,18 +57,24 @@ class NetConfigDeviceGeneric(object):

def up(self): config = self._dev_config + exec_cmd("ip link set %s up" % config["name"]) + + def down(self): + config = self._dev_config + exec_cmd("ip link set %s down" % config["name"]) + + def address_setup(self): + config = self._dev_config if "addresses" in config: for address in config["addresses"]: exec_cmd("ip addr add %s dev %s" % (address, config["name"])) - exec_cmd("ip link set %s up" % config["name"])

- def down(self): + def address_cleanup(self): config = self._dev_config if "addresses" in config: for address in config["addresses"]: exec_cmd("ip addr del %s dev %s" % (address, config["name"]), die_on_err=False) - exec_cmd("ip link set %s down" % config["name"])

def set_addresses(self, ips): self._dev_config["addresses"] = ips @@ -432,9 +438,6 @@ class NetConfigDeviceOvsBridge(NetConfigDeviceGeneric): int_ports = self._dev_config["ovs_conf"]["internals"] br_name = self._dev_config["name"] for iport in int_ports: - if "addresses" in iport: - for address in iport["addresses"]: - exec_cmd("ip addr add %s dev %s" % (address, iport["name"])) exec_cmd("ip link set %s up" % iport["name"])

def down(self): @@ -448,6 +451,27 @@ class NetConfigDeviceOvsBridge(NetConfigDeviceGeneric):

super(NetConfigDeviceOvsBridge, self).down()

+ def address_setup(self): + super(NetConfigDeviceOvsBridge, self).up() + + int_ports = self._dev_config["ovs_conf"]["internals"] + br_name = self._dev_config["name"] + for iport in int_ports: + if "addresses" in iport: + for address in iport["addresses"]: + exec_cmd("ip addr add %s dev %s" % (address, iport["name"])) + + def address_cleanup(self): + super(NetConfigDeviceOvsBridge, self).up() + + int_ports = self._dev_config["ovs_conf"]["internals"] + br_name = self._dev_config["name"] + for iport in int_ports: + if "addresses" in iport: + for address in iport["addresses"]: + exec_cmd("ip addr del %s dev %s" % (address, iport["name"]), + die_on_err=False) + @classmethod def type_init(self): super(NetConfigDeviceOvsBridge, self).type_init() diff --git a/lnst/Slave/NetTestSlave.py b/lnst/Slave/NetTestSlave.py index f0b3183..da1e2f4 100644 --- a/lnst/Slave/NetTestSlave.py +++ b/lnst/Slave/NetTestSlave.py @@ -254,6 +254,20 @@ class SlaveMethods: 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: diff --git a/lnst/Slave/NmConfigDevice.py b/lnst/Slave/NmConfigDevice.py index a9e0c1f..8f9170e 100644 --- a/lnst/Slave/NmConfigDevice.py +++ b/lnst/Slave/NmConfigDevice.py @@ -133,6 +133,34 @@ class NmConfigDeviceGeneric(object): if self._connection_added: self._nm_deactivate_connection()

+ def address_setup(self): + config = self._dev_config + + if len(config["addresses"]) == 0: + return + else: + s_ipv4, s_ipv6 = self._nm_make_ip_settings(config["addresses"]) + + self._connection["ipv4"] = s_ipv4 + self._connection["ipv6"] = s_ipv6 + + self._nm_update_connection() + + self._nm_reapply_connection() + + def address_cleanup(self): + if self._connection['ipv4']['method'] == 'disabled' and\ + self._connection['ipv6']['method'] == 'ignore': + return + + self._connection["ipv4"] = dbus.Dictionary({'method': 'disabled'}, + signature='sv') + self._connection["ipv6"] = dbus.Dictionary({'method': 'ignore'}, + signature='sv') + self._nm_update_connection() + + self._nm_reapply_connection() + @classmethod def type_init(self): pass @@ -217,13 +245,6 @@ class NmConfigDeviceGeneric(object): return (s_ipv4, s_ipv6)

def _nm_add_connection(self): - #NM will succesfully add this connection but is unable to activate it... - if self._connection["ipv4"]["method"] == "disabled" and\ - self._connection["ipv6"]["method"] == "ignore" and\ - "master" not in self._connection["connection"] and\ - self._connection["connection"]["type"] == "802-3-ethernet": - return - if not self._connection_added: bus = self._bus settings_obj = bus.get_object(NM_BUS, OBJ_PRE + "/Settings") @@ -302,6 +323,26 @@ class NmConfigDeviceGeneric(object): raise e self._acon_obj_path = None

+ def _nm_reapply_connection(self): + if self._acon_obj_path == None: + return + + config = self._dev_config + + bus = self._bus + nm_if = self._nm_if + + try: + device_obj_path = nm_if.GetDeviceByIpIface(config["name"]) + except: + logging.error("Device needed to reapply connection doesn't exist") + return + + dev_obj = bus.get_object(NM_BUS, device_obj_path) + dev_if = dbus.Interface(dev_obj, IF_PRE + ".Device") + + dev_if.Reapply("", 0, 0) + def nm_enslave(self, slave_type, master_uuid, slave_conf): if get_nm_version() < "1.0.0": self._connection["connection"]["slave_type"] = slave_type @@ -367,8 +408,6 @@ class NmConfigDeviceEth(NmConfigDeviceGeneric):

hw_addr = self._convert_hwaddr(config)

- s_ipv4, s_ipv6 = self._nm_make_ip_settings(config["addresses"]) - s_eth = dbus.Dictionary({'mac-address': hw_addr}, signature='sv') s_con = dbus.Dictionary({ 'type': '802-3-ethernet', @@ -379,8 +418,9 @@ class NmConfigDeviceEth(NmConfigDeviceGeneric): connection = dbus.Dictionary({ '802-3-ethernet': s_eth, 'connection': s_con, - 'ipv4': s_ipv4, - 'ipv6': s_ipv6}, signature='sa{sv}') + 'ipv4': dbus.Dictionary({'method': 'disabled'}, signature='sv'), + 'ipv6': dbus.Dictionary({'method': 'ignore'}, signature='sv'), + }, signature='sa{sv}')

self._connection = connection self._nm_add_connection() @@ -468,12 +508,11 @@ class NmConfigDeviceBond(NmConfigDeviceGeneric): s_bond = dbus.Dictionary({ 'interface-name': config["name"]})

- s_ipv4, s_ipv6 = self._nm_make_ip_settings(config["addresses"])

connection = dbus.Dictionary({ 'bond': s_bond, - 'ipv4': s_ipv4, - 'ipv6': s_ipv6, + 'ipv4': dbus.Dictionary({'method': 'disabled'}, signature='sv'), + 'ipv6': dbus.Dictionary({'method': 'ignore'}, signature='sv'), 'connection': s_bond_con})

self._connection = connection @@ -559,12 +598,10 @@ class NmConfigDeviceBridge(NmConfigDeviceGeneric): 'interface-name': config["name"], 'stp': dbus.Boolean(False)})

- s_ipv4, s_ipv6 = self._nm_make_ip_settings(config["addresses"]) - connection = dbus.Dictionary({ 'bridge': s_bridge, - 'ipv4': s_ipv4, - 'ipv6': s_ipv6, + 'ipv4': dbus.Dictionary({'method': 'disabled'}, signature='sv'), + 'ipv6': dbus.Dictionary({'method': 'ignore'}, signature='sv'), 'connection': s_bridge_con})

self._connection = connection @@ -686,12 +723,10 @@ class NmConfigDeviceVlan(NmConfigDeviceGeneric): 'parent': realdev, 'id': dbus.UInt32(vlan_tci)}, signature="sv")

- s_ipv4, s_ipv6 = self._nm_make_ip_settings(config["addresses"]) - connection = dbus.Dictionary({ 'vlan': s_vlan, - 'ipv4': s_ipv4, - 'ipv6': s_ipv6, + 'ipv4': dbus.Dictionary({'method': 'disabled'}, signature='sv'), + 'ipv6': dbus.Dictionary({'method': 'ignore'}, signature='sv'), 'connection': s_vlan_con})

self._connection = connection @@ -770,12 +805,10 @@ class NmConfigDeviceTeam(NmConfigDeviceGeneric): 'interface-name': config["name"], 'config': teamd_config})

- s_ipv4, s_ipv6 = self._nm_make_ip_settings(config["addresses"]) - connection = dbus.Dictionary({ 'team': s_team, - 'ipv4': s_ipv4, - 'ipv6': s_ipv6, + 'ipv4': dbus.Dictionary({'method': 'disabled'}, signature='sv'), + 'ipv6': dbus.Dictionary({'method': 'ignore'}, signature='sv'), 'connection': s_team_con})

self._connection = connection