Skip to content

Commit

Permalink
[muxorch] Skip programming SoC IP kernel tunnel route (#2557)
Browse files Browse the repository at this point in the history 
What I did
Let mux orch skip adding kernel tunnel routes for SoC IPs.

Signed-off-by: Longxiang Lyu lolv@microsoft.com

Why I did it
Please refer to sonic-net/SONiC#1132 for more details.

How I verified it
Add unittest.

Signed-off-by: Longxiang Lyu <lolv@microsoft.com>
  • Loading branch information
@lolyu
lolyu committed Dec 13, 2022
1 parent 6695113 commit 242ee11
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 31 deletions.
51 changes: 33 additions & 18 deletions orchagent/muxorch.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,8 +359,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, std::set<IpAddress> skip_neighbors)
:mux_name_(name), srv_ip4_(srv_ip4), srv_ip6_(srv_ip6), peer_ip4_(peer_ip), skip_neighbors_(skip_neighbors)
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)
{
mux_orch_ = gDirectory.get<MuxOrch*>();
mux_cb_orch_ = gDirectory.get<MuxCableOrch*>();
Expand Down Expand Up @@ -549,11 +549,6 @@ 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 All @@ -570,7 +565,6 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
SWSS_LOG_INFO("Neigh %s on %s, add %d, state %d",
nh.ip_address.to_string().c_str(), nh.alias.c_str(), add, state);

MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();
IpPrefix pfx = nh.ip_address.to_string();

if (add)
Expand All @@ -597,7 +591,7 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
case MuxState::MUX_STATE_STANDBY:
neighbors_[nh.ip_address] = tunnelId;
gNeighOrch->disableNeighbor(nh);
mux_cb_orch->addTunnelRoute(nh);
updateTunnelRoute(nh, true);
create_route(pfx, tunnelId);
break;
default:
Expand All @@ -612,7 +606,7 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
if (state == MuxState::MUX_STATE_STANDBY)
{
remove_route(pfx);
mux_cb_orch->removeTunnelRoute(nh);
updateTunnelRoute(nh, false);
}
neighbors_.erase(nh.ip_address);
}
Expand All @@ -621,7 +615,6 @@ void MuxNbrHandler::update(NextHopKey nh, sai_object_id_t tunnelId, bool add, Mu
bool MuxNbrHandler::enable(bool update_rt)
{
NeighborEntry neigh;
MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();

auto it = neighbors_.begin();
while (it != neighbors_.end())
Expand Down Expand Up @@ -677,7 +670,7 @@ bool MuxNbrHandler::enable(bool update_rt)
{
return false;
}
mux_cb_orch->removeTunnelRoute(nh_key);
updateTunnelRoute(nh_key, false);
}

it++;
Expand All @@ -689,7 +682,6 @@ bool MuxNbrHandler::enable(bool update_rt)
bool MuxNbrHandler::disable(sai_object_id_t tnh)
{
NeighborEntry neigh;
MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();

auto it = neighbors_.begin();
while (it != neighbors_.end())
Expand Down Expand Up @@ -735,7 +727,7 @@ bool MuxNbrHandler::disable(sai_object_id_t tnh)
return false;
}

mux_cb_orch->addTunnelRoute(nh_key);
updateTunnelRoute(nh_key, true);

IpPrefix pfx = it->first.to_string();
if (create_route(pfx, it->second) != SAI_STATUS_SUCCESS)
Expand All @@ -760,6 +752,27 @@ sai_object_id_t MuxNbrHandler::getNextHopId(const NextHopKey nhKey)
return SAI_NULL_OBJECT_ID;
}

void MuxNbrHandler::updateTunnelRoute(NextHopKey nh, bool add)
{
MuxOrch* mux_orch = gDirectory.get<MuxOrch*>();
MuxCableOrch* mux_cb_orch = gDirectory.get<MuxCableOrch*>();

if (mux_orch->isSkipNeighbor(nh.ip_address))
{
SWSS_LOG_INFO("Skip updating neighbor %s, add %d", nh.ip_address.to_string().c_str(), add);
return;
}

if (add)
{
mux_cb_orch->addTunnelRoute(nh);
}
else
{
mux_cb_orch->removeTunnelRoute(nh);
}
}

std::map<std::string, AclTable> MuxAclHandler::acl_table_;

