[vnet/vxlan]: Handle SAI failures without crashing swss (sonic-net#3908)

What I did

Remove explicit throwing of exceptions
Use generic orchagent SAI failure handling logic
Why I did it

To avoid unnecessary orchagent crashes when programming vxlan tunnels and nexthops on a vnet in case of SAI errors.
How I verified it
By running mock tests and making sure that there are no crashes when there are vxlan tunnel, mapping or tunnel termination create/remove failure.

Author: prabhataravind

orchagent/vnetorch.cpp

@@ -315,7 +315,12 @@ sai_object_id_t VNetVrfObject::getTunnelNextHop(NextHopKey& nh)
     nh_id = vxlan_orch->createNextHopTunnel(tun_name, nh.ip_address, nh.mac_address, nh.vni);
     if (nh_id == SAI_NULL_OBJECT_ID)
     {
-        throw std::runtime_error("NH Tunnel create failed for " + vnet_name_ + " ip " + nh.ip_address.to_string());
+        /*
+         * Log an error and return. Generic error handling and reporting
+         * is already done in createNextHopTunnel()
+         */
+        SWSS_LOG_ERROR("NH Tunnel create failed for '%s' ip '%s'",
+                vnet_name_.c_str(), nh.ip_address.to_string().c_str());
     }
 
     return nh_id;

orchagent/vxlanorch.cpp

@@ -20,6 +20,7 @@ extern "C" {
 #include "sai_serialize.h"
 #include "flex_counter_manager.h"
 #include "converter.h"
+#include "saihelper.h"
 
 /* Global variables */
 extern sai_object_id_t gSwitchId;
@@ -145,7 +146,12 @@ create_tunnel_map(MAP_T map_t)
                           );
     if (status != SAI_STATUS_SUCCESS)
     {
-        throw std::runtime_error("Can't create tunnel map object");
+        task_process_status handle_status = handleSaiCreateStatus(SAI_API_TUNNEL, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("Can't create tunnel map object");
+            return SAI_NULL_OBJECT_ID;
+        }
     }
 
     return tunnel_map_id;
@@ -157,7 +163,11 @@ remove_tunnel_map(sai_object_id_t tunnel_map_id)
     sai_status_t status = sai_tunnel_api->remove_tunnel_map(tunnel_map_id);
     if (status != SAI_STATUS_SUCCESS)
     {
-        throw std::runtime_error("Can't remove a tunnel map object");
+        task_process_status handle_status = handleSaiRemoveStatus(SAI_API_TUNNEL, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("Can't remove a tunnel map object");
+        }
     }
 }
 
@@ -204,7 +214,12 @@ static sai_object_id_t create_tunnel_map_entry(
 
     if (status != SAI_STATUS_SUCCESS)
     {
-        throw std::runtime_error("Can't create a tunnel map entry object");
+        task_process_status handle_status = handleSaiCreateStatus(SAI_API_TUNNEL, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("Can't create a tunnel map entry object");
+            return SAI_NULL_OBJECT_ID;
+        }
     }
 
     return tunnel_map_entry_id;
@@ -221,7 +236,11 @@ void remove_tunnel_map_entry(sai_object_id_t obj_id)
 
     if (status != SAI_STATUS_SUCCESS)
     {
-        throw std::runtime_error("Can't delete a tunnel map entry object");
+        task_process_status handle_status = handleSaiRemoveStatus(SAI_API_TUNNEL, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("Can't delete a tunnel map entry object");
+        }
     }
 }
 
@@ -383,7 +402,12 @@ create_tunnel(
                           );
     if (status != SAI_STATUS_SUCCESS)
     {
-        throw std::runtime_error("Can't create a tunnel object");
+        task_process_status handle_status = handleSaiCreateStatus(SAI_API_TUNNEL, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("Can't create a tunnel object");
+            return SAI_NULL_OBJECT_ID;
+        }
     }
 
     return tunnel_id;
