Tunneldecaporch fixes

doc/swss-schema.md

@@ -315,25 +315,30 @@ and reflects the LAG ports into the redis under: `LAG_TABLE:<team0>:port`
     key                     = TUNNEL_DECAP_TABLE:name
     ;field                      value
     tunnel_type             = "IPINIP"
+    src_ip                  = IP
     dst_ip                  = IP1,IP2 ;IP addresses separated by ","
     dscp_mode               = "uniform" / "pipe"
     ecn_mode                = "copy_from_outer" / "standard" ;standard: Behavior defined in RFC 6040 section 4.2
     ttl_mode                = "uniform" / "pipe"
 
     IP = dec-octet "." dec-octet "." dec-octet "." dec-octet
 
+    "src_ip" field is optional
+
     Example:
     127.0.0.1:6379> hgetall TUNNEL_DECAP_TABLE:NETBOUNCER
     1) "dscp_mode"
     2) "uniform"
-    3) "dst_ip"
+    3) "src_ip"
     4) "127.0.0.1"
-    5) "ecn_mode"
-    6) "copy_from_outer"
-    7) "ttl_mode"
-    8) "uniform"
-    9) "tunnel_type"
-    10) "IPINIP"
+    5) "dst_ip"
+    6) "127.0.0.1"
+    7) "ecn_mode"
+    8) "copy_from_outer"
+    9) "ttl_mode"
+    10) "uniform"
+    11) "tunnel_type"
+    12) "IPINIP"
 
 ---------------------------------------------
 

orchagent/tunneldecaporch.cpp

@@ -36,6 +36,7 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
 
         IpAddresses ip_addresses;
         IpAddress src_ip;
+        IpAddress* p_src_ip = nullptr;
         string tunnel_type;
         string dscp_mode;
         string ecn_mode;
