Hi all,

Please find answers inline

On 03/06/2025 06:54, Nitin Saxena wrote:
Hi Morten,

Let me take a stab regarding the VRF question.
Please find answers inline

Thanks,
Nitin

On Mon, Jun 2, 2025 at 2:12 PM Morten Brørup <mb@smartsharesystems.com> wrote:
+TO: Robin Jarry, might have relevant feedback for such a node

From: Ankur Dwivedi [mailto:adwivedi@marvell.com]
Sent: Monday, 2 June 2025 08.36

This patch series adds two inbuilt nodes ip4_lookup_fib and
ip6_lookup_fib in graph library. These nodes uses the existing
Forwarding Information Base (FIB) library to create FIB, to do
route addition and route lookup.

Two new commands (for ipv4 and ipv6) for providing the lookup mode
is added in the dpdk-graph application. fib or lpm can be given as
lookup mode. If these new lookup mode commands are not given, the
dpdk-graph uses lpm (Longest Prefix Match) or lpm6 by default.
If fib is given as lookup mode then the ip4_lookup_fib or
ip6_lookup_fib
nodes are used by the application.
@Ankur, @Vladimir, @Jerin,

A couple of high level questions...

1. Thread safety:
I'm not familiar with the thread safety models of the FIB and Graph libraries.
Is the FIB library thread safe, so one thread can invoke the FIB modify operation while other threads invoke the FIB lookup operation?
If it isn't, how does the Graph library make its use of the FIB library thread safe?

Yes, as @Ankur previously mentioned, FIB supports RCU to guarantee that there is no race condition between readers and the writer.

Just a nit-pick here, there are two libraries used by rte_graph applications
- Graph library (lib/graph/*)
- Node library (lib/node/*)

Questions here are pertaining to "Node library" and not to "Graph library"

2. VRF support:
This looks like a global route table for the entire graph.
Correct.

Shouldn't there be a route table per FIB node?
Yes when VRF support is added in library, one FIB would support on route table

Or how are VRFs supported by the Graph library?
Or is there no requirement to support VRFs in the Graph library?

Currently the node library(instead of Graph library) does not support
VRF. There is an intention to add it in lib/node/ip[4|6]-lookup nodes
IMO, @Vladimir can correct me, even FIB support for multiple VRF is
unoptimized because rte_fib_lookup_bulk() function takes single fib
pointer instead of multiple fib pointers
For adding VRF support in ip[4|6]-lookup nodes ideally we would like to have

rte_fib_lookup_bulk(struct rte_fib *fib, uint32_t *ips, uint64_t
next_hops, int n); (OR different new API)

taking an array of fib pointers (or fib index) corresponding to each
nth packet. It should also avoid fib->lookup() callback for every
packet.
We can also add this API as part of "Node library" but we need "static
inline" version of fib->lookup() in rte_fib header file

Yes, the current FIB implementation implies a single routing table, and for VRF support user have to use different FIB, which is not optimal in terms of performance.

Some time ago, I was thinking about VRF support in FIB, and this could be implemented with almost no performance drop compared to the current version. API will look as follows:

rte_fib_lookup_vrf_bulk(struct rte_fib *fib, uint32_t *ips, uint64_t *next_hops, int *vrf_idxes, int n); Thus keeping multiple VRFs inside a single struct rte_fib, and adding new VRF-aware API to the FIB library.

I'll try to find some time to implement this idea.


      
-Morten

-- 
Regards,
Vladimir