Address comments

Signed-off-by: bingwang <[email protected]>

Author: bingwang-ms

orchagent/muxorch.cpp

@@ -33,6 +33,7 @@ extern RouteOrch *gRouteOrch;
 extern AclOrch *gAclOrch;
 extern PortsOrch *gPortsOrch;
 extern FdbOrch *gFdbOrch;
+extern QosOrch *gQosOrch;
 
 extern sai_object_id_t gVirtualRouterId;
 extern sai_object_id_t  gUnderlayIfId;
@@ -1183,13 +1184,13 @@ void MuxOrch::update(SubjectType type, void *cntx)
     }
 }
 
-MuxOrch::MuxOrch(DBConnector *db, const std::vector<std::string> &tables,
+MuxOrch::MuxOrch(DBConnector *cfg_db, DBConnector *app_db, const std::vector<std::string> &tables,
          TunnelDecapOrch* decapOrch, NeighOrch* neighOrch, FdbOrch* fdbOrch) :
-         Orch2(db, tables, request_),
+         Orch2(cfg_db, tables, request_),
          decap_orch_(decapOrch),
          neigh_orch_(neighOrch),
          fdb_orch_(fdbOrch),
-         cfgTunnelTable_(db, CFG_TUNNEL_TABLE_NAME)
+         app_decap_tunnel_table_(app_db, APP_TUNNEL_DECAP_TABLE_NAME)
 
 {
     handler_map_.insert(handler_pair(CFG_MUX_CABLE_TABLE_NAME, &MuxOrch::handleMuxCfg));
@@ -1249,38 +1250,35 @@ bool MuxOrch::handleMuxCfg(const Request& request)
 bool MuxOrch::resolveQosTableIds()
 {
     std::vector<FieldValueTuple> field_value_tuples;
-    if (cfgTunnelTable_.get(MUX_TUNNEL, field_value_tuples))
+    if (app_decap_tunnel_table_.get(MUX_TUNNEL, field_value_tuples))
     {
         KeyOpFieldsValuesTuple tuple{"TUNNEL", MUX_TUNNEL, field_value_tuples};
-        for (auto it = kfvFieldsValues(tuple).begin(); it != kfvFieldsValues(tuple).end(); it++)
+        // Read tc_to_queue_map_id
+        tc_to_queue_map_id_ = gQosOrch->resolveTunnelQosMap(app_decap_tunnel_table_.getTableName(), MUX_TUNNEL, encap_tc_to_queue_field_name, tuple);
+        if (tc_to_queue_map_id_ == SAI_NULL_OBJECT_ID)
         {
-            if (qos_to_ref_table_map.find(fvField(*it)) != qos_to_ref_table_map.end())
-            {
-                sai_object_id_t id;
-                string object_name;
-                string &map_type_name = fvField(*it);
-                string &map_name = fvValue(*it);
-                ref_resolve_status status = resolveFieldRefValue(QosOrch::getTypeMap(), map_type_name, qos_to_ref_table_map.at(map_type_name), tuple, id, object_name);
-                if (status == ref_resolve_status::success)
-                {
-                    if (map_type_name == encap_tc_to_queue_field_name)
-                    {
-                        tc_to_queue_map_id_ = id;
-                    }
-                    else if (map_type_name == encap_tc_to_dscp_field_name)
-                    {
-                        tc_to_dscp_map_id_ = id;
-                    }
-                    setObjectReference(QosOrch::getTypeMap(), CFG_TUNNEL_TABLE_NAME, MUX_TUNNEL, map_type_name, object_name);
-                    SWSS_LOG_NOTICE("Resolved QoS map for tunnel %s type %s name %s", MUX_TUNNEL, map_type_name.c_str(), map_name.c_str());
-                }
-            }
+            SWSS_LOG_NOTICE("QoS map for tunnel %s type %s is not set", MUX_TUNNEL, encap_tc_to_queue_field_name.c_str());
+        }
+        else
+        {
+            SWSS_LOG_NOTICE("Resolved QoS map for tunnel %s type %s id %" PRId64, MUX_TUNNEL, encap_tc_to_queue_field_name.c_str(), tc_to_queue_map_id_);
+        }
+
+        // Read tc_to_dscp_map_id
+        tc_to_dscp_map_id_ = gQosOrch->resolveTunnelQosMap(app_decap_tunnel_table_.getTableName(), MUX_TUNNEL, encap_tc_to_dscp_field_name, tuple);
+        if (tc_to_dscp_map_id_ == SAI_NULL_OBJECT_ID)
+        {
+            SWSS_LOG_NOTICE("QoS map for tunnel %s type %s is not set", MUX_TUNNEL, encap_tc_to_dscp_field_name.c_str());
+        }
+        else
+        {
+            SWSS_LOG_NOTICE("Resolved QoS map for tunnel %s type %s id %" PRId64, MUX_TUNNEL, encap_tc_to_dscp_field_name.c_str(), tc_to_dscp_map_id_);
         }
         return true;
     }
     else
     {
-        SWSS_LOG_ERROR("Failed to read config from CONFIG_DB for %s", MUX_TUNNEL);
+        SWSS_LOG_NOTICE("Entry for table %s not created yet in APP_DB", MUX_TUNNEL);
         return false;
     }
 }
@@ -1306,17 +1304,23 @@ bool MuxOrch::handlePeerSwitch(const Request& request)
                            MUX_TUNNEL, peer_ip.to_string().c_str());
             return false;
         }
