The slaves of an interface are determined during creation time and stored in two objects: * Interface on the controller * NetConfigDeviceGeneric on the slave machine
When slaves are dynamically added / deleted during a Python task the 'slaves' data structures in these objects are not updated, which can cause several problems:
If a slave is removed from its master, then during cleanup the slave machine will try to remove it again and thereby generate an error. This requires us to update the data struct on the slave machine.
If a slave is added to a master and that master is later reset(), then the newly added slave won't be configured correctly after the reset(). This requires us to update the data struct on the controller.
Cc: Andrew Lunn andrew@lunn.ch Fixes: af025186a329 ("InterfaceAPI: Introduce slave_{add, del}") Signed-off-by: Ido Schimmel idosch@mellanox.com --- v1->v2: * Align team devices with bridge and only add / delete slaves from the "slaves" list when dynamically configured. --- lnst/Controller/Machine.py | 12 ++++++++++++ lnst/Slave/NetConfigDevice.py | 18 ++++++++++++++---- 2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/lnst/Controller/Machine.py b/lnst/Controller/Machine.py index 884b315..9897580 100644 --- a/lnst/Controller/Machine.py +++ b/lnst/Controller/Machine.py @@ -591,6 +591,12 @@ class Interface(object): else: self._master["other"].append(master)
+ def del_master(self, master): + if self._master["primary"] is master: + self._master["primary"] = None + else: + self._master["other"].remove(master) + def get_primary_master(self): return self._master["primary"]
@@ -601,6 +607,10 @@ class Interface(object): else: iface.add_master(self)
+ def del_slave(self, iface): + iface.del_master(self) + del self._slaves[iface.get_id()] + def set_slave_option(self, slave_id, name, value): if slave_id not in self._slave_options: self._slave_options[slave_id] = [] @@ -812,8 +822,10 @@ class Interface(object):
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))
def slave_del(self, if_id): + self.del_slave(self._machine.get_interface(if_id)) self._machine._rpc_call_x(self._netns, "slave_del", self._id, if_id)
class StaticInterface(Interface): diff --git a/lnst/Slave/NetConfigDevice.py b/lnst/Slave/NetConfigDevice.py index 60be2ec..dd053a6 100644 --- a/lnst/Slave/NetConfigDevice.py +++ b/lnst/Slave/NetConfigDevice.py @@ -212,8 +212,10 @@ class NetConfigDeviceBridge(NetConfigDeviceGeneric):
def slave_add(self, slave_id): self._add_rm_port("add", slave_id) + self._dev_config["slaves"].append(slave_id)
def slave_del(self, slave_id): + self._dev_config["slaves"].remove(slave_id) self._add_rm_port("del", slave_id)
class NetConfigDeviceMacvlan(NetConfigDeviceGeneric): @@ -379,14 +381,14 @@ class NetConfigDeviceTeam(NetConfigDeviceGeneric): self._ports_down()
for slave_id in get_slaves(self._dev_config): - self.slave_add(slave_id) + self._slave_add(slave_id) self._ports_up()
def deconfigure(self): for slave_id in get_slaves(self._dev_config): - self.slave_del(slave_id) + self._slave_del(slave_id)
- def slave_add(self, slave_id): + def _slave_add(self, slave_id): dev_name = self._dev_config["name"] port_dev = self._if_manager.get_mapped_device(slave_id) port_name = port_dev.get_name() @@ -400,13 +402,21 @@ class NetConfigDeviceTeam(NetConfigDeviceGeneric): port_dev.down() exec_cmd("teamdctl %s %s port add %s" % (dbus_option, dev_name, port_name))
- def slave_del(self, slave_id): + def slave_add(self, slave_id): + self._slave_add(slave_id) + self._dev_config["slaves"].append(slave_id) + + def _slave_del(self, slave_id): dev_name = self._dev_config["name"] port_dev = self._if_manager.get_mapped_device(slave_id) port_name = port_dev.get_name() dbus_option = "-D" if self._should_enable_dbus() else "" exec_cmd("teamdctl %s %s port remove %s" % (dbus_option, dev_name, port_name))
+ def slave_del(self, slave_id): + self._dev_config["slaves"].remove(slave_id) + self._slave_del(slave_id) + class NetConfigDeviceOvsBridge(NetConfigDeviceGeneric): _modulename = "openvswitch" _moduleload = True
Fixes: bfa1f380f017 ("recipes: Add recipeset for testing switchdev based switches") Signed-off-by: Ido Schimmel idosch@mellanox.com --- v1->v2: * No change. --- recipes/switchdev/l2-002-bridge_fdb.py | 15 +++++---------- recipes/switchdev/l2-017-bridge_fdb_vlan1d.py | 15 +++++---------- recipes/switchdev/l2-018-bridge_fdb_team.py | 15 +++++---------- recipes/switchdev/l2-019-bridge_fdb_team_vlan1d.py | 15 +++++---------- 4 files changed, 20 insertions(+), 40 deletions(-)
diff --git a/recipes/switchdev/l2-002-bridge_fdb.py b/recipes/switchdev/l2-002-bridge_fdb.py index 8e35758..0dfb11b 100644 --- a/recipes/switchdev/l2-002-bridge_fdb.py +++ b/recipes/switchdev/l2-002-bridge_fdb.py @@ -121,16 +121,11 @@ def do_task(ctl, hosts, ifaces, aliases): # bridge code will flush it himself, instead of driver. Unlike the # previous case, here we check if the driver correctly removes the static # entry. - # XXX: This currently fails because firmware doesn't flush static FDBs. - # Uncomment it when it's introduced. - #sw_br.slave_del(sw_if1.get_id()) - #sw_br.slave_add(sw_if1.get_id()) - #sw_if1.set_br_learning(on=False, self=True) - #sw_if1.set_br_flooding(on=False, self=True) - #tl.ping_simple(m1_if1, m2_if1, fail_expected=True) - - # XXX: Cleanup because firmware doesn't do it. - sw_if1.del_br_fdb(str(m1_if1.get_hwaddr()), self=True, vlan_tci=1) + sw_br.slave_del(sw_if1.get_id()) + sw_br.slave_add(sw_if1.get_id()) + sw_if1.set_br_learning(on=False, self=True) + sw_if1.set_br_flooding(on=False, self=True) + tl.ping_simple(m1_if1, m2_if1, fail_expected=True)
do_task(ctl, [ctl.get_host("machine1"), ctl.get_host("machine2"), diff --git a/recipes/switchdev/l2-017-bridge_fdb_vlan1d.py b/recipes/switchdev/l2-017-bridge_fdb_vlan1d.py index 3edcfc6..26a0020 100644 --- a/recipes/switchdev/l2-017-bridge_fdb_vlan1d.py +++ b/recipes/switchdev/l2-017-bridge_fdb_vlan1d.py @@ -123,16 +123,11 @@ def do_task(ctl, hosts, ifaces, aliases): # bridge code will flush it himself, instead of driver. Unlike the # previous case, here we check if the driver correctly removes the static # entry. - # XXX: This currently fails because firmware doesn't flush static FDBs. - # Uncomment it when it's introduced. - #sw_br.slave_del(sw_if1_10.get_id()) - #sw_br.slave_add(sw_if1_10.get_id()) - #sw_if1_10.set_br_learning(on=False, self=True) - #sw_if1_10.set_br_flooding(on=False, self=True) - #tl.ping_simple(m1_if1_10, m2_if1_20, fail_expected=True) - - # XXX: Cleanup because firmware doesn't do it. - sw_if1_10.del_br_fdb(str(m1_if1_10.get_hwaddr()), self=True) + sw_br.slave_del(sw_if1_10.get_id()) + sw_br.slave_add(sw_if1_10.get_id()) + sw_if1_10.set_br_learning(on=False, self=True) + sw_if1_10.set_br_flooding(on=False, self=True) + tl.ping_simple(m1_if1_10, m2_if1_20, fail_expected=True)
do_task(ctl, [ctl.get_host("machine1"), ctl.get_host("machine2"), diff --git a/recipes/switchdev/l2-018-bridge_fdb_team.py b/recipes/switchdev/l2-018-bridge_fdb_team.py index a4443f6..0cf75e8 100644 --- a/recipes/switchdev/l2-018-bridge_fdb_team.py +++ b/recipes/switchdev/l2-018-bridge_fdb_team.py @@ -125,16 +125,11 @@ def do_task(ctl, hosts, ifaces, aliases): # bridge code will flush it himself, instead of driver. Unlike the # previous case, here we check if the driver correctly removes the static # entry. - # XXX: This currently fails because firmware doesn't flush static FDBs. - # Uncomment it when it's introduced. - #sw_br.slave_del(sw_lag1.get_id()) - #sw_br.slave_add(sw_lag1.get_id()) - #sw_lag1.set_br_learning(on=False, self=True) - #sw_lag1.set_br_flooding(on=False, self=True) - #tl.ping_simple(m1_lag1, m2_lag1, fail_expected=True) - - # XXX: Cleanup because firmware doesn't do it. - sw_lag1.del_br_fdb(str(m1_lag1.get_hwaddr()), self=True, vlan_tci=1) + sw_br.slave_del(sw_lag1.get_id()) + sw_br.slave_add(sw_lag1.get_id()) + sw_lag1.set_br_learning(on=False, self=True) + sw_lag1.set_br_flooding(on=False, self=True) + tl.ping_simple(m1_lag1, m2_lag1, fail_expected=True)
do_task(ctl, [ctl.get_host("machine1"), ctl.get_host("machine2"), diff --git a/recipes/switchdev/l2-019-bridge_fdb_team_vlan1d.py b/recipes/switchdev/l2-019-bridge_fdb_team_vlan1d.py index 3d588d0..7784997 100644 --- a/recipes/switchdev/l2-019-bridge_fdb_team_vlan1d.py +++ b/recipes/switchdev/l2-019-bridge_fdb_team_vlan1d.py @@ -132,16 +132,11 @@ def do_task(ctl, hosts, ifaces, aliases): # bridge code will flush it himself, instead of driver. Unlike the # previous case, here we check if the driver correctly removes the static # entry. - # XXX: This currently fails because firmware doesn't flush static FDBs. - # Uncomment it when it's introduced. - #sw_br.slave_del(sw_lag1_10.get_id()) - #sw_br.slave_add(sw_lag1_10.get_id()) - #sw_lag1_10.set_br_learning(on=False, self=True) - #sw_lag1_10.set_br_flooding(on=False, self=True) - #tl.ping_simple(m1_lag1_10, m2_lag1_20, fail_expected=True) - - # XXX: Cleanup because firmware doesn't do it. - sw_lag1_10.del_br_fdb(str(m1_lag1_10.get_hwaddr()), self=True) + sw_br.slave_del(sw_lag1_10.get_id()) + sw_br.slave_add(sw_lag1_10.get_id()) + sw_lag1_10.set_br_learning(on=False, self=True) + sw_lag1_10.set_br_flooding(on=False, self=True) + tl.ping_simple(m1_lag1_10, m2_lag1_20, fail_expected=True)
do_task(ctl, [ctl.get_host("machine1"), ctl.get_host("machine2"),
When removing a VLAN interface from a 802.1d bridge one needs to do a proper cleanup, so that the interface will direct traffic to CPU instead.
Remove a VLAN interface from a bridge and check that ping works.
Cc: Andrew Lunn andrew@lunn.ch Cc: Vivien Didelot vivien.didelot@savoirfairelinux.com Signed-off-by: Ido Schimmel idosch@mellanox.com --- v1->v2: * No change. --- recipes/switchdev/l2-010-bridge_vlan1d_sanity.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/recipes/switchdev/l2-010-bridge_vlan1d_sanity.py b/recipes/switchdev/l2-010-bridge_vlan1d_sanity.py index 2c0062f..b30f741 100644 --- a/recipes/switchdev/l2-010-bridge_vlan1d_sanity.py +++ b/recipes/switchdev/l2-010-bridge_vlan1d_sanity.py @@ -34,9 +34,9 @@ def do_task(ctl, hosts, ifaces, aliases): br_options = {"vlan_filtering": 1} sw.create_bridge(slaves=[sw_if1, sw_if2], options=br_options)
- sw_if1_10 = sw.create_vlan(sw_if1, 10) + sw_if1_10 = sw.create_vlan(sw_if1, 10, ip=test_ip(2, 3)) sw_if2_10 = sw.create_vlan(sw_if2, 10) - sw.create_bridge(slaves=[sw_if1_10, sw_if2_10], options=br_options) + sw_br = sw.create_bridge(slaves=[sw_if1_10, sw_if2_10], options=br_options)
sw_if1_20 = sw.create_vlan(sw_if1, 20) sw_if2_21 = sw.create_vlan(sw_if2, 21) @@ -50,6 +50,12 @@ def do_task(ctl, hosts, ifaces, aliases): tl.ping_simple(m1_if1_20, m2_if1_21) tl.ping_simple(m1_if1_30, m2_if1_30, fail_expected=True)
+ sw_br.slave_del(sw_if1_10.get_id()) + + sleep(5) + + tl.ping_simple(sw_if1_10, m1_if1_10) + do_task(ctl, [ctl.get_host("machine1"), ctl.get_host("machine2"), ctl.get_host("switch")],
When removing a port netdev from a 802.1q bridge one needs to do a proper cleanup, so that the interface will direct traffic to CPU instead.
Remove a port netdev from a bridge and check that ping works.
Cc: Andrew Lunn andrew@lunn.ch Cc: Vivien Didelot vivien.didelot@savoirfairelinux.com Signed-off-by: Ido Schimmel idosch@mellanox.com --- v1->v2: * No change. --- recipes/switchdev/l2-008-bridge_vlan1q_sanity.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/recipes/switchdev/l2-008-bridge_vlan1q_sanity.py b/recipes/switchdev/l2-008-bridge_vlan1q_sanity.py index 740de76..5572017 100644 --- a/recipes/switchdev/l2-008-bridge_vlan1q_sanity.py +++ b/recipes/switchdev/l2-008-bridge_vlan1q_sanity.py @@ -44,8 +44,10 @@ def do_task(ctl, hosts, ifaces, aliases): m2_if1_20 = m2.create_vlan(m2_if1, 20, ip=test_ip(3, 2)) m2_if1_30 = m2.create_vlan(m2_if1, 30, ip=test_ip(4, 2))
+ sw_if1.reset(ip=test_ip(1, 3)) + br_options = {"vlan_filtering": 1} - sw.create_bridge(slaves=[sw_if1, sw_if2], options=br_options) + sw_br = sw.create_bridge(slaves=[sw_if1, sw_if2], options=br_options)
sw_if1.add_br_vlan(10) sw_if2.add_br_vlan(10) @@ -76,6 +78,12 @@ def do_task(ctl, hosts, ifaces, aliases): sleep(1) tl.ping_simple(m1_if1, m2_if1)
+ sw_br.slave_del(sw_if1.get_id()) + + sleep(5) + + tl.ping_simple(sw_if1, m1_if1) + do_task(ctl, [ctl.get_host("machine1"), ctl.get_host("machine2"), ctl.get_host("switch")],
Wed, Feb 17, 2016 at 12:21:55PM CET, idosch@mellanox.com wrote:
The slaves of an interface are determined during creation time and stored in two objects:
- Interface on the controller
- NetConfigDeviceGeneric on the slave machine
When slaves are dynamically added / deleted during a Python task the 'slaves' data structures in these objects are not updated, which can cause several problems:
If a slave is removed from its master, then during cleanup the slave machine will try to remove it again and thereby generate an error. This requires us to update the data struct on the slave machine.
If a slave is added to a master and that master is later reset(), then the newly added slave won't be configured correctly after the reset(). This requires us to update the data struct on the controller.
Cc: Andrew Lunn andrew@lunn.ch Fixes: af025186a329 ("InterfaceAPI: Introduce slave_{add, del}") Signed-off-by: Ido Schimmel idosch@mellanox.com
set applied, thanks Ido
lnst-developers@lists.fedorahosted.org
-
Ido Schimmel -
Jiri Pirko