publish route install status in case a duplicate prefix is received

This is needed in case DEL/SET consolidation by ConsumerStateTable Signed-off-by: Stepan Blyschak <[email protected]>
sonic-net · Jan 19, 2023 · 0fa58de · 0fa58de
1 parent 2b80c52
commit 0fa58de
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 5 deletions.
diff --git a/orchagent/routeorch.cpp b/orchagent/routeorch.cpp
@@ -841,7 +841,7 @@ void RouteOrch::doTask(Consumer& consumer)
                         /* The prefix is full mask (/32 or /128) and it is an interface subnet route, so IntfOrch has already
                          * created an IP2ME route for it and we skip programming such route here as it already exists.
                          * However, to keep APPL_DB and APPL_STATE_DB consistent we have to publish it. */
-                        publishRouteState(ctx, ReturnCode(SAI_STATUS_SUCCESS));
+                        publishRouteState(ctx);
                         it = consumer.m_toSync.erase(it);
                     }
                     /* subnet route, vrf leaked route, etc */
@@ -871,7 +871,9 @@ void RouteOrch::doTask(Consumer& consumer)
                 }
                 else
                 {
-                    /* Duplicate entry */
+                    /* Duplicate entry. Publish route state anyway since there could be multiple DEL, SET operations
+                     * consolidated by ConsumerStateTable leading to orchagent receiving only the last SET update. */
+                    publishRouteState(ctx);
                     it = consumer.m_toSync.erase(it);
                 }
 
@@ -2238,7 +2240,7 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey
     notifyNextHopChangeObservers(vrf_id, ipPrefix, nextHops, true);
 
     /* Publish and update APPL STATE DB route entry programming status */
-    publishRouteState(ctx, ReturnCode(SAI_STATUS_SUCCESS));
+    publishRouteState(ctx);
 
     /*
      * If the route uses a temporary synced NHG owned by NhgOrch, return false
@@ -2435,7 +2437,7 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)
             ipPrefix.to_string().c_str(), it_route->second.nhg_key.to_string().c_str());
 
     /* Publish removal status, removes route entry from APPL STATE DB */
-    publishRouteState(ctx, ReturnCode(SAI_STATUS_SUCCESS));
+    publishRouteState(ctx);
 
     if (ipPrefix.isDefaultRoute() && vrf_id == gVirtualRouterId)
     {

diff --git a/orchagent/routeorch.h b/orchagent/routeorch.h
@@ -276,7 +276,7 @@ class RouteOrch : public Orch, public Subject
     void incNhgRefCount(const std::string& nhg_index);
     void decNhgRefCount(const std::string& nhg_index);
 
-    void publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status);
+    void publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status = ReturnCode(SAI_STATUS_SUCCESS));
 };
 
 #endif /* SWSS_ROUTEORCH_H */
diff --git a/tests/mock_tests/routeorch_ut.cpp b/tests/mock_tests/routeorch_ut.cpp
@@ -446,6 +446,12 @@ namespace routeorch_test
         EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
         static_cast<Orch *>(gRouteOrch)->doTask();
 
+        // add entries again to the consumer queue (in case of rapid DEL/SET operations from fpmsyncd, routeorch just gets the last SET update)
+        consumer->addToSync(entries);
+
+        EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
+        static_cast<Orch *>(gRouteOrch)->doTask();
+
         entries.clear();
 
         // Route deletion