[mclagdctl] added peer link state #9416

globaltrouble · 2021-12-01T16:01:22Z

Why I did it

In case peer-link is down mclag feature is not properly working, but mclagdctl doesn't show any error, all statuses are OK. The system will be more traceable when mclagdctl will show peer-link state.


# MCLAG is working properly, some traffic will be forwarded/received using peer-link.

$ mclagdctl dump state
The MCLAG's keepalive is: OK
MCLAG info sync is: completed
Domain id: 1
Local Ip: 192.168.3.1
Peer Ip: 192.168.3.2
Peer Link Interface: PortChannel0002
Keepalive time: 1
sesssion Timeout : 15
Peer Link Mac: 52:51:00:12:34:56
Role: Active
MCLAG Interface: PortChannel0001
Loglevel: NOTICE

$ config interface shut PortChannel0002

# MCLAG is not working properly, peer-link is down, some traffic will be lost, but it is not traceable.

$ mclagdctl dump state
The MCLAG's keepalive is: OK
MCLAG info sync is: completed
Domain id: 1
Local Ip: 192.168.3.1
Peer Ip: 192.168.3.2
Peer Link Interface: PortChannel0002
Keepalive time: 1
sesssion Timeout : 15
Peer Link Mac: 52:51:00:12:34:56
Role: Active
MCLAG Interface: PortChannel0001
Loglevel: NOTICE

How I did it

add peer_link_state to struct mclagd_state
parse peer_link_state in function iccp_mclag_config_dump
show peer_link_state in function mclagdctl_parse_dump_state

How to verify it

setup MCLAG topology
run mclagdctl dump state , keepalive must be OK and Peer Link state: Up, expected output:

$ mclagdctl dump state
The MCLAG's keepalive is: OK
MCLAG info sync is: completed
Domain id: 1
Local Ip: 192.168.3.1
Peer Ip: 192.168.3.2
Peer Link Interface: PortChannel0002
Peer Link state: Up
Keepalive time: 1
sesssion Timeout : 15
Peer Link Mac: 52:51:00:12:34:56
Role: Active
MCLAG Interface: PortChannel0001
Loglevel: NOTICE

shutdown peer link port config interface shut PortChannelForPeerLink
run mclagdctl dump state , keepalive must be OK and Peer Link state: Down, expected output:

$ mclagdctl dump state
The MCLAG's keepalive is: OK
MCLAG info sync is: completed
Domain id: 1
Local Ip: 192.168.3.1
Peer Ip: 192.168.3.2
Peer Link Interface: PortChannel0002
Peer Link state: Down
Keepalive time: 1
sesssion Timeout : 15
Peer Link Mac: 52:51:00:12:34:56
Role: Active
MCLAG Interface: PortChannel0001
Loglevel: NOTICE

Which release branch to backport (provide reason below if selected)

Description for the changelog

mclagdctl dump state will additionally show peer-link port state.

globaltrouble · 2021-12-08T10:08:34Z

@lguohan could you please review it?

globaltrouble · 2022-01-12T12:35:44Z

@Praveen-Brcm could you please review it?

globaltrouble · 2022-01-24T10:07:01Z

@Praveen-Brcm , @lguohan could somebody help with it please?

Praveen-Brcm · 2022-01-25T06:01:00Z

@novikauanton Changes looks good and pls go ahead. Thanks

globaltrouble · 2022-01-27T10:26:00Z

@lguohan , @Praveen-Brcm could somebody merge it please?

Praveen-Brcm · 2022-01-27T19:10:19Z

@lguohan , @Praveen-Brcm could somebody merge it please?

@ sorry I do not have merge rights, would request @Iguohan to help.

globaltrouble · 2022-01-31T09:54:53Z

@lguohan could you please review and merge it?

msosyak · 2022-02-15T14:44:15Z

@gechiang Could we merge this improvement?

gechiang · 2022-02-15T23:32:22Z

src/iccpd/src/iccp_cmd_show.c

            memcpy(state_info.peer_link_if, peer_link_if->name, ICCP_MAX_PORT_NAME);
+            memcpy(state_info.peer_link_mac, peer_link_if->mac_addr, ETHER_ADDR_LEN);


Is this linbe trying to fix something that was not mentioned in your PR?? Did someone reviewed this change? The other changes looks straight forward but not this one...

the magic number 6 was replaced by the appropriate constant instead.

there was a confusing logic with duplicated check.

