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

Fix bug with checking VRF's routes in route_check.py #2301

Merged
merged 2 commits into from
Aug 17, 2022

Conversation

bratashX
Copy link
Contributor

@bratashX bratashX commented Aug 8, 2022

Signed-off-by: Petro Bratash petrox.bratash@intel.com

What I did

Add capability to check routes with VRF

How I did it

All routes in VRF have a specific prefix in name.

Route table when we have VRF:

admin@igk-5-dut:~$ redis-cli -n 0 keys "ROUTE_TABLE*" 
    1) "ROUTE_TABLE:Vrf2:20c0:cb50:0:80::/64"
    2) "ROUTE_TABLE:Vrf2:192.211.48.0/25"
    3) "ROUTE_TABLE:Vrf1:192.246.8.128/25"
    4) "ROUTE_TABLE:Vrf1:192.222.8.0/25"
    5) "ROUTE_TABLE:Vrf2:192.246.64.0/25"
    .......

Route table without VRF:

admin@cab18-2-dut:~$ redis-cli -n 0 keys "ROUTE_TABLE*"
1) "ROUTE_TABLE:192.168.0.0/21"
2) "ROUTE_TABLE:10.1.0.32"
3) "ROUTE_TABLE:fc02:1000::/64"
4) "ROUTE_TABLE:fe80::/64"
5) "ROUTE_TABLE:fc00:1::32"
.......

Remove Vrf prefix from route. Without Vrf, we can compare routes from APP and ASIC DB.

How to verify it

Run next command on SONiC:

  1. sudo config vrf add Vrf1
  2. sudo config interface vrf bind PortChannel101 Vrf1
  3. sudo config interface ip add PortChannel101 10.0.0.56/31
  4. /usr/local/bin/route_check.py

Also can run SONiC TC vrf/test_vrf.py::TestVrfDeletion::test_vrf1_neigh_after_restore and check syslog

Previous command output (if the output of a command-line utility has changed)

$ /usr/local/bin/route_check.py
Failure results: {{
    "Unaccounted_ROUTE_ENTRY_TABLE_entries": [
        "10.0.0.56/31"
    ]
}}
Failed. Look at reported mismatches above
add: []
del: []

New command output (if the output of a command-line utility has changed)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Aug 8, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: bratashX / name: Petro Bratash (388b545)

@prsunny
Copy link
Contributor

prsunny commented Aug 8, 2022

What is the fix? We already have the check without VRF. Can you explain a bit more?

@bratashX bratashX changed the title Create route_check.py compatible with VRF Make route_check.py compatible with VRF Aug 9, 2022
@bratashX
Copy link
Contributor Author

bratashX commented Aug 9, 2022

What is the fix? We already have the check without VRF. Can you explain a bit more?

Previously, we skip routes with VRF prefix (ROUTE_TABLE from APP_DB):

if not is_vrf(k) and not is_local(k):

But from ASIC_DB we read all routes (including routes from VRF) and have a problem with comparing.

@prsunny
Copy link
Contributor

prsunny commented Aug 9, 2022

What is the fix? We already have the check without VRF. Can you explain a bit more?

Previously, we skip routes with VRF prefix (ROUTE_TABLE from APP_DB):

if not is_vrf(k) and not is_local(k):

But from ASIC_DB we read all routes (including routes from VRF) and have a problem with comparing.

ok, in that case i suggest lets fix the proper way. For VRF routes, lets get the VRF ID from ASIC_DB and compare only routes from that VRF.

@bratashX
Copy link
Contributor Author

What is the fix? We already have the check without VRF. Can you explain a bit more?

Previously, we skip routes with VRF prefix (ROUTE_TABLE from APP_DB):

if not is_vrf(k) and not is_local(k):

But from ASIC_DB we read all routes (including routes from VRF) and have a problem with comparing.

ok, in that case i suggest lets fix the proper way. For VRF routes, lets get the VRF ID from ASIC_DB and compare only routes from that VRF.

We can't compare only routes from specific VRF separatelly because:

  • on APP_DB used name of VRF (VRF1, VRF2..etc.)
    Example route entry from APP_DB:
    "ROUTE_TABLE:Vrf1:10.0.0.56/31"

  • on ASIC_DB used virtual router oid
    Example route entry from ASIC_DB:
    ASIC_STATE:SAI_OBJECT_TYPE_ROUTE_ENTRY:{"dest":"10.0.0.56/31","switch_id":"oid:0x21000000000000","vr":"oid:0x30000000003e3"}

Mapping VRF names with OIDs is absent in REDIS (it's saved into swss code).

In my implementation, I compare all routes from APP_DB (without VRF + VRF routes) with all routes from ASIC_DB.

@bratashX bratashX changed the title Make route_check.py compatible with VRF Fix bug with checking VRF's routes in route_check.py Aug 10, 2022
@bratashX bratashX force-pushed the fix_route_check branch 2 times, most recently from 0a8149a to 7f326d0 Compare August 11, 2022 21:44
Signed-off-by: Petro Bratash <petrox.bratash@intel.com>
@bratashX
Copy link
Contributor Author

@prsunny Could you please take a look?

Signed-off-by: Petro Bratash <petrox.bratash@intel.com>
@prsunny prsunny merged commit ece4049 into sonic-net:master Aug 17, 2022
preetham-singh pushed a commit to preetham-singh/sonic-utilities that referenced this pull request Nov 21, 2022
* Create route_check.py compatible with VRF

Signed-off-by: Petro Bratash <petrox.bratash@intel.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants