Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[sonic-swss] Port auto negotiation sonic-swss implementation #2

Closed
wants to merge 18 commits into from
Closed

[sonic-swss] Port auto negotiation sonic-swss implementation #2

wants to merge 18 commits into from

Conversation

Junchao-Mellanox
Copy link
Owner

What I did

  1. Added port auto negotiation attributes handle
  2. Get supported speeds for each port and save them to CONFIG_DB
  3. Added new test cases in VS test to verify the change

Why I did it

To support port auto negotiation feature.

How I verified it

Manual test and vs test

Details if related

Junchao-Mellanox and others added 13 commits February 1, 2021 19:28
@Junchao-Mellanox
Add port autoneg support
574a33b
@Junchao-Mellanox
Fix build issue and add vs test cases
85456ea
@Junchao-Mellanox
Fix vs test failure
cf56dee
@Junchao-Mellanox
Merge commit '288fb40' into port-auto-neg
0e2bb63
@Junchao-Mellanox
Merge branch 'master' into port-auto-neg
1408922 
Conflicts:
	orchagent/port.h
@Junchao-Mellanox
Change autoneg value range fron 0|1 to on|off
62a6e09
@Junchao-Mellanox
Change autoneg value to on|off for VS test case
eb6a2d3
@Junchao-Mellanox
Use boost to_upper and to_lower
078dbb5
@Junchao-Mellanox
Fix issue found in manual test
c36cd3a
@Junchao-Mellanox
Fix issue found in manual test
284b21a
@shi-su @Junchao-Mellanox
Keep attribute order in bulk mode (sonic-net#1659)
8caf7e1 
Keep attributes for the same entry in the order that the attributes are set in bulk mode. And add a unit test for the scenario in bulker.

Some attributes have a dependency on others in SAI operations (e.g., setting nexthop requires packet action to be forward in route set operation). Without keeping the order of the attributes, the configuration may not be successfully applied to hardware. Therefore, it is necessary to keep the order of attributes in bulk mode.
@Junchao-Mellanox
Fix issue: should flap admin status when setting speed
8b9ddd5
@Junchao-Mellanox
Merge branch 'master' into port-auto-neg
5a6a2ee 
Conflicts:
	orchagent/port.h
	orchagent/portsorch.cpp
	tests/test_port_an.py
keboliu
keboliu reviewed Mar 26, 2021
View reviewed changes
orchagent/portsorch.cpp Outdated
@@ -2344,6 +2458,10 @@ void PortsOrch::doPortTask(Consumer &consumer)
{
throw runtime_error("PortsOrch initialization failure.");
}
else
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove else

orchagent/portsorch.cpp Outdated
p.m_autoneg = an;
m_portList[alias] = p;
SWSS_LOG_ERROR("Failed to parse autoneg value: %s", an_str.c_str());
// Invalid auto negotiation mode configured, dont retry
Copy link
Collaborator

Choose a reason for hiding this comment

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

dont -> don't

orchagent/portsorch.cpp Outdated
}
else
{
SWSS_LOG_ERROR("Failed to set port %s AN to %u", alias.c_str(), an);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add previous value in log

orchagent/portsorch.cpp Outdated
* 4. Set port speed
*/

SWSS_LOG_DEBUG("Set port %s speed to %u -> %u", alias.c_str(), p.m_speed, speed);
if (speed != 0)
{
if (speed != p.m_speed)
{
m_portList[alias] = p;
Copy link
Collaborator

Choose a reason for hiding this comment

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

No need update cache here

@Junchao-Mellanox
Make log clearer
abef7f0
@Junchao-Mellanox
Merge remote-tracking branch 'origin/master' into port-auto-neg
f23c7bc 
Conflicts:
	orchagent/orchdaemon.cpp
	orchagent/portsorch.cpp
	orchagent/portsorch.h
	tests/mock_tests/aclorch_ut.cpp
	tests/mock_tests/portsorch_ut.cpp
@Junchao-Mellanox
Merge remote-tracking branch 'origin/master' into port-auto-neg
ae45479 
Conflicts:
	orchagent/portsorch.cpp
@Junchao-Mellanox Junchao-Mellanox deleted the port-auto-neg branch October 29, 2021 01:45
Junchao-Mellanox pushed a commit that referenced this pull request Nov 17, 2021
@TACappleman @abanu-ms
[cbf] Add class-based forwarding support (sonic-net#1963)
339101c 
* [cbf] Added initial CBF support (#2)

* Added CBF NHG support to NhgOrch
* Added NhgMapOrch to handle DSCP_TO_FC and EXP_TO_FC tables
* Added UT for the new NhgOrch function and NhgMapOrch

Support sonic-net/SONiC#796

Co-authored-by: Alexandru Banu <v-albanu@microsoft.com>
Junchao-Mellanox pushed a commit that referenced this pull request Jul 1, 2022
@Yakiv-Huryk
[asan] suppress the static variable leaks (sonic-net#2354)
979d871 
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 <yhuryk@nvidia.com>
Junchao-Mellanox pushed a commit that referenced this pull request Mar 7, 2023
@Yakiv-Huryk @yxieca
[asan] suppress the static variable leaks (sonic-net#2354)
c7f5743 
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 <yhuryk@nvidia.com>
Junchao-Mellanox pushed a commit that referenced this pull request May 6, 2024
@vivekrnv
[ASAN] Fix Indirect Mem Leaks in Orchagent (sonic-net#2869)
23cb2e5 
**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
Reviewers

@keboliu keboliu keboliu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Junchao-Mellanox @keboliu @shi-su