if (peer_link_if) do_some_stuff() else do_another_stuff() if (peer_link_if) do_the_rest()

As those lines needed to be changed in this PR simplified it.

Oh... I overlooked the original code that you changed from. Must have missed my coffee that day. My bad... It makes perfect sense for this change.

gechiang · 2022-02-15T23:40:33Z

@globaltrouble, Please add the necessary unit test code for the code that you are changing.
I don't see any attempt to improve the unit test code over MCLAG area and if we keep allowing changes to go in without proper unit test code added, it will never be done... I will not allow any merge of code going forward if no unit test code is included as part of the PR.
@Praveen-Brcm, can you please start involving the owner of iccpd (if you are not the rightful owner) to have unit test infrastructure implemented? We cannot keep going without it.
Thanks!

#### Why I did it The current redis version of SONiC is `6.0.6`, which contains many high-risky security issues like CVEs that are fixed in the latest version. The Redis release notes also highly recommend to upgrade with SECURITY urgency. ``` ================================================================================ Redis 6.0.16 Released Mon Oct 4 12:00:00 IDT 2021 ================================================================================ Upgrade urgency: SECURITY, contains fixes to security issues. Security Fixes: * (CVE-2021-41099) Integer to heap buffer overflow handling certain string commands and network payloads, when proto-max-bulk-len is manually configured to a non-default, very large value [reported by yiyuaner]. * (CVE-2021-32762) Integer to heap buffer overflow issue in redis-cli and redis-sentinel parsing large multi-bulk replies on some older and less common platforms [reported by Microsoft Vulnerability Research]. * (CVE-2021-32687) Integer to heap buffer overflow with intsets, when set-max-intset-entries is manually configured to a non-default, very large value [reported by Pawel Wieczorkiewicz, AWS]. * (CVE-2021-32675) Denial Of Service when processing RESP request payloads with a large number of elements on many connections. * (CVE-2021-32672) Random heap reading issue with Lua Debugger [reported by Meir Shpilraien]. * (CVE-2021-32628) Integer to heap buffer overflow handling ziplist-encoded data types, when configuring a large, non-default value for hash-max-ziplist-entries, hash-max-ziplist-value, zset-max-ziplist-entries or zset-max-ziplist-value [reported by sundb]. * (CVE-2021-32627) Integer to heap buffer overflow issue with streams, when configuring a non-default, large value for proto-max-bulk-len and client-query-buffer-limit [reported by sundb]. * (CVE-2021-32626) Specially crafted Lua scripts may result with Heap buffer overflow [reported by Meir Shpilraien]. Other bug fixes: * Fix appendfsync to always guarantee fsync before reply, on MacOS and FreeBSD (kqueue) (#9416) * Fix the wrong mis-detection of sync_file_range system call, affecting performance (#9371) * Fix replication issues when repl-diskless-load is used (#9280) ``` #### How I did it Edit `Dockerfile.j2` file #### How to verify it Check redis version #### Description for the changelog This PR will upgrade redis-server version to `6.0.16`.

bratashX · 2022-05-13T23:16:24Z

will be good to include these changes into 202111 branch

#### Why I did it The current redis version of SONiC is `6.0.6`, which contains many high-risky security issues like CVEs that are fixed in the latest version. The Redis release notes also highly recommend to upgrade with SECURITY urgency. ``` ================================================================================ Redis 6.0.16 Released Mon Oct 4 12:00:00 IDT 2021 ================================================================================ Upgrade urgency: SECURITY, contains fixes to security issues. Security Fixes: * (CVE-2021-41099) Integer to heap buffer overflow handling certain string commands and network payloads, when proto-max-bulk-len is manually configured to a non-default, very large value [reported by yiyuaner]. * (CVE-2021-32762) Integer to heap buffer overflow issue in redis-cli and redis-sentinel parsing large multi-bulk replies on some older and less common platforms [reported by Microsoft Vulnerability Research]. * (CVE-2021-32687) Integer to heap buffer overflow with intsets, when set-max-intset-entries is manually configured to a non-default, very large value [reported by Pawel Wieczorkiewicz, AWS]. * (CVE-2021-32675) Denial Of Service when processing RESP request payloads with a large number of elements on many connections. * (CVE-2021-32672) Random heap reading issue with Lua Debugger [reported by Meir Shpilraien]. * (CVE-2021-32628) Integer to heap buffer overflow handling ziplist-encoded data types, when configuring a large, non-default value for hash-max-ziplist-entries, hash-max-ziplist-value, zset-max-ziplist-entries or zset-max-ziplist-value [reported by sundb]. * (CVE-2021-32627) Integer to heap buffer overflow issue with streams, when configuring a non-default, large value for proto-max-bulk-len and client-query-buffer-limit [reported by sundb]. * (CVE-2021-32626) Specially crafted Lua scripts may result with Heap buffer overflow [reported by Meir Shpilraien]. Other bug fixes: * Fix appendfsync to always guarantee fsync before reply, on MacOS and FreeBSD (kqueue) (sonic-net#9416) * Fix the wrong mis-detection of sync_file_range system call, affecting performance (sonic-net#9371) * Fix replication issues when repl-diskless-load is used (sonic-net#9280) ``` #### How I did it Edit `Dockerfile.j2` file #### How to verify it Check redis version #### Description for the changelog This PR will upgrade redis-server version to `6.0.16`.