+        auto it =  dst_ips.getIpAddresses().begin();
+        const IpAddress& dst_ip = *it;
+
+        // Read dscp_mode of MuxTunnel0 from app_db
+        string dscp_mode_name = "pipe";
+        if (!app_decap_tunnel_table_.hget(MUX_TUNNEL, "dscp_mode", dscp_mode_name))
+        {
+            SWSS_LOG_NOTICE("dscp_mode not available for %s", MUX_TUNNEL);
+            return false;
+        }
+
+        // Read tc_to_dscp_map_id and tc_to_queue_map_id
         if (!resolveQosTableIds())
         {
             return false;
         }
-        auto it =  dst_ips.getIpAddresses().begin();
-        const IpAddress& dst_ip = *it;
 
-        // Read dscp_mode of MuxTunnel0 from config_db
-        string dscp_mode_name = "pipe";
-        cfgTunnelTable_.hget(MUX_TUNNEL, "dscp_mode", dscp_mode_name);
-    
         mux_tunnel_id_ = create_tunnel(&peer_ip, &dst_ip, tc_to_dscp_map_id_, tc_to_queue_map_id_, dscp_mode_name);
         SWSS_LOG_NOTICE("Mux peer ip '%s' was added, peer name '%s'",
                          peer_ip.to_string().c_str(), peer_name.c_str());

orchagent/muxorch.h

@@ -156,7 +156,7 @@ class MuxCfgRequest : public Request
 class MuxOrch : public Orch2, public Observer, public Subject
 {
 public:
-    MuxOrch(DBConnector *db, const std::vector<std::string> &tables, TunnelDecapOrch*, NeighOrch*, FdbOrch*);
+    MuxOrch(DBConnector *cfg_db, DBConnector *app_db, const std::vector<std::string> &tables, TunnelDecapOrch*, NeighOrch*, FdbOrch*);
 
     using handler_pair = pair<string, bool (MuxOrch::*) (const Request& )>;
     using handler_map = map<string, bool (MuxOrch::*) (const Request& )>;
@@ -214,7 +214,7 @@ class MuxOrch : public Orch2, public Observer, public Subject
     FdbOrch *fdb_orch_;
 
     MuxCfgRequest request_;
-    Table cfgTunnelTable_;
+    Table app_decap_tunnel_table_;
 };
 
 const request_description_t mux_cable_request_description = {

orchagent/orchdaemon.cpp

@@ -303,7 +303,7 @@ bool OrchDaemon::init()
         CFG_MUX_CABLE_TABLE_NAME,
         CFG_PEER_SWITCH_TABLE_NAME
     };
-    MuxOrch *mux_orch = new MuxOrch(m_configDb, mux_tables, tunnel_decap_orch, gNeighOrch, gFdbOrch);
+    MuxOrch *mux_orch = new MuxOrch(m_configDb, m_applDb, mux_tables, tunnel_decap_orch, gNeighOrch, gFdbOrch);
     gDirectory.set(mux_orch);
 
     MuxCableOrch *mux_cb_orch = new MuxCableOrch(m_applDb, m_stateDb, APP_MUX_CABLE_TABLE_NAME);

