Skip to content

Commit

Permalink
[RouteOrch] Record ROUTE_TABLE entry programming status to APPL_STATE…
Browse files Browse the repository at this point in the history
…_DB (sonic-net#2512)

* [RouteOrch] Record ROUTE_TABLE entry programming status to APPL_STATE_DB
* Support route supression in BGP. Orchagent has to sends route programming feedback back to fpmsyncd which communicates with zebra.
  • Loading branch information
stepanblyschak authored Feb 14, 2023
1 parent 0704f78 commit 35385ad
Show file tree
Hide file tree
Showing 5 changed files with 132 additions and 7 deletions.
42 changes: 40 additions & 2 deletions orchagent/routeorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ RouteOrch::RouteOrch(DBConnector *db, vector<table_name_with_pri_t> &tableNames,
{
SWSS_LOG_ENTER();

m_publisher.setBuffered(true);

sai_attribute_t attr;
attr.id = SAI_SWITCH_ATTR_NUMBER_OF_ECMP_GROUPS;

Expand Down Expand Up @@ -499,7 +501,7 @@ void RouteOrch::doTask(Consumer& consumer)

auto rc = toBulk.emplace(std::piecewise_construct,
std::forward_as_tuple(key, op),
std::forward_as_tuple());
std::forward_as_tuple(key, (op == SET_COMMAND)));

bool inserted = rc.second;
auto& ctx = rc.first->second;
Expand Down Expand Up @@ -630,6 +632,11 @@ void RouteOrch::doTask(Consumer& consumer)

if (fvField(i) == "seg_src")
srv6_source = fvValue(i);

if (fvField(i) == "protocol")
{
ctx.protocol = fvValue(i);
}
}

/*
Expand Down Expand Up @@ -831,6 +838,10 @@ void RouteOrch::doTask(Consumer& consumer)
/* fullmask subnet route is same as ip2me route */
else if (ip_prefix.isFullMask() && m_intfsOrch->isPrefixSubnet(ip_prefix, alsv[0]))
{
/* The prefix is full mask (/32 or /128) and it is an interface subnet route, so IntfOrch has already
* created an IP2ME route for it and we skip programming such route here as it already exists.
* However, to keep APPL_DB and APPL_STATE_DB consistent we have to publish it. */
publishRouteState(ctx);
it = consumer.m_toSync.erase(it);
}
/* subnet route, vrf leaked route, etc */
Expand Down Expand Up @@ -860,7 +871,9 @@ void RouteOrch::doTask(Consumer& consumer)
}
else
{
/* Duplicate entry */
/* Duplicate entry. Publish route state anyway since there could be multiple DEL, SET operations
* consolidated by ConsumerStateTable leading to orchagent receiving only the last SET update. */
publishRouteState(ctx);
it = consumer.m_toSync.erase(it);
}

Expand Down Expand Up @@ -2226,6 +2239,9 @@ bool RouteOrch::addRoutePost(const RouteBulkContext& ctx, const NextHopGroupKey

notifyNextHopChangeObservers(vrf_id, ipPrefix, nextHops, true);

/* Publish and update APPL STATE DB route entry programming status */
publishRouteState(ctx);

/*
* If the route uses a temporary synced NHG owned by NhgOrch, return false
* in order to keep trying to update the route in case the NHG is updated,
Expand Down Expand Up @@ -2419,6 +2435,9 @@ bool RouteOrch::removeRoutePost(const RouteBulkContext& ctx)

SWSS_LOG_INFO("Remove route %s with next hop(s) %s",
ipPrefix.to_string().c_str(), it_route->second.nhg_key.to_string().c_str());

/* Publish removal status, removes route entry from APPL STATE DB */
publishRouteState(ctx);

if (ipPrefix.isDefaultRoute() && vrf_id == gVirtualRouterId)
{
Expand Down Expand Up @@ -2574,3 +2593,22 @@ void RouteOrch::decNhgRefCount(const std::string &nhg_index)
gCbfNhgOrch->decNhgRefCount(nhg_index);
}
}

