Disable FDB hardware learning on all bridge ports before planned warm…

… restart (sonic-net#628)

* Disable FDB learning on all bridge ports before freezing orchagent
* Refine create_bridge_port attr
* Flush before freeze
* Add vs test
* Fix error message
* Generalize setBridgePortLearningFDB()
* Tune test

orchagent/orchdaemon.cpp

@@ -363,6 +363,14 @@ void OrchDaemon::start()
  // Should sleep here or continue handling timers and etc.??
  if (!gSwitchOrch->checkRestartNoFreeze())
  {
+ // Disable FDB learning on all bridge ports
+ for (auto& pair: gPortsOrch->getAllPorts())
+ {
+ auto& port = pair.second;
+ gPortsOrch->setBridgePortLearningFDB(port, SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE);
+ }
+ flush();
+
  SWSS_LOG_WARN("Orchagent is frozen for warm restart!");
  sleep(UINT_MAX);
  }

orchagent/portsorch.cpp

@@ -2312,6 +2312,27 @@ bool PortsOrch::addHostIntfs(Port &port, string alias, sai_object_id_t &host_int
  return true;
 }
 
+bool PortsOrch::setBridgePortLearningFDB(Port &port, sai_bridge_port_fdb_learning_mode_t mode)
+{
+ // TODO: how to support 1D bridge?
+ if (port.m_type != Port::PHY) return false;
+
+ auto bridge_port_id = port.m_bridge_port_id;
+ if (bridge_port_id == SAI_NULL_OBJECT_ID) return false;
+
+ sai_attribute_t bport_attr;
+ bport_attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE;
+ bport_attr.value.s32 = mode;
+ auto status = sai_bridge_api->set_bridge_port_attribute(bridge_port_id, &bport_attr);
+ if (status != SAI_STATUS_SUCCESS)
+ {
+ SWSS_LOG_ERROR("Failed to set bridge port %lx learning_mode attribute: %d", bridge_port_id, status);
+ return false;
+ }
+ SWSS_LOG_NOTICE("Disable FDB learning on bridge port %s(%lx)", port.m_alias.c_str(), bridge_port_id);
+ return true;
+}
+
 bool PortsOrch::addBridgePort(Port &port)
 {
  SWSS_LOG_ENTER();
@@ -2350,6 +2371,11 @@ bool PortsOrch::addBridgePort(Port &port)
  attr.value.booldata = true;
  attrs.push_back(attr);
 
+ /* And with hardware FDB learning mode set to HW (explicit default value) */
+ attr.id = SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE;
+ attr.value.s32 = SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW;
+ attrs.push_back(attr);
+
  sai_status_t status = sai_bridge_api->create_bridge_port(&port.m_bridge_port_id, gSwitchId, (uint32_t)attrs.size(), attrs.data());
  if (status != SAI_STATUS_SUCCESS)
  {

orchagent/portsorch.h

@@ -59,6 +59,7 @@ class PortsOrch : public Orch, public Subject
  bool bake() override;
  void cleanPortTable(const vector<string>& keys);
  bool getBridgePort(sai_object_id_t id, Port &port);
+ bool setBridgePortLearningFDB(Port &port, sai_bridge_port_fdb_learning_mode_t mode);
  bool getPort(string alias, Port &port);
  bool getPort(sai_object_id_t id, Port &port);
  bool getPortByBridgePortId(sai_object_id_t bridge_port_id, Port &port);

tests/conftest.py

@@ -269,15 +269,28 @@ def copy_file(self, path, filename):
  self.ctn.put_archive(path, tarstr.getvalue())
  tarstr.close()
 
+ def get_map_iface_bridge_port_id(self, asic_db):
+ port_id_2_iface = self.asicdb.portoidmap
+ tbl = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT")
+ iface_2_bridge_port_id = {}
+ for key in tbl.getKeys():
+ status, data = tbl.get(key)
+ assert status
+ values = dict(data)
+ iface_id = values["SAI_BRIDGE_PORT_ATTR_PORT_ID"]
+ iface_name = port_id_2_iface[iface_id]
+ iface_2_bridge_port_id[iface_name] = key
+
+ return iface_2_bridge_port_id
+
  def is_table_entry_exists(self, db, table, keyregex, attributes):
  tbl = swsscommon.Table(db, table)
  keys = tbl.getKeys()
 
- exists = False
  extra_info = []
- key_found = False
  for key in keys:
- key_found = re.match(keyregex, key)
+ if re.match(keyregex, key) is None:
+ continue
 
  status, fvs = tbl.get(key)
  assert status, "Error reading from table %s" % table
@@ -288,17 +301,13 @@ def is_table_entry_exists(self, db, table, keyregex, attributes):
  del d_attributes[k]
 
  if len(d_attributes) != 0:
- exists = False
  extra_info.append("Desired attributes %s was not found for key %s" % (str(d_attributes), key))
  else:
- exists = True
- break
-
- if not key_found:
- exists = False
- extra_info.append("Desired key with parameters %s was not found" % str(key_values))
-
- return exists, extra_info
+ return True, extra_info
+ else:
+ if not extra_info:
+ extra_info.append("Desired key regex %s was not found" % str(keyregex))
+ return False, extra_info
 
  def is_fdb_entry_exists(self, db, table, key_values, attributes):
  tbl = swsscommon.Table(db, table)

tests/test_fdb_cold.py

@@ -20,21 +20,6 @@ def create_entry_pst(db, table, key, pairs):
  tbl = swsscommon.ProducerStateTable(db, table)
  create_entry(tbl, key, pairs)
 
-
-def get_map_iface_bridge_port_id(asic_db, dvs):
- port_id_2_iface = dvs.asicdb.portoidmap
- tbl = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT")
- iface_2_bridge_port_id = {}
- for key in tbl.getKeys():
- status, data = tbl.get(key)
- assert status
- values = dict(data)
- iface_id = values["SAI_BRIDGE_PORT_ATTR_PORT_ID"]
- iface_name = port_id_2_iface[iface_id]
- iface_2_bridge_port_id[iface_name] = key
-
- return iface_2_bridge_port_id
-
 def how_many_entries_exist(db, table):
  tbl = swsscommon.Table(db, table)
  return len(tbl.getKeys())
@@ -90,7 +75,7 @@ def test_FDBAddedAfterMemberCreated(dvs):
  assert vm_after - vm_before == 1, "The vlan member wasn't added"
 
  # Get mapping between interface name and its bridge port_id
- iface_2_bridge_port_id = get_map_iface_bridge_port_id(dvs.adb, dvs)
+ iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb)
 
  # check that the FDB entry was inserted into ASIC DB
  assert how_many_entries_exist(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY") == 1, "The fdb entry wasn't inserted to ASIC"

tests/test_fdb_warm.py

@@ -20,29 +20,14 @@ def create_entry_pst(db, table, key, pairs):
  tbl = swsscommon.ProducerStateTable(db, table)
  create_entry(tbl, key, pairs)
 
-
-def get_map_iface_bridge_port_id(asic_db, dvs):
- port_id_2_iface = dvs.asicdb.portoidmap
- tbl = swsscommon.Table(asic_db, "ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT")
- iface_2_bridge_port_id = {}
- for key in tbl.getKeys():
- status, data = tbl.get(key)
- assert status
- values = dict(data)
- iface_id = values["SAI_BRIDGE_PORT_ATTR_PORT_ID"]
- iface_name = port_id_2_iface[iface_id]
- iface_2_bridge_port_id[iface_name] = key
-
- return iface_2_bridge_port_id
-
 def how_many_entries_exist(db, table):
  tbl = swsscommon.Table(db, table)
  return len(tbl.getKeys())
 
 def test_fdb_notifications(dvs):
  dvs.setup_db()
 
- #dvs.runcmd("sonic-clear fdb all")
+ dvs.runcmd("sonic-clear fdb all")
 
  dvs.runcmd("crm config polling interval 1")
  dvs.setReadOnlyAttr('SAI_OBJECT_TYPE_SWITCH', 'SAI_SWITCH_ATTR_AVAILABLE_FDB_ENTRY', '1000')
@@ -55,6 +40,20 @@ def test_fdb_notifications(dvs):
  dvs.create_vlan_member("6", "Ethernet64")
  dvs.create_vlan_member("6", "Ethernet68")
 
+ # Get mapping between interface name and its bridge port_id
+ time.sleep(2)
+ iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb)
+
+ # check FDB learning mode
+ ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT',
+ iface_2_bridge_port_id["Ethernet64"],
+ [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")])
+ assert ok, str(extra)
+ ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT',
+ iface_2_bridge_port_id["Ethernet68"],
+ [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")])
+ assert ok, str(extra)
+
  # bring up vlan and member
  dvs.set_interface_status("Vlan6", "up")
  dvs.add_ip_address("Vlan6", "6.6.6.1/24")
@@ -72,9 +71,6 @@ def test_fdb_notifications(dvs):
  rc = dvs.servers[17].runcmd("ping -c 1 6.6.6.6")
  assert rc == 0
 
- # Get mapping between interface name and its bridge port_id
- time.sleep(2)
- iface_2_bridge_port_id = get_map_iface_bridge_port_id(dvs.adb, dvs)
 
  # check that the FDB entries were inserted into ASIC DB
  ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY",
@@ -113,24 +109,34 @@ def test_fdb_notifications(dvs):
  assert ok, str(extra)
 
  # enable warm restart
- # TODO: use cfg command to config it
- create_entry_tbl(
- dvs.cdb,
- swsscommon.CFG_WARM_RESTART_TABLE_NAME, "swss",
- [
- ("enable", "true"),
- ]
- )
+ (exitcode, result) = dvs.runcmd("config warm_restart enable swss")
+ assert exitcode == 0
+
+ # freeze orchagent for warm restart
+ (exitcode, result) = dvs.runcmd("/usr/bin/orchagent_restart_check")
+ assert result == "RESTARTCHECK succeeded\n"
+ time.sleep(2)
 
  try:
  # restart orchagent
  dvs.stop_swss()
+
+ # check FDB learning mode
+ ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT',
+ iface_2_bridge_port_id["Ethernet64"],
+ [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE")])
+ assert ok, str(extra)
+ ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT',
+ iface_2_bridge_port_id["Ethernet68"],
+ [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_DISABLE")])
+ assert ok, str(extra)
+
  dvs.start_swss()
  time.sleep(2)
 
  # Get mapping between interface name and its bridge port_id
  # Note: they are changed
- iface_2_bridge_port_id = get_map_iface_bridge_port_id(dvs.adb, dvs)
+ iface_2_bridge_port_id = dvs.get_map_iface_bridge_port_id(dvs.adb)
 
  # check that the FDB entries were inserted into ASIC DB
  ok, extra = dvs.is_fdb_entry_exists(dvs.adb, "ASIC_STATE:SAI_OBJECT_TYPE_FDB_ENTRY",
@@ -148,17 +154,22 @@ def test_fdb_notifications(dvs):
  )
  assert ok, str(extra)
 
+ # check FDB learning mode
+ ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT',
+ iface_2_bridge_port_id["Ethernet64"],
+ [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")])
+ assert ok, str(extra)
+ ok, extra = dvs.is_table_entry_exists(dvs.adb, 'ASIC_STATE:SAI_OBJECT_TYPE_BRIDGE_PORT',
+ iface_2_bridge_port_id["Ethernet68"],
+ [("SAI_BRIDGE_PORT_ATTR_FDB_LEARNING_MODE", "SAI_BRIDGE_PORT_FDB_LEARNING_MODE_HW")])
+ assert ok, str(extra)
+
  time.sleep(2)
  counter_restarted = dvs.getCrmCounterValue('STATS', 'crm_stats_fdb_entry_used')
  assert counter_inserted == counter_restarted
 
  finally:
- # disable warm restart
- # TODO: use cfg command to config it
- create_entry_tbl(
- dvs.cdb,
- swsscommon.CFG_WARM_RESTART_TABLE_NAME, "swss",
- [
- ("enable", "false"),
- ]
- )
+ # enable warm restart
+ dvs.runcmd("config warm_restart enable swss")
+ # slow down crm polling
+ dvs.runcmd("crm config polling interval 10000")