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 1 commit
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
174 changes: 168 additions & 6 deletions orchagent/bfdorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ BfdOrch::BfdOrch(DBConnector *db, string tableName, TableConnector stateDbBfdSes

Orch::addExecutor(bfdStateNotificatier);
register_state_change_notif = false;
BgpGolbalStateOrch* bgp_global_state_orch = gDirectory.get<BgpGolbalStateOrch*>();
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
tsa_enabled = bgp_global_state_orch->getTsaState();
}

BfdOrch::~BfdOrch(void)
Expand All @@ -96,18 +98,62 @@ 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);
if (fvField(i) == "tsa_shutdown" && value == "true" )
{
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 +344,10 @@ bool BfdOrch::create_bfd_session(const string& key, const vector<FieldValueTuple
{
tos = to_uint<uint8_t>(value);
}
else if (fvField(i) == "tsa_shutdown")
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 the purpose of this section?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added a comment in the code and I have changed the name of the field to shutdown_bfd_during_tsa. The BFD tsa shutdown is handled by the bfd cache in the caller function. This block is added to avoid getting the unknown parameter syslog.

{
continue;
}
else
SWSS_LOG_ERROR("Unsupported BFD attribute %s\n", fvField(i).c_str());
}
Expand Down Expand Up @@ -551,3 +601,115 @@ 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.

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 ot remove BFD session %s\n", it.first.c_str());
}
} 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 ot create BFD session %s\n", it.first.c_str());
}
}
}
}

BgpGolbalStateOrch::BgpGolbalStateOrch(DBConnector *db, string tableName):
Orch(db, tableName)
{
SWSS_LOG_ENTER();
tsa_enabled = false;
SWSS_LOG_ERROR("BgpGolbalStateOrch init complete\n");
}

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

bool BgpGolbalStateOrch::getTsaState()
{
SWSS_LOG_ENTER();
SWSS_LOG_ERROR("SOME ONE CALLED ME\n");
siqbal1986 marked this conversation as resolved.
Show resolved Hide resolved
return tsa_enabled;
}
void BgpGolbalStateOrch::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)
{
SWSS_LOG_INFO("SET on key %s\n", key.c_str());
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;
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 BgpGolbalStateOrch : public Orch
{
public:
void doTask(Consumer &consumer);
BgpGolbalStateOrch(swss::DBConnector *db, std::string tableName);
virtual ~BgpGolbalStateOrch(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 @@ -144,9 +144,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);

BgpGolbalStateOrch* bgp_global_state_orch;
bgp_global_state_orch = new BgpGolbalStateOrch(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 @@ -397,7 +403,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
1 change: 1 addition & 0 deletions orchagent/vnetorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1817,6 +1817,7 @@ 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");
data.emplace_back("tsa_shutdown", "true");
Copy link
Collaborator

Choose a reason for hiding this comment

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

not clear of the meaning of this variable - 'tsa_shutdown' ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed it already and added comments.

bfd_session_producer_.set(key, data);
bfd_sessions_[monitor_addr].bfd_state = SAI_BFD_SESSION_STATE_DOWN;
}
Expand Down