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

sonic-swss: bfdorch changes to support software bfd sessions #3406

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
52 changes: 50 additions & 2 deletions orchagent/bfdorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "sai_serialize.h"
#include "directory.h"
#include "notifications.h"
#include "schema.h"

using namespace std;
using namespace swss;
Expand All @@ -21,12 +22,17 @@ using namespace swss;
#define BFD_SRCPORTMAX 65536
#define NUM_BFD_SRCPORT_RETRIES 3

//TODO: remove once this definition is committed in swss-common schema
#define STATE_BFD_SOFTWARE_SESSION_TABLE_NAME "SOFTWARE_BFD_SESSION_TABLE"

extern sai_bfd_api_t* sai_bfd_api;
extern sai_object_id_t gSwitchId;
extern sai_object_id_t gVirtualRouterId;
extern PortsOrch* gPortsOrch;
extern sai_switch_api_t* sai_switch_api;
extern Directory<Orch*> gDirectory;
extern string gMySwitchType;

Copy link
Contributor

Choose a reason for hiding this comment

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

why extra space


const map<string, sai_bfd_session_type_t> session_type_map =
{
Expand Down Expand Up @@ -62,16 +68,29 @@ BfdOrch::BfdOrch(DBConnector *db, string tableName, TableConnector stateDbBfdSes
m_bfdStateNotificationConsumer = new swss::NotificationConsumer(notificationsDb, "NOTIFICATIONS");
auto bfdStateNotificatier = new Notifier(m_bfdStateNotificationConsumer, this, "BFD_STATE_NOTIFICATIONS");

// Clean up state database BFD entries
m_stateDbConnector = std::make_unique<swss::DBConnector>("STATE_DB", 0);
m_stateSoftBfdSessionTable = std::make_unique<swss::Table>(m_stateDbConnector.get(), STATE_BFD_SOFTWARE_SESSION_TABLE_NAME);

SWSS_LOG_NOTICE("Switch type is: %s", gMySwitchType.c_str());

vector<string> keys;

// Clean up state database BFD entries
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove extra new line below this

m_stateBfdSessionTable.getKeys(keys);

for (auto alias : keys)
{
m_stateBfdSessionTable.del(alias);
}

// Clean up state database software BFD entries
if (gMySwitchType == "dpu") {
m_stateSoftBfdSessionTable->getKeys(keys);
for (auto alias : keys)
{
m_stateSoftBfdSessionTable->del(alias);
}
}

Orch::addExecutor(bfdStateNotificatier);
register_state_change_notif = false;
}
Expand All @@ -81,6 +100,21 @@ BfdOrch::~BfdOrch(void)
SWSS_LOG_ENTER();
}

std::string BfdOrch::replaceFirstTwoColons(const std::string &input) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please rename to something like createStateDBKey.

std::string result = input;
size_t pos = result.find(':'); // Find the first colon
if (pos != std::string::npos) {
result[pos] = '|'; // Replace the first colon with '|'

// Find the second colon
pos = result.find(':', pos + 1);
if (pos != std::string::npos) {
result[pos] = '|'; // Replace the second colon with '|'
}
}
return result;
}

void BfdOrch::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();
Expand All @@ -101,6 +135,13 @@ void BfdOrch::doTask(Consumer &consumer)

