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

Bfd support for TSA state. #2926

Merged
merged 15 commits into from
Feb 7, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
189 changes: 183 additions & 6 deletions orchagent/bfdorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ BfdOrch::BfdOrch(DBConnector *db, string tableName, TableConnector stateDbBfdSes

Orch::addExecutor(bfdStateNotificatier);
register_state_change_notif = false;
BgpGlobalStateOrch* bgp_global_state_orch = gDirectory.get<BgpGlobalStateOrch*>();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you don't need this section. Just initialize tsa_enabled to false in class and handle TSA change in the normal update path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

if (bgp_global_state_orch)
{
tsa_enabled = bgp_global_state_orch->getTsaState();
}
else
{
tsa_enabled = false;
}

}

BfdOrch::~BfdOrch(void)
Expand All @@ -96,18 +106,66 @@ void BfdOrch::doTask(Consumer &consumer)

if (op == SET_COMMAND)
{
if (!create_bfd_session(key, data))
bool tsa_shutdown_enabled = false;
for (auto i : data)
{
it++;
continue;
auto value = fvValue(i);
//shutdown_bfd_during_tsa parameter is used by the BFD session creator to ensure that the the
//specified session gets removed when the device goes into TSA state.
//if this parameter is not specified or set to false for a session, the
// corrosponding BFD session would be maintained even in TSA state.
if (fvField(i) == "shutdown_bfd_during_tsa" && value == "true" )
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arlakshm , fyi (This param can control the individual BFD session state during TSA)

{
tsa_shutdown_enabled = true;
break;
}
}
if (tsa_shutdown_enabled)
{
bfd_session_cache[key] = data;
if (!tsa_enabled)
{
if (!create_bfd_session(key, data))
{
it++;
continue;
}
}
else
{
notify_session_state_down(key);
}
}
else
{
if (!create_bfd_session(key, data))
{
it++;
continue;
}
}
}
else if (op == DEL_COMMAND)
{
if (!remove_bfd_session(key))
if (bfd_session_cache.find(key) != bfd_session_cache.end() )
{
it++;
continue;
bfd_session_cache.erase(key);
if (!tsa_enabled)
{
if (!remove_bfd_session(key))
{
it++;
continue;
}
}
}
else
{
if (!remove_bfd_session(key))
{
it++;
continue;
}
}
}
else
Expand Down Expand Up @@ -298,6 +356,12 @@ bool BfdOrch::create_bfd_session(const string& key, const vector<FieldValueTuple
{
tos = to_uint<uint8_t>(value);
}
else if (fvField(i) == "shutdown_bfd_during_tsa")
{
//since we are handling shutdown_bfd_during_tsa in the caller function, we need to ignore it here.
//failure to ignore this parameter would cause error log.
continue;
}
else
SWSS_LOG_ERROR("Unsupported BFD attribute %s\n", fvField(i).c_str());
}
Expand Down Expand Up @@ -551,3 +615,116 @@ uint32_t BfdOrch::bfd_src_port(void)
return (port++);
}

void BfdOrch::notify_session_state_down(const string& key)
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
{
size_t found_vrf = key.find(delimiter);
if (found_vrf == string::npos)
{
SWSS_LOG_ERROR("Failed to parse key %s, no vrf is given", key.c_str());
return;
}

size_t found_ifname = key.find(delimiter, found_vrf + 1);
if (found_ifname == string::npos)
{
SWSS_LOG_ERROR("Failed to parse key %s, no ifname is given", key.c_str());
return;
}
string vrf_name = key.substr(0, found_vrf);
string alias = key.substr(found_vrf + 1, found_ifname - found_vrf - 1);
IpAddress peer_address(key.substr(found_ifname + 1));
BfdUpdate update;
update.peer = get_state_db_key(vrf_name, alias, peer_address);
update.state = SAI_BFD_SESSION_STATE_DOWN;
notify(SUBJECT_TYPE_BFD_SESSION_STATE_CHANGE, static_cast<void *>(&update));
return;
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
}

