[Neighsync] Handle Mac address 'none' during neigh add #2593

prsunny · 2022-12-30T00:32:18Z

What I did
In some rare cases, kernel neighbor add notifications are received with Mac string as 'none'. This is from the following libnl code:

https://www.infradead.org/~tgr/libnl/doc/api/lib_2addr_8c_source.html

This can cause a neighsyncd crash as below from here

Dec 12 07:36:37.149555 ST12-M0 INFO swss#/supervisord: neighsyncd terminate called after throwing an instance of 'std::invalid_argument'
Dec 12 07:36:37.149555 ST12-01M0 INFO swss#/supervisord: neighsyncd   what():  can't parse mac address 'none'

Why I did it

Prevent crash by checking if MacString is 'none' during neighbor 'add' operation.

How I verified it

Details if related

theasianpianist · 2022-12-30T23:03:56Z

neighsyncd/neighsync.cpp

@@ -143,6 +143,12 @@ void NeighSync::onMsg(int nlmsg_type, struct nl_object *obj)
        nl_addr2str(rtnl_neigh_get_lladdr(neigh), macStr, MAX_ADDR_SIZE);
    }

+    if (!delete_key && !strncmp(macStr, "none", MAX_ADDR_SIZE))


Is it possible for the MAC to be set to "none" on a dual ToR device? If this happens and also triggers the zero MAC logic directly above, we could have the "none" MAC be overwritten by an empty MAC which would cause unexpected tunnel routes to be created.

I thought about dualtor case and it is unlikely we hit this issue in a stable system. In case it happens, i think treating it as zero mac should be fine.

weiliu-ivy · 2023-01-30T07:16:03Z

@prsunny @theasianpianist could you help add the reason that this PR can bypass coverage failure? Thank you!

