Skip to content

Commit

Permalink
Tunneldecaporch fixes (sonic-net#573)
Browse files Browse the repository at this point in the history 
* Allow decap tunnel configuration without "src_ip"

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>

* Remove overlay loopback interface when removing tunnel

After tunnel removal there was still overlay loopback
interface in ASIC DB.

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>

* Add more test cases to test_tunnel.py

Seperate symmetric and decap tunnel test
Add checks for tunnel removal

Signed-off-by: Stepan Blyschak <stepanb@mellanox.com>
  • Loading branch information
@stepanblyschak @sihuihan88
stepanblyschak authored and sihuihan88 committed Aug 20, 2018
1 parent fb5c389 commit 7ac5101
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 34 deletions.
19 changes: 12 additions & 7 deletions doc/swss-schema.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

---------------------------------------------

Expand Down
28 changes: 21 additions & 7 deletions orchagent/tunneldecaporch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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.");
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down
3 changes: 2 additions & 1 deletion orchagent/tunneldecaporch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
};

Expand All @@ -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);
Expand Down
108 changes: 89 additions & 19 deletions tests/test_tunnel.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)

Expand All @@ -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":
Expand All @@ -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 """
Expand All @@ -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")

0 comments on commit 7ac5101

Please sign in to comment.