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.

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

PortChannel creation with out of range(0-255) value of "min-links", leads to Exception during update_data() #6781

Closed
raphaelt-nvidia opened this issue Feb 14, 2021 · 13 comments · Fixed by #7265 or sonic-net/sonic-utilities#1630
Assignees
Labels
Triaged this issue has been triaged YANG YANG model related changes

Comments

@raphaelt-nvidia
Copy link
Contributor

Description

Creating a portchannel with an invalid value for min-links results in repeated errors in the log, as well as a stack dump when executing "show interfaces portchannel".

Steps to reproduce the issue:

  1. config portchannel add PortChannel10 --min-links 256

Describe the results you received:

show logging:

Feb 10 11:19:27.156197 r-lionfish-07 ERR snmp#snmp-subagent [ax_interface] ERROR: MIBUpdater.start() caught an unexpected exception during update_data()#012Traceback (most recent call last):#12 File "/usr/local/lib/python3.7/dist-packages/ax_interface/mib.py", line 43, in start#012 self.update_data()#12 File "/usr/local/lib/python3.7/dist-packages/sonic_ax_impl/mibs/ietf/rfc4292.py", line 70, in update_data#012 nexthops = ent["nexthop"]#12 File "/usr/lib/python3/dist-packages/swsscommon/swsscommon.py", line 345, in getitem#12 return swsscommon.FieldValueMap___getitem_(self, key)#012IndexError: key not found

May not be observed in the first couple of minutes, but once it starts, it repeats about every 5 seconds.

Similarly "show interfaces portchannel" may initially produce output like this:

Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available,
S - selected, D - deselected, * - not synced
No. Team Dev Protocol Ports


10 PortChannel10 N/A

but after that all invocations result in this stack dump:

Traceback (most recent call last):
File "/usr/local/bin/show", line 8, in
sys.exit(cli())
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 764, in call
return self.main(*args, **kwargs)
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 717, in main
rv = self.invoke(ctx)
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 1137, in invoke
return _process_result(sub_ctx.command.invoke(sub_ctx))
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 956, in invoke
return ctx.invoke(self.callback, **ctx.params)
File "/usr/local/lib/python3.7/dist-packages/click/core.py", line 555, in invoke
return callback(*args, **kwargs)
File "/usr/local/lib/python3.7/dist-packages/show/interfaces/portchannel.py", line 165, in portchannel
team.get_teams_info()
File "/usr/local/lib/python3.7/dist-packages/utilities_common/multi_asic.py", line 137, in wrapped_run_on_all_asics
func(self, *args, **kwargs)
File "/usr/local/lib/python3.7/dist-packages/show/interfaces/portchannel.py", line 49, in get_teams_info
self.get_teamshow_result()
File "/usr/local/lib/python3.7/dist-packages/show/interfaces/portchannel.py", line 111, in get_teamshow_result
info['protocol'] += "(A)" if state['runner.active'] == "true" else '(I)'
KeyError: 'runner.active'

Describe the results you expected:

Invalid value rejected in CLI.
No errors on show command or in log.

Output of show version:

root@r-spider-05:/home/admin# show version

SONiC Software Version: SONiC.master.548-1d99d14e
Distribution: Debian 10.8
Kernel: 4.19.0-12-2-amd64
Build commit: 1d99d14
Build date: Tue Feb 9 12:12:26 UTC 2021
Built by: johnar@jenkins-worker-6

Platform: x86_64-mlnx_msn2410-r0
HwSKU: ACS-MSN2410
ASIC: mellanox
ASIC Count: 1
Serial Number: MT1921X01546
Uptime: 17:59:10 up 1:43, 1 user, load average: 8.46, 8.48, 8.45

