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

[teamd]: different portchannels configured with same LACP key #4009

Closed
mkovnir opened this issue Jan 10, 2020 · 9 comments · Fixed by sonic-net/sonic-swss#1660
Closed

[teamd]: different portchannels configured with same LACP key #4009

mkovnir opened this issue Jan 10, 2020 · 9 comments · Fixed by sonic-net/sonic-swss#1660
Labels

Comments

@mkovnir
Copy link

mkovnir commented Jan 10, 2020

Description

According to 6.3.3 of IEEE 802.1AX-2014:
An Aggregator also shall be assigned an integer identifier that is used by Link Aggregation Control to uniquely identify the Aggregator within the System.

SONIC system starts teamd (separate process for each portchannel) without specifying LACP key, and default value ("0") is used for different portchannels. This lead to situation when neighboring device is not able to differentiate links belonging to different portchannels.

Steps to reproduce the issue:

  1. Build 2 (or more) links between 2 switches.
  2. On switchA configure links to belong to one portchannel
  3. On switchB configure links to belong to different portchannels

Describe the results you expected:
SwitchA aggregates both link to one portchannel, switchB aggregates links to different portchannels.

Describe the results you expected:
System should assign locally unique integer as LACP key for different portchannels and described above assymetric topology will not be possible.

LACP_key

Output of show version:

SONiC Software Version: SONiC.HEAD.129-0c9040de
Distribution: Debian 9.11
Kernel: 4.9.0-9-2-amd64
Build commit: 0c9040de
Build date: Thu Nov 21 12:50:41 UTC 2019
Built by: johnar@jenkins-worker-4

Platform: x86_64-mlnx_msn2700-r0
HwSKU: Mellanox-SN2700
ASIC: mellanox
Serial Number: MT1829X20804
Uptime: 11:43:35 up  2:24,  1 user,  load average: 3.41, 3.30, 3.41

