Skip to content

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

Merged
prsunny merged 2 commits intosonic-net:masterfrom
bratashX:fix_route_check
Aug 17, 2022
Merged

Fix bug with checking VRF's routes in route_check.py #2301
prsunny merged 2 commits intosonic-net:masterfrom
bratashX:fix_route_check

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 prsunny requested a review from renukamanavalan August 8, 2022 22:43
@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