Change nexthop key to ip & ifname (#977)

* Change nexthop key to ip & ifname
pphuchar · Oct 2, 2019 · bd36751 · bd36751
1 parent fee1aaa
commit bd36751
Show file tree

Hide file tree

Showing 15 changed files with 468 additions and 209 deletions.
diff --git a/debian/changelog b/debian/changelog
@@ -1,5 +1,9 @@
 sonic (1.0.0) stable; urgency=medium
 
+  * next-hop key add ifname
+
+ -- Tyler Li <tyler.li@mediatek.com>  Thu, 19 Sep 2019 04:00:00 -0700
+
   * Initial release.
 
  -- Shuotian Cheng <shuche@microsoft.com>  Wed, 09 Mar 2016 12:00:00 -0800

diff --git a/doc/swss-schema.md b/doc/swss-schema.md
@@ -456,8 +456,10 @@ Stores rules associated with a specific ACL table on the switch.
                                                ; it could be:
                                                : name of physical port.          Example: "Ethernet10"
                                                : name of LAG port                Example: "PortChannel5"
-                                               : next-hop ip address             Example: "10.0.0.1"
-                                               : next-hop group set of addresses Example: "10.0.0.1,10.0.0.3"
+                                               : next-hop ip address (in global) Example: "10.0.0.1"
+                                               : next-hop ip address and vrf     Example: "10.0.0.2@Vrf2"
+                                               : next-hop ip address and ifname  Example: "10.0.0.3@Ethernet1"
+                                               : next-hop group set of next-hop  Example: "10.0.0.1,10.0.0.3@Ethernet1"
 
     redirect_action = 1*255CHAR                ; redirect parameter
                                                ; This parameter defines a destination for redirected packets

diff --git a/fpmsyncd/routesync.cpp b/fpmsyncd/routesync.cpp
@@ -334,6 +334,17 @@ string RouteSync::getNextHopGw(struct rtnl_route *route_obj)
             nl_addr2str(addr, gw_ip, MAX_ADDR_SIZE);
             result += gw_ip;
         }