@@ -397,7 +421,11 @@ remove_tunnel(sai_object_id_t tunnel_id)
         sai_status_t status = sai_tunnel_api->remove_tunnel(tunnel_id);
         if (status != SAI_STATUS_SUCCESS)
         {
-            throw std::runtime_error("Can't remove a tunnel object");
+            task_process_status handle_status = handleSaiRemoveStatus(SAI_API_TUNNEL, status);
+            if (handle_status != task_success)
+            {
+                SWSS_LOG_ERROR("Can't remove a tunnel object");
+            }
         }
     }
     else
@@ -459,7 +487,12 @@ create_tunnel_termination(
                           );
     if (status != SAI_STATUS_SUCCESS)
     {
-        throw std::runtime_error("Can't create a tunnel term table object");
+        task_process_status handle_status = handleSaiCreateStatus(SAI_API_TUNNEL, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("Can't create a tunnel term table object");
+            return SAI_NULL_OBJECT_ID;
+        }
     }
 
     return term_table_id;
@@ -473,7 +506,11 @@ remove_tunnel_termination(sai_object_id_t term_table_id)
         sai_status_t status = sai_tunnel_api->remove_tunnel_term_table_entry(term_table_id);
         if (status != SAI_STATUS_SUCCESS)
         {
-            throw std::runtime_error("Can't remove a tunnel term table object");
+            task_process_status handle_status = handleSaiRemoveStatus(SAI_API_TUNNEL, status);
+            if (handle_status != task_success)
+            {
+                SWSS_LOG_ERROR("Can't remove a tunnel term table object");
+            }
         }
     }
     else
