Skip to content

Commit

Permalink
[muxorch] Fixing bug with updateRoute and mux neighbors
Browse files Browse the repository at this point in the history
mux neighbors that were not the configured mux ip were being treated as
active.

Signed-off-by: Nikola Dancejic <ndancejic@microsoft.com>
  • Loading branch information
Ndancejic committed Jun 7, 2024
1 parent 6568193 commit 3645154
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 9 deletions.
17 changes: 8 additions & 9 deletions orchagent/muxorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1130,14 +1130,13 @@ void MuxOrch::updateRoute(const IpPrefix &pfx, bool add)
for (auto it = nextHops.begin(); it != nextHops.end(); it++)
{
NextHopKey nexthop = *it;
/* This will only work for configured MUX neighbors (most cases)
* TODO: add way to find MUX from neighbor
*/
MuxCable* cable = findMuxCableInSubnet(nexthop.ip_address);
auto standalone = standalone_tunnel_neighbors_.find(nexthop.ip_address);
NeighborEntry neighbor;
MacAddress mac;

gNeighOrch->getNeighborEntry(nexthop, neighbor, mac);
auto standalone = standalone_tunnel_neighbors_.find(neighbor.ip_address);

if ((cable == nullptr && standalone == standalone_tunnel_neighbors_.end()) ||
cable->isActive())
if (isNeighborActive(neighbor.ip_address, mac, neighbor.alias) && standalone == standalone_tunnel_neighbors_.end())
{
/* Here we pull from local nexthop ID because neighbor update occurs during state change
* before nexthopID is updated in neighorch. This ensures that if a neighbor is Active
Expand All @@ -1149,11 +1148,11 @@ void MuxOrch::updateRoute(const IpPrefix &pfx, bool add)
if (status != SAI_STATUS_SUCCESS)
{
SWSS_LOG_ERROR("Failed to set route entry %s to nexthop %s",
pfx.to_string().c_str(), nexthop.to_string().c_str());
pfx.to_string().c_str(), neighbor.to_string().c_str());
continue;
}
SWSS_LOG_NOTICE("setting route %s with nexthop %s %" PRIx64 "",
pfx.to_string().c_str(), nexthop.to_string().c_str(), next_hop_id);
pfx.to_string().c_str(), neighbor.to_string().c_str(), next_hop_id);
active_found = true;
break;
}
Expand Down
8 changes: 8 additions & 0 deletions tests/test_mux.py
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,8 @@ def create_and_test_multi_nexthop_routes(self, dvs, dvs_route, appdb, macs, new_
new_ipv6_nexthop = self.SERV3_IPV6
non_mux_ipv4 = "11.11.11.11"
non_mux_ipv6 = "2222::100"
mux_neighbor_ipv4 = "192.170.0.100"
mux_neighbor_ipv6 = "fc02:1000:100::100"
non_mux_mac = "00:aa:aa:aa:aa:aa"
mux_ports = ["Ethernet0", "Ethernet4"]
new_mux_port = "Ethernet8"
Expand All @@ -746,6 +748,8 @@ def create_and_test_multi_nexthop_routes(self, dvs, dvs_route, appdb, macs, new_
self.add_neighbor(dvs, new_ipv6_nexthop, new_mac)
self.add_neighbor(dvs, non_mux_ipv4, non_mux_mac)
self.add_neighbor(dvs, non_mux_ipv6, non_mux_mac)
self.add_neighbor(dvs, mux_neighbor_ipv4, macs[1])
self.add_neighbor(dvs, mux_neighbor_ipv6, macs[1])

for port in mux_ports:
self.set_mux_state(appdb, port, ACTIVE)
Expand All @@ -772,6 +776,8 @@ def create_and_test_multi_nexthop_routes(self, dvs, dvs_route, appdb, macs, new_
self.add_route(dvs, route_B_ipv4, ipv4_nexthops)
self.add_route(dvs, route_B_ipv6, ipv6_nexthops)

self.multi_nexthop_test_toggle(appdb, asicdb, dvs_route, route_ipv4, mux_ports, [self.SERV1_IPV4, mux_neighbor_ipv4])
self.multi_nexthop_test_toggle(appdb, asicdb, dvs_route, route_ipv6, mux_ports, [self.SERV1_IPV6, mux_neighbor_ipv6])
self.multi_nexthop_test_fdb(appdb, asicdb, dvs, dvs_route, [route_ipv4, route_B_ipv4], mux_ports, ipv4_nexthops, macs)
self.multi_nexthop_test_fdb(appdb, asicdb, dvs, dvs_route, [route_ipv6, route_B_ipv6], mux_ports, ipv6_nexthops, macs)
self.multi_nexthop_test_neighbor_add(appdb, asicdb, dvs, dvs_route, [route_ipv4, route_B_ipv4], mux_ports, ipv4_nexthops, macs)
Expand All @@ -790,6 +796,8 @@ def create_and_test_multi_nexthop_routes(self, dvs, dvs_route, appdb, macs, new_
self.del_neighbor(dvs, neighbor)
self.del_neighbor(dvs, new_ipv4_nexthop)
self.del_neighbor(dvs, new_ipv6_nexthop)
self.del_neighbor(dvs, mux_neighbor_ipv4)
self.del_neighbor(dvs, mux_neighbor_ipv6)

def create_and_test_NH_routes(self, appdb, asicdb, dvs, dvs_route, mac):
'''
Expand Down

0 comments on commit 3645154

Please sign in to comment.