Docker images:
REPOSITORY TAG IMAGE ID SIZE
docker-syncd-mlnx latest f8ef4eb1c8da 542MB
docker-syncd-mlnx master.548-1d99d14e f8ef4eb1c8da 542MB
docker-snmp latest a1aa1fb1f61a 438MB
docker-snmp master.548-1d99d14e a1aa1fb1f61a 438MB
docker-teamd latest a4cb179f0f42 407MB
docker-teamd master.548-1d99d14e a4cb179f0f42 407MB
docker-nat latest 8006db83fd09 410MB
docker-nat master.548-1d99d14e 8006db83fd09 410MB
docker-router-advertiser latest 085d154c2ec6 397MB
docker-router-advertiser master.548-1d99d14e 085d154c2ec6 397MB
docker-platform-monitor latest 6848278b9fab 687MB
docker-platform-monitor master.548-1d99d14e 6848278b9fab 687MB
docker-lldp latest 9262a7fe0431 437MB
docker-lldp master.548-1d99d14e 9262a7fe0431 437MB
docker-database latest 826e389613f8 397MB
docker-database master.548-1d99d14e 826e389613f8 397MB
docker-orchagent latest aaef75da45b8 426MB
docker-orchagent master.548-1d99d14e aaef75da45b8 426MB
docker-macsec latest 41289c3e9c5b 411MB
docker-macsec master.548-1d99d14e 41289c3e9c5b 411MB
docker-sonic-telemetry latest 3cd1c1a5fe8c 471MB
docker-sonic-telemetry master.548-1d99d14e 3cd1c1a5fe8c 471MB
docker-dhcp-relay latest a79611919486 404MB
docker-dhcp-relay master.548-1d99d14e a79611919486 404MB
docker-sonic-mgmt-framework latest 530109438706 615MB
docker-sonic-mgmt-framework master.548-1d99d14e 530109438706 615MB
docker-fpm-frr latest fc77af2bce80 426MB
docker-fpm-frr master.548-1d99d14e fc77af2bce80 426MB
docker-sflow latest 4a255fc9a653 408MB
docker-sflow master.548-1d99d14e 4a255fc9a653 408MB

Additional information you deem important (e.g. issue happens only occasionally):

The delay in errors is something I only noticed in recent versions, suggesting that perhaps a fix was attempted, but this may just be coincidence, e.g. timing.

Unable to attach dump, 31 MB size, 10 MB allowed.

@ghost
Copy link

ghost commented Feb 15, 2021

Took into investigation.

@ghost
Copy link

ghost commented Feb 16, 2021

I have investigated the issue. The root cause of the issue is that the system cannot create portchannel due to value range restriction of min_ports parameter(min-links in terminology of sonic-utilities) inside teamd daemon of libteam library. min_ports parameter has allowable range 1...255. To fix the issue is just need to set range of allowable value of the parameter.
But, this is my concern. I have an information that 802.3ad standard, in which the Link Aggregation feature is described, allows only 8 active ports per LAG. If it so, it could be transformed to the top limit of the min-links parameter. Unfortunately, I have not copy of the standard and I cannot confirm the limitation. I found no related restrictions in SONiC design documents and source code, except the range above in libteam. In addition, I found yang model, in which the range of min-links is 1...128: https://github.com/Azure/sonic-buildimage/blob/ab0ee0bfae5354adbf36f6fac0144883a5222037/src/sonic-yang-models/yang-models/sonic-portchannel.yang.
Due to that, the allowable range of min-links is unclear for me..
@lguohan, I am not sure that you could help me, but could I ask you to clarify is there any restrictions of value of min-links in SONiC? Or, maybe, you could forward me to a person, who could help me in this question?

@anshuv-mfst
Copy link

Liat: Please confirm with Lib team on the max limit (i.e. 128), thanks.
Sonic portchannel yang model also reflects the same range for min links (1..128).

Next steps:
Might require code changes for this issue.
Also, to be discussed in YANG model subgroup for CLI check

@anshuv-mfst anshuv-mfst added the Triaged this issue has been triaged label Feb 17, 2021
@ghost
Copy link

ghost commented Feb 19, 2021

@liat-grozovik, here is the place in libteam, where the range of min_ports is being checked: https://github.com/jpirko/libteam/blob/267f24839536234ca621d4fd0466d3b57cb6dccd/teamd/teamd_runner_lacp.c#L286
Could you ask guys from libteam to clarify from where they got the top limit value of min_ports? Maybe, there was some reason to limit it on value of UCHAR_MAX? IMO, the value is too high.

@liat-grozovik
Copy link
Collaborator

There is a newer version of libteam which has a fix for it
Please check https://git.kernel.org/pub/scm/linux/kernel/git/jpirko/libteam.git/commit/?id=69a7494bb77dc10bb27076add07b380dbd778592

@lguohan is there any plan to move to a newer version of libteam or should we consider backporting the change to sonic libteam?

@ghost
Copy link

ghost commented Feb 22, 2021

@liat-grozovik, could you clarify will the system be checking the value range of "min-links", according to updates of libteam, or we still should allign on information, defined in https://github.com/Azure/sonic-buildimage/blob/ab0ee0bfae5354adbf36f6fac0144883a5222037/src/sonic-yang-models/yang-models/sonic-portchannel.yang?
To fix the issue a proper range of the input argument should be set in the CLI function of sonic-utilities.

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Feb 22, 2021 via email

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Feb 22, 2021 via email

@ghost
Copy link

ghost commented Mar 23, 2021

@anshuv-mfst, is there any updates from yang workgroup regarding this issue?

@anshuv-mfst anshuv-mfst added the YANG YANG model related changes label Mar 23, 2021
@anshuv-mfst
Copy link