@@ -615,12 +652,16 @@ bool VxlanTunnel::removeNextHop(IpAddress& ipAddr, MacAddress macAddress, uint32
 
     if (!nh_tunnels_[key].ref_count)
     {
-        if (sai_next_hop_api->remove_next_hop(nh_tunnels_[key].nh_id) != SAI_STATUS_SUCCESS)
+        sai_status_t status = sai_next_hop_api->remove_next_hop(nh_tunnels_[key].nh_id);
+        if (status != SAI_STATUS_SUCCESS)
         {
-            SWSS_LOG_INFO("delete NH tunnel for ip '%s', mac '%s' vni %d failed",
+            task_process_status handle_status = handleSaiRemoveStatus(SAI_API_NEXT_HOP, status);
+            if (handle_status != task_success)
+            {
+                SWSS_LOG_ERROR("delete NH tunnel for ip '%s', mac '%s' vni %d failed",
                             ipAddr.to_string().c_str(), macAddress.to_string().c_str(), vni);
-            string err_msg = "NH tunnel delete failed for " + ipAddr.to_string();
-            throw std::runtime_error(err_msg);
+                return false;
+            }
         }
 
         nh_tunnels_.erase(key);
@@ -869,11 +910,30 @@ bool VxlanTunnel::createTunnelHw(uint8_t mapper_list, tunnel_map_use_t map_src,
         {
             tunnel_orch->addTunnelToFlexCounter(ids_.tunnel_id, tunnel_name_);
         }
+        else
+        {
+            // Undo changes in createMapperHw if create_tunnel fails.
+            deleteMapperHw(mapper_list, map_src);
+            ids_.tunnel_id = SAI_NULL_OBJECT_ID;
+            ids_.tunnel_term_id = SAI_NULL_OBJECT_ID;
+            active_ = false;
+            return false;
+        }
 
         if (with_term)
         {
             ids_.tunnel_term_id = create_tunnel_termination(ids_.tunnel_id, ips, 
                                                             ip, gVirtualRouterId);
+            if (ids_.tunnel_term_id == SAI_NULL_OBJECT_ID)
+            {
+                // Undo changes if create_tunnel_termination fails
+                tunnel_orch->removeTunnelFromFlexCounter(ids_.tunnel_id, tunnel_name_);
+                remove_tunnel(ids_.tunnel_id);
+                deleteMapperHw(mapper_list, map_src);
+                ids_.tunnel_id = SAI_NULL_OBJECT_ID;
+                active_ = false;
+                return false;
+            }
         }
 
         active_ = true;
@@ -1366,10 +1426,17 @@ VxlanTunnelOrch::createNextHopTunnel(string tunnelName, IpAddress& ipAddr,
         macptr = &mac;
     }
 
-    if (create_nexthop_tunnel(host_ip, vni, macptr, tunnel_id, &nh_id) != SAI_STATUS_SUCCESS)
+    sai_status_t status = create_nexthop_tunnel(host_ip, vni, macptr, tunnel_id, &nh_id);
+    if (status != SAI_STATUS_SUCCESS)
     {
-        string err_msg = "NH tunnel create failed for " + ipAddr.to_string() + " " + to_string(vni);
-        throw std::runtime_error(err_msg);
+        task_process_status handle_status = handleSaiCreateStatus(SAI_API_NEXT_HOP, status);
+        if (handle_status != task_success)
+        {
+            SWSS_LOG_ERROR("NH vxlan tunnel create failed for %s, ip %s, mac %s, vni %d",
+                        tunnelName.c_str(), ipAddr.to_string().c_str(),
+                        macAddress.to_string().c_str(), vni);
+            return SAI_NULL_OBJECT_ID;
+        }
     }
 
     //Store the nh tunnel id
@@ -1412,6 +1479,7 @@ bool VxlanTunnelOrch::createVxlanTunnelMap(string tunnelName, tunnel_map_type_t
     }
 
     auto tunnel_obj = getVxlanTunnel(tunnelName);
+    bool tunnel_created = false;
 
     if (!tunnel_obj->isActive())
     {
@@ -1420,13 +1488,21 @@ bool VxlanTunnelOrch::createVxlanTunnelMap(string tunnelName, tunnel_map_type_t
             uint8_t mapper_list = 0;
             TUNNELMAP_SET_VLAN(mapper_list);
             TUNNELMAP_SET_VRF(mapper_list);
-            tunnel_obj->createTunnelHw(mapper_list, TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP , true, encap_ttl);
+            tunnel_created = tunnel_obj->createTunnelHw(mapper_list, TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP , true, encap_ttl);
+            if (!tunnel_created)
+            {
+                return false;
+            }
         }
         else if (map == TUNNEL_MAP_T_BRIDGE)
         {
             uint8_t mapper_list = 0;
             TUNNELMAP_SET_BRIDGE(mapper_list);
-            tunnel_obj->createTunnelHw(mapper_list,  TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP, true, encap_ttl);
+            tunnel_created = tunnel_obj->createTunnelHw(mapper_list,  TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP, true, encap_ttl);
+            if (!tunnel_created)
+            {
+                return false;
+            }
         }
     }
 
@@ -1990,7 +2066,12 @@ bool VxlanTunnelMapOrch::addOperation(const Request& request)
         uint8_t mapper_list = 0;
         TUNNELMAP_SET_VLAN(mapper_list);
         TUNNELMAP_SET_VRF(mapper_list);
-        tunnel_obj->createTunnelHw(mapper_list,TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
+        bool tunnel_created = tunnel_obj->createTunnelHw(mapper_list,
+                                            TUNNEL_MAP_USE_DEDICATED_ENCAP_DECAP);
+        if (!tunnel_created)
+        {
+            return false;
+        }
         if (!tunnel_orch->isDipTunnelsSupported())
         {
             Port tunPort;

tests/mock_tests/Makefile.am

@@ -31,6 +31,7 @@ tests_INCLUDES = -I $(FLEX_CTR_DIR) -I $(DEBUG_CTR_DIR) -I $(top_srcdir)/lib -I$
 tests_SOURCES = aclorch_ut.cpp \
                 aclorch_rule_ut.cpp \
                 portsorch_ut.cpp \
+                vxlanorch_ut.cpp \
                 routeorch_ut.cpp \
                 qosorch_ut.cpp \
                 bufferorch_ut.cpp \

tests/mock_tests/mock_sai_tunnel.h

@@ -0,0 +1,80 @@
+#pragma once
+
+#include <gmock/gmock.h>
+
+extern "C"
+{
+#include "sai.h"
+}
+
+// Mock Class mapping methods to tunnel object SAI APIs.
+class MockSaiTunnel
+{
+  public:
+    MOCK_METHOD4(create_tunnel, sai_status_t(_Out_ sai_object_id_t *tunnel_id, _In_ sai_object_id_t switch_id,
+                                             _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list));
+
+    MOCK_METHOD4(create_tunnel_map, sai_status_t(_Out_ sai_object_id_t *tunnel_map_id, _In_ sai_object_id_t switch_id,
+                                             _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list));
+
+    MOCK_METHOD4(create_tunnel_map_entry, sai_status_t(_Out_ sai_object_id_t *tunnel_map_entry_id, _In_ sai_object_id_t switch_id,
+                                             _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list));
+
+    MOCK_METHOD4(create_tunnel_term_table_entry, sai_status_t(_Out_ sai_object_id_t *tunnel_term_table_entry_id, _In_ sai_object_id_t switch_id,
+                                             _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list));
+
+    MOCK_METHOD1(remove_tunnel, sai_status_t(_In_ sai_object_id_t tunnel_id));
+
+    MOCK_METHOD1(remove_tunnel_map, sai_status_t(_In_ sai_object_id_t tunnel_map_id));
+
+    MOCK_METHOD1(remove_tunnel_map_entry, sai_status_t(sai_object_id_t tunnel_map_entry_id));
+
+    MOCK_METHOD1(remove_tunnel_term_table_entry, sai_status_t(sai_object_id_t tunnel_term_table_entry_id));
+
+};
+
+MockSaiTunnel *mock_sai_tunnel;
+
+sai_status_t mock_create_tunnel(_Out_ sai_object_id_t *tunnel_id, _In_ sai_object_id_t switch_id,
+                                _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list)
+{
+    return mock_sai_tunnel->create_tunnel(tunnel_id, switch_id, attr_count, attr_list);
+}
+
+sai_status_t mock_create_tunnel_map(_Out_ sai_object_id_t *tunnel_map_id, _In_ sai_object_id_t switch_id,
+                                _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list)
+{
+    return mock_sai_tunnel->create_tunnel_map(tunnel_map_id, switch_id, attr_count, attr_list);
+}
+
+sai_status_t mock_create_tunnel_map_entry(_Out_ sai_object_id_t *tunnel_map_entry_id, _In_ sai_object_id_t switch_id,
+                                _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list)
+{
+    return mock_sai_tunnel->create_tunnel_map_entry(tunnel_map_entry_id, switch_id, attr_count, attr_list);
+}
+
+sai_status_t mock_create_tunnel_term_table_entry(_Out_ sai_object_id_t *tunnel_term_table_entry_id, _In_ sai_object_id_t switch_id,
+                                _In_ uint32_t attr_count, _In_ const sai_attribute_t *attr_list)
+{
+    return mock_sai_tunnel->create_tunnel_term_table_entry(tunnel_term_table_entry_id, switch_id, attr_count, attr_list);
+}
+
+sai_status_t mock_remove_tunnel(_In_ sai_object_id_t tunnel_id)
+{
+    return mock_sai_tunnel->remove_tunnel(tunnel_id);
+}
+
+sai_status_t mock_remove_tunnel_map(_In_ sai_object_id_t tunnel_map_id)
+{
+    return mock_sai_tunnel->remove_tunnel_map(tunnel_map_id);
+}
+
+sai_status_t mock_remove_tunnel_map_entry(_In_ sai_object_id_t tunnel_map_entry_id)
+{
+    return mock_sai_tunnel->remove_tunnel_map_entry(tunnel_map_entry_id);
+}
+
+sai_status_t mock_remove_tunnel_term_table_entry(_In_ sai_object_id_t tunnel_term_table_entry_id)
+{
+    return mock_sai_tunnel->remove_tunnel_term_table_entry(tunnel_term_table_entry_id);
+}