Handle dual ToR neighbor miss scenario by theasianpianist · Pull Request #2137 · sonic-net/sonic-swss

theasianpianist · 2022-02-08T01:26:38Z

What I did

If unable to resolve a neighbor on a dual ToR system, neighsyncd will write a neighbor table entry for that neighbor with a zero MAC
When orchagent receives a neighbor update with a zero MAC:
- If the neighbor IP is configured for a specific mux cable port in the MUX_CABLE table in CONFIG_DB, handle the neighbor normally (if active for the port, no action is needed. if standby, a tunnel route is created for the neighbor IP)
- If the neighbor IP is not configured for a specific port, create a tunnel route for the IP to the peer switch.
  - When these neighbor IPs are eventually resolved, remove the tunnel route and handle the neighbor normally.
When creating/initializing a mux cable object, set the internal state to standby to match the constructor behavior.

Test changes

Various formatting fixes inside test_mux.py
Remove references to deprecated @pytest.yield_fixture
Add dual ToR neighbor miss test cases:
- Test cases and expected results are described in mux_neigh_miss_tests.py. These descriptions are used by the generic test runner test_neighbor_miss function to execute the test actions and verify expected results
- Various setup fixtures and test info fixtures were added
- Existing test cases were changed to use these setup fixtures for consistency

Test case descriptions:
Expectations:

On dual ToR devices, any unresolved IP in the kernel neighbor table should have a corresponding tunnel route created
If an IP is resolved, tunnel route creation depends on cable state (this is the existing/normal muxorch code flow)
If kernel entry deleted, always remove tunnel route

For the purposes of this test, there is a distinction made between 'server' IPs and 'neighbor' IPs. Server IPs are IP addresses explicitly configured on a specific mux cable interface in the MUX_CABLE table in config DB. Mux cable IPs (or neighbor IPs) are any other IPs within the VLAN subnet.

Standby Mux Cable IP Unresolved Test Cases:

	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. switch to active -> tunnel route remains
	3. IP resolved -> tunnel route teardown, neighbor added
	4. kernel neighbor deleted -> no tunnel route, no neighbor entry
	
	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. IP resolved -> tunnel route remains
	3. switch to active -> tunnel route teardown, neighbor added
	4. kernel neighbor deleted -> no tunnel route, no neighbor entry
	
	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. kernel neighbor deleted -> tunnel deleted

Active Mux Cable IP Unresolved Test Cases:

	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. switch to standby -> tunnel route remains
	3. IP resolved -> tunnel route remains
	4. kernel neighbor deleted -> no tunnel route, no neighbor entry

	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. IP resolved -> tunnel route teardown, neighbor added
	3. switch to standby -> tunnel route created, no neighbor
	4. kernel neighbor deleted -> no tunnel route, no neighbor entry
		
	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. kernel neighbor deleted -> no tunnel route

Neighbor IPs Unresolved Test Cases:

	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. resolved on standby -> tunnel route remains
	3. switch to active -> tunnel route teardown, neighbor added
	4. kernel neighbor deleted -> no tunnel route, no neighbor entry

	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. resolved on active -> tunnel route teardown, neighbor added
	3. switch to standby -> tunnel route created, no neighbor
	4. kernel neighbor deleted -> no tunnel route, no neighbor entry

	   [action] -> [expected result]
	1. netlink fail -> tunnel route created
	2. kernel neighbor deleted -> tunnel route teardown

Why I did it

How I verified it

❯ sudo py.test -v --imgname=docker-sonic-vs:latest "test_mux.py::TestMuxTunnel"
=================================================================================== test session starts ====================================================================================
platform linux -- Python 3.6.9, pytest-7.0.0, pluggy-1.0.0 -- /usr/bin/python3
cachedir: .pytest_cache
rootdir: /home/lawlee/sonic-buildimage/src/sonic-swss/tests
plugins: flaky-3.7.0
collected 26 items

