Fix sai api loglevel #365

shi-su · 2020-07-17T23:58:04Z

Why I did it
This PR contains the code changes to fix the SAI API log level issue "swssloglevel -l SAI_LOG_LEVEL_ERROR -s -c switch" or any SAI API component no longer works in Master branch #366
Why this issue happens
In the initialization of syncd, the main function first calls linkToDbNative function to initialize the log level for syncd. Then the log levels for SAI APIs are initialized using linkToDb. Since the settingThread of logger is set up by linkToDbNative function and the ConsumerStateTables are set up only in the initialization of the thread, the SAI API log levels in only added to m_settingChangeObservers, whereas the corresponding ConsumerStateTables are not created. Therefore, the SAI API log levels are not successfully initialized.
How I did it
Instead of creating ConsumerStateTables at the initialization of settingThread, I create ConsumerStateTables in the execution loop of the thread. Such a change fixes the issue by allowing to create the corresponding ConsumerStateTables when there are new items added to m_settingChangeObservers.
How to verify it
Verify all SAI API log levels are correctly initialized and can be modified in vs tests. A unit test is also added to simulate the scenario and verify the log levels can be properly handled.

Add support for attribute capability query in lua script (sonic-net#362)

…added to m_settingChangeObservers

qiluo-msft · 2020-07-18T00:31:35Z

@gechiang Could you help test this PR? #Closed

qiluo-msft · 2020-07-18T00:34:08Z

common/logger.cpp

+        {
+            for (const auto& i : m_settingChangeObservers)
+            {
+                if(selectableKeys.find(i.first) == selectableKeys.end())


i.first [](start = 39, length = 7)

i.first is used multiple times. Could you give it a name to improve readability? #Closed

qiluo-msft · 2020-07-18T00:35:58Z

common/logger.cpp

+            {
+                if(selectableKeys.find(i.first) == selectableKeys.end())
+                {
+                    std::shared_ptr<ConsumerStateTable> table = std::make_shared<ConsumerStateTable>(&db, i.first);


std::shared_ptr [](start = 20, length = 35)

auto #Closed

lguohan · 2020-07-18T00:39:05Z

@shi-su, can you provide more detail description of the pr? what is the bug, how the bug is fixed.

qiluo-msft · 2020-07-18T00:42:25Z

common/logger.cpp

+        {
+            for (const auto& i : m_settingChangeObservers)
+            {
+                if(selectableKeys.find(i.first) == selectableKeys.end())


selectableKeys [](start = 19, length = 14)

You can insert and check return value. No need to find beforehand. #Closed

Since the vector selectables and the set selectableKeys in now combined as a map, inserting into the map requires a pointer to a consumerStateTable. In such a case, I think it might be better to find before creating the consumerStateTable.

qiluo-msft · 2020-07-18T00:44:56Z

common/logger.cpp

+                {
+                    std::shared_ptr<ConsumerStateTable> table = std::make_shared<ConsumerStateTable>(&db, i.first);
+                    selectableKeys.insert(i.first);
+                    selectables.push_back(table);


push_back [](start = 32, length = 9)

emplace_back? #Closed

It now becomes inserting into a map.

https://en.cppreference.com/w/cpp/container/map/emplace

In reply to: 456727155 [](ancestors = 456727155)

Changed to emplace.

qiluo-msft · 2020-07-18T00:46:16Z

common/logger.cpp

-        select.addSelectable(table.get());
-    }
+    std::vector<std::shared_ptr<ConsumerStateTable>> selectables;
+    std::set<std::string> selectableKeys;

    while(true)


while(true) [](start = 4, length = 11)

Add one blank after while #Closed

Blank added.

common/logger.cpp

gechiang · 2020-07-18T00:52:16Z

@gechiang Could you help test this PR?

sure I will help test this out
Is there a buildimage that I can find to load this to my DUT?

qiluo-msft · 2020-07-18T00:52:25Z

common/logger.cpp

-        select.addSelectable(table.get());
-    }
+    std::vector<std::shared_ptr<ConsumerStateTable>> selectables;
+    std::set<std::string> selectableKeys;


Seems selectables is only used to manage memory. You may use a map instead of vector + set. #Closed

Fixed as prescribed.

qiluo-msft · 2020-07-18T01:29:53Z

common/logger.cpp

        Selectable *selectable = nullptr;

-        int ret = select.select(&selectable);
+        int ret = select.select(&selectable, 1000);


1000 [](start = 45, length = 4)

Add comment about constant unit. #Closed

qiluo-msft · 2020-07-18T01:30:28Z

common/logger.cpp

        Selectable *selectable = nullptr;

-        int ret = select.select(&selectable);
+        int ret = select.select(&selectable, 1000);


select [](start = 25, length = 6)

Add a unit test case to simulate the fixed bug. #Closed

A unit test is added.

qiluo-msft

As comments

kcudnik · 2020-07-18T17:14:51Z

common/logger.cpp


        if (ret == Select::ERROR)
        {
            SWSS_LOG_NOTICE("%s select error %s", __PRETTY_FUNCTION__, strerror(errno));
            continue;
        }

+        if (ret == Select::TIMEOUT)
+        {
+            continue;


if this is not intended to happen, please add log line

Since a timeout is added to select selectables, the timeout is actually intended. I added an info level log to the timeout.

gechiang · 2020-07-19T20:21:12Z

@gechiang Could you help test this PR?

sure I will help test this out
Is there a buildimage that I can find to load this to my DUT?

I have tested this change and find it resolved the issue I raised. I was able to set the levels of all the SAI component via the "-a" option as well as individual SAI component via the "-c" option. Fix looks good.
Thanks!

qiluo-msft · 2020-07-20T17:53:34Z

common/logger.cpp

+        {
+            for (const auto& i : m_settingChangeObservers)
+            {
+                std::string dbName = i.first;


std::string [](start = 16, length = 11)

const std::string& or const auto& #Closed

lguohan · 2020-07-22T01:48:18Z

@shi-su, can you updat the submodule pointer on sonic-buildimage?

Signed-off-by: Mihir Patel <[email protected]>

shi-su and others added 2 commits July 17, 2020 12:27

Merge pull request #1 from Azure/master

8a3cd04

Add support for attribute capability query in lua script (sonic-net#362)

Support creation of ConsumerStateTables that correspond to new items …

548744f

…added to m_settingChangeObservers

lguohan linked an issue Jul 18, 2020 that may be closed by this pull request

"swssloglevel -l SAI_LOG_LEVEL_ERROR -s -c switch" or any SAI API component no longer works in Master branch #366

Closed

qiluo-msft reviewed Jul 18, 2020

View reviewed changes

common/logger.cpp Show resolved Hide resolved

qiluo-msft reviewed Jul 18, 2020

View reviewed changes

qiluo-msft requested changes Jul 18, 2020

View reviewed changes

kcudnik reviewed Jul 18, 2020

View reviewed changes

shi-su added 4 commits July 20, 2020 16:55

Add unit test for log level

cf89baa

Address review comments

2a73631

Address review comments-2

51c2727

Handle nullptr in the log level unit test

b9e805a

shi-su requested a review from qiluo-msft July 20, 2020 17:26

qiluo-msft reviewed Jul 20, 2020

View reviewed changes

Address review comments-3

ac4f118

qiluo-msft approved these changes Jul 22, 2020

View reviewed changes

lguohan merged commit ed9c497 into sonic-net:master Jul 22, 2020

prgeor pushed a commit to prgeor/sonic-swss-common that referenced this pull request Feb 27, 2025

Adding electrical for 800G and 100G (sonic-net#365)

85a4507

Signed-off-by: Mihir Patel <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix sai api loglevel #365

Fix sai api loglevel #365

shi-su commented Jul 17, 2020 •

edited

Loading

qiluo-msft commented Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

lguohan commented Jul 18, 2020

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

qiluo-msft Jul 20, 2020

shi-su Jul 21, 2020

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

gechiang commented Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

qiluo-msft Jul 18, 2020 •

edited

Loading

shi-su Jul 20, 2020

qiluo-msft left a comment

kcudnik Jul 18, 2020

shi-su Jul 20, 2020

gechiang commented Jul 19, 2020

qiluo-msft Jul 20, 2020 •

edited

Loading

shi-su Jul 21, 2020

lguohan commented Jul 22, 2020

Fix sai api loglevel #365

Fix sai api loglevel #365

Conversation

shi-su commented Jul 17, 2020 • edited Loading

qiluo-msft commented Jul 18, 2020 • edited Loading

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Jul 18, 2020

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gechiang commented Jul 18, 2020 • edited Loading

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gechiang commented Jul 19, 2020

qiluo-msft Jul 20, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan commented Jul 22, 2020

shi-su commented Jul 17, 2020 •

edited

Loading

qiluo-msft commented Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

gechiang commented Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 18, 2020 •

edited

Loading

qiluo-msft Jul 20, 2020 •

edited

Loading