orchagent/qosorch.cpp

@@ -80,7 +80,7 @@ type_map QosOrch::m_qos_maps = {
     {CFG_DSCP_TO_FC_MAP_TABLE_NAME, new object_reference_map()},
     {CFG_EXP_TO_FC_MAP_TABLE_NAME, new object_reference_map()},
     {CFG_TC_TO_DSCP_MAP_TABLE_NAME, new object_reference_map()},
-    {CFG_TUNNEL_TABLE_NAME, new object_reference_map()}
+    {APP_TUNNEL_DECAP_TABLE_NAME, new object_reference_map()}
 };
 
 map<string, string> qos_to_ref_table_map = {
@@ -1949,3 +1949,35 @@ void QosOrch::doTask(Consumer &consumer)
         }
     }
 }
+
+/**
+ * Function Description:
+ *    @brief Resolve the id of QoS map that is referenced by tunnel
+ *
+ * Arguments:
+ *    @param[in] referencing_table_name - The name of table that is referencing the QoS map
+ *    @param[in] tunnle_name - The name of tunnel
+ *    @param[in] map_type_name - The type of referenced QoS map
+ *    @param[in] tuple - The KeyOpFieldsValuesTuple that contains keys - values
+ *
+ * Return Values:
+ *    @return The sai_object_id of referenced map, or SAI_NULL_OBJECT_ID  if there's an error
+ */
+sai_object_id_t QosOrch::resolveTunnelQosMap(std::string referencing_table_name, std::string tunnel_name, std::string map_type_name, KeyOpFieldsValuesTuple& tuple)
+{
+    sai_object_id_t id;
+    string object_name;
+    ref_resolve_status status = resolveFieldRefValue(m_qos_maps, map_type_name, qos_to_ref_table_map.at(map_type_name), tuple, id, object_name);
+    if (status == ref_resolve_status::success)
+    {
+        
+        setObjectReference(m_qos_maps, referencing_table_name, tunnel_name, map_type_name, object_name);
+        SWSS_LOG_INFO("Resolved QoS map for table %s tunnel %s type %s name %s", referencing_table_name.c_str(), tunnel_name.c_str(), map_type_name.c_str(), object_name.c_str());
+        return id;
+    }
+    else
+    {
+        SWSS_LOG_ERROR("Failed to resolve QoS map for table %s tunnel %s type %s", referencing_table_name.c_str(), tunnel_name.c_str(), map_type_name.c_str());
+        return SAI_NULL_OBJECT_ID;
+    }
+}

orchagent/qosorch.h

@@ -60,8 +60,6 @@ const string ecn_green_red                      = "ecn_green_red";
 const string ecn_green_yellow                   = "ecn_green_yellow";
 const string ecn_all                            = "ecn_all";
 
