Administratively shutdown port channel has member ports in deselected state and traffic is not forwarded. #1771 #2882

phanindra-tv · 2019-05-10T09:00:04Z

- What I did
Added code in teamd for the issue reported in #1771.

- How I did it
Added a conditional check in teamd runner (lacp) code to ensure LACP frames received on an administratively shutdown port channel, are not processed. This results in the member ports of the port channel staying in 'deselected' state. Hardware does not have these ports part of the port channel and thereby traffic is not forwarded.

- How to verify it

Configure a port channel with two physical ports connected back-to-back between two devices.
Execute 'teamshow' or 'show interfaces portchannel' to ensure the member ports are in selected (S) state after LACP exchange.
Shutdown the port channel.
Execute 'teamshow' or 'show interfaces portchannel'. Notice that the member ports remain in deselected (D) state. Send bi-directional traffic to confirm the flow across devices has stopped.
Bring up the port channel. Member ports need to be placed in selected (S) state. And traffic flow across the devices needs to resume.

Without the fix:
root@sonic:/home/admin# show int port
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

1  PortChannel1  LACP(A)(Up)  Ethernet2(S) Ethernet4(S)

root@sonic:/home/admin# config interface shutdown PortChannel1
root@sonic:/home/admin# show int port
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

1  PortChannel1  LACP(A)(Dw)  Ethernet2(D) Ethernet4(D)

root@sonic:/home/admin# show int port
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

1  PortChannel1  LACP(A)(Dw)  Ethernet2(S) Ethernet4(S)

With the fix:
root@sonic:/home/admin# show int port
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

1  PortChannel1  LACP(A)(Up)  Ethernet2(S) Ethernet4(S)

root@sonic:/home/admin# config interface shutdown PortChannel1
root@sonic:/home/admin# show int port
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

1  PortChannel1  LACP(A)(Dw)  Ethernet2(D) Ethernet4(D)

root@sonic:/home/admin# config interface startup PortChannel1
root@sonic:/home/admin# show int port
Flags: A - active, I - inactive, Up - up, Dw - Down, N/A - not available, S - selected, D - deselected
No. Team Dev Protocol Ports

1  PortChannel1  LACP(A)(Up)  Ethernet2(S) Ethernet4(S)

- Description for the changelog
Administratively shutdown port channel has member ports in deselected state and traffic is not forwarded.

- A picture of a cute animal (not mandatory but encouraged)

…m trunk in the hardware and thereby traffic stops forwarding. sonic-net#1771

msftclas · 2019-05-10T09:00:18Z

All CLA requirements met.

lguohan · 2019-05-15T17:42:50Z

I can repro this issue on the master branch. The issue looks like to be even though the port channel is shutdown, but the member port are not shutdown. Then, the member ports still receive the lacp pdu and make the member port to be selected.

another approach to fix the problem is to shutdown the member ports when port channel is administrative down.

phanindra-tv · 2019-05-16T07:47:46Z

lguohan: Were you able to recreate the issue with the code changes listed in this PR?

pavel-shirshov

Please check my comment.
Have you contacted libteam maintainer? Possible it worth raise a bug with them.

pavel-shirshov · 2019-05-16T21:07:15Z

src/libteam/0013-teamd-lacp-port-admin-down-recv-not-processing.patch

+
+        admin_state = team_get_ifinfo_admin_state(lacp_port->ctx->ifinfo);
+        if (!admin_state) {
+                teamd_log_info("%s received PDU in admin down state.",


I'd recommend to make it debug message. Otherwise we will create syslog message every 30 seconds on each member port of disabled port channel.

Sure, the log message should be debug. I have changed it.

I will email libteam maintainer. But, as SONiC has more than 12 patches in libteam, it might consume significant time before the maintainer works/merges (or perhaps requests changes or provides comments on the proposed patches) these into libteam.

Thanks for the fix. I'll check it later today.
Please email only your patch which fixing only one issue.

Sure. Thanks.

pavel-shirshov

I approve.
But I think we should think about the message.
In sonic teamd has debug log level by default. That means that the message will spam our logs every 30 seconds while a portchannel is down. Should we remove log output? Do we need it at all?

phanindra-tv · 2019-05-21T03:11:30Z

Thank you. I too had a similar thought regarding the need for a debug log message. I will remove it.

pavel-shirshov · 2019-05-21T21:06:10Z

retest this please

phanindra-tv · 2019-05-22T03:23:06Z

Tested the code changes provided in the patch file.

pavel-shirshov · 2019-05-23T01:48:19Z

can you please fix your patch?

error: corrupt patch at line 24

phanindra-tv · 2019-05-23T05:42:49Z

Apologies. Added proper patch file.

…eselected state and traffic is not forwarded. #1771 (#2882)

Update sonic-utilities submodule pointer to include the following: * 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882)) * 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839)) * fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789)) * 659ba24b [syslog] Adjust runningconfiguration syslog command ([sonic-net#2843](sonic-net/sonic-utilities#2843)) * 46fba26f [db_migrator] add required protocol field in ROUTE_TABLE ([sonic-net#2766](sonic-net/sonic-utilities#2766)) * f186376e Fix issue: show interfaces transceiver eeprom -d should display same entry for CMIS cable ([sonic-net#2864](sonic-net/sonic-utilities#2864)) * de491798 fix precedence in portstat CLI ([sonic-net#2874](sonic-net/sonic-utilities#2874)) Signed-off-by: dprital <drorp@nvidia.com>

Update sonic-utilities submodule pointer to include the following: * ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580)) * 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895)) * 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885)) * 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750)) * 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887)) * b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676)) * 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698)) * dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737)) * 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525)) * 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882)) * 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839)) * fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789)) Signed-off-by: dgsudharsan <sudharsand@nvidia.com>

Update sonic-utilities submodule pointer to include the following: * ff380e04 [hash]: Implement GH frontend ([#2580](sonic-net/sonic-utilities#2580)) * 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([#2895](sonic-net/sonic-utilities#2895)) * 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([#2885](sonic-net/sonic-utilities#2885)) * 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([#2750](sonic-net/sonic-utilities#2750)) * 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([#2887](sonic-net/sonic-utilities#2887)) * b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([#2676](sonic-net/sonic-utilities#2676)) * 316b14c0 Add support for secure upgrade ([#2698](sonic-net/sonic-utilities#2698)) * dc2945bc [dns] Implement config and show commands for static DNS. ([#2737](sonic-net/sonic-utilities#2737)) * 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([#2525](sonic-net/sonic-utilities#2525)) * 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([#2882](sonic-net/sonic-utilities#2882)) * 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([#2839](sonic-net/sonic-utilities#2839)) * fceef2ed [chassis][voq] Clear fabric counters queue/port ([#2789](sonic-net/sonic-utilities#2789)) Signed-off-by: dgsudharsan <sudharsand@nvidia.com>

…atically (#15456) #### Why I did it src/sonic-utilities ``` * ff380e04 - (HEAD -> master, origin/master, origin/HEAD) [hash]: Implement GH frontend (#2580) (13 hours ago) [Nazarii Hnydyn] * 61bad064 - [db_migrator] Set correct CURRENT_VERSION, extend UT (#2895) (4 days ago) [Vadym Hlushko] * 6b8ee47c - [CLI][Show][BGP] Show BGP Change for no neighbor scenario (#2885) (6 days ago) [Dev Ojha] * 73d8d633 - [doc] Update Command-Reference.md, change "show bgp peer" command to "show bfd peer" (#2750) (11 days ago) [PinghaoQu] * 7bc08c28 - [db_migrator] Remove hardcoded config and migrate config from minigraph (#2887) (11 days ago) [Vaibhav Hemant Dixit] * b1aa9426 - [generate_dump]: Enhance show techsupport for Marvell platform (#2676) (11 days ago) [pavannaregundi] * 316b14c0 - Add support for secure upgrade (#2698) (2 weeks ago) [ycoheNvidia] * dc2945bc - [dns] Implement config and show commands for static DNS. (#2737) (2 weeks ago) [Oleksandr Ivantsiv] * 8414a709 - [chassis][multi asic] change acl_loader to use tcp socket for db communication (#2525) (2 weeks ago) [Arvindsrinivasan Lakshmi Narasimhan] * 0b629ba1 - Revert "[chassis][voq] Clear fabric counters queue/port (#2789)" (#2882) (3 weeks ago) [RoRonoa] * 3ba8241a - [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. (#2839) (4 weeks ago) [Vadym Hlushko] * fceef2ed - [chassis][voq] Clear fabric counters queue/port (#2789) (4 weeks ago) [jfeng-arista] ``` #### How I did it #### How to verify it #### Description for the changelog

Update sonic-utilities submodule pointer to include the following: * ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580)) * 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895)) * 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885)) * 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750)) * 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887)) * b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676)) * 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698)) * dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737)) * 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525)) * 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882)) * 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839)) * fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789)) Signed-off-by: dgsudharsan <sudharsand@nvidia.com>

…lly (#16834) src/sonic-swss * 561cfd94 - (HEAD -> 202205, origin/202205) [202205][buffers] Add handler for the 'create_only_config_db_buffers' configuration knob (#2882) (11 hours ago) [Vadym Hlushko]

…sonic-buildimage into internal Fix conflict for rsyslog. Skip partial DNS unit test in internal branch after confirmed with Gang. Related work items: sonic-net#113, sonic-net#131, sonic-net#132, sonic-net#134, sonic-net#321, sonic-net#331, sonic-net#381, sonic-net#382, sonic-net#2525, sonic-net#2676, sonic-net#2698, sonic-net#2737, sonic-net#2789, sonic-net#2839, sonic-net#2845, sonic-net#2850, sonic-net#2882, sonic-net#2885, sonic-net#2887, sonic-net#2890, sonic-net#2895, sonic-net#13338, sonic-net#14105, sonic-net#15142, sonic-net#15223, sonic-net#15456, sonic-net#15487, sonic-net#15520, sonic-net#15726, sonic-net#15727, sonic-net#15758, sonic-net#15764, sonic-net#15765, sonic-net#15772, sonic-net#15779, sonic-net#15782, sonic-net#15785, sonic-net#15797, sonic-net#15798, sonic-net#15810, sonic-net#15811, sonic-net#15821

Administratively shutting down port channel will remove the ports fro…

7dcf099

…m trunk in the hardware and thereby traffic stops forwarding. sonic-net#1771

lguohan requested a review from pavel-shirshov May 15, 2019 17:37

pavel-shirshov suggested changes May 16, 2019

View reviewed changes

sonic-net#2882

0705258

pavel-shirshov approved these changes May 20, 2019

View reviewed changes

sonic-net#2882

0c45e08

pavel-shirshov approved these changes May 21, 2019

View reviewed changes

sonic-net#2882

03d2db5

pavel-shirshov approved these changes May 23, 2019

View reviewed changes

lguohan merged commit df149cd into sonic-net:master May 29, 2019

phanindra-tv deleted the 1771_fix branch May 29, 2019 05:55

yxieca added the Request for 201811 Branch label May 30, 2019

yxieca pushed a commit that referenced this pull request May 30, 2019

[teamd]: Administratively shutdown port channel has member ports in d…

abc25df

…eselected state and traffic is not forwarded. #1771 (#2882)

yxieca added the Included in 201811 Branch label May 30, 2019

stcheng added the Enhancement ➕ label Jun 6, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Administratively shutdown port channel has member ports in deselected state and traffic is not forwarded. #1771 #2882

Administratively shutdown port channel has member ports in deselected state and traffic is not forwarded. #1771 #2882

phanindra-tv commented May 10, 2019

msftclas commented May 10, 2019 •

edited

Loading

lguohan commented May 15, 2019

phanindra-tv commented May 16, 2019

pavel-shirshov left a comment

pavel-shirshov May 16, 2019

phanindra-tv May 17, 2019

pavel-shirshov May 17, 2019

phanindra-tv May 18, 2019

pavel-shirshov left a comment

phanindra-tv commented May 21, 2019

pavel-shirshov commented May 21, 2019

phanindra-tv commented May 22, 2019

pavel-shirshov commented May 23, 2019

phanindra-tv commented May 23, 2019

Administratively shutdown port channel has member ports in deselected state and traffic is not forwarded. #1771 #2882

Administratively shutdown port channel has member ports in deselected state and traffic is not forwarded. #1771 #2882

Conversation

phanindra-tv commented May 10, 2019

msftclas commented May 10, 2019 • edited Loading

lguohan commented May 15, 2019

phanindra-tv commented May 16, 2019

pavel-shirshov left a comment

Choose a reason for hiding this comment

pavel-shirshov May 16, 2019

Choose a reason for hiding this comment

phanindra-tv May 17, 2019

Choose a reason for hiding this comment

pavel-shirshov May 17, 2019

Choose a reason for hiding this comment

phanindra-tv May 18, 2019

Choose a reason for hiding this comment

pavel-shirshov left a comment

Choose a reason for hiding this comment

phanindra-tv commented May 21, 2019

pavel-shirshov commented May 21, 2019

phanindra-tv commented May 22, 2019

pavel-shirshov commented May 23, 2019

phanindra-tv commented May 23, 2019

msftclas commented May 10, 2019 •

edited

Loading