MuxAclHandler::MuxAclHandler(sai_object_id_t port, string alias)
Expand Down Expand Up @@ -977,7 +990,7 @@ bool MuxOrch::isNeighborActive(const IpAddress& nbr, const MacAddress& mac, stri

if (ptr)
{
return (ptr->isActive() || ptr->isSkipNeighbor(nbr));
return ptr->isActive();
}

string port;
Expand All @@ -991,15 +1004,15 @@ bool MuxOrch::isNeighborActive(const IpAddress& nbr, const MacAddress& mac, stri
if (!port.empty() && isMuxExists(port))
{
MuxCable* ptr = getMuxCable(port);
return (ptr->isActive() || ptr->isSkipNeighbor(nbr));
return ptr->isActive();
}

NextHopKey nh_key = NextHopKey(nbr, alias);
string curr_port = getNexthopMuxName(nh_key);
if (port.empty() && !curr_port.empty() && isMuxExists(curr_port))
{
MuxCable* ptr = getMuxCable(curr_port);
return (ptr->isActive() || ptr->isSkipNeighbor(nbr));
return ptr->isActive();
}

return true;
Expand Down Expand Up @@ -1295,7 +1308,8 @@ 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_, skip_neighbors));
(MuxCable(port_name, srv_ip, srv_ip6, mux_peer_switch_));
addSkipNeighbors(skip_neighbors);

SWSS_LOG_NOTICE("Mux entry for port '%s' was added", port_name.c_str());
}
Expand All @@ -1307,6 +1321,7 @@ bool MuxOrch::handleMuxCfg(const Request& request)
return true;
}

removeSkipNeighbors(skip_neighbors);
mux_cable_tb_.erase(port_name);

SWSS_LOG_NOTICE("Mux cable for port '%s' was removed", port_name.c_str());
Expand Down
30 changes: 23 additions & 7 deletions orchagent/muxorch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ class MuxNbrHandler

sai_object_id_t getNextHopId(const NextHopKey);

private:
inline void updateTunnelRoute(NextHopKey, bool = true);

private:
MuxNeighbor neighbors_;
string alias_;
Expand All @@ -76,7 +79,7 @@ class MuxNbrHandler
class MuxCable
{
public:
MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip, std::set<IpAddress> skip_neighbors);
MuxCable(string name, IpPrefix& srv_ip4, IpPrefix& srv_ip6, IpAddress peer_ip);

bool isActive() const
{
Expand All @@ -97,10 +100,6 @@ class MuxCable
{
return nbr_handler_->getNextHopId(nh);
}
bool isSkipNeighbor(const IpAddress& nbr)
{
return (skip_neighbors_.find(nbr) != skip_neighbors_.end());
}

private:
bool stateActive();
Expand All @@ -119,8 +118,6 @@ 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 Down Expand Up @@ -180,6 +177,11 @@ class MuxOrch : public Orch2, public Observer, public Subject
return mux_cable_tb_.at(portName).get();
}

bool isSkipNeighbor(const IpAddress& nbr)
{
return (skip_neighbors_.find(nbr) != skip_neighbors_.end());
}

MuxCable* findMuxCableInSubnet(IpAddress);
bool isNeighborActive(const IpAddress&, const MacAddress&, string&);
void update(SubjectType, void *);
Expand Down Expand Up @@ -212,6 +214,19 @@ class MuxOrch : public Orch2, public Observer, public Subject
void createStandaloneTunnelRoute(IpAddress neighborIp);
void removeStandaloneTunnelRoute(IpAddress neighborIp);

void addSkipNeighbors(const std::set<IpAddress> &neighbors)
{
skip_neighbors_.insert(neighbors.begin(), neighbors.end());
}

void removeSkipNeighbors(const std::set<IpAddress> &neighbors)
{
for (const IpAddress &neighbor : neighbors)
{
skip_neighbors_.erase(neighbor);
}
}

IpAddress mux_peer_switch_ = 0x0;
sai_object_id_t mux_tunnel_id_ = SAI_NULL_OBJECT_ID;

Expand All @@ -227,6 +242,7 @@ class MuxOrch : public Orch2, public Observer, public Subject

MuxCfgRequest request_;
std::set<IpAddress> standalone_tunnel_neighbors_;
std::set<IpAddress> skip_neighbors_;
};