if (op == SET_COMMAND)
{
if (gMySwitchType == "dpu") {
//program entry in software BFD table
m_stateSoftBfdSessionTable->set(replaceFirstTwoColons(key), data);
it = consumer.m_toSync.erase(it);
continue;
}

bool tsa_shutdown_enabled = false;
for (auto i : data)
{
Expand Down Expand Up @@ -142,6 +183,13 @@ void BfdOrch::doTask(Consumer &consumer)
}
else if (op == DEL_COMMAND)
{
if (gMySwitchType == "dpu") {
//delete entry from software BFD table
m_stateSoftBfdSessionTable->del(replaceFirstTwoColons(key));
it = consumer.m_toSync.erase(it);
continue;
}

if (bfd_session_cache.find(key) != bfd_session_cache.end() )
{
bfd_session_cache.erase(key);
Expand Down
5 changes: 4 additions & 1 deletion orchagent/bfdorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,19 @@ class BfdOrch: public Orch, public Subject
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);
std::string replaceFirstTwoColons(const std::string &input);

std::map<std::string, sai_object_id_t> bfd_session_map;
std::map<sai_object_id_t, BfdUpdate> bfd_session_lookup;

swss::Table m_stateBfdSessionTable;

std::unique_ptr<swss::DBConnector> m_stateDbConnector;
std::unique_ptr<swss::Table> m_stateSoftBfdSessionTable;

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

};

class BgpGlobalStateOrch : public Orch
Expand Down
189 changes: 189 additions & 0 deletions tests/test_soft_bfd.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
import pytest

Check notice

Code scanning / CodeQL

Unused import Note test

Import of 'pytest' is not used.
import time

Check notice

Code scanning / CodeQL

Unused import Note test

Import of 'time' is not used.

from swsscommon import swsscommon

#Replace with swsscommon.SOFTWARE_BFD_SESSION_STATE_TABLE once those changes are in master
#SOFT_BFD__STATE_TABLE = swsscommon.STATE_BFD_SOFTWARE_SESSION_TABLE_NAME
SOFT_BFD_STATE_TABLE = "SOFTWARE_BFD_SESSION_TABLE"

class TestSoftBfd(object):
def setup_db(self, dvs):
dvs.setup_db()
self.pdb = dvs.get_app_db()
self.sdb = dvs.get_state_db()
self.cdb = dvs.get_config_db()

self.cdb.db_connection.hset('DEVICE_METADATA|localhost', "switch_type", "dpu")

#Restart swss to pick up new switch type
dvs.stop_swss()
dvs.start_swss()

def get_exist_bfd_session(self):
return set(self.sdb.get_keys(SOFT_BFD_STATE_TABLE))

def create_bfd_session(self, key, pairs):
tbl = swsscommon.ProducerStateTable(self.pdb.db_connection, "BFD_SESSION_TABLE")
fvs = swsscommon.FieldValuePairs(list(pairs.items()))
tbl.set(key, fvs)

def remove_bfd_session(self, key):
tbl = swsscommon.ProducerStateTable(self.pdb.db_connection, "BFD_SESSION_TABLE")
tbl._del(key)

def check_state_bfd_session_value(self, key, expected_values):
#Key format is different in STATE_DB compared to APP_DB
key = key.replace(":", "|", 2)
fvs = self.sdb.get_entry(SOFT_BFD_STATE_TABLE, key)
for k, v in expected_values.items():
assert fvs[k] == v

def test_addRemoveBfdSession(self, dvs):
self.setup_db(dvs)
bfd_session_key = "default:default:10.0.0.2"
bfdSessions = self.get_exist_bfd_session()

# Create BFD session
fieldValues = {"local_addr": "10.0.0.1", "tos": "64", "multiplier": "5", "tx_interval": "300",
"rx_interval": "500"}
self.create_bfd_session(bfd_session_key, fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Check created BFD session in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
assert len(createdSessions) == 1

session = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(bfd_session_key, fieldValues)

# Remove the BFD session
self.remove_bfd_session(bfd_session_key)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session)

def test_addRemoveBfdSession_ipv6(self, dvs):
self.setup_db(dvs)
bfd_session_key = "default:default:2000::2"
bfdSessions = self.get_exist_bfd_session()

# Create BFD session
fieldValues = {"local_addr": "2000::1", "multihop": "true", "multiplier": "3", "tx_interval": "400",
"rx_interval": "200"}
self.create_bfd_session(bfd_session_key, fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Check created BFD session in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
assert len(createdSessions) == 1

session = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(bfd_session_key, fieldValues)

# Remove the BFD session
self.remove_bfd_session(bfd_session_key)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session)

def test_addRemoveBfdSession_interface(self, dvs):
self.setup_db(dvs)
bfd_session_key = "default:Ethernet0:10.0.0.2"
bfdSessions = self.get_exist_bfd_session()

# Create BFD session
fieldValues = {"local_addr": "10.0.0.1", "dst_mac": "00:02:03:04:05:06", "type": "async_passive"}
self.create_bfd_session("default:Ethernet0:10.0.0.2", fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Check created BFD session in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
assert len(createdSessions) == 1

session = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(bfd_session_key, fieldValues)

# Remove the BFD session
self.remove_bfd_session(bfd_session_key)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session)

def test_multipleBfdSessions(self, dvs):
self.setup_db(dvs)
bfdSessions = self.get_exist_bfd_session()

# Create BFD session 1
key1 = "default:default:10.0.0.2"
fieldValues = {"local_addr": "10.0.0.1"}
self.create_bfd_session(key1, fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Checked BFD session 1 in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
bfdSessions = self.get_exist_bfd_session()
assert len(createdSessions) == 1

session1 = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(key1, fieldValues)

# Create BFD session 2
key2 = "default:default:10.0.1.2"
fieldValues = {"local_addr": "10.0.0.1", "tx_interval": "300", "rx_interval": "500"}
self.create_bfd_session(key2, fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Check BFD session 2 in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
bfdSessions = self.get_exist_bfd_session()
assert len(createdSessions) == 1

session2 = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(key2, fieldValues)

# Create BFD session 3
key3 = "default:default:2000::2"
fieldValues = {"local_addr": "2000::1", "type": "demand_active"}
self.create_bfd_session(key3, fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Check BFD session 3 in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
bfdSessions = self.get_exist_bfd_session()
Copy link
Contributor

Choose a reason for hiding this comment

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

can you please add a test in test_vnet.py where a vnet route with BFD session is created and added, that way we can be sure that it works fine with VNET routes as well.

assert len(createdSessions) == 1

session3 = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(key3, fieldValues)

# Create BFD session 4
key4 = "default:default:3000::2"
fieldValues = {"local_addr": "3000::1"}
self.create_bfd_session(key4, fieldValues)
self.sdb.wait_for_n_keys(SOFT_BFD_STATE_TABLE, len(bfdSessions) + 1)

# Check BFD session 3 in STATE_DB
createdSessions = self.get_exist_bfd_session() - bfdSessions
bfdSessions = self.get_exist_bfd_session()

Check notice

Code scanning / CodeQL

Unused local variable Note test

Variable bfdSessions is not used.
assert len(createdSessions) == 1

session4 = createdSessions.pop()

# Check STATE_DB entry related to the BFD session
self.check_state_bfd_session_value(key4, fieldValues)

# Remove the BFD sessions
self.remove_bfd_session(key1)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session1)
self.remove_bfd_session(key2)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session2)
self.remove_bfd_session(key3)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session3)
self.remove_bfd_session(key4)
self.sdb.wait_for_deleted_entry(SOFT_BFD_STATE_TABLE, session4)
Loading