Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
87501b2
[FC] process FC after apply view
stepanblyschak Oct 11, 2024
47fb40f
Remove m_flexCounterConfigTable
stepanblyschak Oct 15, 2024
017a569
delay counters by 60 sec
stepanblyschak Nov 12, 2024
ada9e59
Delay for warm/fast start
stepanblyschak Nov 13, 2024
956f1f6
Handle review comment
stepanblyschak Nov 14, 2024
12eea05
use env variable to delay
stepanblyschak Nov 14, 2024
c8e8b41
Add unit test
stepanblyschak Nov 16, 2024
18b3606
Revert "Add unit test"
stepanblyschak Nov 19, 2024
71e05f5
Revert "use env variable to delay"
stepanblyschak Nov 19, 2024
e2a68ae
Update UT
stepanblyschak Nov 19, 2024
a1e320d
Merge branch 'master' into fc-after-apply-view
bingwang-ms Nov 21, 2024
829d529
Merge branch 'master' into fc-after-apply-view
stepanblyschak Dec 5, 2024
3eeb06e
Merge branch 'master' into fc-after-apply-view
stepanblyschak Dec 9, 2024
bd84f92
Merge branch 'master' into fc-after-apply-view
stepanblyschak Dec 18, 2024
03e31eb
Merge branch 'master' into fc-after-apply-view
liat-grozovik Dec 24, 2024
9da6b4d
Merge branch 'master' into fc-after-apply-view
dgsudharsan Jan 15, 2025
0d4fd74
Replace new with unique_ptr
stepanblyschak Jan 17, 2025
7255d8d
Merge branch 'master' into fc-after-apply-view
stepanblyschak Jan 27, 2025
c412aae
Merge branch 'master' into fc-after-apply-view
stepanblyschak Jan 29, 2025
5600064
Merge branch 'master' into fc-after-apply-view
qiluo-msft Jan 30, 2025
00455a5
Merge branch 'master' into fc-after-apply-view
bingwang-ms Feb 5, 2025
71b6513
Merge branch 'master' into fc-after-apply-view
stepanblyschak Feb 7, 2025
bacf16b
Merge branch 'master' into fc-after-apply-view
liat-grozovik Feb 11, 2025
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
80 changes: 35 additions & 45 deletions orchagent/flexcounterorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "macsecorch.h"
#include "dash/dashorch.h"
#include "flowcounterrouteorch.h"
#include "warm_restart.h"

extern sai_port_api_t *sai_port_api;
extern sai_switch_api_t *sai_switch_api;
Expand All @@ -28,6 +29,8 @@ extern CoppOrch *gCoppOrch;
extern FlowCounterRouteOrch *gFlowCounterRouteOrch;
extern sai_object_id_t gSwitchId;

#define FLEX_COUNTER_DELAY_SEC 60

#define BUFFER_POOL_WATERMARK_KEY "BUFFER_POOL_WATERMARK"
#define PORT_KEY "PORT"
#define PORT_BUFFER_DROP_KEY "PORT_BUFFER_DROP"
Expand Down Expand Up @@ -69,12 +72,22 @@ unordered_map<string, string> flexCounterGroupMap =

FlexCounterOrch::FlexCounterOrch(DBConnector *db, vector<string> &tableNames):
Orch(db, tableNames),
m_flexCounterConfigTable(db, CFG_FLEX_COUNTER_TABLE_NAME),
m_bufferQueueConfigTable(db, CFG_BUFFER_QUEUE_TABLE_NAME),
m_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME),
m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME)
{
SWSS_LOG_ENTER();
m_delayTimer = std::make_unique<SelectableTimer>(timespec{.tv_sec = FLEX_COUNTER_DELAY_SEC, .tv_nsec = 0});
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@vaibhavhd , could you review the warm boot part?

if (WarmStart::isWarmStart())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just to confirm this will also handle fast-reboot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes

{
m_delayExecutor = std::make_unique<ExecutableTimer>(m_delayTimer.get(), this, "FLEX_COUNTER_DELAY");
Orch::addExecutor(m_delayExecutor.get());
m_delayTimer->start();
}
else
{
m_delayTimerExpired = true;
}
}