+        else
+        {
+            if (rtnl_route_get_family(route_obj) == AF_INET)
+            {
+                result += "0.0.0.0";
+            }
+            else
+            {
+                result += "::";
+            }
+        }
 
         if (i + 1 < rtnl_route_get_nnexthops(route_obj))
         {

diff --git a/orchagent/aclorch.cpp b/orchagent/aclorch.cpp
@@ -546,12 +546,12 @@ void AclRule::decreaseNextHopRefCount()
 {
     if (!m_redirect_target_next_hop.empty())
     {
-        m_pAclOrch->m_neighOrch->decreaseNextHopRefCount(IpAddress(m_redirect_target_next_hop));
+        m_pAclOrch->m_neighOrch->decreaseNextHopRefCount(NextHopKey(m_redirect_target_next_hop));
         m_redirect_target_next_hop.clear();
     }
     if (!m_redirect_target_next_hop_group.empty())
     {
-        IpAddresses target = IpAddresses(m_redirect_target_next_hop_group);
+        NextHopGroupKey target = NextHopGroupKey(m_redirect_target_next_hop_group);
         m_pAclOrch->m_routeOrch->decreaseNextHopRefCount(target);
         // remove next hop group in case it's not used by anything else
         if (m_pAclOrch->m_routeOrch->isRefCounterZero(target))
@@ -880,44 +880,44 @@ sai_object_id_t AclRuleL3::getRedirectObjectId(const string& redirect_value)
         }
     }
 
-    // Try to parse nexthop ip address
+    // Try to parse nexthop ip address and interface name
     try
     {
-        IpAddress ip(target);
-        if (!m_pAclOrch->m_neighOrch->hasNextHop(ip))
+        NextHopKey nh(target);
+        if (!m_pAclOrch->m_neighOrch->hasNextHop(nh))
         {
-            SWSS_LOG_ERROR("ACL Redirect action target next hop ip: '%s' doesn't exist on the switch", ip.to_string().c_str());
+            SWSS_LOG_ERROR("ACL Redirect action target next hop ip: '%s' doesn't exist on the switch", nh.to_string().c_str());
             return SAI_NULL_OBJECT_ID;
         }
 
         m_redirect_target_next_hop = target;
-        m_pAclOrch->m_neighOrch->increaseNextHopRefCount(ip);
-        return m_pAclOrch->m_neighOrch->getNextHopId(ip);
+        m_pAclOrch->m_neighOrch->increaseNextHopRefCount(nh);
+        return m_pAclOrch->m_neighOrch->getNextHopId(nh);
     }
     catch (...)
     {
         // no error, just try next variant
     }
 
-    // try to parse nh group ip addresses
+    // try to parse nh group the set of <ip address, interface name>
     try
     {
-        IpAddresses ips(target);
-        if (!m_pAclOrch->m_routeOrch->hasNextHopGroup(ips))
+        NextHopGroupKey nhg(target);
+        if (!m_pAclOrch->m_routeOrch->hasNextHopGroup(nhg))
         {
-            SWSS_LOG_INFO("ACL Redirect action target next hop group: '%s' doesn't exist on the switch. Creating it.", ips.to_string().c_str());
+            SWSS_LOG_INFO("ACL Redirect action target next hop group: '%s' doesn't exist on the switch. Creating it.", nhg.to_string().c_str());
 
-            if (!m_pAclOrch->m_routeOrch->addNextHopGroup(ips))
+            if (!m_pAclOrch->m_routeOrch->addNextHopGroup(nhg))
             {
-                SWSS_LOG_ERROR("Can't create required target next hop group '%s'", ips.to_string().c_str());
+                SWSS_LOG_ERROR("Can't create required target next hop group '%s'", nhg.to_string().c_str());
                 return SAI_NULL_OBJECT_ID;
             }
-            SWSS_LOG_DEBUG("Created acl redirect target next hop group '%s'", ips.to_string().c_str());
+            SWSS_LOG_DEBUG("Created acl redirect target next hop group '%s'", nhg.to_string().c_str());
         }
 
         m_redirect_target_next_hop_group = target;
-        m_pAclOrch->m_routeOrch->increaseNextHopRefCount(ips);
-        return m_pAclOrch->m_routeOrch->getNextHopGroupId(ips);
+        m_pAclOrch->m_routeOrch->increaseNextHopRefCount(nhg);
+        return m_pAclOrch->m_routeOrch->getNextHopGroupId(nhg);
     }
     catch (...)
     {

diff --git a/orchagent/intfsorch.cpp b/orchagent/intfsorch.cpp
@@ -79,10 +79,35 @@ sai_object_id_t IntfsOrch::getRouterIntfsId(const string &alias)
 {
     Port port;
     gPortsOrch->getPort(alias, port);
-    assert(port.m_rif_id);
     return port.m_rif_id;
 }
 
+string IntfsOrch::getRouterIntfsAlias(const IpAddress &ip, const string &vrf_name)
+{
+    sai_object_id_t vrf_id = gVirtualRouterId;
+
+    if (!vrf_name.empty())
+    {
+        vrf_id = m_vrfOrch->getVRFid(vrf_name);
+    }
+
+    for (const auto &it_intfs: m_syncdIntfses)
+    {
+        if (it_intfs.second.vrf_id != vrf_id)
+        {
+            continue;
+        }
+        for (const auto &prefixIt: it_intfs.second.ip_addresses)
+        {
+            if (prefixIt.isAddressInSubnet(ip))
+            {
+                return it_intfs.first;
+            }
+        }
+    }
+    return string();
+}
+
 void IntfsOrch::increaseRouterIntfsRefCount(const string &alias)
 {
     SWSS_LOG_ENTER();
@@ -154,6 +179,7 @@ bool IntfsOrch::setIntf(const string& alias, sai_object_id_t vrf_id, const IpPre
             gPortsOrch->increasePortRefCount(alias);
             IntfsEntry intfs_entry;
             intfs_entry.ref_count = 0;
+            intfs_entry.vrf_id = vrf_id;
             m_syncdIntfses[alias] = intfs_entry;
         }
         else
@@ -328,6 +354,7 @@ void IntfsOrch::doTask(Consumer &consumer)
                     IntfsEntry intfs_entry;
 
                     intfs_entry.ref_count = 0;
+                    intfs_entry.vrf_id = vrf_id;
                     intfs_entry.ip_addresses.insert(ip_prefix);
                     m_syncdIntfses[alias] = intfs_entry;
                     addIp2Me = true;
@@ -629,7 +656,7 @@ void IntfsOrch::addSubnetRoute(const Port &port, const IpPrefix &ip_prefix)
         gCrmOrch->incCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE);
     }
 
-    gRouteOrch->notifyNextHopChangeObservers(ip_prefix, IpAddresses(), true);
+    gRouteOrch->notifyNextHopChangeObservers(ip_prefix, NextHopGroupKey(), true);
 }
 
 void IntfsOrch::removeSubnetRoute(const Port &port, const IpPrefix &ip_prefix)
@@ -661,7 +688,7 @@ void IntfsOrch::removeSubnetRoute(const Port &port, const IpPrefix &ip_prefix)
         gCrmOrch->decCrmResUsedCounter(CrmResourceType::CRM_IPV6_ROUTE);
     }
 
-    gRouteOrch->notifyNextHopChangeObservers(ip_prefix, IpAddresses(), false);
+    gRouteOrch->notifyNextHopChangeObservers(ip_prefix, NextHopGroupKey(), false);
 }
 
 void IntfsOrch::addIp2MeRoute(sai_object_id_t vrf_id, const IpPrefix &ip_prefix)

diff --git a/orchagent/intfsorch.h b/orchagent/intfsorch.h
@@ -22,6 +22,7 @@ struct IntfsEntry
 {
     std::set<IpPrefix>  ip_addresses;
     int                 ref_count;
+    sai_object_id_t     vrf_id;
 };
 
 typedef map<string, IntfsEntry> IntfsTable;