void RouteOrch::publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status)
{
SWSS_LOG_ENTER();

std::vector<FieldValueTuple> fvs;

/* Leave the fvs empty if the operation type is "DEL".
* An empty fvs makes ResponsePublisher::publish() remove the state entry from APPL_STATE_DB
*/
if (ctx.is_set)
{
fvs.emplace_back("protocol", ctx.protocol);
}

const bool replace = false;

m_publisher.publish(APP_ROUTE_TABLE_NAME, ctx.key, fvs, status, replace);
}
12 changes: 10 additions & 2 deletions orchagent/routeorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,12 @@ struct RouteBulkContext
// using_temp_nhg will track if the NhgOrch's owned NHG is temporary or not
bool using_temp_nhg;

RouteBulkContext()
: excp_intfs_flag(false), using_temp_nhg(false)
std::string key; // Key in database table
std::string protocol; // Protocol string
bool is_set; // True if set operation

RouteBulkContext(const std::string& key, bool is_set)
: key(key), excp_intfs_flag(false), using_temp_nhg(false), is_set(is_set)
{
}

Expand All @@ -139,6 +143,8 @@ struct RouteBulkContext
excp_intfs_flag = false;
vrf_id = SAI_NULL_OBJECT_ID;
using_temp_nhg = false;
key.clear();
protocol.clear();
}
};

Expand Down Expand Up @@ -269,6 +275,8 @@ class RouteOrch : public Orch, public Subject
const NhgBase &getNhg(const std::string& nhg_index);
void incNhgRefCount(const std::string& nhg_index);
void decNhgRefCount(const std::string& nhg_index);

void publishRouteState(const RouteBulkContext& ctx, const ReturnCode& status = ReturnCode(SAI_STATUS_SUCCESS));
};

#endif /* SWSS_ROUTEORCH_H */
2 changes: 1 addition & 1 deletion tests/mock_tests/Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ tests_intfmgrd_INCLUDES = $(tests_INCLUDES) -I$(top_srcdir)/cfgmgr -I$(top_srcdi
tests_intfmgrd_CFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI)
tests_intfmgrd_CPPFLAGS = $(DBGFLAGS) $(AM_CFLAGS) $(CFLAGS_COMMON) $(CFLAGS_GTEST) $(CFLAGS_SAI) $(tests_intfmgrd_INCLUDES)
tests_intfmgrd_LDADD = $(LDADD_GTEST) $(LDADD_SAI) -lnl-genl-3 -lhiredis -lhiredis \
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread
-lswsscommon -lswsscommon -lgtest -lgtest_main -lzmq -lnl-3 -lnl-route-3 -lpthread -lgmock -lgmock_main

## fpmsyncd unit tests

Expand Down
21 changes: 19 additions & 2 deletions tests/mock_tests/fake_response_publisher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,36 @@
#include <vector>

#include "response_publisher.h"
#include "mock_response_publisher.h"

/* This mock plugs into this fake response publisher implementation
* when needed to test code that uses response publisher. */
std::unique_ptr<MockResponsePublisher> gMockResponsePublisher;

ResponsePublisher::ResponsePublisher(bool buffered) : m_db(std::make_unique<swss::DBConnector>("APPL_STATE_DB", 0)), m_buffered(buffered) {}

void ResponsePublisher::publish(
const std::string& table, const std::string& key,
const std::vector<swss::FieldValueTuple>& intent_attrs,
const ReturnCode& status,
const std::vector<swss::FieldValueTuple>& state_attrs, bool replace) {}
const std::vector<swss::FieldValueTuple>& state_attrs, bool replace)
{
if (gMockResponsePublisher)
{
gMockResponsePublisher->publish(table, key, intent_attrs, status, state_attrs, replace);
}
}

