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 --- lnst/Controller/Machine.py | 12 ++++++++++++ lnst/Slave/NetConfigDevice.py | 4 ++++ 2 files changed, 16 insertions(+)
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..8adcf2b 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): @@ -399,8 +401,10 @@ class NetConfigDeviceTeam(NetConfigDeviceGeneric): exec_cmd("teamdctl %s %s port config update %s "%s"" % (dbus_option, dev_name, port_name, teamd_port_config)) port_dev.down() exec_cmd("teamdctl %s %s port add %s" % (dbus_option, dev_name, port_name)) + self._dev_config["slaves"].append(slave_id)
def slave_del(self, slave_id): + self._dev_config["slaves"].remove(slave_id) dev_name = self._dev_config["name"] port_dev = self._if_manager.get_mapped_device(slave_id) port_name = port_dev.get_name()
Fixes: bfa1f380f017 ("recipes: Add recipeset for testing switchdev based switches") Signed-off-by: Ido Schimmel idosch@mellanox.com --- 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 --- 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 --- 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")],
Tue, Feb 16, 2016 at 12:11:38PM IST, 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
lnst/Controller/Machine.py | 12 ++++++++++++ lnst/Slave/NetConfigDevice.py | 4 ++++ 2 files changed, 16 insertions(+)
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"] = Noneelse: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..8adcf2b 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): @@ -399,8 +401,10 @@ class NetConfigDeviceTeam(NetConfigDeviceGeneric): exec_cmd("teamdctl %s %s port config update %s "%s"" % (dbus_option, dev_name, port_name, teamd_port_config)) port_dev.down() exec_cmd("teamdctl %s %s port add %s" % (dbus_option, dev_name, port_name))
self._dev_config["slaves"].append(slave_id)
Unlike bridge, this function is called during configure(), so we can get into an infinity loop. I'll fix that and send a V2.
def slave_del(self, slave_id):
self._dev_config["slaves"].remove(slave_id) dev_name = self._dev_config["name"] port_dev = self._if_manager.get_mapped_device(slave_id) port_name = port_dev.get_name()-- 2.4.10
lnst-developers@lists.fedorahosted.org
-
Ido Schimmel