[FlexCounterManager] Add ACL_COUNTER FC type #12

stepanblyschak · 2021-09-20T14:14:10Z

Signed-off-by: Stepan Blyschak [email protected]

What I did

Why I did it

How I verified it

Details if related

*Fixed the failed DPB LAG tests

…onic-net#1929) *Add sleep in route perf test to ensure the test starts when route table is stable.

* QOS field reference ABNF format to string changes

Signed-off-by: Stepan Blyschak <[email protected]>

* Created a map to store single nexthop routes. This is for faster retrieval when you've to fetch the routes for a particular nexthop. No changes to ECMP route handling.

* [VS Test] Reduce route count for route perf test

- If the fake_platform does not change between test modules, re-use the same DVS container (but still restart it between modules and do some cleanup/re-init to ensure a consistent start state for each test module) - Add a CLI option --force-recreate-dvs to revert to the previous behavior of creating a new DVS per test module Signed-off-by: Lawrence Lee <[email protected]>

orchagent/aclorch.h

orchagent/aclorch.cpp

tests/dvslib/dvs_acl.py

Signed-off-by: Stepan Blyshchak <[email protected]>

…ConfigDone (sonic-net#1861) *continue the execution of handlePortConfigFromConfigDB when no ports on config db -> continue and set PortConfigDone *function handlePortConfigFromConfigDB is no longer boolean - change it to void Signed-off-by: tomeri <[email protected]>

* Add test_gearbox * Add DVS_ENV for module specific dvs env variables

What I did Reverted skipped test_buffer_dynamic as part of sonic-net#1754 Why I did it How I verified it sudo pytest --dvsname=vs --forcedvs -sv --keeptb test_buffer_dynamic.py ======================================================= test session starts ======================================================== platform linux -- Python 3.6.9, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /usr/bin/python3 cachedir: .pytest_cache rootdir: /home/ashokd/swss-vs/ashok-swss/sonic-swss/tests plugins: flaky-3.7.0 collected 9 items test_buffer_dynamic.py::TestBufferMgrDyn::test_changeSpeed remove extra link dummy PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_changeCableLen PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_MultipleLosslessPg PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_headroomOverride PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_mtuUpdate PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_nonDefaultAlpha PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_sharedHeadroomPool PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_shutdownPort PASSED test_buffer_dynamic.py::TestBufferMgrDyn::test_autoNegPort PASSED

nazariig · 2021-10-01T11:21:45Z

orchagent/aclorch.cpp

+        return false;
+    }
+
+    auto res = removeRanges();


@stepanblyschak not sure if it's ok to use auto here...

This is questionable. As a rule of thumb I would prefer the code with auto, especially considering that return type of the function can be also deduced in newer c++. I don't see a value in having to explicitelly say it's bool.

nazariig · 2021-10-01T11:23:14Z

orchagent/aclorch.cpp

-    attr.id = SAI_ACL_COUNTER_ATTR_ENABLE_PACKET_COUNT;
-    attr.value.booldata = true;
-    counter_attrs.push_back(attr);
+    for (auto counterAttrPair: aclCounterLookup)


@stepanblyschak why not to use const auto& ?

Better to use.

nazariig · 2021-10-01T11:29:16Z

orchagent/aclorch.cpp

@@ -3165,7 +3183,17 @@ bool AclOrch::addAclRule(shared_ptr<AclRule> newRule, string table_id)
        return false;
    }

-    return m_AclTables[table_oid].add(newRule);
+    if (!m_AclTables[table_oid].add(newRule))


@stepanblyschak do we need to add if (!newRule) prior to this?

We do, currently it is undefined behaviour if we pass nullptr regardless of this change. I will try to push it in another PR.

nazariig · 2021-10-01T11:36:04Z

orchagent/aclorch.cpp

+    auto counterOidStr = sai_serialize_object_id(rule.getCounterOid());
+
+    unordered_set<string> serializedCounterStatAttrs;
+    for (auto counterAttrPair: aclCounterLookup)


@stepanblyschak why not to use const auto& ?

Better to use.

Signed-off-by: Stepan Blyschak <[email protected]>

