Skip to content

Commit

Permalink
Fix issue config qos reload causing orchagent aborted via tracking de…
Browse files Browse the repository at this point in the history
…pendencies among QoS tables (sonic-net#2116)

- What I did
Fix issue config qos reload causing orchagent aborted via tracking dependencies among QoS tables

1. Track dependencies among QoS tables.
2. Won't call SAI remove API for an object if it is still referenced.
3. Support removing/replacing one field in PORT_QOS_MAP and QUEUE tables.
4. Optimize logic to handle QUEUE table.
5. Remove switch level DSCP_TO_TC map before the map is removed.
6. Add mock test

- Why I did it
Fix issue.

- How I verified it
Manually test and mock test.

Signed-off-by: Stephen Sun <[email protected]>
  • Loading branch information
stephenxs authored Mar 9, 2022
1 parent 6e5ed1c commit 12f980c
Show file tree
Hide file tree
Showing 9 changed files with 1,070 additions and 84 deletions.
43 changes: 39 additions & 4 deletions orchagent/orch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -410,7 +410,8 @@ void Orch::removeMeFromObjsReferencedByMe(
const string &table,
const string &obj_name,
const string &field,
const string &old_referenced_obj_name)
const string &old_referenced_obj_name,
bool remove_field)
{
vector<string> objects = tokenize(old_referenced_obj_name, list_item_delimiter);
for (auto &obj : objects)
Expand All @@ -426,6 +427,12 @@ void Orch::removeMeFromObjsReferencedByMe(
referenced_table.c_str(), ref_obj_name.c_str(),
to_string(old_referenced_obj.m_objsDependingOnMe.size()).c_str());
}

if (remove_field)
{
auto &referencing_object = (*type_maps[table])[obj_name];
referencing_object.m_objsReferencingByMe.erase(field);
}
}

void Orch::setObjectReference(
Expand All @@ -439,7 +446,7 @@ void Orch::setObjectReference(
auto field_ref = obj.m_objsReferencingByMe.find(field);

if (field_ref != obj.m_objsReferencingByMe.end())
removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field, field_ref->second);
removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field, field_ref->second, false);

obj.m_objsReferencingByMe[field] = referenced_obj;

Expand All @@ -459,16 +466,44 @@ void Orch::setObjectReference(
}
}

bool Orch::doesObjectExist(
type_map &type_maps,
const string &table,
const string &obj_name,
const string &field,
string &referenced_obj)
{
auto &&searchRef = (*type_maps[table]).find(obj_name);
if (searchRef != (*type_maps[table]).end())
{
auto &obj = searchRef->second;
auto &&searchReferencingObjectRef = obj.m_objsReferencingByMe.find(field);
if (searchReferencingObjectRef != obj.m_objsReferencingByMe.end())
{
referenced_obj = searchReferencingObjectRef->second;
return true;
}
}

return false;
}

void Orch::removeObject(
type_map &type_maps,
const string &table,
const string &obj_name)
{
auto &obj = (*type_maps[table])[obj_name];
auto &&searchRef = (*type_maps[table]).find(obj_name);
if (searchRef == (*type_maps[table]).end())
{
return;
}

auto &obj = searchRef->second;

for (auto field_ref : obj.m_objsReferencingByMe)
{
removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field_ref.first, field_ref.second);
removeMeFromObjsReferencedByMe(type_maps, table, obj_name, field_ref.first, field_ref.second, false);
}

// Update the field store
Expand Down
3 changes: 2 additions & 1 deletion orchagent/orch.h
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,11 @@ class Orch
bool parseReference(type_map &type_maps, std::string &ref, const std::string &table_name, std::string &object_name);
ref_resolve_status resolveFieldRefArray(type_map&, const std::string&, const std::string&, swss::KeyOpFieldsValuesTuple&, std::vector<sai_object_id_t>&, std::string&);
void setObjectReference(type_map&, const std::string&, const std::string&, const std::string&, const std::string&);
bool doesObjectExist(type_map&, const std::string&, const std::string&, const std::string&, std::string&);
void removeObject(type_map&, const std::string&, const std::string&);
bool isObjectBeingReferenced(type_map&, const std::string&, const std::string&);
std::string objectReferenceInfo(type_map&, const std::string&, const std::string&);
void removeMeFromObjsReferencedByMe(type_map &type_maps, const std::string &table, const std::string &obj_name, const std::string &field, const std::string &old_referenced_obj_name, bool remove_field=true);

/* Note: consumer will be owned by this class */
void addExecutor(Executor* executor);
Expand All @@ -250,7 +252,6 @@ class Orch

ResponsePublisher m_publisher;
private:
void removeMeFromObjsReferencedByMe(type_map &type_maps, const std::string &table, const std::string &obj_name, const std::string &field, const std::string &old_referenced_obj_name);
void addConsumer(swss::DBConnector *db, std::string tableName, int pri = default_orch_pri);
};

Expand Down
5 changes: 3 additions & 2 deletions orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ PbhOrch *gPbhOrch;
MirrorOrch *gMirrorOrch;
CrmOrch *gCrmOrch;
BufferOrch *gBufferOrch;
QosOrch *gQosOrch;
SwitchOrch *gSwitchOrch;
Directory<Orch*> gDirectory;
NatOrch *gNatOrch;
Expand Down Expand Up @@ -216,7 +217,7 @@ bool OrchDaemon::init()
CFG_DSCP_TO_FC_MAP_TABLE_NAME,
CFG_EXP_TO_FC_MAP_TABLE_NAME
};
QosOrch *qos_orch = new QosOrch(m_configDb, qos_tables);
gQosOrch = new QosOrch(m_configDb, qos_tables);

vector<string> buffer_tables = {
APP_BUFFER_POOL_TABLE_NAME,
Expand Down Expand Up @@ -325,7 +326,7 @@ bool OrchDaemon::init()
* when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map.
* For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask()
*/
m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, qos_orch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, debug_counter_orch, gMacsecOrch, gBfdOrch, gSrv6Orch};
m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, mux_orch, mux_cb_orch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, policer_orch, tunnel_decap_orch, sflow_orch, debug_counter_orch, gMacsecOrch, gBfdOrch, gSrv6Orch};

bool initialize_dtel = false;
if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING)
Expand Down
Loading

0 comments on commit 12f980c

Please sign in to comment.