Add Mapping From Table ID to VRF

[sworleys]

In zebra/rt_netlink.c, we call vrf_lookup_by_id() when we get a route to find the associated vrf object for that table.

This should be improved to be a hash table mapping of Table ID to zebra_vrf object.

You would need to create a hash table probably in zebra/zebra_vrf.h that uses the table_id hash as a key and then update the code in vrf_lookup_by_id() to use this.

This should be a fun one! Good luck!

To Reproduce

1. Create a vrf in the kernel with iproute2
2. ip link add vrf-red type vrf table 1111
3. Put an interface in the vrf
4. ip link add test type dummy
5. ip link set dev test master vrf-red
6. ip link set test up
7. Create a route in the kernel with iproute2 in the vrf
8. ip route add 3.3.3.3/32 dev test vrf vrf-red
9. Zebra reads in the route in the vrf and calls the function

With the new code you should be able to use that table ID 1111 to lookup the vrf.

Versions

• OS Kernel: [linux] [5.3]
• FRR Version [master]

[proelbtn]

I'll try it.

[pguibert6WIND]

can the hash_lookup() function be avoided when vrf backend is not netns ?

[sworleys]

can the hash_lookup() function be avoided when vrf backend is not netns ?

sure that makes sense to me.

[sworleys]

@proelbtn you will need to a vrf_is_backend_netns() check or something similar in the codepath before to doing the lookup. If it is netns, you will need to continue the old way probably.

[85ayush]

I would love to work on this issue, I am new to open source contribution. Can you please guide me?

[sworleys]

I would love to work on this issue, I am new to open source contribution. Can you please guide me?

Definitely!

[85ayush]

Thank you, so I am starting working on it, where should I start from? And if I get stuck somewhere how can I contact you for help? Please reply.

…

On Tue, 23 Feb 2021, 10:30 pm Stephen Worley, ***@***.***> wrote: I would love to work on this issue, I am new to open source contribution. Can you please guide me? Definitely! — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#5982 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AR2RQWCG66TOS2LHUYNB7O3TAPNMTANCNFSM4LF2PXLA> .

[nathan-duddles]

I see in zebra/rt_netlink.c that vrf_lookup_by_id() is used at lines 1405 and 1650. This function is defined in lib/vrf.c starting at line 275 and uses a find function on a red-black tree (lib/vrf.h includes openbsd-tree.h).

Is this issue for updating the implementation of vrf_lookup_by_table() which starts at line 367 in zebra/rt_netlink.c?

It seems the implementation of vrf_lookup_by_table() is iterating through a red-black tree. It looks like this function is only used at line 768 in zebra/rt_netlink.c and line 343 in if_netlink.c .

Edit: grammar

[sworleys]

I see in zebra/rt_netlink.c that vrf_lookup_by_id() is used at lines 1405 and 1650. This function is defined in lib/vrf.c starting at line 275 and uses a find function on a red-black tree (lib/vrf.h includes openbsd-tree.h).

Is this issue for updating the implementation of vrf_lookup_by_table() which starts at line 367 in zebra/rt_netlink.c?

It seems the implementation of vrf_lookup_by_table() is iterating through a red-black tree. It looks like this function is only used at line 768 in zebra/rt_netlink.c and line 343 in if_netlink.c .

Edit: grammar

Yea the idea is to create a table or tree to maintain a lookup from ID -> vrf

so that we don't have to iterate over all the VRFs and match them.

Yes you update the implementation here and move the function to reside in lib/vrf.c[h] as a more general API.

[nathan-duddles]

Thanks for your reply!

I have another basic question:
Do you recommend using lib/hash.c for this?

I see there is some documentation on this in frr/doc/developer/lists.rst. Besides this and the source code, is there anything else I should be looking at?

[sworleys]

You won't use lib/hash directly. That is the old style. We have a newer API provided by lib/typesafe.h that acts as a wrapper for all our list/table/tree implementations to provide common macros for lookup/iteration.

if you look here you can see the macros available to use and how to use them: http://docs.frrouting.org/projects/dev-guide/en/latest/lists.html#cheat-sheet

there are also examples scattered about the code base.

[nathan-duddles]

Thank you, @sworleys! This is super helpful!

[nathan-duddles]

I think I may be ready to submit a pull request for this. I have my patch on this branch.

I have gone through the steps in the Pre-submission Checklist, except when I run make test in the frr directory, I get the message: make: *** No rule to make target 'test'. Stop. I'm running all topotests on my branch now.

I have also done my manual testing by running ospf with vrf in GNS3 between alpine docker images built from my branch, and everything worked as it did when built from the master branch.

Are there any other ways you would suggest testing for this?

Before submitting a pull request, should I rebase my branch?