**What I did** Fix the Mem Leak by moving the raw pointers in type_maps to use smart pointers **Why I did it** ``` Indirect leak of 83776 byte(s) in 476 object(s) allocated from: #0 0x7f0a2a414647 in operator new(unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:99 #1 0x5555590cc923 in __gnu_cxx::new_allocator, std::allocator > const, referenced_object> > >::allocate(unsigned long, void const*) /usr/include/c++/10/ext/new_allocator.h:115 #2 0x5555590cc923 in std::allocator_traits, std::allocator > const, referenced_object> > > >::allocate(std::allocator, std::allocator > const, referenced_object> > >&, unsigned long) /usr/include/c++/10/bits/alloc_traits.h:460 #3 0x5555590cc923 in std::_Rb_tree, std::allocator >, std::pair, std::allocator > const, referenced_object>, std::_Select1st, std::allocator > const, referenced_object> >, std::less, std::allocator > >, std::allocator, std::allocator > const, referenced_object> > >::_M_get_node() /usr/include/c++/10/bits/stl_tree.h:584 #4 0x5555590cc923 in std::_Rb_tree_node, std::allocator > const, referenced_object> >* std::_Rb_tree, std::allocator >, std::pair, std::allocator > const, referenced_object>, std::_Select1st, std::allocator > const, referenced_object> >, std::less, std::allocator > >, std::allocator, std::allocator > const, referenced_object> > >::_M_create_node, std::allocator > const&>, std::tuple<> >(std::piecewise_construct_t const&, std::tuple, std::allocator > const&>&&, std::tuple<>&&) /usr/include/c++/10/bits/stl_tree.h:634 #5 0x5555590cc923 in std::_Rb_tree_iterator, std::allocator > const, referenced_object> > std::_Rb_tree, std::allocator >, std::pair, std::allocator > const, referenced_object>, std::_Select1st, std::allocator > const, referenced_object> >, std::less, std::allocator > >, std::allocator, std::allocator > const, referenced_object> > >::_M_emplace_hint_unique, std::allocator > const&>, std::tuple<> >(std::_Rb_tree_const_iterator, std::allocator > const, referenced_object> >, std::piecewise_construct_t const&, std::tuple, std::allocator > const&>&&, std::tuple<>&&) /usr/include/c++/10/bits/stl_tree.h:2461 #6 0x5555590e8757 in std::map, std::allocator >, referenced_object, std::less, std::allocator > >, std::allocator, std::allocator > const, referenced_object> > >::operator[](std::__cxx11::basic_string, std::allocator > const&) /usr/include/c++/10/bits/stl_map.h:501 #7 0x5555590d48b0 in Orch::setObjectReference(std::map, std::allocator >, std::map, std::allocator >, referenced_object, std::less, std::allocator > >, std::allocator, std::allocator > const, referenced_object> > >*, std::less, std::allocator > >, std::allocator, std::allocator > const, std::map, std::allocator >, referenced_object, std::less, std::allocator > >, std::allocator, std::allocator > const, referenced_object> > >*> > >&, std::__cxx11::basic_string, std::allocator > const&, std::__cxx11::basic_string, std::allocator > const&, std::__cxx11::basic_string, std::allocator > const&, std::__cxx11::basic_string, std::allocator > const&) orchagent/orch.cpp:450 #8 0x5555594ff66b in QosOrch::handleQueueTable(Consumer&, std::tuple, std::allocator >, std::__cxx11::basic_string, std::allocator >, std::vector, std::allocator >, std::__cxx11::basic_string, std::allocator > >, std::allocator, std::allocator >, std::__cxx11::basic_string, std::allocator > > > > >&) orchagent/qosorch.cpp:1763 #9 0x5555594edbd6 in QosOrch::doTask(Consumer&) orchagent/qosorch.cpp:2179 #10 0x5555590c8743 in Consumer::drain() orchagent/orch.cpp:241 #11 0x5555590c8743 in Consumer::drain() orchagent/orch.cpp:238 #12 0x5555590c8743 in Consumer::execute() orchagent/orch.cpp:235 #13 0x555559090dad in OrchDaemon::start() orchagent/orchdaemon.cpp:755 #14 0x555558e9be25 in main orchagent/main.cpp:766 #15 0x7f0a299b6d09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09) ```

stepanblyschak force-pushed the acl-fc branch 10 times, most recently from 51a64b9 to b65b49f Compare September 22, 2021 14:10