Hi@maksymbelei95, @liat-grozovik - Sorry, I missed the notification.
Feature will be reviewed in next YANG subgroup meeting 4/8/2021. @raphaelt-nvidia - please plan to join to drive PR review, thanks.

@anshuv-mfst
Copy link

Hi @prsunny - could you please take look, thanks.

@prsunny
Copy link
Contributor

prsunny commented Apr 22, 2021

@anshuv-mfst, i think this would be fixed by #7265

@liat-grozovik
Copy link
Collaborator

/azp run

qiluo-msft pushed a commit that referenced this issue Jul 8, 2021
#### Why I did it

Restrict the min-links parameter in "config portchannel" to the range 1-1024.
Fixes #6781 in conjunction with #1630.
Align YANG model with limits in libteam and sonic-utilties.

#### How I did it

PR 1630 in sonic-utilities prevents CLI user from entering a value outside the allowed range.  This PR does the following:

- Increases the maximum value of min-links from 128 to 1024.
- Provides validation in libteam, incorporating as a patch the code in https://git.kernel.org/pub/scm/linux/kernel/git/jpirko/libteam.git/commit/?id=69a7494bb77dc10bb27076add07b380dbd778592.
- Updates the Yang model upper limit from 128 to 1024 (was inconsistent with libteam value).
- Updates the Yang model lower limit from 1 to 0, since 0 is set as default in sonic-utilities which would fail its new range check otherwise.
- Added Yang tests for valid and invalid value.

#### How to verify it

config portchannel add PortChannel0004 --min-links 1024

Command should be accepted.

show interfaces portchannel

Output should show PortChannel0004, no errors on CLI.

config portchannel add PortChannel0005 --min-links 1025

Command should be rejected

show interfaces portchannel

Output should not show PortChannel0005 , no errors on CLI.

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


#### Description for the changelog

Updates YANG model to allow up to 1024 min_links for portchannel.  Fixes #6781 in conjunction with #1630.
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this issue Aug 7, 2021
…#7265)

#### Why I did it

Restrict the min-links parameter in "config portchannel" to the range 1-1024.
Fixes sonic-net#6781 in conjunction with sonic-net#1630.
Align YANG model with limits in libteam and sonic-utilties.

#### How I did it

PR 1630 in sonic-utilities prevents CLI user from entering a value outside the allowed range.  This PR does the following:

- Increases the maximum value of min-links from 128 to 1024.
- Provides validation in libteam, incorporating as a patch the code in https://git.kernel.org/pub/scm/linux/kernel/git/jpirko/libteam.git/commit/?id=69a7494bb77dc10bb27076add07b380dbd778592.
- Updates the Yang model upper limit from 128 to 1024 (was inconsistent with libteam value).
- Updates the Yang model lower limit from 1 to 0, since 0 is set as default in sonic-utilities which would fail its new range check otherwise.
- Added Yang tests for valid and invalid value.

#### How to verify it

config portchannel add PortChannel0004 --min-links 1024

Command should be accepted.

show interfaces portchannel

Output should show PortChannel0004, no errors on CLI.

config portchannel add PortChannel0005 --min-links 1025

Command should be rejected

show interfaces portchannel

Output should not show PortChannel0005 , no errors on CLI.

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


#### Description for the changelog

Updates YANG model to allow up to 1024 min_links for portchannel.  Fixes sonic-net#6781 in conjunction with sonic-net#1630.
judyjoseph pushed a commit that referenced this issue Oct 14, 2021
#### Why I did it

Restrict the min-links parameter in "config portchannel" to the range 1-1024.
Fixes #6781 in conjunction with #1630.
Align YANG model with limits in libteam and sonic-utilties.

#### How I did it

PR 1630 in sonic-utilities prevents CLI user from entering a value outside the allowed range.  This PR does the following:

- Increases the maximum value of min-links from 128 to 1024.
- Provides validation in libteam, incorporating as a patch the code in https://git.kernel.org/pub/scm/linux/kernel/git/jpirko/libteam.git/commit/?id=69a7494bb77dc10bb27076add07b380dbd778592.
- Updates the Yang model upper limit from 128 to 1024 (was inconsistent with libteam value).
- Updates the Yang model lower limit from 1 to 0, since 0 is set as default in sonic-utilities which would fail its new range check otherwise.
- Added Yang tests for valid and invalid value.

#### How to verify it

config portchannel add PortChannel0004 --min-links 1024

Command should be accepted.

show interfaces portchannel

Output should show PortChannel0004, no errors on CLI.

config portchannel add PortChannel0005 --min-links 1025

Command should be rejected

show interfaces portchannel

Output should not show PortChannel0005 , no errors on CLI.

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


#### Description for the changelog

Updates YANG model to allow up to 1024 min_links for portchannel.  Fixes #6781 in conjunction with #1630.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment