From 7ac510173d034c3254c3d27116c313a7f06c40c1 Mon Sep 17 00:00:00 2001 From: stepanblyschak <38952541+stepanblyschak@users.noreply.github.com> Date: Mon, 20 Aug 2018 20:26:13 +0300 Subject: [PATCH] Tunneldecaporch fixes (#573) * Allow decap tunnel configuration without "src_ip" Signed-off-by: Stepan Blyschak * Remove overlay loopback interface when removing tunnel After tunnel removal there was still overlay loopback interface in ASIC DB. Signed-off-by: Stepan Blyschak * Add more test cases to test_tunnel.py Seperate symmetric and decap tunnel test Add checks for tunnel removal Signed-off-by: Stepan Blyschak --- doc/swss-schema.md | 19 +++--- orchagent/tunneldecaporch.cpp | 28 ++++++--- orchagent/tunneldecaporch.h | 3 +- tests/test_tunnel.py | 108 ++++++++++++++++++++++++++++------ 4 files changed, 124 insertions(+), 34 deletions(-) diff --git a/doc/swss-schema.md b/doc/swss-schema.md index 124cb6d590..663574ce48 100644 --- a/doc/swss-schema.md +++ b/doc/swss-schema.md @@ -315,6 +315,7 @@ and reflects the LAG ports into the redis under: `LAG_TABLE::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 @@ -322,18 +323,22 @@ and reflects the LAG ports into the redis under: `LAG_TABLE::port` 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" --------------------------------------------- diff --git a/orchagent/tunneldecaporch.cpp b/orchagent/tunneldecaporch.cpp index 23bcf356ea..78470775aa 100644 --- a/orchagent/tunneldecaporch.cpp +++ b/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,7 +177,7 @@ 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) @@ -183,7 +185,7 @@ void TunnelDecapOrch::doTask(Consumer& consumer) * 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; } diff --git a/orchagent/tunneldecaporch.h b/orchagent/tunneldecaporch.h index 892cebafff..dbd3126df9 100644 --- a/orchagent/tunneldecaporch.h +++ b/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 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); diff --git a/tests/test_tunnel.py b/tests/test_tunnel.py index 866ec3ae4d..5b21b9e993 100644 --- a/tests/test_tunnel.py +++ b/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")