-// Declaration for being referenced in muxorch and decaporch
-extern std::map<string, string> qos_to_ref_table_map;
 class QosMapHandler
 {
 public:
@@ -168,6 +166,8 @@ class QosOrch : public Orch
 
     static type_map& getTypeMap();
     static type_map m_qos_maps;
+
+    sai_object_id_t resolveTunnelQosMap(std::string referencing_table_name, std::string tunnel_name, std::string map_type_name, KeyOpFieldsValuesTuple& tuple);
 private:
     void doTask() override;
     virtual void doTask(Consumer& consumer);

orchagent/tunneldecaporch.cpp

@@ -18,6 +18,7 @@ extern sai_object_id_t  gUnderlayIfId;
 extern sai_object_id_t  gSwitchId;
 extern PortsOrch*       gPortsOrch;
 extern CrmOrch*         gCrmOrch;
+extern QosOrch*         gQosOrch;
 
 TunnelDecapOrch::TunnelDecapOrch(DBConnector *db, string tableName) : Orch(db, tableName)
 {
@@ -32,7 +33,7 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
     {
         return;
     }
-
+    string table_name = consumer.getTableName();
     auto it = consumer.m_toSync.begin();
     while (it != consumer.m_toSync.end())
     {
@@ -41,8 +42,8 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
         string key = kfvKey(t);
         string op = kfvOp(t);
 
-        IpAddresses dst_ip_addresses;
-        IpAddress src_ip_address("0.0.0.0");
+        IpAddresses ip_addresses;
+        IpAddress src_ip;
         IpAddress* p_src_ip = nullptr;
         string tunnel_type;
         string dscp_mode;
@@ -51,7 +52,6 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
         string ttl_mode;
         sai_object_id_t dscp_to_dc_map_id = SAI_NULL_OBJECT_ID;
         sai_object_id_t tc_to_pg_map_id = SAI_NULL_OBJECT_ID;
-        TunnelTermType term_type = TUNNEL_TERM_TYPE_P2MP;
 
         bool valid = true;
 
@@ -83,7 +83,7 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
                 {
                     try
                     {
-                        dst_ip_addresses = IpAddresses(fvValue(i));
+                        ip_addresses = IpAddresses(fvValue(i));
                     }
                     catch (const std::invalid_argument &e)
                     {
@@ -93,17 +93,15 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
                     }
                     if (exists)
                     {
-                        setIpAttribute(key, dst_ip_addresses, tunnelTable.find(key)->second.tunnel_id);
+                        setIpAttribute(key, ip_addresses, tunnelTable.find(key)->second.tunnel_id);
                     }
                 }
                 else if (fvField(i) == "src_ip")
                 {
                     try
                     {
-                        src_ip_address = IpAddress(fvValue(i));
-                        p_src_ip = &src_ip_address;
-                        //Tunnel term type is set to P2P when source ip is present
-                        term_type = TUNNEL_TERM_TYPE_P2P;
+                        src_ip = IpAddress(fvValue(i));
+                        p_src_ip = &src_ip;
                     }
                     catch (const std::invalid_argument &e)
                     {
@@ -174,15 +172,15 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
                 }
                 else if (fvField(i) == decap_dscp_to_tc_field_name)
                 {
-                    dscp_to_dc_map_id = resolveQosMapId(key, decap_dscp_to_tc_field_name, t); 
+                    dscp_to_dc_map_id = gQosOrch->resolveTunnelQosMap(table_name, key, decap_dscp_to_tc_field_name, t); 
                     if (exists && dscp_to_dc_map_id != SAI_NULL_OBJECT_ID)
                     {
                         setTunnelAttribute(fvField(i), dscp_to_dc_map_id, tunnel_id);
                     }
                 }
                 else if (fvField(i) == decap_tc_to_pg_field_name)
                 {
-                    tc_to_pg_map_id = resolveQosMapId(key, decap_tc_to_pg_field_name, t); 
+                    tc_to_pg_map_id = gQosOrch->resolveTunnelQosMap(table_name, key, decap_tc_to_pg_field_name, t); 
                     if (exists && tc_to_pg_map_id != SAI_NULL_OBJECT_ID)
                     {
                         setTunnelAttribute(fvField(i), tc_to_pg_map_id, tunnel_id);
@@ -194,8 +192,8 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
             if (valid && !exists)
             {
 
-                if (addDecapTunnel(key, tunnel_type, dst_ip_addresses, p_src_ip, dscp_mode, ecn_mode, encap_ecn_mode, ttl_mode,
-                                    term_type, dscp_to_dc_map_id, tc_to_pg_map_id))
+                if (addDecapTunnel(key, tunnel_type, ip_addresses, p_src_ip, dscp_mode, ecn_mode, encap_ecn_mode, ttl_mode,
+                                    dscp_to_dc_map_id, tc_to_pg_map_id))
                 {
                     SWSS_LOG_NOTICE("Tunnel(s) added to ASIC_DB.");
                 }
@@ -233,7 +231,6 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
  *    @param[in] dscp - dscp mode (uniform/pipe)
  *    @param[in] ecn - ecn mode (copy_from_outer/standard)
  *    @param[in] ttl - ttl mode (uniform/pipe)
- *    @param[in] term_type - The type of tunnel term
  *    @param[in] dscp_to_tc_map_id - Map ID for remapping DSCP to TC (decap)
  *    @param[in] tc_to_pg_map_id   - Map ID for remapping TC to PG (decap)
  *
@@ -249,7 +246,6 @@ bool TunnelDecapOrch::addDecapTunnel(
     string ecn,
     string encap_ecn,
     string ttl,
-    TunnelTermType term_type,
     sai_object_id_t dscp_to_tc_map_id,
     sai_object_id_t tc_to_pg_map_id)
 {
@@ -262,6 +258,7 @@ bool TunnelDecapOrch::addDecapTunnel(
     sai_attribute_t attr;
     vector<sai_attribute_t> tunnel_attrs;
     sai_object_id_t overlayIfId;
+    TunnelTermType term_type = TUNNEL_TERM_TYPE_P2MP;
 
     // create the overlay router interface to create a LOOPBACK type router interface (decap)
     vector<sai_attribute_t> overlay_intf_attrs;
@@ -310,6 +307,7 @@ bool TunnelDecapOrch::addDecapTunnel(
         copy(attr.value.ipaddr, p_src_ip->to_string());
         tunnel_attrs.push_back(attr);
         src_ip = *p_src_ip;
+        term_type = TUNNEL_TERM_TYPE_P2P;
     }
 
     // decap ecn mode (copy from outer/standard)
@@ -474,7 +472,7 @@ bool TunnelDecapOrch::addDecapTunnelTermEntries(string tunnelKey, swss::IpAddres
         // check if the there's an entry already for the key pair
         if (existingIps.find(key) != existingIps.end())
         {
-            SWSS_LOG_ERROR("%s already exists. Did not create entry.", key.c_str());
+            SWSS_LOG_NOTICE("%s already exists. Did not create entry.", key.c_str());
         }
         else
         {
@@ -941,34 +939,3 @@ IpAddresses TunnelDecapOrch::getDstIpAddresses(std::string tunnelKey)
     return tunnelTable[tunnelKey].dst_ip_addrs;
 }
 
-/**
- * Function Description:
- *    @brief Resolve the map id from QosOrch
- *
- * Arguments:
- *    @param[in] tunnle_name - The name of tunnel
- *    @param[in] map_type_name - The type of referenced QoS map
- *    @param[in] tuple - The KeyOpFieldsValuesTuple that contains keys - values
- *
- * Return Values:
- *    @return The sai_object_id of referenced map, or SAI_NULL_OBJECT_ID  if there's an error
- */
-sai_object_id_t TunnelDecapOrch::resolveQosMapId(std::string tunnle_name, std::string map_type_name, KeyOpFieldsValuesTuple& tuple)
-{
-    sai_object_id_t id;
-    string object_name;
-    ref_resolve_status status = resolveFieldRefValue(QosOrch::getTypeMap(), map_type_name, qos_to_ref_table_map.at(map_type_name), tuple, id, object_name);
-    if (status == ref_resolve_status::success)
-    {
-        
-        setObjectReference(QosOrch::getTypeMap(), CFG_TUNNEL_TABLE_NAME, tunnle_name, map_type_name, object_name);
-        SWSS_LOG_INFO("Resolved QoS map for tunnel %s type %s name %s", tunnle_name.c_str(), map_type_name.c_str(), object_name.c_str());
-        return id;
-    }
-    else
-    {
-        SWSS_LOG_ERROR("Failed to resolce QoS map for tunnel %s type %s", tunnle_name.c_str(), map_type_name.c_str());
-        return SAI_NULL_OBJECT_ID;
-    }
-
-}

orchagent/tunneldecaporch.h

@@ -72,7 +72,7 @@ class TunnelDecapOrch : public Orch
     TunnelNhs   tunnelNhs;
 
     bool addDecapTunnel(std::string key, std::string type, swss::IpAddresses dst_ip, swss::IpAddress* p_src_ip,
-                        std::string dscp, std::string ecn, std::string encap_ecn, std::string ttl, TunnelTermType term_type,
+                        std::string dscp, std::string ecn, std::string encap_ecn, std::string ttl,
                         sai_object_id_t dscp_to_tc_map_id, sai_object_id_t tc_to_pg_map_id);
     bool removeDecapTunnel(std::string key);
 
@@ -88,7 +88,5 @@ class TunnelDecapOrch : public Orch
     int decNextHopRef(std::string tunnelKey, swss::IpAddress& ipAddr);
 
     void doTask(Consumer& consumer);
-
-    sai_object_id_t resolveQosMapId(std::string tunnle_name, std::string map_type_name, swss::KeyOpFieldsValuesTuple& tuple);
 };
 #endif