const request_description_t mux_cable_request_description = {
Expand Down
41 changes: 35 additions & 6 deletions tests/test_mux.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class TestMuxTunnelBase():
APP_MUX_CABLE = "MUX_CABLE_TABLE"
APP_NEIGH_TABLE = "NEIGH_TABLE"
APP_TUNNEL_DECAP_TABLE_NAME = "TUNNEL_DECAP_TABLE"
APP_TUNNEL_ROUTE_TABLE_NAME = "TUNNEL_ROUTE_TABLE"
ASIC_TUNNEL_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL"
ASIC_TUNNEL_TERM_ENTRIES = "ASIC_STATE:SAI_OBJECT_TYPE_TUNNEL_TERM_TABLE_ENTRY"
ASIC_RIF_TABLE = "ASIC_STATE:SAI_OBJECT_TYPE_ROUTER_INTERFACE"
Expand Down Expand Up @@ -166,6 +167,14 @@ def get_vlan_rif_oid(self, asicdb):

return vlan_oid

def check_tunnel_route_in_app_db(self, dvs, destinations, expected=True):
appdb = dvs.get_app_db()

if expected:
appdb.wait_for_matching_keys(self.APP_TUNNEL_ROUTE_TABLE_NAME, destinations)
else:
appdb.wait_for_deleted_keys(self.APP_TUNNEL_ROUTE_TABLE_NAME, destinations)

def check_neigh_in_asic_db(self, asicdb, ip, expected=True):
rif_oid = self.get_vlan_rif_oid(asicdb)
switch_oid = self.get_switch_oid(asicdb)
Expand Down Expand Up @@ -275,9 +284,6 @@ 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")
srv1_v6 = self.check_neigh_in_asic_db(asicdb, self.SERV1_IPV6)

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)

existing_keys = asicdb.get_keys(self.ASIC_NEIGH_TABLE)

self.add_neighbor(dvs, self.SERV2_IPV4, "00:00:00:00:00:02")
Expand All @@ -291,7 +297,7 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
)

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

# Change state to Standby. This will delete Neigh and add Route
self.set_mux_state(appdb, "Ethernet0", "standby")
Expand All @@ -301,8 +307,6 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
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)
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")
Expand All @@ -313,6 +317,26 @@ def create_and_test_neighbor(self, confdb, appdb, asicdb, dvs, dvs_route):
self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV4)
self.check_neigh_in_asic_db(asicdb, self.SERV2_IPV6)

def create_and_test_soc(self, appdb, asicdb, dvs, dvs_route):

self.set_mux_state(appdb, "Ethernet0", "active")

self.add_fdb(dvs, "Ethernet0", "00-00-00-00-00-01")
self.add_neighbor(dvs, self.SERV1_SOC_IPV4, "00:00:00:00:00:01")

time.sleep(1)

srv1_soc_v4 = self.check_neigh_in_asic_db(asicdb, self.SERV1_SOC_IPV4)
self.check_tunnel_route_in_app_db(dvs, [self.SERV1_SOC_IPV4+self.IPV4_MASK], expected=False)

self.set_mux_state(appdb, "Ethernet0", "standby")

asicdb.wait_for_deleted_entry(self.ASIC_NEIGH_TABLE, srv1_soc_v4)
dvs_route.check_asicdb_route_entries(
[self.SERV1_SOC_IPV4+self.IPV4_MASK]
)
self.check_tunnel_route_in_app_db(dvs, [self.SERV1_SOC_IPV4+self.IPV4_MASK], expected=False)

marker = dvs.add_log_marker()

self.set_mux_state(appdb, "Ethernet0", "active")
Expand Down Expand Up @@ -1122,6 +1146,11 @@ def test_neighbor_miss_no_peer(
for ip in test_ips:
self.check_neighbor_state(dvs, dvs_route, ip, expect_route=False)

def test_soc_ip(self, dvs, dvs_route, setup_vlan, setup_mux_cable, testlog):
appdb = swsscommon.DBConnector(swsscommon.APPL_DB, dvs.redis_sock, 0)
asicdb = dvs.get_asic_db()

self.create_and_test_soc(appdb, asicdb, dvs, dvs_route)

# Add Dummy always-pass test at end as workaroud
# for issue when Flaky fail on final test it invokes module tear-down before retrying
Expand Down

0 comments on commit 242ee11

Please sign in to comment.