FlexCounterOrch::~FlexCounterOrch(void)
Expand All @@ -86,6 +99,11 @@ void FlexCounterOrch::doTask(Consumer &consumer)
{
SWSS_LOG_ENTER();

if (!m_delayTimerExpired)
{
return;
}

VxlanTunnelOrch* vxlan_tunnel_orch = gDirectory.get<VxlanTunnelOrch*>();
DashOrch* dash_orch = gDirectory.get<DashOrch*>();
if (gPortsOrch && !gPortsOrch->allPortsReady())
Expand Down Expand Up @@ -116,16 +134,9 @@ void FlexCounterOrch::doTask(Consumer &consumer)

if (op == SET_COMMAND)
{
auto itDelay = std::find(std::begin(data), std::end(data), FieldValueTuple(FLEX_COUNTER_DELAY_STATUS_FIELD, "true"));
string poll_interval;
string bulk_chunk_size;
string bulk_chunk_size_per_counter;

if (itDelay != data.end())
{
consumer.m_toSync.erase(it++);
continue;
}
for (auto valuePair:data)
{
const auto &field = fvField(valuePair);
Expand Down Expand Up @@ -255,12 +266,6 @@ void FlexCounterOrch::doTask(Consumer &consumer)
}
}
}
else if(field == FLEX_COUNTER_DELAY_STATUS_FIELD)
{
// This field is ignored since it is being used before getting into this loop.
// If it is exist and the value is 'true' we need to skip the iteration in order to delay the counter creation.
// The field will clear out and counter will be created when enable_counters script is called.
}
else
{
SWSS_LOG_NOTICE("Unsupported field %s", field.c_str());
Expand All @@ -285,6 +290,20 @@ void FlexCounterOrch::doTask(Consumer &consumer)
}
}

