Closed
Conversation
|
This PR includes changes that were made not by you. Please remove them from the diff. |
arfeigin
force-pushed
the
fast_reboot_improve
branch
from
7fd2ffa to
6abbb0b
Compare
both fast/warm boot scenarios. Align all orch's in orchagent to advanced reboot infra. Align all sync daemons to advanced reboot infra. Align all config daemons to advanced reboot infra. Signed-off-by: Shlomi Bitton <[email protected]>
arfeigin
force-pushed
the
fast_reboot_improve
branch
from
6abbb0b to
95e01bb
Compare
stepanblyschak
approved these changes
Jun 23, 2022
stepanblyschak
left a comment
stepanblyschak
left a comment
There was a problem hiding this comment.
LGTM. Seems this PR only changes names. If I missed some logic change, please point me to it. If you have logic change, please create one PR with name changes and another once with logic change. I do not see you change any logic, just want to confirm with you.
arfeigin
pushed a commit
that referenced
this pull request
Jul 3, 2022
Currently, ASAN sometimes reports the BufferOrch::m_buffer_type_maps and QosOrch::m_qos_maps as leaked. However, their lifetime is the lifetime of a process so they are not really 'leaked'.
This also adds a simple way to add more suppressions later if required.
Example of ASAN report:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f96aa952d30 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xead30)
#1 0x55ca1da9f789 in __static_initialization_and_destruction_0 /__w/2/s/orchagent/bufferorch.cpp:39
#2 0x55ca1daa02af in _GLOBAL__sub_I_bufferorch.cpp /__w/2/s/orchagent/bufferorch.cpp:1321
#3 0x55ca1e2a9cd4 (/usr/bin/orchagent+0xe89cd4)
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f96aa952d30 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xead30)
#1 0x55ca1da6d2da in __static_initialization_and_destruction_0 /__w/2/s/orchagent/qosorch.cpp:80
#2 0x55ca1da6ecf2 in _GLOBAL__sub_I_qosorch.cpp /__w/2/s/orchagent/qosorch.cpp:2000
#3 0x55ca1e2a9cd4 (/usr/bin/orchagent+0xe89cd4)
- What I did
Added an lsan suppression config with static variable leak suppression
- Why I did it
To suppress ASAN false positives
- How I verified it
Run a test that produces the static variable leaks report and checked that report has these leaks suppressed.
Signed-off-by: Yakiv Huryk <[email protected]>
arfeigin
pushed a commit
that referenced
this pull request
Jul 4, 2022
Currently, ASAN sometimes reports the BufferOrch::m_buffer_type_maps and QosOrch::m_qos_maps as leaked. However, their lifetime is the lifetime of a process so they are not really 'leaked'.
This also adds a simple way to add more suppressions later if required.
Example of ASAN report:
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f96aa952d30 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xead30)
#1 0x55ca1da9f789 in __static_initialization_and_destruction_0 /__w/2/s/orchagent/bufferorch.cpp:39
#2 0x55ca1daa02af in _GLOBAL__sub_I_bufferorch.cpp /__w/2/s/orchagent/bufferorch.cpp:1321
#3 0x55ca1e2a9cd4 (/usr/bin/orchagent+0xe89cd4)
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f96aa952d30 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.5+0xead30)
#1 0x55ca1da6d2da in __static_initialization_and_destruction_0 /__w/2/s/orchagent/qosorch.cpp:80
#2 0x55ca1da6ecf2 in _GLOBAL__sub_I_qosorch.cpp /__w/2/s/orchagent/qosorch.cpp:2000
#3 0x55ca1e2a9cd4 (/usr/bin/orchagent+0xe89cd4)
- What I did
Added an lsan suppression config with static variable leak suppression
- Why I did it
To suppress ASAN false positives
- How I verified it
Run a test that produces the static variable leaks report and checked that report has these leaks suppressed.
Signed-off-by: Yakiv Huryk <[email protected]>
arfeigin
pushed a commit
that referenced
this pull request
Mar 13, 2024
**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
sonic-net#13 0x555559090dad in OrchDaemon::start() orchagent/orchdaemon.cpp:755
sonic-net#14 0x555558e9be25 in main orchagent/main.cpp:766
sonic-net#15 0x7f0a299b6d09 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x23d09)
```
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
What I did
Change warmRestartAssist to advancedRestartAssist since it is used for both fast/warm boot scenarios.
Align all orchs in orchagent to use advanced reboot infrastructure.
Align all sync daemons to use advanced reboot infrastructure.
Align all config daemons to use advanced reboot infrastructure.
Why I did it
Improving fast reboot, following this HLD: https://github.com/sonic-net/SONiC/blob/master/doc/fast-reboot/Fast-reboot_Flow_Improvements_HLD.md
How I verified it
Manual and automated tests, compatibility check.
Details if related