void BfdOrch::handleTsaStateChange(bool tsaState)
{
SWSS_LOG_INFO("BfdOrch TSA state Changed to %d.\n", int(tsaState));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please change this to NOTICE? , also log the existing state

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. now log is in the caller function.

tsa_enabled = tsaState;
for (auto it : bfd_session_cache)
{
if (tsaState == true)
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
{
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
notify_session_state_down(it.first);
if (!remove_bfd_session(it.first))
{
SWSS_LOG_ERROR("Failed to remove BFD session %s\n", it.first.c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove the error log as its handled in the remove function already

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
} else
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

else -> next line

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

{
if (!create_bfd_session(it.first, it.second))
{
SWSS_LOG_ERROR("Failed to create BFD session %s\n", it.first.c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the error log

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}
}
}
}

BgpGlobalStateOrch::BgpGlobalStateOrch(DBConnector *db, string tableName):
Orch(db, tableName)
{
SWSS_LOG_ENTER();
tsa_enabled = false;
SWSS_LOG_ERROR("BgpGlobalStateOrch init complete\n");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is this ERROR log, please remove

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

BgpGlobalStateOrch::~BgpGlobalStateOrch(void)
{
SWSS_LOG_ENTER();
}

bool BgpGlobalStateOrch::getTsaState()
{
SWSS_LOG_ENTER();
return tsa_enabled;
}
void BgpGlobalStateOrch::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();

auto it = consumer.m_toSync.begin();
while (it != consumer.m_toSync.end())
{
KeyOpFieldsValuesTuple t = it->second;

string key = kfvKey(t);
string op = kfvOp(t);
auto data = kfvFieldsValues(t);

if (op == SET_COMMAND)
{
for (auto i : data)
{
auto value = fvValue(i);
auto type = fvField(i);
SWSS_LOG_INFO("SET on key %s, data T %s, V %s\n", key.c_str(), type.c_str(), value.c_str());
if (type == "tsa_enabled")
{
bool state = true ? value == "true" : false;
if (tsa_enabled != state)
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
{
tsa_enabled = state;
BfdOrch* bfd_orch = gDirectory.get<BfdOrch*>();
if (bfd_orch)
{
bfd_orch->handleTsaStateChange(state);
}
}
}
}
}
else if (op == DEL_COMMAND)
{
SWSS_LOG_ERROR("DEL on key %s is not expected.\n", key.c_str());
}
else
{
SWSS_LOG_ERROR("Unknown operation type %s\n", op.c_str());
}
it = consumer.m_toSync.erase(it);
}
}

17 changes: 17 additions & 0 deletions orchagent/bfdorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ class BfdOrch: public Orch, public Subject
void doTask(swss::NotificationConsumer &consumer);
BfdOrch(swss::DBConnector *db, std::string tableName, TableConnector stateDbBfdSessionTable);
virtual ~BfdOrch(void);
void handleTsaStateChange(bool tsaState);

private:
bool create_bfd_session(const std::string& key, const std::vector<swss::FieldValueTuple>& data);
Expand All @@ -26,6 +27,7 @@ class BfdOrch: public Orch, public Subject
uint32_t bfd_gen_id(void);
uint32_t bfd_src_port(void);

void notify_session_state_down(const std::string& key);
bool register_bfd_state_change_notification(void);
void update_port_number(std::vector<sai_attribute_t> &attrs);
sai_status_t retry_create_bfd_session(sai_object_id_t &bfd_session_id, vector<sai_attribute_t> attrs);
Expand All @@ -37,6 +39,21 @@ class BfdOrch: public Orch, public Subject

swss::NotificationConsumer* m_bfdStateNotificationConsumer;
bool register_state_change_notif;
std::map<std::string, vector<FieldValueTuple>> bfd_session_cache;
bool tsa_enabled;

};

class BgpGlobalStateOrch : public Orch
{
public:
void doTask(Consumer &consumer);
BgpGlobalStateOrch(swss::DBConnector *db, std::string tableName);
virtual ~BgpGlobalStateOrch(void);
bool getTsaState();

private:
bool tsa_enabled;

};
#endif /* SWSS_BFDORCH_H */
10 changes: 8 additions & 2 deletions orchagent/orchdaemon.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,9 +145,15 @@ bool OrchDaemon::init()
TableConnector stateDbFdb(m_stateDb, STATE_FDB_TABLE_NAME);
TableConnector stateMclagDbFdb(m_stateDb, STATE_MCLAG_REMOTE_FDB_TABLE_NAME);
gFdbOrch = new FdbOrch(m_applDb, app_fdb_tables, stateDbFdb, stateMclagDbFdb, gPortsOrch);

TableConnector stateDbBfdSessionTable(m_stateDb, STATE_BFD_SESSION_TABLE_NAME);
gBfdOrch = new BfdOrch(m_applDb, APP_BFD_SESSION_TABLE_NAME, stateDbBfdSessionTable);

BgpGlobalStateOrch* bgp_global_state_orch;
bgp_global_state_orch = new BgpGlobalStateOrch(m_configDb, CFG_BGP_DEVICE_GLOBAL_TABLE_NAME);
gDirectory.set(bgp_global_state_orch);

gBfdOrch = new BfdOrch(m_applDb, APP_BFD_SESSION_TABLE_NAME, stateDbBfdSessionTable);
gDirectory.set(gBfdOrch);
static const vector<string> route_pattern_tables = {
CFG_FLOW_COUNTER_ROUTE_PATTERN_TABLE_NAME,
};
Expand Down Expand Up @@ -399,7 +405,7 @@ bool OrchDaemon::init()
* when iterating ConsumerMap. This is ensured implicitly by the order of keys in ordered map.
* For cases when Orch has to process tables in specific order, like PortsOrch during warm start, it has to override Orch::doTask()
*/
m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gFlowCounterRouteOrch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, gPolicerOrch, tunnel_decap_orch, sflow_orch, gDebugCounterOrch, gMacsecOrch, gBfdOrch, gSrv6Orch, mux_orch, mux_cb_orch, gMonitorOrch};
m_orchList = { gSwitchOrch, gCrmOrch, gPortsOrch, gBufferOrch, gFlowCounterRouteOrch, gIntfsOrch, gNeighOrch, gNhgMapOrch, gNhgOrch, gCbfNhgOrch, gRouteOrch, gCoppOrch, gQosOrch, wm_orch, gPolicerOrch, tunnel_decap_orch, sflow_orch, gDebugCounterOrch, gMacsecOrch, bgp_global_state_orch, gBfdOrch, gSrv6Orch, mux_orch, mux_cb_orch, gMonitorOrch};

bool initialize_dtel = false;
if (platform == BFN_PLATFORM_SUBSTRING || platform == VS_PLATFORM_SUBSTRING)
Expand Down
4 changes: 4 additions & 0 deletions orchagent/vnetorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,10 @@ void VNetRouteOrch::createBfdSession(const string& vnet, const NextHopKey& endpo
FieldValueTuple fvTuple("local_addr", src_ip.to_string());
data.push_back(fvTuple);
data.emplace_back("multihop", "true");
// The BFD sessions established by the Vnet routes with monitoring need to be brought down
// when the device goes into TSA. The following parameter ensures that these session are
// brought down while transitioning to TSA and brought back up when transitioning to TSB.
data.emplace_back("shutdown_bfd_during_tsa", "true");
bfd_session_producer_.set(key, data);
bfd_sessions_[monitor_addr].bfd_state = SAI_BFD_SESSION_STATE_DOWN;
}
Expand Down