Skip to content

Commit

Permalink
[muxorch] Always use direct link for SoC IPs (sonic-net#2369)
Browse files Browse the repository at this point in the history 
What I did
For SoC IPs of ports in active-active cable type, if mux is toggled to standby, still use direct link instead of changing next-hop to the tunnel.

Why I did it
In an active-active dualtor setup, changing the nexthop of route to SoC IP to the tunnel will have the following problem:
If lower ToR is already standby, and somehow the upper ToR decides to toggle itself to standby, the toggle will fail:
linkmgrd decide to toggle to standby --> muxorch disables the SoC IP neighbor and change the route next-hop to the tunnel --> ycabled could not setup gRPC connection.

How I verified it
Add unittest and verify the change on active-active testbeds.
  • Loading branch information
@lolyu
lolyu authored Jul 14, 2022
1 parent 3fd812d commit 24a0797
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 10 deletions.
29 changes: 26 additions & 3 deletions orchagent/muxorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -351,8 +351,8 @@ static bool remove_nh_tunnel(sai_object_id_t nh_id, IpAddress& ipAddr)
return true;
}

MuxCable::MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip)
:mux_name_(name), srv_ip4_(srv_ip4), srv_ip6_(srv_ip6), peer_ip4_(peer_ip)
MuxCable::MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip, std::set<IpAddress> skip_neighbors)
:mux_name_(name), srv_ip4_(srv_ip4), srv_ip6_(srv_ip6), peer_ip4_(peer_ip), skip_neighbors_(skip_neighbors)
{
mux_orch_ = gDirectory.get<MuxOrch*>();
mux_cb_orch_ = gDirectory.get<MuxCableOrch*>();
Expand Down Expand Up @@ -534,6 +534,11 @@ bool MuxCable::nbrHandler(bool enable, bool update_rt)
void MuxCable::updateNeighbor(NextHopKey nh, bool add)
{
sai_object_id_t tnh = mux_orch_->getNextHopTunnelId(MUX_TUNNEL, peer_ip4_);
if (add && skip_neighbors_.find(nh.ip_address) != skip_neighbors_.end())
{
SWSS_LOG_INFO("Skip update neighbor %s on %s", nh.ip_address.to_string().c_str(), nh.alias.c_str());
return;
}
nbr_handler_->update(nh, tnh, add, state_);
if (add)
{
Expand Down Expand Up @@ -1208,9 +1213,27 @@ bool MuxOrch::handleMuxCfg(const Request& request)
auto srv_ip = request.getAttrIpPrefix("server_ipv4");
auto srv_ip6 = request.getAttrIpPrefix("server_ipv6");

std::set<IpAddress> skip_neighbors;

const auto& port_name = request.getKeyString(0);
auto op = request.getOperation();

for (const auto &name : request.getAttrFieldNames())
{
if (name == "soc_ipv4")
{
auto soc_ip = request.getAttrIpPrefix("soc_ipv4");
SWSS_LOG_NOTICE("%s: %s was added to ignored neighbor list", port_name.c_str(), soc_ip.getIp().to_string().c_str());
skip_neighbors.insert(soc_ip.getIp());
}
else if (name == "soc_ipv6")
{
auto soc_ip6 = request.getAttrIpPrefix("soc_ipv6");
SWSS_LOG_NOTICE("%s: %s was added to ignored neighbor list", port_name.c_str(), soc_ip6.getIp().to_string().c_str());
skip_neighbors.insert(soc_ip6.getIp());
}
}

if (op == SET_COMMAND)
{
if(isMuxExists(port_name))
Expand All @@ -1226,7 +1249,7 @@ bool MuxOrch::handleMuxCfg(const Request& request)
}

mux_cable_tb_[port_name] = std::make_unique<MuxCable>
(MuxCable(port_name, srv_ip, srv_ip6, mux_peer_switch_));
(MuxCable(port_name, srv_ip, srv_ip6, mux_peer_switch_, skip_neighbors));

SWSS_LOG_NOTICE("Mux entry for port '%s' was added", port_name.c_str());
}
Expand Down
5 changes: 4 additions & 1 deletion orchagent/muxorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class MuxNbrHandler
class MuxCable
{
public:
MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip);
MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip, std::set<IpAddress> skip_neighbors);

bool isActive() const
{
Expand Down Expand Up @@ -115,6 +115,8 @@ class MuxCable
IpPrefix srv_ip4_, srv_ip6_;
IpAddress peer_ip4_;

std::set<IpAddress> skip_neighbors_;

MuxOrch *mux_orch_;
MuxCableOrch *mux_cb_orch_;
MuxStateOrch *mux_state_orch_;
Expand All @@ -132,6 +134,7 @@ const request_description_t mux_cfg_request_description = {
{ "server_ipv6", REQ_T_IP_PREFIX },
{ "address_ipv4", REQ_T_IP },
{ "soc_ipv4", REQ_T_IP_PREFIX },
{ "soc_ipv6", REQ_T_IP_PREFIX },
{ "cable_type", REQ_T_STRING },
},
{ }
Expand Down
23 changes: 17 additions & 6 deletions tests/test_mux.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ class TestMuxTunnelBase(object):

SERV1_IPV4 = "192.168.0.100"
SERV1_IPV6 = "fc02:1000::100"
SERV1_SOC_IPV4 = "192.168.0.102"
SERV2_IPV4 = "192.168.0.101"
SERV2_IPV6 = "fc02:1000::101"
IPV4_MASK = "/32"
Expand Down Expand Up @@ -74,7 +75,12 @@ def create_vlan_interface(self, confdb, asicdb, dvs):

def create_mux_cable(self, confdb):

fvs = { "server_ipv4":self.SERV1_IPV4+self.IPV4_MASK, "server_ipv6":self.SERV1_IPV6+self.IPV6_MASK }
fvs = {
"server_ipv4":self.SERV1_IPV4 + self.IPV4_MASK,
"server_ipv6":self.SERV1_IPV6 + self.IPV6_MASK,
"soc_ipv4": self.SERV1_SOC_IPV4 + self.IPV4_MASK,
"cable_type": "active-active"
}
confdb.create_entry(self.CONFIG_MUX_CABLE, "Ethernet0", fvs)

fvs = { "server_ipv4":self.SERV2_IPV4+self.IPV4_MASK, "server_ipv6":self.SERV2_IPV6+self.IPV6_MASK }
Expand Down Expand Up @@ -201,6 +207,9 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
self.add_neighbor(dvs, self.SERV1_IPV6, "00:00:00:00:00:01", True)
srv1_v6 = self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV6, 3)

self.add_neighbor(dvs, self.SERV1_SOC_IPV4, "00:00:00:00:00:01")
self.check_neigh_in_asic_db(asicdb, self.SERV1_SOC_IPV4, 4)

existing_keys = asicdb.get_keys(self.ASIC_NEIGH_TABLE)

self.add_neighbor(dvs, self.SERV2_IPV4, "00:00:00:00:00:02")
Expand All @@ -212,21 +221,23 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
dvs_route.check_asicdb_route_entries([self.SERV2_IPV4+self.IPV4_MASK, self.SERV2_IPV6+self.IPV6_MASK])

# The first standby route also creates as tunnel Nexthop
self.check_tnl_nexthop_in_asic_db(asicdb, 3)
self.check_tnl_nexthop_in_asic_db(asicdb, 4)

# Change state to Standby. This will delete Neigh and add Route
self.set_mux_state(appdb, "Ethernet0", "standby")

asicdb.wait_for_deleted_entry(self.ASIC_NEIGH_TABLE, srv1_v4)
asicdb.wait_for_deleted_entry(self.ASIC_NEIGH_TABLE, srv1_v6)
dvs_route.check_asicdb_route_entries([self.SERV1_IPV4+self.IPV4_MASK, self.SERV1_IPV6+self.IPV6_MASK])
self.check_neigh_in_asic_db(asicdb, self.SERV1_SOC_IPV4, 2)
dvs_route.check_asicdb_deleted_route_entries([self.SERV1_SOC_IPV4+self.IPV4_MASK])

# Change state to Active. This will add Neigh and delete Route
self.set_mux_state(appdb, "Ethernet4", "active")

dvs_route.check_asicdb_deleted_route_entries([self.SERV2_IPV4+self.IPV4_MASK, self.SERV2_IPV6+self.IPV6_MASK])
self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV4, 3)
self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV6, 3)
self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV4, 4)
self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV6, 4)


def create_and_test_fdb(self, appdb, asicdb, dvs, dvs_route):
Expand All @@ -244,7 +255,7 @@ def create_and_test_fdb(self, appdb, asicdb, dvs, dvs_route):
self.add_neighbor(dvs, ip_2, "00:00:00:00:00:12", True)

# ip_1 is on Active Mux, hence added to Host table
self.check_neigh_in_asic_db(asicdb, ip_1, 4)
self.check_neigh_in_asic_db(asicdb, ip_1, 5)

# ip_2 is on Standby Mux, hence added to Route table
dvs_route.check_asicdb_route_entries([ip_2+self.IPV6_MASK])
Expand All @@ -260,7 +271,7 @@ def create_and_test_fdb(self, appdb, asicdb, dvs, dvs_route):

# ip_2 moved to active Mux, hence remove from Route table
dvs_route.check_asicdb_deleted_route_entries([ip_2+self.IPV6_MASK])
self.check_neigh_in_asic_db(asicdb, ip_2, 4)
self.check_neigh_in_asic_db(asicdb, ip_2, 5)

# Simulate FDB aging out test case
ip_3 = "192.168.0.200"
Expand Down

0 comments on commit 24a0797

Please sign in to comment.