void ResponsePublisher::publish(
const std::string& table, const std::string& key,
const std::vector<swss::FieldValueTuple>& intent_attrs,
const ReturnCode& status, bool replace) {}
const ReturnCode& status, bool replace)
{
if (gMockResponsePublisher)
{
gMockResponsePublisher->publish(table, key, intent_attrs, status, replace);
}
}

void ResponsePublisher::writeToDB(
const std::string& table, const std::string& key,
Expand Down
62 changes: 62 additions & 0 deletions tests/mock_tests/routeorch_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@
#include "ut_helper.h"
#include "mock_orchagent_main.h"
#include "mock_table.h"
#include "mock_response_publisher.h"
#include "bulker.h"

extern string gMySwitchType;

extern std::unique_ptr<MockResponsePublisher> gMockResponsePublisher;

using ::testing::_;

namespace routeorch_test
{
Expand Down Expand Up @@ -282,6 +286,10 @@ namespace routeorch_test
{"mac_addr", "00:00:00:00:00:00" }});
intfTable.set("Ethernet0:10.0.0.1/24", { { "scope", "global" },
{ "family", "IPv4" }});
intfTable.set("Ethernet4", { {"NULL", "NULL" },
{"mac_addr", "00:00:00:00:00:00" }});
intfTable.set("Ethernet4:11.0.0.1/32", { { "scope", "global" },
{ "family", "IPv4" }});
gIntfsOrch->addExistingData(&intfTable);
static_cast<Orch *>(gIntfsOrch)->doTask();

Expand Down Expand Up @@ -422,4 +430,58 @@ namespace routeorch_test
ASSERT_EQ(current_set_count + 1, set_route_count);
ASSERT_EQ(sai_fail_count, 0);
}

TEST_F(RouteOrchTest, RouteOrchTestSetDelResponse)
{
gMockResponsePublisher = std::make_unique<MockResponsePublisher>();

std::deque<KeyOpFieldsValuesTuple> entries;
std::string key = "2.2.2.0/24";
std::vector<FieldValueTuple> fvs{{"ifname", "Ethernet0,Ethernet0"}, {"nexthop", "10.0.0.2,10.0.0.3"}, {"protocol", "bgp"}};
entries.push_back({key, "SET", fvs});

auto consumer = dynamic_cast<Consumer *>(gRouteOrch->getExecutor(APP_ROUTE_TABLE_NAME));
consumer->addToSync(entries);

EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
static_cast<Orch *>(gRouteOrch)->doTask();

// add entries again to the consumer queue (in case of rapid DEL/SET operations from fpmsyncd, routeorch just gets the last SET update)
consumer->addToSync(entries);

EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
static_cast<Orch *>(gRouteOrch)->doTask();

entries.clear();

// Route deletion

entries.clear();
entries.push_back({key, "DEL", {}});

consumer->addToSync(entries);

EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
static_cast<Orch *>(gRouteOrch)->doTask();

gMockResponsePublisher.reset();
}

TEST_F(RouteOrchTest, RouteOrchSetFullMaskSubnetPrefix)
{
gMockResponsePublisher = std::make_unique<MockResponsePublisher>();

std::deque<KeyOpFieldsValuesTuple> entries;
std::string key = "11.0.0.1/32";
std::vector<FieldValueTuple> fvs{{"ifname", "Ethernet4"}, {"nexthop", "0.0.0.0"}, {"protocol", "bgp"}};
entries.push_back({key, "SET", fvs});

auto consumer = dynamic_cast<Consumer *>(gRouteOrch->getExecutor(APP_ROUTE_TABLE_NAME));
consumer->addToSync(entries);

EXPECT_CALL(*gMockResponsePublisher, publish(APP_ROUTE_TABLE_NAME, key, std::vector<FieldValueTuple>{{"protocol", "bgp"}}, ReturnCode(SAI_STATUS_SUCCESS), false)).Times(1);
static_cast<Orch *>(gRouteOrch)->doTask();

gMockResponsePublisher.reset();
}
}

0 comments on commit 35385ad

Please sign in to comment.