dgsudharsan and others added 2 commits September 23, 2021 14:28

Fix failing DPB LAG tests (sonic-net#1919)

a89d1f8

*Fixed the failed DPB LAG tests

Add sleep to ensure starting route perf test after the vs is stable (s…

a8fcadf

…onic-net#1929) *Add sleep in route perf test to ensure the test starts when route table is stable.

stepanblyschak force-pushed the acl-fc branch from b65b49f to 451c230 Compare September 24, 2021 18:59

QOS fieldvalue reference ABNF format to string changes (sonic-net#1754)

a3fdaf4

* QOS field reference ABNF format to string changes

stepanblyschak force-pushed the acl-fc branch from 451c230 to 99b2a63 Compare September 27, 2021 09:28

[FlexCounterManager] Add ACL_COUNTER FC type

c07fc3f

Signed-off-by: Stepan Blyschak <[email protected]>

stepanblyschak force-pushed the acl-fc branch from 99b2a63 to c07fc3f Compare September 27, 2021 09:38

prsunny and others added 3 commits September 27, 2021 08:12

Cache routes for single nexthop for faster retrieval (sonic-net#1922)

f1aca8a

* Created a map to store single nexthop routes. This is for faster retrieval when you've to fetch the routes for a particular nexthop. No changes to ECMP route handling.

Reduce route count for route perf test (sonic-net#1928)

a875d60

* [VS Test] Reduce route count for route perf test

stepanblyschak commented Sep 28, 2021

View reviewed changes

orchagent/aclorch.h Outdated Show resolved Hide resolved

stepanblyschak commented Sep 28, 2021

View reviewed changes

orchagent/aclorch.cpp Outdated Show resolved Hide resolved

stepanblyschak commented Sep 28, 2021

View reviewed changes

orchagent/aclorch.cpp Outdated Show resolved Hide resolved

orchagent/aclorch.cpp Outdated Show resolved Hide resolved

orchagent/aclorch.cpp Outdated Show resolved Hide resolved

orchagent/aclorch.cpp Outdated Show resolved Hide resolved

tests/dvslib/dvs_acl.py Outdated Show resolved Hide resolved

stepanblyschak and others added 4 commits September 28, 2021 17:30

internal review comments handling

c07009e

Signed-off-by: Stepan Blyshchak <[email protected]>

[gearbox] Add gearbox unit test (sonic-net#1920)

d23924f

* Add test_gearbox * Add DVS_ENV for module specific dvs env variables

nazariig reviewed Oct 1, 2021

View reviewed changes

nazariig approved these changes Oct 1, 2021

View reviewed changes

stepanblyschak added 2 commits October 3, 2021 18:27

use const auto& in for loops

1a08896

Signed-off-by: Stepan Blyschak <[email protected]>

Merge branch 'master' of github.com:azure/sonic-swss into acl-fc

4eef944

stepanblyschak closed this Oct 6, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FlexCounterManager] Add ACL_COUNTER FC type #12

[FlexCounterManager] Add ACL_COUNTER FC type #12

stepanblyschak commented Sep 20, 2021

nazariig Oct 1, 2021

stepanblyschak Oct 3, 2021

nazariig Oct 1, 2021 •

edited

Loading

stepanblyschak Oct 3, 2021

nazariig Oct 1, 2021

stepanblyschak Oct 3, 2021

nazariig Oct 1, 2021

stepanblyschak Oct 3, 2021

[FlexCounterManager] Add ACL_COUNTER FC type #12

[FlexCounterManager] Add ACL_COUNTER FC type #12

Conversation

stepanblyschak commented Sep 20, 2021

nazariig Oct 1, 2021

Choose a reason for hiding this comment

stepanblyschak Oct 3, 2021

Choose a reason for hiding this comment

nazariig Oct 1, 2021 • edited Loading

Choose a reason for hiding this comment

stepanblyschak Oct 3, 2021

Choose a reason for hiding this comment

nazariig Oct 1, 2021

Choose a reason for hiding this comment

stepanblyschak Oct 3, 2021

Choose a reason for hiding this comment

nazariig Oct 1, 2021

Choose a reason for hiding this comment

stepanblyschak Oct 3, 2021

Choose a reason for hiding this comment

nazariig Oct 1, 2021 •

edited

Loading