test_mux.py::TestMuxTunnel::test_Tunnel PASSED                                                                                                                                       [  3%]
test_mux.py::TestMuxTunnel::test_Peer PASSED                                                                                                                                         [  7%]
test_mux.py::TestMuxTunnel::test_Neighbor PASSED                                                                                                                                     [ 11%]
test_mux.py::TestMuxTunnel::test_Fdb PASSED                                                                                                                                          [ 15%]
test_mux.py::TestMuxTunnel::test_Route PASSED                                                                                                                                        [ 19%]
test_mux.py::TestMuxTunnel::test_acl PASSED                                                                                                                                          [ 23%]
test_mux.py::TestMuxTunnel::test_mux_metrics PASSED                                                                                                                                  [ 26%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_serv->standby->resolve_entry->delete_entry-IPv4] PASSED                                                                  [ 30%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_serv->standby->resolve_entry->delete_entry-IPv6] PASSED                                                                  [ 34%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_serv->resolve_entry->standby->delete_entry-IPv4] PASSED                                                                  [ 38%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_serv->resolve_entry->standby->delete_entry-IPv6] PASSED                                                                  [ 42%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_serv->delete_entry-IPv4] PASSED                                                                                          [ 46%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_serv->delete_entry-IPv6] PASSED                                                                                          [ 50%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_serv->active->resolve_entry->delete_entry-IPv4] PASSED                                                                  [ 53%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_serv->active->resolve_entry->delete_entry-IPv6] PASSED                                                                  [ 57%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_serv->resolve_entry->active->delete_entry-IPv4] PASSED                                                                  [ 61%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_serv->resolve_entry->active->delete_entry-IPv6] PASSED                                                                  [ 65%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_serv->delete_entry-IPv4] PASSED                                                                                         [ 69%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_serv->delete_entry-IPv6] PASSED                                                                                         [ 73%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_neigh->resolve_entry->active->delete_entry-IPv4] PASSED                                                                 [ 76%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[standby->ping_neigh->resolve_entry->active->delete_entry-IPv6] PASSED                                                                 [ 80%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_neigh->resolve_entry->standby->delete_entry-IPv4] PASSED                                                                 [ 84%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[active->ping_neigh->resolve_entry->standby->delete_entry-IPv6] PASSED                                                                 [ 88%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[ping_neigh->delete_entry-IPv4] PASSED                                                                                                 [ 92%]
test_mux.py::TestMuxTunnel::test_neighbor_miss[ping_neigh->delete_entry-IPv6] PASSED                                                                                                 [ 96%]
test_mux.py::TestMuxTunnel::test_neighbor_miss_no_peer PASSED                                                                                                                        [100%]

============================================================================== 26 passed in 207.79s (0:03:27) ==============================================================================

Details if related

- If unable to resolve a neighbor on a dual ToR system, write a neighbor table entry for that neighbor with a zero MAC Signed-off-by: Lawrence Lee <[email protected]>

- When receiving a neighbor update with a zero MAC: - If the neighbor IP is configured for a specific mux cable port in the MUX_CABLE table in CONFIG_DB, handle the neighbor normally (if active for the port, no action is needed. if standby, a tunnel route is created for the neighbor IP) - If the neighbor IP is not configured for a specific port, create a tunnel route for the IP to the peer switch. - When these neighbor IPs are eventually resolved, remove the tunnel route and handle the neighbor normally. Signed-off-by: Lawrence Lee <[email protected]>

Signed-off-by: Lawrence Lee <[email protected]>

neighsyncd/neighsync.cpp

orchagent/muxorch.cpp

orchagent/neighorch.cpp

- Insert newline before open braces - Use MacAddress boolean conversion Signed-off-by: Lawrence Lee <[email protected]>

Signed-off-by: Lawrence Lee <[email protected]>

- Move standalone tunnel neighbor handling to completely separate code path Signed-off-by: Lawrence Lee <[email protected]>

Signed-off-by: Lawrence Lee <[email protected]>

- Add dual ToR neighbor miss test cases: - Create generic test runner function that executes test steps described in mux_neigh_miss_tests.py - Add neighbor miss test case with missing PEER_SWITCH config - Create setup fixtures for dual ToR DB entries - CONFIG_DB: VLAN, MUX_CABLE, PEER_SWITCH - APPL_DB: TUNNEL_DECAP_TABLE - Refactor existing tests to use setup fixtures for consistency - Create ASIC_DB and APPL_DB verification functions - Verify APPL_DB NEIGH_TABLE entry presence/absence - Verify ASIC_DB ROUTE_ENTRY and NEIGH_ENTRY entry presence/absence - Create various fixtures to generate test information Signed-off-by: Lawrence Lee <[email protected]>

Signed-off-by: Lawrence Lee <[email protected]>

lgtm-com · 2022-02-14T22:32:16Z

This pull request introduces 2 alerts and fixes 1 when merging 193a00d into 4bff5c6 - view on LGTM.com

new alerts:

1 for Unused local variable
1 for Unused import

fixed alerts:

1 for Unused import

Signed-off-by: Lawrence Lee <[email protected]>

lgtm-com · 2022-02-14T23:59:00Z

This pull request fixes 1 alert when merging dc04459 into 4bff5c6 - view on LGTM.com

fixed alerts:

1 for Unused import

prsunny

overall lgtm, lets chat on the test changes.

prsunny · 2022-02-15T01:02:32Z

orchagent/neighorch.cpp

 #include "routeorch.h"
 #include "directory.h"
 #include "muxorch.h"
+#include "observer.h"


Please remove this. Its already in .h file

prsunny · 2022-02-15T01:03:20Z

orchagent/neighorch.cpp

 extern FgNhgOrch *gFgNhgOrch;
 extern Directory<Orch*> gDirectory;

+#define MUX_TUNNEL "MuxTunnel0"


Please remove. Its unused and also not relevant to neighorch

Signed-off-by: Lawrence Lee <[email protected]>

lgtm-com · 2022-02-15T19:13:10Z

This pull request fixes 1 alert when merging 499475e into 4bff5c6 - view on LGTM.com

fixed alerts:

1 for Unused import

lgtm-com · 2022-02-15T23:24:00Z

This pull request fixes 1 alert when merging 499475e into 4bff5c6 - view on LGTM.com

fixed alerts:

1 for Unused import

- When orchagent receives a neighbor update with a zero MAC: - If the neighbor IP is configured for a specific mux cable port in the MUX_CABLE table in CONFIG_DB, handle the neighbor normally (if active for the port, no action is needed. if standby, a tunnel route is created for the neighbor IP) - If the neighbor IP is not configured for a specific port, create a tunnel route for the IP to the peer switch. - When these neighbor IPs are eventually resolved, remove the tunnel route and handle the neighbor normally. - When creating/initializing a mux cable object, set the internal state to standby to match the constructor behavior. - Various formatting fixes inside test_mux.py - Remove references to deprecated `@pytest.yield_fixture` - Add dual ToR neighbor miss test cases: - Test cases and expected results are described in `mux_neigh_miss_tests.py`. These descriptions are used by the generic test runner `test_neighbor_miss` function to execute the test actions and verify expected results - Various setup fixtures and test info fixtures were added - Existing test cases were changed to use these setup fixtures for consistency Signed-off-by: Lawrence Lee <[email protected]> Co-authored-by: Sumukha Tumkur Vani <[email protected]>

* Handle dual ToR neighbor miss scenario (#2137) - When orchagent receives a neighbor update with a zero MAC: - If the neighbor IP is configured for a specific mux cable port in the MUX_CABLE table in CONFIG_DB, handle the neighbor normally (if active for the port, no action is needed. if standby, a tunnel route is created for the neighbor IP) - If the neighbor IP is not configured for a specific port, create a tunnel route for the IP to the peer switch. - When these neighbor IPs are eventually resolved, remove the tunnel route and handle the neighbor normally. - When creating/initializing a mux cable object, set the internal state to standby to match the constructor behavior. - Various formatting fixes inside test_mux.py - Remove references to deprecated `@pytest.yield_fixture` - Add dual ToR neighbor miss test cases: - Test cases and expected results are described in `mux_neigh_miss_tests.py`. These descriptions are used by the generic test runner `test_neighbor_miss` function to execute the test actions and verify expected results - Various setup fixtures and test info fixtures were added - Existing test cases were changed to use these setup fixtures for consistency Signed-off-by: Lawrence Lee <[email protected]> Co-authored-by: Sumukha Tumkur Vani <[email protected]>

When checking for route entry mismatches, ignore mismatched routes where an APPL_DB neighbor entry with a zero MAC is present. These routes are introduced by this change in SWSS and are expected: sonic-net/sonic-swss#2137 Signed-off-by: Lawrence Lee <[email protected]>

When checking for route entry mismatches, ignore mismatched routes where an APPL_DB neighbor entry with a zero MAC is present. These routes are introduced by this change in SWSS and are expected: sonic-net/sonic-swss#2137 Signed-off-by: Lawrence Lee <[email protected]> Signed-off-by: Lawrence Lee <[email protected]>

When checking for route entry mismatches, ignore mismatched routes where an APPL_DB neighbor entry with a zero MAC is present. These routes are introduced by this change in SWSS and are expected: sonic-net/sonic-swss#2137 Signed-off-by: Lawrence Lee <[email protected]>

* Handle dual ToR neighbor miss scenario (sonic-net#2137) - When orchagent receives a neighbor update with a zero MAC: - If the neighbor IP is configured for a specific mux cable port in the MUX_CABLE table in CONFIG_DB, handle the neighbor normally (if active for the port, no action is needed. if standby, a tunnel route is created for the neighbor IP) - If the neighbor IP is not configured for a specific port, create a tunnel route for the IP to the peer switch. - When these neighbor IPs are eventually resolved, remove the tunnel route and handle the neighbor normally. - When creating/initializing a mux cable object, set the internal state to standby to match the constructor behavior. - Various formatting fixes inside test_mux.py - Remove references to deprecated `@pytest.yield_fixture` - Add dual ToR neighbor miss test cases: - Test cases and expected results are described in `mux_neigh_miss_tests.py`. These descriptions are used by the generic test runner `test_neighbor_miss` function to execute the test actions and verify expected results - Various setup fixtures and test info fixtures were added - Existing test cases were changed to use these setup fixtures for consistency Signed-off-by: Lawrence Lee <[email protected]> Co-authored-by: Sumukha Tumkur Vani <[email protected]>

theasianpianist added 2 commits February 8, 2022 01:16

[neighsyncd]: Support dualtor neighbor miss

3a1805d

- If unable to resolve a neighbor on a dual ToR system, write a neighbor table entry for that neighbor with a zero MAC Signed-off-by: Lawrence Lee <[email protected]>

theasianpianist requested a review from prsunny February 8, 2022 01:26

[neighorch]: Remove duplicate neighbor entry

e6869f4

Signed-off-by: Lawrence Lee <[email protected]>

prsunny reviewed Feb 8, 2022

View reviewed changes

theasianpianist and others added 8 commits February 9, 2022 21:52

[swss]: Address review comments

549e891

- Insert newline before open braces - Use MacAddress boolean conversion Signed-off-by: Lawrence Lee <[email protected]>

Add VS test

11f9589

[neighsyncd]: Handle dual ToR neighbor deletion

7f4b0e9

Signed-off-by: Lawrence Lee <[email protected]>

[neighorch]: Reduce zero mac tunnel log level

fb01e0c

Signed-off-by: Lawrence Lee <[email protected]>

[muxorch]: Handle standalone tunnel neighbors

5ff5dd7

- Move standalone tunnel neighbor handling to completely separate code path Signed-off-by: Lawrence Lee <[email protected]>

[tests]: Update fixture declarations

1feea32

Signed-off-by: Lawrence Lee <[email protected]>

[test_mux]: Formatting fixes

193a00d

Signed-off-by: Lawrence Lee <[email protected]>

theasianpianist force-pushed the dualtor-neigh-miss-202012 branch from 58859e8 to 193a00d Compare February 14, 2022 22:27

theasianpianist requested a review from prsunny February 14, 2022 22:31

theasianpianist marked this pull request as ready for review February 14, 2022 22:33

[tests]: Address lgtm

dc04459

Signed-off-by: Lawrence Lee <[email protected]>

prsunny reviewed Feb 15, 2022

View reviewed changes

[neighorch]: Remove unused include and define

499475e

Signed-off-by: Lawrence Lee <[email protected]>

prsunny approved these changes Feb 15, 2022

View reviewed changes

theasianpianist merged commit 081dd01 into sonic-net:202012 Feb 16, 2022

theasianpianist mentioned this pull request Feb 19, 2022

Handle dual ToR neighbor miss scenario #2151

Merged

theasianpianist mentioned this pull request Jul 29, 2022

[dualtor]: Resolve neighbor after neighbor removal sonic-net/sonic-mgmt#6071

Merged

5 tasks

theasianpianist mentioned this pull request Aug 22, 2022

[route_check]: Ignore standalone tunnel routes sonic-net/sonic-utilities#2325

Merged

theasianpianist mentioned this pull request Sep 1, 2022

[route_check]: Ignore standalone tunnel routes sonic-net/sonic-utilities#2346

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle dual ToR neighbor miss scenario#2137

Handle dual ToR neighbor miss scenario#2137
theasianpianist merged 13 commits intosonic-net:202012from
theasianpianist:dualtor-neigh-miss-202012

theasianpianist commented Feb 8, 2022 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Feb 14, 2022

Uh oh!

lgtm-com bot commented Feb 14, 2022

Uh oh!

prsunny left a comment

Uh oh!

prsunny Feb 15, 2022

Uh oh!

prsunny Feb 15, 2022

Uh oh!

lgtm-com bot commented Feb 15, 2022

Uh oh!

lgtm-com bot commented Feb 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

theasianpianist commented Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lgtm-com bot commented Feb 14, 2022

Uh oh!

lgtm-com bot commented Feb 14, 2022

Uh oh!

prsunny left a comment

Choose a reason for hiding this comment

Uh oh!

prsunny Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

prsunny Feb 15, 2022

Choose a reason for hiding this comment

Uh oh!

lgtm-com bot commented Feb 15, 2022

Uh oh!

lgtm-com bot commented Feb 15, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

theasianpianist commented Feb 8, 2022 •

edited

Loading