Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[202211][muxorch] Skip programming SoC IP kernel tunnel route #2841

Merged
merged 3 commits into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 33 additions & 18 deletions orchagent/muxorch.cpp
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 @@ -547,11 +547,6 @@ void MuxCable::updateNeighbor(NextHopKey nh, bool add)
SWSS_LOG_NOTICE("Processing update on neighbor %s for mux %s, add %d, state %d",
nh.ip_address.to_string().c_str(), mux_name_.c_str(), add, state_);
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 @@ -568,7 +563,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 @@ -595,7 +589,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 @@ -610,7 +604,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 @@ -619,7 +613,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 @@ -675,7 +668,7 @@ bool MuxNbrHandler::enable(bool update_rt)
{
return false;
}
mux_cb_orch->removeTunnelRoute(nh_key);
updateTunnelRoute(nh_key, false);
}

it++;
Expand All @@ -687,7 +680,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 @@ -733,7 +725,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 @@ -758,6 +750,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 @@ -981,7 +994,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 @@ -995,15 +1008,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 @@ -1299,7 +1312,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 @@ -1311,6 +1325,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
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ class MuxNbrHandler

sai_object_id_t getNextHopId(const NextHopKey);

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

private:
MuxNeighbor neighbors_;
string alias_;
Expand All @@ -77,7 +80,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 @@ -98,10 +101,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 @@ -120,8 +119,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 @@ -181,6 +178,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 @@ -215,6 +217,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 @@ -230,6 +245,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
50 changes: 44 additions & 6 deletions tests/test_mux.py
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 @@ -162,6 +163,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 @@ -271,9 +280,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 @@ -287,7 +293,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 @@ -297,8 +303,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 @@ -309,6 +313,35 @@ 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")
self.set_mux_state(appdb, "Ethernet0", "active")
self.check_syslog(dvs, marker, "Maintaining current MUX state", 1)

self.set_mux_state(appdb, "Ethernet0", "init")
self.check_syslog(dvs, marker, "State transition from active to init is not-handled", 1)

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

self.set_mux_state(appdb, "Ethernet0", "active")
Expand Down Expand Up @@ -1140,6 +1173,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