@@ -32,6 +33,7 @@ class IntfsOrch : public Orch
     IntfsOrch(DBConnector *db, string tableName, VRFOrch *vrf_orch);
 
     sai_object_id_t getRouterIntfsId(const string&);
+    string getRouterIntfsAlias(const IpAddress &ip, const string &vrf_name = "");
 
     void increaseRouterIntfsRefCount(const string&);
     void decreaseRouterIntfsRefCount(const string&);

diff --git a/orchagent/mirrororch.cpp b/orchagent/mirrororch.cpp
@@ -60,7 +60,7 @@ MirrorEntry::MirrorEntry(const string& platform) :
     }
 
     nexthopInfo.prefix = IpPrefix("0.0.0.0/0");
-    nexthopInfo.nexthop = IpAddress("0.0.0.0");
+    nexthopInfo.nexthop = NextHopKey("0.0.0.0", "");
 }
 
 MirrorOrch::MirrorOrch(TableConnector stateDbConnector, TableConnector confDbConnector,
@@ -456,7 +456,7 @@ bool MirrorOrch::getNeighborInfo(const string& name, MirrorEntry& session)
     // 3) Otherwise, return false.
     if (!m_neighOrch->getNeighborEntry(session.dstIp,
                 session.neighborInfo.neighbor, session.neighborInfo.mac) &&
-            (session.nexthopInfo.nexthop.isZero() ||
+            (session.nexthopInfo.nexthop.ip_address.isZero() ||
             !m_neighOrch->getNeighborEntry(session.nexthopInfo.nexthop,
                 session.neighborInfo.neighbor, session.neighborInfo.mac)))
     {
@@ -894,8 +894,8 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update)
 
         // This is the ECMP scenario that the new next hop group contains the previous
         // next hop. There is no need to update this session's monitor port.
-        if (update.nexthopGroup != IpAddresses() &&
-                update.nexthopGroup.getIpAddresses().count(session.nexthopInfo.nexthop))
+        if (update.nexthopGroup != NextHopGroupKey() &&
+                update.nexthopGroup.getNextHops().count(session.nexthopInfo.nexthop))
 
         {
             continue;
@@ -904,18 +904,18 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update)
         SWSS_LOG_NOTICE("Updating mirror session %s with route %s",
                 name.c_str(), update.prefix.to_string().c_str());
 
-        if (update.nexthopGroup != IpAddresses())
+        if (update.nexthopGroup != NextHopGroupKey())
         {
             SWSS_LOG_NOTICE("    next hop IPs: %s", update.nexthopGroup.to_string().c_str());
 
             // Recover the session based on the state database information
             if (m_recoverySessionMap.find(name) != m_recoverySessionMap.end())
             {
-                IpAddress nexthop = IpAddress(tokenize(m_recoverySessionMap[name],
+                NextHopKey nexthop = NextHopKey(tokenize(m_recoverySessionMap[name],
                             state_db_key_delimiter, 1)[1]);
 
                 // Check if recovered next hop IP is within the update's next hop IPs
-                if (update.nexthopGroup.getIpAddresses().count(nexthop))
+                if (update.nexthopGroup.getNextHops().count(nexthop))
                 {
                     SWSS_LOG_NOTICE("Recover mirror session %s with next hop %s",
                             name.c_str(), nexthop.to_string().c_str());
@@ -927,18 +927,18 @@ void MirrorOrch::updateNextHop(const NextHopUpdate& update)
                     SWSS_LOG_NOTICE("Correct mirror session %s next hop from %s to %s",
                             name.c_str(), session.nexthopInfo.nexthop.to_string().c_str(),
                     nexthop.to_string().c_str());
-                    session.nexthopInfo.nexthop = *update.nexthopGroup.getIpAddresses().begin();
+                    session.nexthopInfo.nexthop = *update.nexthopGroup.getNextHops().begin();
                 }
             }
             else
             {
                 // Pick the first one from the next hop group
-                session.nexthopInfo.nexthop = *update.nexthopGroup.getIpAddresses().begin();
+                session.nexthopInfo.nexthop = *update.nexthopGroup.getNextHops().begin();
             }
         }
         else
         {
-            session.nexthopInfo.nexthop = IpAddress(0);
+            session.nexthopInfo.nexthop = NextHopKey("0.0.0.0", "");
         }
 
         // Resolve the neighbor of the new next hop
@@ -960,7 +960,7 @@ void MirrorOrch::updateNeighbor(const NeighborUpdate& update)
         // Check if the session's destination IP matches the neighbor's update IP
         // or if the session's next hop IP matches the neighbor's update IP
         if (session.dstIp != update.entry.ip_address &&
-                session.nexthopInfo.nexthop != update.entry.ip_address)
+                session.nexthopInfo.nexthop.ip_address != update.entry.ip_address)
         {
             continue;
         }

diff --git a/orchagent/mirrororch.h b/orchagent/mirrororch.h
@@ -36,7 +36,7 @@ struct MirrorEntry
     struct
     {
         IpPrefix prefix;
-        IpAddress nexthop;
+        NextHopKey nexthop;
     } nexthopInfo;
 
     struct