Docker images:
REPOSITORY                 TAG                 IMAGE ID            SIZE
docker-syncd-mlnx          HEAD.129-0c9040de   4f3b126274c2        373MB
docker-syncd-mlnx          latest              4f3b126274c2        373MB
docker-fpm-frr             HEAD.129-0c9040de   7345e105bf26        321MB
docker-fpm-frr             latest              7345e105bf26        321MB
docker-sflow               HEAD.129-0c9040de   f240b81ea842        305MB
docker-sflow               latest              f240b81ea842        305MB
docker-lldp-sv2            HEAD.129-0c9040de   f1b94915bf13        299MB
docker-lldp-sv2            latest              f1b94915bf13        299MB
docker-dhcp-relay          HEAD.129-0c9040de   7ba3cee353b6        289MB
docker-dhcp-relay          latest              7ba3cee353b6        289MB
docker-database            HEAD.129-0c9040de   c5c92210277c        281MB
docker-database            latest              c5c92210277c        281MB
docker-snmp-sv2            HEAD.129-0c9040de   ad01f8a547b1        335MB
docker-snmp-sv2            latest              ad01f8a547b1        335MB
docker-orchagent           HEAD.129-0c9040de   7fc54899dc92        322MB
docker-orchagent           latest              7fc54899dc92        322MB
docker-teamd               HEAD.129-0c9040de   c0316e26bbbd        304MB
docker-teamd               latest              c0316e26bbbd        304MB
docker-sonic-telemetry     HEAD.129-0c9040de   43df42f49e91        304MB
docker-sonic-telemetry     latest              43df42f49e91        304MB
docker-router-advertiser   HEAD.129-0c9040de   b0aff9280842        281MB
docker-router-advertiser   latest              b0aff9280842        281MB
docker-platform-monitor    HEAD.129-0c9040de   10013f82e4d7        565MB
docker-platform-monitor    latest              10013f82e4d7        565MB ```


@liat-grozovik
Copy link
Collaborator

@pavel-shirshov can you please check the below? how do you think the key should be assigned?

@pavel-shirshov pavel-shirshov self-assigned this Jan 16, 2020
@pavel-shirshov
Copy link
Contributor

Thank you @mkovnir for reporting the issue. @liat-grozovik We should assign unique keys for each portchannel in sonic to prevent the issue. I'll create a patch as soon as I have time.

@pavel-shirshov
Copy link
Contributor

@mkovnir Would you like to create such patch? You need to fix the configuration here: https://github.com/Azure/sonic-swss/blob/master/cfgmgr/teammgr.cpp#L507
I can help you with the PR

@mkovnir
Copy link
Author

mkovnir commented Jan 20, 2020

@pavel-shirshov Sorry, I am not a developer and dont know how to patch the code.

@pavel-shirshov
Copy link
Contributor

@mkovnir No problem. I'll try to fix the issue as soon as I have some time

@pavel-shirshov
Copy link
Contributor

Another approach sonic-net/sonic-swss#1117

@liat-grozovik
Copy link
Collaborator

@pavel-shirshov and @lguohan what is the status of this issue? planning to be fixed for 201911?

@anshuv-mfst
Copy link

Hi @lguohan - could you please re-assign this PR.

@liat-grozovik
Copy link
Collaborator

liat-grozovik commented Jan 28, 2021

@anshuv-mfst Nvidia team will take it. Please assign @DavidZagury (i was not able to do so)

lguohan pushed a commit to sonic-net/sonic-swss that referenced this issue May 5, 2021
Fix issue - sonic-net/sonic-buildimage#4009

When a member port is added to port-channel, create a unique LACP key.
When adding a member port to port-channel set the LACP key to a unique number.
The number is extracted from the port-channel name and will be the number at the end of the port-channel name with an additional digit at the beginning in order to make sure that this number will be unique in the system.

Why I did it
If LACP key is not set, then the peer will not be able to distinguish the ports which are connected to different port-channels, as it will receive the LACP key as 0 for all the ports from different port-channels.

How I verified it
I configure a SONiC switch to have two port-channels and on a second switch, I created one port-channel for both links between the switches.
On the second switch only one of the ports comes up in the PO and the other one stayed down.
judyjoseph pushed a commit to sonic-net/sonic-utilities that referenced this issue Jun 21, 2021
What I did
Fix issue - sonic-net/sonic-buildimage#4009
Change the LACP key to be generated from the Port Channel name instead of always being 0.
When upgrading without warm-reboot update old port channels to use the new default.

How I did it
When adding a new port-channel add by default the key lacp_key with the value 'auto' to the port-channel table, this is done to change the port-channel LACP key to be generated from the Port Channel name instead of always being 0.

When upgrading without warm-reboot, also update old port channels to use the new default.
This is not done on warm-reboot to avoid the link from going down.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-utilities that referenced this issue Aug 10, 2021
What I did
Fix issue - sonic-net/sonic-buildimage#4009
Change the LACP key to be generated from the Port Channel name instead of always being 0.
When upgrading without warm-reboot update old port channels to use the new default.

How I did it
When adding a new port-channel add by default the key lacp_key with the value 'auto' to the port-channel table, this is done to change the port-channel LACP key to be generated from the Port Channel name instead of always being 0.

When upgrading without warm-reboot, also update old port channels to use the new default.
This is not done on warm-reboot to avoid the link from going down.
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this issue Oct 5, 2021
Fix issue - sonic-net/sonic-buildimage#4009

When a member port is added to port-channel, create a unique LACP key.
When adding a member port to port-channel set the LACP key to a unique number.
The number is extracted from the port-channel name and will be the number at the end of the port-channel name with an additional digit at the beginning in order to make sure that this number will be unique in the system.

Why I did it
If LACP key is not set, then the peer will not be able to distinguish the ports which are connected to different port-channels, as it will receive the LACP key as 0 for all the ports from different port-channels.

How I verified it
I configure a SONiC switch to have two port-channels and on a second switch, I created one port-channel for both links between the switches.
On the second switch only one of the ports comes up in the PO and the other one stayed down.
abdosi added a commit to abdosi/sonic-buildimage that referenced this issue Nov 14, 2022
abdosi added a commit that referenced this issue Nov 15, 2022
…aph (#12694)

Add lacp_key as auto in portchannel configuration when parsing minigraph
Needed as mention in issue: #4009
davidpil2002 pushed a commit to davidpil2002/sonic-buildimage that referenced this issue Nov 15, 2022
…aph (sonic-net#12694)

Add lacp_key as auto in portchannel configuration when parsing minigraph
Needed as mention in issue: sonic-net#4009
yxieca pushed a commit that referenced this issue Nov 28, 2022
…aph (#12694)

Add lacp_key as auto in portchannel configuration when parsing minigraph
Needed as mention in issue: #4009
malletvapid23 added a commit to malletvapid23/Sonic-Utility that referenced this issue Aug 3, 2023
What I did
Fix issue - sonic-net/sonic-buildimage#4009
Change the LACP key to be generated from the Port Channel name instead of always being 0.
When upgrading without warm-reboot update old port channels to use the new default.

How I did it
When adding a new port-channel add by default the key lacp_key with the value 'auto' to the port-channel table, this is done to change the port-channel LACP key to be generated from the Port Channel name instead of always being 0.

When upgrading without warm-reboot, also update old port channels to use the new default.
This is not done on warm-reboot to avoid the link from going down.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants