[teamd] retry creating team_port after interface info changed #2699
Conversation
Race condition has been noticed after warm reboot: sometimes when port_changed notification was received, the link message didn't have the device name. Without device name, creating team port would fail. Registering to the interface information change notification, so later when device name becomes available, retry creating team port. Signed-off-by: Ying Xie <ying.xie@microsoft.com>
|
Could you share the error log collected for the problem? I don't quite understand how "port_changed notification was received, the link message didn't have the device name" happened. Also I'd like to know whether it is related to the incorrect teammgrd then teamsyncd startup order. |
|
The logs came with libteamd doesn't have all the information. I had to add quite some extra log entries to eventually piece together the puzzle (keep in mind that anything started with '===' was added or enhanced:
Mostly, the race was between the libnl and libteamd, but teamd has to be able to handle it. |
|
Currently TeamMgr::doLagMemberTask() makes sure member port state is ok before adding the port to LAG. Port state ok was set by portsyncd upon receiving RTM_NEWLINK message which contains the correct device name. Why when the processing reaches teamd later, device/link name becomes unavailable? bug in teamd? |
|
I had the same question, but with the information from you question, I think I have an answer now:
Why we need to do it again? Because after warm reboot, the LAGs are restored by teamd. This operation bypassed the teammgrd protection. So I guess we really need to fix it in libnl, I looked at libnl while investigating this issue. I didn't find a good way of patching libnl. Maybe someone more familiar with libnl could take another look? Cheers, |
|
Overnight test reached 172 iterations, all 4 peers see lags up for ~21 hours. yinxi@acs-trusty8:$ ssh vm-1 "show interface po1" |
|
"It is a known issue that initial RTM_NEWLINK could come up without device name. For system warm reboot, the kernel is clean, though lacp state will be restored from the saved lacp pdu file, LAG and lag member are managed by teammmgrd. How this operation bypassed the teammgrd protection? Your change looks fixing the problem. I just want to get a clear understanding of the problem scenario |
|
Things are getting more interesting. The libnl has an 'optimization' that it won't report interface information change if it has been changed once. Looks like we might be hitting that too. At 182th iteration, test hit an issue where teamd repeated retry creating port_obj with empty name. Now we need even longer test cycle to proof safety. |
|
Jipan, I saw some change under warm reboot, teamd would do things differently. That might be mainly to make sure that we use same lag member ID to prevent peer tearing down the LAGs. What you claimed might be right. I didn't fully track down who created the new LAGs. One thing I know is that the LAGs are created very early on after warm reboot, they are usually already created by the time I can ssh in. Cheers, |
Race condition has been noticed after warm reboot: sometimes when port_changed notification was received, the link message didn't have the device name. Without device name, creating team port would fail. Registering to the interface information change notification, so later when device name becomes available, retry creating team port. Signed-off-by: Ying Xie <ying.xie@microsoft.com>
…net#2699) Race condition has been noticed after warm reboot: sometimes when port_changed notification was received, the link message didn't have the device name. Without device name, creating team port would fail. Registering to the interface information change notification, so later when device name becomes available, retry creating team port. Signed-off-by: Ying Xie <ying.xie@microsoft.com>
…net#2699) Race condition has been noticed after warm reboot: sometimes when port_changed notification was received, the link message didn't have the device name. Without device name, creating team port would fail. Registering to the interface information change notification, so later when device name becomes available, retry creating team port. Signed-off-by: Ying Xie <ying.xie@microsoft.com>
…net#2699) Race condition has been noticed after warm reboot: sometimes when port_changed notification was received, the link message didn't have the device name. Without device name, creating team port would fail. Registering to the interface information change notification, so later when device name becomes available, retry creating team port. Signed-off-by: Ying Xie <ying.xie@microsoft.com>
Update sonic-utilities submodule pointer to include the following: * 88ffb167 [config]config reload should generate sysinfo if missing ([sonic-net#2778](sonic-net/sonic-utilities#2778)) * 7443b9e5 [sonic-package-manager] support extension with multiple YANG modules ([sonic-net#2752](sonic-net/sonic-utilities#2752)) * 522c3a9e [sonic-package-manager] add support for multiple CLI plugin files ([sonic-net#2753](sonic-net/sonic-utilities#2753)) * b38fcfd1 [show][muxcable] fix RC ([sonic-net#2812](sonic-net/sonic-utilities#2812)) * 7e24463f [chassis]: remote cli commands infra for sonic chassis ([sonic-net#2701](sonic-net/sonic-utilities#2701)) * bee593e4 [DPB]Fixing typo in config breakout output ([sonic-net#2802](sonic-net/sonic-utilities#2802)) * ada603c5 [config]Support multi-asic Golden Config override ([sonic-net#2738](sonic-net/sonic-utilities#2738)) * 88a7daa8 [show][barefoot] replace shell=True ([sonic-net#2699](sonic-net/sonic-utilities#2699)) * 5e99edb5 [sonic_package_manager] replace shell=True ([sonic-net#2726](sonic-net/sonic-utilities#2726)) * b547bb45 [acl-loader] Only add default deny rule when table is L3 or L3V6 ([sonic-net#2796](sonic-net/sonic-utilities#2796)) Signed-off-by: dprital <drorp@nvidia.com>
Why I did it 69abbc3c - (HEAD, origin/master, origin/HEAD) Revert "[GCU] Complete RDMA Platform Validation Checks [device][platform] Update Inventec new platform d6356 #2791" DellEMC S6100 Watchdog Support #2854 (8 minutes ago) 4fead896 - [sonic-package-manager] fix CLI plugin compatibility issue [sonic-utilities] advance submodule head to latest #2842 (27 hours ago) db61efca - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan ([201811] [services] Restart SwSS service upon unexpected critical process exit #2852) (33 hours ago) d5544b4a - [config] Generate sysinfo as needed when override config ([minigraph]: Add mirror type v6 condition #2836) (6 days ago) f258e2a3 - [GCU] Complete RDMA Platform Validation Checks ([device][platform] Update Inventec new platform d6356 #2791) (6 days ago) b4f4e63e - Revert "Revert frr route check ([mlnx] fix url inconsistency in fw.mk #2761)" (Support TACACS Accounting #2762) (7 days ago) 3d89589f - Update pcieutil error message on loading common pcie module (Enable Debugs in BCM Kernel-bde and Knet Modules #2786) (11 days ago) e6aacd37 - Update TRANSCEIVER_INFO table after CDB FW upgrade (Remove unused packages in docker images and host (#2807) #2837) (2 weeks ago) 33d665c4 - replace shell=True, replace xml, and replace exit() ([mellanox-simx] add ability to build simx-compatiable image #2664) (2 weeks ago) 9e510a83 - [chassis][voq[Add "config fabric port ..." commands and tests. (Watchdog enable/disable in DellEMC S6100 #2730) (2 weeks ago) aeb0dbc1 - Fix the invalid variable issue when set-fips in uboot (fix bug in file sonic-cfggen #2834) (3 weeks ago) 1e73632d - [test]: add UT coverage for GCU (Feed device info to orchagent process #2818) (3 weeks ago) 3a9995b6 - [config]Support multi-asic Golden Config override with fix ([mellanox] Update Mellanox MFT packedge #2825) (3 weeks ago) 3fb32588 - Revert "[chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources #2701)" ([dhcp_relay] Base DHCP Relay Docker container on Debian Stretch #2832) (3 weeks ago) 2ffe6e37 - [show][mlnx] replace shell=True, replace xml (Add support of HwSKU Mellanox-SN2700-C28D8 #2700) (3 weeks ago) a5091bba - [sonic_sku_create] remove shell=True, replace exit() with sys.exit() (removed exec from script which that prevents the further lines to be … #2816) (3 weeks ago) 71ef4f16 - [build] Fix base OS compilation issue caused by incompatibility with requests >= 2.29.0. ([201811][sairedis][utilities] advance sub module heads #2830) (3 weeks ago) 1097373b - [show] Added alias interface mode support for 'show interfaces counters ...' command ([kernel]: update sonic kernel to 4.9.0-8-2 #2468) (4 weeks ago) <Julian Chang - TW> 589375fc - correctly parsing complete ipv6 vnet info ([201811][mellanox] Update Mellanox FW version to 13.1910.0928 #2827) (4 weeks ago) 634ac77c - LAG keepalive script to reduce lacp session wait during warm-reboot (Set proper hostname on containers startup #2806) (4 weeks ago) 331c9de0 - [config]: Dynamically start and stop ndppd ([Arista] Add QoS needed files for Arista 7170 #2814) (4 weeks ago) d1f307d0 - [GCU]Fix rdma check failure ([device/celestica]: Add fwutil #2824) (4 weeks ago) ce81a340 - Revert "[config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete #2738)" ([BGP docker]: start bgp_eoiu_mark service to populate bgp eoiu marker… #2823) (4 weeks ago) 61e0e810 - Added platform plugin support in load_minigraph ([db migrator] migrate the DB to latest schema when needed #2808) (4 weeks ago) d4355a96 - Change default CDB run mode to non-hitless (Revert "Watchdog enable/disable in DellEMC S6100 " #2817) (4 weeks ago) 88ffb167 - [config]config reload should generate sysinfo if missing ([Mellanox] Update SAI #2778) (4 weeks ago) 7443b9e5 - [sonic-package-manager] support extension with multiple YANG modules (dhcp_relay service stopped with "systemctl stop swss" but not restarted with "systemctl restart swss" #2752) (4 weeks ago) 522c3a9e - [sonic-package-manager] add support for multiple CLI plugin files (Updated Makefile infrastructure to build debug images. #2753) (4 weeks ago) b38fcfd1 - [show][muxcable] fix show mux hwmode muxdirection RC (syncd-rpc.mk: Fix stretch dockers build failure #2812) (5 weeks ago) 7e24463f - [chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources #2701) (6 weeks ago) bee593e4 - [DPB]Fixing typo in config breakout output ([submodule update]: Quagga bgpd crash fix #2802) (6 weeks ago) ada603c5 - [config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete #2738) (6 weeks ago) 88a7daa8 - [show][barefoot] replace shell=True ([teamd] retry creating team_port after interface info changed #2699) (6 weeks ago) 5e99edb5 - [sonic_package_manager] replace shell=True (Upgrade Mellanox HW-MGMT: fix high CPU utilization issue #2726) (6 weeks ago) b547bb45 - [acl-loader] Only add default deny rule when table is L3 or L3V6 ([201811] [radvd] Build radvd from source; Patch so as not to treat out-of-range MTU as an error #2796) (6 weeks ago)
…#2699) **What I did** Enforce the order when the shared headroom pool is enabled. **Why I did it** The current flow to enable the shared headroom pool 1. Configure the shared headroom pool size or over-subscribe ratio 2. Update lossless buffer profiles with `xon == size` 3. Calculate and update the shared headroom pool size. In step 2, the lossless buffer profiles have been updated to values as if the shared headroom pool is enabled. However, it is enabled only in step 3, which is inconsistent between steps 2 and 3. Therefore, we open the PR to guarantee the order. The new flow 1. A user configures the shared headroom pool size or over-subscribe ratio 2. The dynamic buffer manager invokes the vendor-specific Lua plugin to calculate the shared headroom pool size - This is the step introduced in this PR to guarantee the shared headroom pool will be enabled in advance - On Nvidia platform, a non-zero shared headroom pool is returned in this stage if the user configures the over-subscribe ratio 3. If a non-zero shared headroom pool is returned, the dynamic buffer manager pushes the shared headroom pool size to APPL_DB.ingress_lossless_pool and blocks until it has been updated into APPL_STATE_DB.ingress_lossless_pool (which indicates the buffer orchagent finishes handling it) 4. The buffer manager updates the lossless buffer profiles 5. The buffer manager invokes the Lua plugin to calculate the shared headroom pool size. 6. The flow continues as normal. **How I verified it** Manually test and regression test
Why I did it 69abbc3c - (HEAD, origin/master, origin/HEAD) Revert "[GCU] Complete RDMA Platform Validation Checks [device][platform] Update Inventec new platform d6356 sonic-net#2791" DellEMC S6100 Watchdog Support sonic-net#2854 (8 minutes ago) 4fead896 - [sonic-package-manager] fix CLI plugin compatibility issue [sonic-utilities] advance submodule head to latest sonic-net#2842 (27 hours ago) db61efca - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan ([201811] [services] Restart SwSS service upon unexpected critical process exit sonic-net#2852) (33 hours ago) d5544b4a - [config] Generate sysinfo as needed when override config ([minigraph]: Add mirror type v6 condition sonic-net#2836) (6 days ago) f258e2a3 - [GCU] Complete RDMA Platform Validation Checks ([device][platform] Update Inventec new platform d6356 sonic-net#2791) (6 days ago) b4f4e63e - Revert "Revert frr route check ([mlnx] fix url inconsistency in fw.mk sonic-net#2761)" (Support TACACS Accounting sonic-net#2762) (7 days ago) 3d89589f - Update pcieutil error message on loading common pcie module (Enable Debugs in BCM Kernel-bde and Knet Modules sonic-net#2786) (11 days ago) e6aacd37 - Update TRANSCEIVER_INFO table after CDB FW upgrade (Remove unused packages in docker images and host (sonic-net#2807) sonic-net#2837) (2 weeks ago) 33d665c4 - replace shell=True, replace xml, and replace exit() ([mellanox-simx] add ability to build simx-compatiable image sonic-net#2664) (2 weeks ago) 9e510a83 - [chassis][voq[Add "config fabric port ..." commands and tests. (Watchdog enable/disable in DellEMC S6100 sonic-net#2730) (2 weeks ago) aeb0dbc1 - Fix the invalid variable issue when set-fips in uboot (fix bug in file sonic-cfggen sonic-net#2834) (3 weeks ago) 1e73632d - [test]: add UT coverage for GCU (Feed device info to orchagent process sonic-net#2818) (3 weeks ago) 3a9995b6 - [config]Support multi-asic Golden Config override with fix ([mellanox] Update Mellanox MFT packedge sonic-net#2825) (3 weeks ago) 3fb32588 - Revert "[chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources sonic-net#2701)" ([dhcp_relay] Base DHCP Relay Docker container on Debian Stretch sonic-net#2832) (3 weeks ago) 2ffe6e37 - [show][mlnx] replace shell=True, replace xml (Add support of HwSKU Mellanox-SN2700-C28D8 sonic-net#2700) (3 weeks ago) a5091bba - [sonic_sku_create] remove shell=True, replace exit() with sys.exit() (removed exec from script which that prevents the further lines to be … sonic-net#2816) (3 weeks ago) 71ef4f16 - [build] Fix base OS compilation issue caused by incompatibility with requests >= 2.29.0. ([201811][sairedis][utilities] advance sub module heads sonic-net#2830) (3 weeks ago) 1097373b - [show] Added alias interface mode support for 'show interfaces counters ...' command ([kernel]: update sonic kernel to 4.9.0-8-2 sonic-net#2468) (4 weeks ago) <Julian Chang - TW> 589375fc - correctly parsing complete ipv6 vnet info ([201811][mellanox] Update Mellanox FW version to 13.1910.0928 sonic-net#2827) (4 weeks ago) 634ac77c - LAG keepalive script to reduce lacp session wait during warm-reboot (Set proper hostname on containers startup sonic-net#2806) (4 weeks ago) 331c9de0 - [config]: Dynamically start and stop ndppd ([Arista] Add QoS needed files for Arista 7170 sonic-net#2814) (4 weeks ago) d1f307d0 - [GCU]Fix rdma check failure ([device/celestica]: Add fwutil sonic-net#2824) (4 weeks ago) ce81a340 - Revert "[config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete sonic-net#2738)" ([BGP docker]: start bgp_eoiu_mark service to populate bgp eoiu marker… sonic-net#2823) (4 weeks ago) 61e0e810 - Added platform plugin support in load_minigraph ([db migrator] migrate the DB to latest schema when needed sonic-net#2808) (4 weeks ago) d4355a96 - Change default CDB run mode to non-hitless (Revert "Watchdog enable/disable in DellEMC S6100 " sonic-net#2817) (4 weeks ago) 88ffb167 - [config]config reload should generate sysinfo if missing ([Mellanox] Update SAI sonic-net#2778) (4 weeks ago) 7443b9e5 - [sonic-package-manager] support extension with multiple YANG modules (dhcp_relay service stopped with "systemctl stop swss" but not restarted with "systemctl restart swss" sonic-net#2752) (4 weeks ago) 522c3a9e - [sonic-package-manager] add support for multiple CLI plugin files (Updated Makefile infrastructure to build debug images. sonic-net#2753) (4 weeks ago) b38fcfd1 - [show][muxcable] fix show mux hwmode muxdirection RC (syncd-rpc.mk: Fix stretch dockers build failure sonic-net#2812) (5 weeks ago) 7e24463f - [chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources sonic-net#2701) (6 weeks ago) bee593e4 - [DPB]Fixing typo in config breakout output ([submodule update]: Quagga bgpd crash fix sonic-net#2802) (6 weeks ago) ada603c5 - [config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete sonic-net#2738) (6 weeks ago) 88a7daa8 - [show][barefoot] replace shell=True ([teamd] retry creating team_port after interface info changed sonic-net#2699) (6 weeks ago) 5e99edb5 - [sonic_package_manager] replace shell=True (Upgrade Mellanox HW-MGMT: fix high CPU utilization issue sonic-net#2726) (6 weeks ago) b547bb45 - [acl-loader] Only add default deny rule when table is L3 or L3V6 ([201811] [radvd] Build radvd from source; Patch so as not to treat out-of-range MTU as an error sonic-net#2796) (6 weeks ago)
- What I did
Race condition has been noticed after warm reboot: sometimes when
port_changed notification was received, the link message didn't
have the device name. Without device name, creating team port
would fail.
Registering to the interface information change notification, so
later when device name becomes available, retry creating team port.
Signed-off-by: Ying Xie ying.xie@microsoft.com
- How to verify it
Continuous warm reboot on my DUT. The retry has been verified with debug messages.
Without the change, continuous warm reboot would fail within 20 iterations. With the fix, the test count has gone up to 78 and still running.
for f in
ls *.log; do cnt=grep iteration $f | wc -l; echo $f $cnt; donewb-test-20190313-0437.log 13
wb-test-20190319-2212.log 2
wb-test-20190319-2307.log 1
wb-test-20190319-2313.log 3
wb-test-20190319-2348.log 6
wb-test-20190320-0119.log 20
wb-test-20190321-0020.log 2
wb-test-20190321-0207.log 10
wb-test-20190321-1733.log 7
wb-test-20190321-1839.log 5
wb-test-20190321-2131.log 7
wb-test-20190322-0133.log 4
wb-test-20190322-0219.log 5
wb-test-20190322-0429.log 3
wb-test-20190322-1656.log 19
wb-test-20190322-2158.log 2
wb-test-20190323-0006.log 7
wb-test-20190323-0650.log 4
wb-test-20190323-0729.log 3
wb-test-20190323-1855.log 2
wb-test-20190323-1911.log 1
wb-test-20190323-1924.log 10
wb-test-20190324-1911.log 78