void FlexCounterOrch::doTask(SelectableTimer&)
{
SWSS_LOG_ENTER();

if (m_delayTimerExpired)
{
return;
}

SWSS_LOG_NOTICE("Processing counters");
m_delayTimer->stop();
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How about following?

if (!m_delayTimerExpired)
{
    m_delayTimer->stop();
    m_delayTimerExpired = true;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

m_delayTimerExpired = true;
}

bool FlexCounterOrch::getPortCountersState() const
{
return m_port_counter_enabled;
Expand Down Expand Up @@ -322,39 +341,10 @@ bool FlexCounterOrch::bake()
* By default, it should fetch items from the tables the sub agents listen to,
* and then push them into m_toSync of each sub agent.
* The motivation is to make sub agents handle the saved entries first and then handle the upcoming entries.
* The FCs are not data plane configuration required during reconciling process, hence don't do anything in bake.
*/

std::deque<KeyOpFieldsValuesTuple> entries;
vector<string> keys;
m_flexCounterConfigTable.getKeys(keys);
for (const auto &key: keys)
{
if (!flexCounterGroupMap.count(key))
{
SWSS_LOG_NOTICE("FlexCounterOrch: Invalid flex counter group intput %s is skipped during reconciling", key.c_str());
continue;
}

if (key == BUFFER_POOL_WATERMARK_KEY)
{
SWSS_LOG_NOTICE("FlexCounterOrch: Do not handle any FLEX_COUNTER table for %s update during reconciling",
BUFFER_POOL_WATERMARK_KEY);
continue;
}

KeyOpFieldsValuesTuple kco;

kfvKey(kco) = key;
kfvOp(kco) = SET_COMMAND;

if (!m_flexCounterConfigTable.get(key, kfvFieldsValues(kco)))
{
continue;
}
entries.push_back(kco);
}
Consumer* consumer = dynamic_cast<Consumer *>(getExecutor(CFG_FLEX_COUNTER_TABLE_NAME));
return consumer->addToSync(entries);
return true;
}

static bool isCreateOnlyConfigDbBuffers(Table& deviceMetadataConfigTable)
Expand Down
6 changes: 5 additions & 1 deletion orchagent/flexcounterorch.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include "orch.h"
#include "port.h"
#include "producertable.h"
#include "selectabletimer.h"
#include "table.h"

extern "C" {
Expand Down Expand Up @@ -40,6 +41,7 @@ class FlexCounterOrch: public Orch
{
public:
void doTask(Consumer &consumer);
void doTask(SelectableTimer &timer);
FlexCounterOrch(swss::DBConnector *db, std::vector<std::string> &tableNames);
virtual ~FlexCounterOrch(void);
bool getPortCountersState() const;
Expand All @@ -63,10 +65,12 @@ class FlexCounterOrch: public Orch
bool m_pg_watermark_enabled = false;
bool m_hostif_trap_counter_enabled = false;
bool m_route_flow_counter_enabled = false;
Table m_flexCounterConfigTable;
bool m_delayTimerExpired = false;
Table m_bufferQueueConfigTable;
Table m_bufferPgConfigTable;
Table m_deviceMetadataConfigTable;
std::unique_ptr<SelectableTimer> m_delayTimer;
std::unique_ptr<Executor> m_delayExecutor;
std::unordered_set<std::string> m_groupsWithBulkChunkSize;
};

Expand Down
6 changes: 5 additions & 1 deletion orchagent/p4orch/tests/fake_flexcounterorch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
#include "flexcounterorch.h"

FlexCounterOrch::FlexCounterOrch(swss::DBConnector *db, std::vector<std::string> &tableNames)
: Orch(db, tableNames), m_flexCounterConfigTable(db, CFG_FLEX_COUNTER_TABLE_NAME),
: Orch(db, tableNames),
m_bufferQueueConfigTable(db, CFG_BUFFER_QUEUE_TABLE_NAME), m_bufferPgConfigTable(db, CFG_BUFFER_PG_TABLE_NAME),
m_deviceMetadataConfigTable(db, CFG_DEVICE_METADATA_TABLE_NAME)
{
Expand All @@ -16,6 +16,10 @@ void FlexCounterOrch::doTask(Consumer &consumer)
{
}

void FlexCounterOrch::doTask(SelectableTimer &timer)
{
}

bool FlexCounterOrch::getPortCountersState() const
{
return true;
Expand Down
53 changes: 46 additions & 7 deletions tests/mock_tests/flexcounter_ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,13 @@ namespace flexcounter_test
sai_switch_api = pold_sai_switch_api;
}

struct FlexCounterTest : public ::testing::TestWithParam<std::tuple<bool, bool>>
enum class StartType
{
Cold,
Warm,
};

struct FlexCounterTest : public ::testing::TestWithParam<std::tuple<bool, bool, StartType>>
{
shared_ptr<swss::DBConnector> m_app_db;
shared_ptr<swss::DBConnector> m_config_db;
Expand All @@ -230,6 +236,7 @@ namespace flexcounter_test
shared_ptr<swss::DBConnector> m_asic_db;
shared_ptr<swss::DBConnector> m_flex_counter_db;
bool create_only_config_db_buffers;
StartType m_start_type;

FlexCounterTest()
{
Expand All @@ -256,6 +263,8 @@ namespace flexcounter_test

gTraditionalFlexCounter = get<0>(GetParam());
create_only_config_db_buffers = get<1>(GetParam());
m_start_type = get<2>(GetParam());

if (gTraditionalFlexCounter)
{
initFlexCounterTables();
Expand Down Expand Up @@ -300,7 +309,19 @@ namespace flexcounter_test
vector<string> flex_counter_tables = {
CFG_FLEX_COUNTER_TABLE_NAME
};

if (m_start_type == StartType::Warm)
{
WarmStart::getInstance().m_enabled = true;
}

auto* flexCounterOrch = new FlexCounterOrch(m_config_db.get(), flex_counter_tables);

if (m_start_type == StartType::Warm)
{
WarmStart::getInstance().m_enabled = false;
}

gDirectory.set(flexCounterOrch);

vector<string> buffer_tables = { APP_BUFFER_POOL_TABLE_NAME,
Expand Down Expand Up @@ -542,8 +563,7 @@ namespace flexcounter_test
ASSERT_TRUE(gPortsOrch->allPortsReady());

// Enable and check counters
const std::vector<FieldValueTuple> values({ {FLEX_COUNTER_DELAY_STATUS_FIELD, "false"},
{FLEX_COUNTER_STATUS_FIELD, "enable"} });
const std::vector<FieldValueTuple> values({ {FLEX_COUNTER_STATUS_FIELD, "enable"} });
flexCounterCfg.set("PG_WATERMARK", values);
flexCounterCfg.set("QUEUE_WATERMARK", values);
flexCounterCfg.set("QUEUE", values);
Expand All @@ -557,6 +577,13 @@ namespace flexcounter_test
flexCounterOrch->addExistingData(&flexCounterCfg);
static_cast<Orch *>(flexCounterOrch)->doTask();

if (m_start_type == StartType::Warm)
{
// Expire timer
flexCounterOrch->doTask(*flexCounterOrch->m_delayTimer);
static_cast<Orch *>(flexCounterOrch)->doTask();
}

ASSERT_TRUE(checkFlexCounterGroup(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP,
{
{POLL_INTERVAL_FIELD, "60000"},
Expand Down Expand Up @@ -878,16 +905,28 @@ namespace flexcounter_test
entries.clear();
static_cast<Orch *>(gBufferOrch)->doTask();
ASSERT_TRUE(checkFlexCounter(BUFFER_POOL_WATERMARK_STAT_COUNTER_FLEX_COUNTER_GROUP, pool_oid));

// Warm/fast-boot case - no FC processing done before APPLY_VIEW
std::vector<std::string> ts;

gDirectory.get<FlexCounterOrch*>()->bake();
gDirectory.get<FlexCounterOrch*>()->dumpPendingTasks(ts);

ASSERT_TRUE(ts.empty());
}

INSTANTIATE_TEST_CASE_P(
FlexCounterTests,
FlexCounterTest,
::testing::Values(
std::make_tuple(false, true),
std::make_tuple(false, false),
std::make_tuple(true, true),
std::make_tuple(true, false))
std::make_tuple(false, true, StartType::Cold),
std::make_tuple(false, false, StartType::Cold),
std::make_tuple(true, true, StartType::Cold),
std::make_tuple(true, false, StartType::Cold),
std::make_tuple(false, true, StartType::Warm),
std::make_tuple(false, false, StartType::Warm),
std::make_tuple(true, true, StartType::Warm),
std::make_tuple(true, false, StartType::Warm))
);

using namespace mock_orch_test;
Expand Down
Loading