globaltrouble requested a review from lguohan as a code owner December 1, 2021 16:01

globaltrouble marked this pull request as draft December 1, 2021 16:01

globaltrouble force-pushed the feature/mclagdctl-show-port-state branch from 9cd92b2 to c5da929 Compare December 1, 2021 16:03

globaltrouble marked this pull request as ready for review December 1, 2021 16:09

globaltrouble changed the title ~~[WIP] [mclagsyncd] added peer link state~~ [mclagsyncd] added peer link state Dec 1, 2021

globaltrouble changed the title ~~[mclagsyncd] added peer link state~~ [mclagdctl] added peer link state Dec 1, 2021

Anton added 4 commits December 1, 2021 08:30

added peer link state to mclagdctl dump state

c5da929

Merge branch 'Azure:master' into feature/mclagdctl-show-port-state

8a39748

Merge branch 'Azure:master' into feature/mclagdctl-show-port-state

dba564b

Merge branch 'Azure:master' into feature/mclagdctl-show-port-state

a18f3ff

novikauanton added 2 commits December 20, 2021 10:53

Merge branch 'Azure:master' into feature/mclagdctl-show-port-state

ce952f5

Merge branch 'Azure:master' into feature/mclagdctl-show-port-state

3340a6d

msosyak approved these changes Jan 18, 2022

View reviewed changes

Praveen-Brcm approved these changes Jan 25, 2022

View reviewed changes

gechiang reviewed Feb 15, 2022

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[mclagdctl] added peer link state #9416

[mclagdctl] added peer link state #9416

globaltrouble commented Dec 1, 2021

globaltrouble commented Dec 8, 2021

globaltrouble commented Jan 12, 2022

globaltrouble commented Jan 24, 2022

Praveen-Brcm commented Jan 25, 2022 •

edited

Loading

globaltrouble commented Jan 27, 2022

Praveen-Brcm commented Jan 27, 2022

globaltrouble commented Jan 31, 2022

msosyak commented Feb 15, 2022

gechiang Feb 15, 2022

globaltrouble Feb 17, 2022

gechiang Feb 17, 2022

gechiang commented Feb 15, 2022

bratashX commented May 13, 2022

		memcpy(state_info.peer_link_if, peer_link_if->name, ICCP_MAX_PORT_NAME);
		memcpy(state_info.peer_link_mac, peer_link_if->mac_addr, ETHER_ADDR_LEN);

[mclagdctl] added peer link state #9416

Are you sure you want to change the base?

[mclagdctl] added peer link state #9416

Conversation

globaltrouble commented Dec 1, 2021

Why I did it

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Description for the changelog

globaltrouble commented Dec 8, 2021

globaltrouble commented Jan 12, 2022

globaltrouble commented Jan 24, 2022

Praveen-Brcm commented Jan 25, 2022 • edited Loading

globaltrouble commented Jan 27, 2022

Praveen-Brcm commented Jan 27, 2022

globaltrouble commented Jan 31, 2022

msosyak commented Feb 15, 2022

gechiang Feb 15, 2022

Choose a reason for hiding this comment

globaltrouble Feb 17, 2022

Choose a reason for hiding this comment

gechiang Feb 17, 2022

Choose a reason for hiding this comment

gechiang commented Feb 15, 2022

bratashX commented May 13, 2022

Praveen-Brcm commented Jan 25, 2022 •

edited

Loading