Update sonic-swss submodule pointer to include the following: * a2a483d [acl] Add new ACL key BTH_OPCODE and AETH_SYNDROME ([sonic-net#2617](sonic-net/sonic-swss#2617)) * 9d1f66b [bfdorch] add local discriminator to state DB ([sonic-net#2629](sonic-net/sonic-swss#2629)) * c54b3d1 Vxlan tunnel endpoint custom monitoring APPL DB table. ([sonic-net#2589](sonic-net/sonic-swss#2589)) * 7f03db2 Fix potential risks ([sonic-net#2516](sonic-net/sonic-swss#2516)) * 383ee68 [refactor]Refactoring sai handle status ([sonic-net#2621](sonic-net/sonic-swss#2621)) * cd95972 Fix issue 13341 ARP entry can be out of sync between kernel and APPL_DB if multiple updates are received from RTNL ([sonic-net#2619](sonic-net/sonic-swss#2619)) * a01470f Remove TODO comments that are no longer relevant ([sonic-net#2622](sonic-net/sonic-swss#2622)) * d058390 Changed the BFD default detect multiplier to 10x ([sonic-net#2614](sonic-net/sonic-swss#2614)) * d78b528 [MuxOrch] Enabling neighbor when adding in active state ([sonic-net#2601](sonic-net/sonic-swss#2601)) * 4ebdad1 [routesync] Fix for stale dynamic neighbor ([sonic-net#2553](sonic-net/sonic-swss#2553)) * 8857f92 Added new attributes for Vnet and Vxlan ecmp configurations. ([sonic-net#2584](sonic-net/sonic-swss#2584)) * b6bbc3e Revert [voq][chassis]Add show fabric counters port/queue commands (2522) ([sonic-net#2611](sonic-net/sonic-swss#2611)) * 52406e2 Add missing parameter to on_switch_shutdown_request method. ([sonic-net#2567](sonic-net/sonic-swss#2567)) * 4ac9ad9 Increase diff coverage to 80% ([sonic-net#2599](sonic-net/sonic-swss#2599)) * 8a0bb36 Handle Mac address 'none' ([sonic-net#2593](sonic-net/sonic-swss#2593)) * f496ab3 [vstest] Only collect stdout of orchagent_restart_check in vstest ([sonic-net#2597](sonic-net/sonic-swss#2597)) * 1dab495 Avoid aborting orchagent when setting TUNNEL attributes ([sonic-net#2591](sonic-net/sonic-swss#2591)) * 4395cea Fix neighbor doesn't update all attribute ([sonic-net#2577](sonic-net/sonic-swss#2577)) Signed-off-by: dprital <[email protected]>

prsunny · 2023-01-31T02:06:01Z

@prsunny @theasianpianist could you help add the reason that this PR can bypass coverage failure? Thank you!

hi @weiliu-ivy , this cannot be simulated by the test and hence bypassed. This can be treated as an exception.

weiliu-ivy · 2023-01-31T11:41:23Z

@prsunny @theasianpianist could you help add the reason that this PR can bypass coverage failure? Thank you!

hi @weiliu-ivy , this cannot be simulated by the test and hence bypassed. This can be treated as an exception.

@prsunny Got it. Thank you!

Update sonic-swss submodule pointer to include the following: * a2a483d [acl] Add new ACL key BTH_OPCODE and AETH_SYNDROME ([#2617](sonic-net/sonic-swss#2617)) * 9d1f66b [bfdorch] add local discriminator to state DB ([#2629](sonic-net/sonic-swss#2629)) * c54b3d1 Vxlan tunnel endpoint custom monitoring APPL DB table. ([#2589](sonic-net/sonic-swss#2589)) * 7f03db2 Fix potential risks ([#2516](sonic-net/sonic-swss#2516)) * 383ee68 [refactor]Refactoring sai handle status ([#2621](sonic-net/sonic-swss#2621)) * cd95972 Fix issue 13341 ARP entry can be out of sync between kernel and APPL_DB if multiple updates are received from RTNL ([#2619](sonic-net/sonic-swss#2619)) * a01470f Remove TODO comments that are no longer relevant ([#2622](sonic-net/sonic-swss#2622)) * d058390 Changed the BFD default detect multiplier to 10x ([#2614](sonic-net/sonic-swss#2614)) * d78b528 [MuxOrch] Enabling neighbor when adding in active state ([#2601](sonic-net/sonic-swss#2601)) * 4ebdad1 [routesync] Fix for stale dynamic neighbor ([#2553](sonic-net/sonic-swss#2553)) * 8857f92 Added new attributes for Vnet and Vxlan ecmp configurations. ([#2584](sonic-net/sonic-swss#2584)) * b6bbc3e Revert [voq][chassis]Add show fabric counters port/queue commands (2522) ([#2611](sonic-net/sonic-swss#2611)) * 52406e2 Add missing parameter to on_switch_shutdown_request method. ([#2567](sonic-net/sonic-swss#2567)) * 4ac9ad9 Increase diff coverage to 80% ([#2599](sonic-net/sonic-swss#2599)) * 8a0bb36 Handle Mac address 'none' ([#2593](sonic-net/sonic-swss#2593)) * f496ab3 [vstest] Only collect stdout of orchagent_restart_check in vstest ([#2597](sonic-net/sonic-swss#2597)) * 1dab495 Avoid aborting orchagent when setting TUNNEL attributes ([#2591](sonic-net/sonic-swss#2591)) * 4395cea Fix neighbor doesn't update all attribute ([#2577](sonic-net/sonic-swss#2577)) Signed-off-by: dprital <[email protected]>

*Merge remote-tracking branch 'upstream/master' into dash (#2663) * Modify coppmgr mergeConfig to support preserving copp tables through reboot. (#2548) * Avoid aborting orchagent when setting TUNNEL attributes (#2591) * Handle Mac address 'none' (#2593) * Increase diff coverage to 80% (#2599) * Add missing parameter to on_switch_shutdown_request method. (#2567) * Add ZMQ based ProducerStateTable and CustomerStateTable. * Revert "[voq][chassis]Add show fabric counters port/queue commands (#2522)" (#2611) * Added new attributes for Vnet and Vxlan ecmp configurations. (#2584) * added support for monitoring, primary and adv_prefix and overlay_dmac. * [routesync] Fix for stale dynamic neighbor (#2553) * [MuxOrch] Enabling neighbor when adding in active state (#2601) * Changed the BFD default detect multiplier to 10x (#2614) * Remove TODO comments that are no longer relevant (#2622) * Fix issue #13341 ARP entry can be out of sync between kernel and APPL_DB if multiple updates are received from RTNL (#2619) * [refactor]Refactoring sai handle status (#2621) * Vxlan tunnel endpoint custom monitoring APPL DB table. (#2589) * added support for monitoring, primary and adv_prefix. changed filter_mac to overlay_dmac * Data Structures and code to write APP_DB VNET_MONITOR table entries for custom monitoring of Vxlan tunnel endpoints. * [bfdorch] add local discriminator to state DB (#2629) * [acl] Add new ACL key BTH_OPCODE and AETH_SYNDROME (#2617) * [voq][chassis] Remove created ports from the default vlan. (#2607) * [EVPN]Handling race condition when remote VNI arrives before tunnel map entry (#2642) *Added check in remote VNI add to ensure vxlan tunnel map is created before adding the remote end point. * [test_mux] add sleep in test_NH (#2648) * [autoneg]Fixing adv interface types to be set when AN is disabled (#2638) * [hash]: Add UT infra. (#2660) *Added UT infra for Generic Hash feature *Aligned PBH tests with Generic Hash UT infra * [sai_failure_dump]Invoking dump during SAI failure (#2644) * [ResponsePublisher] add pipeline support (#2511) * [dash] Fix compilation issue caused by missing include.

) *Merge remote-tracking branch 'upstream/master' into dash (sonic-net#2663) * Modify coppmgr mergeConfig to support preserving copp tables through reboot. (sonic-net#2548) * Avoid aborting orchagent when setting TUNNEL attributes (sonic-net#2591) * Handle Mac address 'none' (sonic-net#2593) * Increase diff coverage to 80% (sonic-net#2599) * Add missing parameter to on_switch_shutdown_request method. (sonic-net#2567) * Add ZMQ based ProducerStateTable and CustomerStateTable. * Revert "[voq][chassis]Add show fabric counters port/queue commands (sonic-net#2522)" (sonic-net#2611) * Added new attributes for Vnet and Vxlan ecmp configurations. (sonic-net#2584) * added support for monitoring, primary and adv_prefix and overlay_dmac. * [routesync] Fix for stale dynamic neighbor (sonic-net#2553) * [MuxOrch] Enabling neighbor when adding in active state (sonic-net#2601) * Changed the BFD default detect multiplier to 10x (sonic-net#2614) * Remove TODO comments that are no longer relevant (sonic-net#2622) * Fix issue #13341 ARP entry can be out of sync between kernel and APPL_DB if multiple updates are received from RTNL (sonic-net#2619) * [refactor]Refactoring sai handle status (sonic-net#2621) * Vxlan tunnel endpoint custom monitoring APPL DB table. (sonic-net#2589) * added support for monitoring, primary and adv_prefix. changed filter_mac to overlay_dmac * Data Structures and code to write APP_DB VNET_MONITOR table entries for custom monitoring of Vxlan tunnel endpoints. * [bfdorch] add local discriminator to state DB (sonic-net#2629) * [acl] Add new ACL key BTH_OPCODE and AETH_SYNDROME (sonic-net#2617) * [voq][chassis] Remove created ports from the default vlan. (sonic-net#2607) * [EVPN]Handling race condition when remote VNI arrives before tunnel map entry (sonic-net#2642) *Added check in remote VNI add to ensure vxlan tunnel map is created before adding the remote end point. * [test_mux] add sleep in test_NH (sonic-net#2648) * [autoneg]Fixing adv interface types to be set when AN is disabled (sonic-net#2638) * [hash]: Add UT infra. (sonic-net#2660) *Added UT infra for Generic Hash feature *Aligned PBH tests with Generic Hash UT infra * [sai_failure_dump]Invoking dump during SAI failure (sonic-net#2644) * [ResponsePublisher] add pipeline support (sonic-net#2511) * [dash] Fix compilation issue caused by missing include.

In some rare cases, kernel neighbor add notifications are received with Mac string as 'none'. Fixing this to prevent crash.

Handle Mac address 'none'

930ddff

prsunny requested review from theasianpianist and dgsudharsan December 30, 2022 00:32

theasianpianist reviewed Dec 30, 2022

View reviewed changes

theasianpianist approved these changes Jan 5, 2023

View reviewed changes

prsunny merged commit 8a0bb36 into sonic-net:master Jan 5, 2023

prsunny deleted the neigh branch January 5, 2023 02:58

Junchao-Mellanox mentioned this pull request Jan 30, 2023

[submodule] Update sonic-swss pointer sonic-net/sonic-buildimage#13538

Closed

dprital mentioned this pull request Jan 30, 2023

[submodule] Advance sonic-swss pointer sonic-net/sonic-buildimage#13551

Merged

8 tasks

prsunny added Request for 202205 Branch Request for 202211 Branch Request for 202305 Branch labels Oct 28, 2023

yxieca pushed a commit that referenced this pull request Nov 6, 2023

Handle Mac address 'none' (#2593)

1e264e0

In some rare cases, kernel neighbor add notifications are received with Mac string as 'none'. Fixing this to prevent crash.

yxieca pushed a commit that referenced this pull request Nov 6, 2023

Handle Mac address 'none' (#2593)

178166e

In some rare cases, kernel neighbor add notifications are received with Mac string as 'none'. Fixing this to prevent crash.

yxieca added Included in 202205 Branch Included in 202211 Branch labels Nov 6, 2023

StormLiangMS added the Included in 202305 Branch label Nov 9, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Neighsync] Handle Mac address 'none' during neigh add #2593

[Neighsync] Handle Mac address 'none' during neigh add #2593

prsunny commented Dec 30, 2022

theasianpianist Dec 30, 2022

prsunny Dec 30, 2022

weiliu-ivy commented Jan 30, 2023

prsunny commented Jan 31, 2023

weiliu-ivy commented Jan 31, 2023

[Neighsync] Handle Mac address 'none' during neigh add #2593

[Neighsync] Handle Mac address 'none' during neigh add #2593

Conversation

prsunny commented Dec 30, 2022

theasianpianist Dec 30, 2022

Choose a reason for hiding this comment

prsunny Dec 30, 2022

Choose a reason for hiding this comment

weiliu-ivy commented Jan 30, 2023

prsunny commented Jan 31, 2023

weiliu-ivy commented Jan 31, 2023