[aclorch] Add ACL_TABLE_TYPE configuration #15
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
stepanblyschak
force-pushed
the
acl_table_type
branch
8 times, most recently
from
9321144 to
3c09253
Compare
Added an API to create a table with configurable ACL table type (matches, bpoints, actions). Implemented a handler for new ACL_TABLE_TYPE CONFIG DB table. Implemented UT for the above. Signed-off-by: Stepan Blyshchak <[email protected]>
stepanblyschak
force-pushed
the
acl_table_type
branch
5 times, most recently
from
4c6a6bd to
3843110
Compare
Signed-off-by: Stepan Blyshchak <[email protected]>
stepanblyschak
force-pushed
the
acl_table_type
branch
from
3843110 to
0e85da1
Compare
stepanblyschak
commented
Oct 22, 2021
orchagent/aclorch.cpp
Outdated
|
|
||
| if (type == ACL_TABLE_MIRROR || type == ACL_TABLE_MIRRORV6) | ||
| for (const auto& actionType: type.getActions()) |
There was a problem hiding this comment.
vector<int32_t> action_types_list = {type.getActions().begin(), type.getActions().end()};
stepanblyschak
commented
Oct 22, 2021
orchagent/aclorch.cpp
Outdated
| @@ -2152,7 +2070,7 @@ bool AclRuleDTelFlowWatchListEntry::create() | |||
| return false; | |||
| } | |||
|
|
|||
| return true; | |||
| return true; | |||
There was a problem hiding this comment.
remove trailing spaces
stepanblyschak
commented
Oct 22, 2021
orchagent/aclorch.cpp
Outdated
| // The upper layer can although ensure that | ||
| // user does not remove table type that is referenced | ||
| // by an ACL table. | ||
| auto erased = m_AclTableTypes.erase(tableTypeName); |
There was a problem hiding this comment.
if (!m_AclTableTypes.erase(tableTypeName))
{
return false;
}
return truel
stepanblyschak
commented
Oct 22, 2021
tests/mock_tests/aclorch_ut.cpp
Outdated
| @@ -631,23 +678,16 @@ namespace aclorch_test | |||
| auto const &resourceMap = Portal::CrmOrchInternal::getResourceMap(crmOrch); | |||
|
|
|||
| // Verify the ACL tables | |||
| uint32_t crmAclTableBindingCount = 0; | |||
| ssize_t crmAclTableBindingCount = 0; | |||
Signed-off-by: Stepan Blyshchak <[email protected]>
Signed-off-by: Stepan Blyshchak <[email protected]>
nazariig
approved these changes
Oct 22, 2021
…they should not use IN_PORTS, OUT_PORTS on L3 tables Signed-off-by: Stepan Blyshchak <[email protected]>
stepanblyschak
added a commit
that referenced
this pull request
Mar 29, 2022
**What I did** It could be that queue counters are populated to FLEX COUNTER DB before the COUNTER_QUEUE_INDEX_MAP is populated which is in accordance to PortsOrch implementation. Fixing it by replacing the order - first the COUNTER_QUEUE_INDEX_MAP then the FLEX COUNTER DB might produce other issues if readers start reading the map and try to find the corresponding entry in COUNTERS DB. Such an order is also in alignment with other maps and counters. So seems like fixing it in the reader is more suitable fix. Errors observed only for short period of time at system start when PFC WD is enabled: ``` Nov 10 15:12:19.001472 tgs-sonic-n2-s1 ERR syncd#SDK: :- guard: RedisReply catches system_error: command: *135#015#012$7#015#012EVALSHA#015#012$40#015#012b62081cc93943a4cbfec30de42638b435d31197c#015#012$3#015#012128#015#012$20#015#012oid:0x15000000000290#015#012$20#015#012oid:0x15000000000291#015#012$20#015#012oid:0x150000000002b1#015#012$20#015#012oid:0x150000000002b2#015#012$20#015#012oid:0x150000000002d2#015#012$20#015#012oid:0x150000000002d3#015#012$20#015#012oid:0x150000000002f3#015#012$20#015#012oid:0x150000000002f4#015#012$20#015#012oid:0x15000000000314#015#012$20#015#012oid:0x15000000000315#015#012$20#015#012oid:0x15000000000335#015#012$20#015#012oid:0x15000000000336#015#012$20#015#012oid:0x15000000000356#015#012$20#015#012oid:0x15000000000357#015#012$20#015#012oid:0x15000000000377#015#012$20#015#012oid:0x15000000000378#015#012$20#015#012oid:0x15000000000398#015#012$20#015#012oid:0x15000000000399#015#012$20#015#012oid:0x150000000003b9#015#012$20#015#012oid:0x150000000003ba#015#012$20#015#012oid:0x150000000003da#015#012$20#015#012oid:0x150000000003db#015#012$20#015#012oid:0x150000000003fb#015#012$20#015#012oid:0x150000000003fc#015#012$20#015#012oid:0x1500000000041c#015#012$20#015#012oid:0x1500000000041d#015#012$20#015#012oid:0x1500000000043d#015#012$20#015#012oid:0x1500000000043e#015#012$20#015#012oid:0x1500000000045e#015#012$20#015#012oid:0x1500000000045f#015#012$20#015#012oid:0x1500000000047f#015#012$20#015#012oid:0x15000000000480#015#012$20#015#012oid:0x150000000004a0#015#012$20#015#012oid:0x150000000004a1#015#012$20#015#012oid:0x150000000004c1#015#012$20#015#012oid:0x150000000004c2#015#012$20#015#012oid:0x150000000004e2#015#012$20#015#012oid:0x150000000004e3#015#012$20#015#012oid:0x15000000000503#015#012$20#015#012oid:0x15000000000504#015#012$20#015#012oid:0x15000000000524#015#012$20#015#012oid:0x15000000000525#015#012$20#015#012oid:0x15000000000545#015#012$20#015#012oid:0x15000000000546#015#012$20#015#012oid:0x15000000000566#015#012$20#015#012oid:0x15000000000567#015#012$20#015#012oid:0x15000000000587#015#012$20#015#012oid:0x15000000000588#015#012$20#015#012oid:0x150000000005a8#015#012$20#015#012oid:0x150000000005a9#015#012$20#015#012oid:0x150000000005c9#015#012$20#015#012oid:0x150000000005ca#015#012$20#015#012oid:0x150000000005ea#015#012$20#015#012oid:0x150000000005eb#015#012$20#015#012oid:0x1500000000060b#015#012$20#015#012oid:0x1500000000060c#015#012$20#015#012oid:0x1500000000062c#015#012$20#015#012oid:0x1500000000062d#015#012$20#015#012oid:0x1500000000064d#015#012$20#015#012oid:0x1500000000064e#015#012$20#015#012oid:0x1500000000066e#015#012$20#015#012oid:0x1500000000066f#015#012$20#015#012oid:0x1500000000068f#015#012$20#015#012oid:0x15000000000690#015#012$20#015#012oid:0x150000000006b0#015#012$20#015#012oid:0x150000000006b1#015#012$20#015#012oid:0x150000000006d1#015#012$20#015#012oid:0x150000000006d2#015#012$20#015#012oid:0x150000000006f2#015#012$20#015#012oid:0x150000000006f3#015#012$20#015#012oid:0x15000000000713#015#012$20#015#012oid:0x15000000000714#015#012$20#015#012oid:0x15000000000734#015#012$20#015#012oid:0x15000000000735#015#012$20#015#012oid:0x15000000000755#015#012$20#015#012oid:0x15000000000756#015#012$20#015#012oid:0x15000000000776#015#012$20#015#012oid:0x15000000000777#015#012$20#015#012oid:0x15000000000797#015#012$20#015#012oid:0x15000000000798#015#012$20#015#012oid:0x150000000007b8#015#012$20#015#012oid:0x150000000007b9#015#012$20#015#012oid:0x150000000007d9#015#012$20#015#012oid:0x150000000007da#015#012$20#015#012oid:0x150000000007fa#015#012$20#015#012oid:0x150000000007fb#015#012$20#015#012oid:0x1500000000081b#015#012$20#015#012oid:0x1500000000081c#015#012$20#015#012oid:0x1500000000083c#015#012$20#015#012oid:0x1500000000083d#015#012$20#015#012oid:0x1500000000085d#015#012$20#015#012oid:0x1500000000085e#015#012$20#015#012oid:0x1500000000087e#015#012$20#015#012oid:0x1500000000087f#015#012$20#015#012oid:0x1500000000089f#015#012$20#015#012oid:0x150000000008a0#015#012$20#015#012oid:0x150000000008c0#015#012$20#015#012oid:0x150000000008c1#015#012$20#015#012oid:0x150000000008e1#015#012$20#015#012oid:0x150000000008e2#015#012$20#015#012oid:0x15000000000902#015#012$20#015#012oid:0x15000000000903#015#012$20#015#012oid:0x15000000000923#015#012$20#015#012oid:0x15000000000924#015#012$20#015#012oid:0x15000000000944#015#012$20#015#012oid:0x15000000000945#015#012$20#015#012oid:0x15000000000965#015#012$20#015#012oid:0x15000000000966#015#012$20#015#012oid:0x15000000000986#015#012$20#015#012oid:0x15000000000987#015#012$20#015#012oid:0x150000000009a7#015#012$20#015#012oid:0x150000000009a8#015#012$20#015#012oid:0x150000000009c8#015#012$20#015#012oid:0x150000000009c9#015#012$20#015#012oid:0x150000000009e9#015#012$20#015#012oid:0x150000000009ea#015#012$20#015#012oid:0x15000000000a0a#015#012$20#015#012oid:0x15000000000a0b#015#012$20#015#012oid:0x15000000000a2b#015#012$20#015#012oid:0x15000000000a2c#015#012$20#015#012oid:0x15000000000a4c#015#012$20#015#012oid:0x15000000000a4d#015#012$20#015#012oid:0x15000000000a6d#015#012$20#015#012oid:0x15000000000a6e#015#012$20#015#012oid:0x15000000000a8e#015#012$20#015#012oid:0x15000000000a8f#015#012$20#015#012oid:0x15000000000aaf#015#012$20#015#012oid:0x15000000000ab0#015#012$1#015#0122#015#012$8#015#012COUNTERS#015#012$3#015#012100#015#012$2#015#012''#15#012, reason: ERR Error running script (call to f_b62081cc93943a4cbfec30de42638b435d31197c): @user_script:39: user_script:39: attempt to concatenate local 'queue_index' (a boolean value) : Input/output error ``` **Why I did it** If queue_index or port_id is not defined don't run the rest of the LUA script logic. **How I verified it** Running it on the switch and verify no errors.
stepanblyschak
pushed a commit
that referenced
this pull request
Dec 19, 2023
**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
pushed a commit
that referenced
this pull request
Dec 20, 2023
**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)
```
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added an API to create a table with configurable ACL table type
(matches, bpoints, actions). Implemented a handler for new
ACL_TABLE_TYPE CONFIG DB table.
Implemented UT for the above.
Signed-off-by: Stepan Blyshchak [email protected]
What I did
I implemented ACL table type concept. Till this change, there are predefined ACL table types orchagent knows about (L3, L3V6, etc.) and if other orch requires a custom table a new table type needs to be defined in aclorch.
This PR addresses this limitation by introducing AclTableType which can be constructed from a set of matches, actions and bpoint types user needs. There is also a new handler for ACL_TABLE_TYPE table which is used for user to define table types.
Currently, some of built-in ACL table types that requires special handling are distinguished from others by their names (TABLE_TYPE_MIRROR, TABLE_TYPE_MIRRORV6) and a special handling is performed by an AclOrch.
Why I did it
To allow users (developers and end users) creating custom ACL tables without modifying AclOrch source code.
How I verified it
Unit tests and VS tests. Manual test on the switch.
Details if related