@@ -82,6 +83,7 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
                     try
                     {
                         src_ip = IpAddress(fvValue(i));
+                        p_src_ip = &src_ip;
                     }
                     catch (const std::invalid_argument &e)
                     {
@@ -141,7 +143,7 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
             // create new tunnel if it doesn't exists already
             if (valid && !exists)
             {
-                if (addDecapTunnel(key, tunnel_type, ip_addresses, src_ip, dscp_mode, ecn_mode, ttl_mode))
+                if (addDecapTunnel(key, tunnel_type, ip_addresses, p_src_ip, dscp_mode, ecn_mode, ttl_mode))
                 {
                     SWSS_LOG_NOTICE("Tunnel(s) added to ASIC_DB.");
                 }
@@ -175,15 +177,15 @@ void TunnelDecapOrch::doTask(Consumer& consumer)
  * Arguments:
  *    @param[in] type - type of tunnel
  *    @param[in] dst_ip - destination ip address to decap
- *    @param[in] src_ip - source ip address to decap
+ *    @param[in] p_src_ip - source ip address for encap (nullptr to skip this)
  *    @param[in] dscp - dscp mode (uniform/pipe)
  *    @param[in] ecn - ecn mode (copy_from_outer/standard)
  *    @param[in] ttl - ttl mode (uniform/pipe)
  *
  * Return Values:
  *    @return true on success and false if there's an error
  */
-bool TunnelDecapOrch::addDecapTunnel(string key, string type, IpAddresses dst_ip, IpAddress src_ip, string dscp, string ecn, string ttl)
+bool TunnelDecapOrch::addDecapTunnel(string key, string type, IpAddresses dst_ip, IpAddress* p_src_ip, string dscp, string ecn, string ttl)
 {
 
     SWSS_LOG_ENTER();
@@ -228,9 +230,12 @@ bool TunnelDecapOrch::addDecapTunnel(string key, string type, IpAddresses dst_ip
     tunnel_attrs.push_back(attr);
 
     // tunnel src ip
-    attr.id = SAI_TUNNEL_ATTR_ENCAP_SRC_IP;
-    copy(attr.value.ipaddr, src_ip.to_string());
-    tunnel_attrs.push_back(attr);
+    if (p_src_ip != nullptr)
+    {
+        attr.id = SAI_TUNNEL_ATTR_ENCAP_SRC_IP;
+        copy(attr.value.ipaddr, p_src_ip->to_string());
+        tunnel_attrs.push_back(attr);
+    }
 
     // decap ecn mode (copy from outer/standard)
     attr.id = SAI_TUNNEL_ATTR_DECAP_ECN_MODE;
@@ -277,7 +282,7 @@ bool TunnelDecapOrch::addDecapTunnel(string key, string type, IpAddresses dst_ip
         return false;
     }
 
-    tunnelTable[key] = { tunnel_id, {} };
+    tunnelTable[key] = { tunnel_id, overlayIfId, {} };
 
     // TODO:
     // there should also be "business logic" for netbouncer in the "tunnel application" code, which is a different source file and daemon process
@@ -523,6 +528,15 @@ bool TunnelDecapOrch::removeDecapTunnel(string key)
         SWSS_LOG_ERROR("Failed to remove tunnel: %lu", tunnel_info->tunnel_id);
         return false;
     }
+
+    // delete overlay loopback interface
+    status = sai_router_intfs_api->remove_router_interface(tunnel_info->overlay_intf_id);
+    if (status != SAI_STATUS_SUCCESS)
+    {
+        SWSS_LOG_ERROR("Failed to remove tunnel overlay interface: %lu", tunnel_info->overlay_intf_id);
+        return false;
+    }
+
     tunnelTable.erase(key);
     return true;
 }

orchagent/tunneldecaporch.h

@@ -18,6 +18,7 @@ struct TunnelTermEntry
 struct TunnelEntry
 {
     sai_object_id_t            tunnel_id;              // tunnel id
+    sai_object_id_t            overlay_intf_id;        // overlay interface id
     vector<TunnelTermEntry>    tunnel_term_info;       // tunnel_entry ids related to the tunnel abd ips related to the tunnel (all ips for tunnel entries that refer to this tunnel)
 };
 
@@ -36,7 +37,7 @@ class TunnelDecapOrch : public Orch
     TunnelTable tunnelTable;
     ExistingIps existingIps;
 
-    bool addDecapTunnel(string key, string type, IpAddresses dst_ip, IpAddress src_ip, string dscp, string ecn, string ttl);
+    bool addDecapTunnel(string key, string type, IpAddresses dst_ip, IpAddress* p_src_ip, string dscp, string ecn, string ttl);
     bool removeDecapTunnel(string key);
 
     bool addDecapTunnelTermEntries(string tunnelKey, IpAddresses dst_ip, sai_object_id_t tunnel_id);

tests/test_tunnel.py

@@ -4,7 +4,7 @@
 def create_fvs(**kwargs):
     return swsscommon.FieldValuePairs(kwargs.items())
 
-class TestTunnel(object):
+class TestTunnelBase(object):
     APP_TUNNEL_DECAP_TABLE_NAME = "TUNNEL_DECAP_TABLE"
     ASIC_TUNNEL_TABLE           = "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL"
     ASIC_TUNNEL_TERM_ENTRIES    = "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY"
@@ -63,14 +63,15 @@ def check_tunnel_termination_entry_exists_in_asicdb(self, asicdb, tunnel_sai_oid
                 else:
                     assert False, "Field %s is not tested" % field
 
-    def create_and_test_tunnel(self, db, asicdb, tunnel_name, src_ip,
-                               dst_ips, dscp_mode, ecn_mode, ttl_mode):
+    def create_and_test_tunnel(self, db, asicdb, tunnel_name, **kwargs):
         """ Create tunnel and verify all needed enties in ASIC DB exists """
 
+        is_symmetric_tunnel = "src_ip" in kwargs;
+
         # create tunnel entry in DB
         ps = swsscommon.ProducerStateTable(db, self.APP_TUNNEL_DECAP_TABLE_NAME)
-        fvs = create_fvs(tunnel_type="IPINIP", src_ip=src_ip, dst_ip=",".join(dst_ips),
-                         dscp_mode=dscp_mode, ecn_mode=ecn_mode, ttl_mode=ttl_mode)
+
+        fvs = create_fvs(**kwargs)
 
         ps.set(tunnel_name, fvs)
 
@@ -88,17 +89,19 @@ def create_and_test_tunnel(self, db, asicdb, tunnel_name, src_ip,
         status, fvs = tunnel_table.get(tunnel_sai_obj)
 
         assert status == True
-        assert len(fvs) == 7
+        # 6 parameters to check in case of decap tunnel
+        # + 1 (SAI_TUNNEL_ATTR_ENCAP_SRC_IP) in case of symmetric tunnel
+        assert len(fvs) == 7 if is_symmetric_tunnel else 6
 
-        expected_ecn_mode = self.ecn_modes_map[ecn_mode]
-        expected_dscp_mode = self.dscp_modes_map[dscp_mode]
-        expected_ttl_mode = self.ttl_modes_map[ttl_mode]
+        expected_ecn_mode = self.ecn_modes_map[kwargs["ecn_mode"]]
+        expected_dscp_mode = self.dscp_modes_map[kwargs["dscp_mode"]]
+        expected_ttl_mode = self.ttl_modes_map[kwargs["ttl_mode"]]
 
         for field, value in fvs:
             if field == "SAI_TUNNEL_ATTR_TYPE":
                 assert value == "SAI_TUNNEL_TYPE_IPINIP"
             elif field == "SAI_TUNNEL_ATTR_ENCAP_SRC_IP":
-                assert value == src_ip
+                assert value == kwargs["src_ip"]
             elif field == "SAI_TUNNEL_ATTR_DECAP_ECN_MODE":
                 assert value == expected_ecn_mode
             elif field == "SAI_TUNNEL_ATTR_DECAP_TTL_MODE":
@@ -112,7 +115,34 @@ def create_and_test_tunnel(self, db, asicdb, tunnel_name, src_ip,
             else:
                 assert False, "Field %s is not tested" % field
 
-        self.check_tunnel_termination_entry_exists_in_asicdb(asicdb, tunnel_sai_obj, dst_ips)
+        self.check_tunnel_termination_entry_exists_in_asicdb(asicdb, tunnel_sai_obj, kwargs["dst_ip"].split(","))
+
+    def remove_and_test_tunnel(self, db, asicdb, tunnel_name):
+        """ Removes tunnel and checks that ASIC db is clear"""
+
+        tunnel_table = swsscommon.Table(asicdb, self.ASIC_TUNNEL_TABLE)
+        tunnel_term_table = swsscommon.Table(asicdb, self.ASIC_TUNNEL_TERM_ENTRIES)
+        tunnel_app_table = swsscommon.Table(asicdb, self.APP_TUNNEL_DECAP_TABLE_NAME)
+
+        tunnels = tunnel_table.getKeys()
+        tunnel_sai_obj = tunnels[0]
+
+        status, fvs = tunnel_table.get(tunnel_sai_obj)
+
+        # get overlay loopback interface oid to check if it is deleted with the tunnel
+        overlay_infs_id = {f:v for f,v in fvs}["SAI_TUNNEL_ATTR_OVERLAY_INTERFACE"] 
+
+        ps = swsscommon.ProducerStateTable(db, self.APP_TUNNEL_DECAP_TABLE_NAME)
+        ps.set(tunnel_name, create_fvs(), 'DEL')
+
+        # wait till config will be applied
+        time.sleep(1)
+
+        assert len(tunnel_table.getKeys()) == 0
+        assert len(tunnel_term_table.getKeys()) == 0
+        assert len(tunnel_app_table.getKeys()) == 0
+        assert not self.check_interface_exists_in_asicdb(asicdb, overlay_infs_id)
+
 
     def cleanup_left_over(self, db, asicdb):
         """ Cleanup APP and ASIC tables """
@@ -129,29 +159,69 @@ def cleanup_left_over(self, db, asicdb):
         for key in tunnel_app_table.getKeys():
             tunnel_table._del(key)
 
+
+class TestDecapTunnel(TestTunnelBase):
+    """ Tests for decap tunnel creation and removal """
+
     def test_TunnelDecap_v4(self, dvs):
         """ test IPv4 tunnel creation """
 
-        db = swsscommon.DBConnector(0, dvs.redis_sock, 0)
-        asicdb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
+        db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        asicdb = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
 
         self.cleanup_left_over(db, asicdb)
 
         # create tunnel IPv4 tunnel
-        self.create_and_test_tunnel(db, asicdb, tunnel_name="IPINIPv4", src_ip="1.1.1.1",
-                                   dst_ips=["2.2.2.2", "3.3.3.3"], dscp_mode="uniform",
+        self.create_and_test_tunnel(db, asicdb, tunnel_name="IPINIPv4Decap", tunnel_type="IPINIP",
+                                   dst_ip="2.2.2.2,3.3.3.3", dscp_mode="uniform",
                                    ecn_mode="standard", ttl_mode="pipe")
+        self.remove_and_test_tunnel(db, asicdb, "IPINIPv4Decap")
 
     def test_TunnelDecap_v6(self, dvs):
         """ test IPv6 tunnel creation """
 
-        db = swsscommon.DBConnector(0, dvs.redis_sock, 0)
-        asicdb = swsscommon.DBConnector(1, dvs.redis_sock, 0)
+        db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        asicdb = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
 
         self.cleanup_left_over(db, asicdb)
 
         # create tunnel IPv6 tunnel
-        self.create_and_test_tunnel(db, asicdb, tunnel_name="IPINIPv6", src_ip="1::1",
-                                    dst_ips=["2::2", "3::3"], dscp_mode="pipe",
+        self.create_and_test_tunnel(db, asicdb, tunnel_name="IPINIPv6Decap", tunnel_type="IPINIP",
+                                    dst_ip="2::2,3::3", dscp_mode="pipe",
                                     ecn_mode="copy_from_outer", ttl_mode="uniform")
+        self.remove_and_test_tunnel(db, asicdb,"IPINIPv6Decap")
+
+
+class TestSymmetricTunnel(TestTunnelBase):
+    """ Tests for symmetric tunnel creation and removal """
+
+    def test_TunnelSymmetric_v4(self, dvs):
+        """ test IPv4 tunnel creation """
+
+        db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        asicdb = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
+
+        self.cleanup_left_over(db, asicdb)
+
+        # create tunnel IPv4 tunnel
+        self.create_and_test_tunnel(db, asicdb, tunnel_name="IPINIPv4Symmetric", tunnel_type="IPINIP",
+                                   src_ip="1.1.1.1",
+                                   dst_ip="2.2.2.2,3.3.3.3", dscp_mode="pipe",
+                                   ecn_mode="copy_from_outer", ttl_mode="uniform")
+        self.remove_and_test_tunnel(db, asicdb, "IPINIPv4Symmetric")
+
+    def test_TunnelSymmetric_v6(self, dvs):
+        """ test IPv6 tunnel creation """
+
+        db = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
+        asicdb = swsscommon.DBConnector(swsscommon.ASIC_DB, dvs.redis_sock, 0)
+
+        self.cleanup_left_over(db, asicdb)
+
+        # create tunnel IPv6 tunnel
+        self.create_and_test_tunnel(db, asicdb, tunnel_name="IPINIPv6Symmetric", tunnel_type="IPINIP",
+                                    src_ip="1::1",
+                                    dst_ip="2::2,3::3", dscp_mode="uniform",
+                                    ecn_mode="standard", ttl_mode="pipe")
+        self.remove_and_test_tunnel(db, asicdb, "IPINIPv6Symmetric")