Re: [dpdk-dev] [PATCH v3 3/5] examples/l3fwd: add FIB infrastructure

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 19-Feb-21 3:09 PM, Conor Walsh wrote:
> The purpose of this commit is to add the necessary function calls
> and supporting infrastructure to allow the Forwarding Information Base
> (FIB) library to be integrated into the l3fwd sample app.
> The flag '-F' has been added to the applications options to allow
> the user to specify that l3fwd uses FIB as its lookup method.
> 
> Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> ---
>   examples/l3fwd/Makefile      |  2 +-
>   examples/l3fwd/l3fwd.h       | 27 ++++++++++++++--
>   examples/l3fwd/l3fwd_event.c |  9 ++++++
>   examples/l3fwd/l3fwd_event.h |  1 +
>   examples/l3fwd/l3fwd_fib.c   | 60 ++++++++++++++++++++++++++++++++++++
>   examples/l3fwd/main.c        | 43 +++++++++++++++++++-------
>   examples/l3fwd/meson.build   |  4 +--
>   7 files changed, 129 insertions(+), 17 deletions(-)
>   create mode 100644 examples/l3fwd/l3fwd_fib.c
> 
> diff --git a/examples/l3fwd/Makefile b/examples/l3fwd/Makefile
> index 7e70bbd826..5f7baffbf7 100644
> --- a/examples/l3fwd/Makefile
> +++ b/examples/l3fwd/Makefile
> @@ -5,7 +5,7 @@
>   APP = l3fwd
>   
>   # all source are stored in SRCS-y
> -SRCS-y := main.c l3fwd_lpm.c l3fwd_em.c l3fwd_event.c
> +SRCS-y := main.c l3fwd_lpm.c l3fwd_fib.c l3fwd_em.c l3fwd_event.c
>   SRCS-y += l3fwd_event_generic.c l3fwd_event_internal_port.c
>   
>   # Build using pkg-config variables if possible
> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> index 2cf06099e0..a808d60247 100644
> --- a/examples/l3fwd/l3fwd.h
> +++ b/examples/l3fwd/l3fwd.h
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright(c) 2010-2016 Intel Corporation
> + * Copyright(c) 2010-2021 Intel Corporation
>    */
>   
>   #ifndef __L3_FWD_H__
> @@ -180,13 +180,16 @@ is_valid_ipv4_pkt(struct rte_ipv4_hdr *pkt, uint32_t link_len)
>   int
>   init_mem(uint16_t portid, unsigned int nb_mbuf);
>   
> -/* Function pointers for LPM or EM functionality. */
> +/* Function pointers for LPM, EM or FIB functionality. */
>   void
>   setup_lpm(const int socketid);
>   
>   void
>   setup_hash(const int socketid);
>   
> +void
> +setup_fib(const int socketid);
> +
>   int
>   em_check_ptype(int portid);
>   
> @@ -207,6 +210,9 @@ em_main_loop(__rte_unused void *dummy);
>   int
>   lpm_main_loop(__rte_unused void *dummy);
>   
> +int
> +fib_main_loop(__rte_unused void *dummy);
> +
>   int
>   lpm_event_main_loop_tx_d(__rte_unused void *dummy);
>   int
> @@ -225,8 +231,17 @@ em_event_main_loop_tx_q(__rte_unused void *dummy);
>   int
>   em_event_main_loop_tx_q_burst(__rte_unused void *dummy);
>   
> +int
> +fib_event_main_loop_tx_d(__rte_unused void *dummy);
> +int
> +fib_event_main_loop_tx_d_burst(__rte_unused void *dummy);
> +int
> +fib_event_main_loop_tx_q(__rte_unused void *dummy);
> +int
> +fib_event_main_loop_tx_q_burst(__rte_unused void *dummy);
> +
>   
> -/* Return ipv4/ipv6 fwd lookup struct for LPM or EM. */
> +/* Return ipv4/ipv6 fwd lookup struct for LPM, EM or FIB. */
>   void *
>   em_get_ipv4_l3fwd_lookup_struct(const int socketid);
>   
> @@ -239,4 +254,10 @@ lpm_get_ipv4_l3fwd_lookup_struct(const int socketid);
>   void *
>   lpm_get_ipv6_l3fwd_lookup_struct(const int socketid);
>   
> +void *
> +fib_get_ipv4_l3fwd_lookup_struct(const int socketid);
> +
> +void *
> +fib_get_ipv6_l3fwd_lookup_struct(const int socketid);
> +
>   #endif  /* __L3_FWD_H__ */
> diff --git a/examples/l3fwd/l3fwd_event.c b/examples/l3fwd/l3fwd_event.c
> index 4d31593a0a..961860ea18 100644
> --- a/examples/l3fwd/l3fwd_event.c
> +++ b/examples/l3fwd/l3fwd_event.c
> @@ -227,6 +227,12 @@ l3fwd_event_resource_setup(struct rte_eth_conf *port_conf)
>   		[1][0] = em_event_main_loop_tx_q,
>   		[1][1] = em_event_main_loop_tx_q_burst,
>   	};
> +	const event_loop_cb fib_event_loop[2][2] = {
> +		[0][0] = fib_event_main_loop_tx_d,
> +		[0][1] = fib_event_main_loop_tx_d_burst,
> +		[1][0] = fib_event_main_loop_tx_q,
> +		[1][1] = fib_event_main_loop_tx_q_burst,
> +	};
>   	uint32_t event_queue_cfg;
>   	int ret;
>   
> @@ -264,4 +270,7 @@ l3fwd_event_resource_setup(struct rte_eth_conf *port_conf)
>   
>   	evt_rsrc->ops.em_event_loop = em_event_loop[evt_rsrc->tx_mode_q]
>   						       [evt_rsrc->has_burst];
> +
> +	evt_rsrc->ops.fib_event_loop = fib_event_loop[evt_rsrc->tx_mode_q]
> +						       [evt_rsrc->has_burst];
>   }
> diff --git a/examples/l3fwd/l3fwd_event.h b/examples/l3fwd/l3fwd_event.h
> index 0e46164170..3ad1902ab5 100644
> --- a/examples/l3fwd/l3fwd_event.h
> +++ b/examples/l3fwd/l3fwd_event.h
> @@ -55,6 +55,7 @@ struct l3fwd_event_setup_ops {
>   	adapter_setup_cb adapter_setup;
>   	event_loop_cb lpm_event_loop;
>   	event_loop_cb em_event_loop;
> +	event_loop_cb fib_event_loop;
>   };
>   
>   struct l3fwd_event_resources {
> diff --git a/examples/l3fwd/l3fwd_fib.c b/examples/l3fwd/l3fwd_fib.c
> new file mode 100644
> index 0000000000..0a2d02db2f
> --- /dev/null
> +++ b/examples/l3fwd/l3fwd_fib.c
> @@ -0,0 +1,60 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2021 Intel Corporation
> + */
> +
> +#include <rte_fib.h>
> +#include <rte_fib6.h>
> +
> +#include "l3fwd.h"
> +#include "l3fwd_event.h"
> +#include "l3fwd_common_route.h"
> +
> +/* Main fib processing loop. */
> +int
> +fib_main_loop(__rte_unused void *dummy)
> +{
> +	return 0;
> +}
> +
> +int __rte_noinline
> +fib_event_main_loop_tx_d(__rte_unused void *dummy)
> +{
> +	return 0;
> +}
> +
> +int __rte_noinline
> +fib_event_main_loop_tx_d_burst(__rte_unused void *dummy)
> +{
> +	return 0;
> +}
> +
> +int __rte_noinline
> +fib_event_main_loop_tx_q(__rte_unused void *dummy)
> +{
> +	return 0;
> +}
> +
> +int __rte_noinline
> +fib_event_main_loop_tx_q_burst(__rte_unused void *dummy)
> +{
> +	return 0;
> +}
> +
> +/* Function to setup fib. */
> +void
> +setup_fib(__rte_unused const int socketid)
> +{}
> +
> +/* Return ipv4 fib lookup struct. */
> +void *
> +fib_get_ipv4_l3fwd_lookup_struct(__rte_unused const int socketid)
> +{
> +	return 0;
> +}
> +
> +/* Return ipv6 fib lookup struct. */
> +void *
> +fib_get_ipv6_l3fwd_lookup_struct(__rte_unused const int socketid)
> +{
> +	return 0;
> +}
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index bb49e5faff..6881b49478 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -1,5 +1,5 @@
>   /* SPDX-License-Identifier: BSD-3-Clause
> - * Copyright(c) 2010-2016 Intel Corporation
> + * Copyright(c) 2010-2021 Intel Corporation
>    */
>   
>   #include <stdio.h>
> @@ -60,9 +60,10 @@ static uint16_t nb_txd = RTE_TEST_TX_DESC_DEFAULT;
>   /**< Ports set in promiscuous mode off by default. */
>   static int promiscuous_on;
>   
> -/* Select Longest-Prefix or Exact match. */
> +/* Select Longest-Prefix, Exact match or Forwarding Information Base. */
>   static int l3fwd_lpm_on;
>   static int l3fwd_em_on;
> +static int l3fwd_fib_on;
>   
>   /* Global variables. */
>   
> @@ -162,10 +163,19 @@ static struct l3fwd_lkp_mode l3fwd_lpm_lkp = {
>   	.get_ipv6_lookup_struct = lpm_get_ipv6_l3fwd_lookup_struct,
>   };
>   
> +static struct l3fwd_lkp_mode l3fwd_fib_lkp = {
> +	.setup                  = setup_fib,
> +	.check_ptype            = lpm_check_ptype,
> +	.cb_parse_ptype         = lpm_cb_parse_ptype,
> +	.main_loop              = fib_main_loop,
> +	.get_ipv4_lookup_struct = fib_get_ipv4_l3fwd_lookup_struct,
> +	.get_ipv6_lookup_struct = fib_get_ipv6_l3fwd_lookup_struct,
> +};
> +
>   /*
>    * Setup lookup methods for forwarding.
> - * Currently exact-match and longest-prefix-match
> - * are supported ones.
> + * Currently exact-match, longest-prefix-match and forwarding information
> + * base are the supported ones.
>    */
>   static void
>   setup_l3fwd_lookup_tables(void)
> @@ -173,6 +183,9 @@ setup_l3fwd_lookup_tables(void)
>   	/* Setup HASH lookup functions. */
>   	if (l3fwd_em_on)
>   		l3fwd_lkp = l3fwd_em_lkp;
> +	/* Setup FIB lookup functions. */
> +	else if (l3fwd_fib_on)
> +		l3fwd_lkp = l3fwd_fib_lkp;
>   	/* Setup LPM lookup functions. */
>   	else
>   		l3fwd_lkp = l3fwd_lpm_lkp;
> @@ -292,6 +305,7 @@ print_usage(const char *prgname)
>   		"  -P : Enable promiscuous mode\n"
>   		"  -E : Enable exact match\n"
>   		"  -L : Enable longest prefix match (default)\n"
> +		"  -F : Enable forwarding information base\n"
>   		"  --config (port,queue,lcore): Rx queue configuration\n"
>   		"  --eth-dest=X,MM:MM:MM:MM:MM:MM: Ethernet destination for port X\n"
>   		"  --enable-jumbo: Enable jumbo frames\n"
> @@ -492,6 +506,7 @@ static const char short_options[] =
>   	"P"   /* promiscuous */
>   	"L"   /* enable long prefix match */
>   	"E"   /* enable exact match */
> +	"F"   /* forwarding information base */
>   	;
>   
>   #define CMD_LINE_OPT_CONFIG "config"
> @@ -596,6 +611,10 @@ parse_args(int argc, char **argv)
>   			l3fwd_lpm_on = 1;
>   			break;
>   
> +		case 'F':
> +			l3fwd_fib_on = 1;
> +			break;
> +
>   		/* long options */

IMO this necessitates creating an enum instead of a set of mutually 
exclusive binary switches. For example:

enum L3FWD_LOOKUP_MODE {
     L3FWD_LOOKUP_DEFAULT,
     L3FWD_LOOKUP_LPM,
     L3FWD_LOOKUP_EM,
     L3FWD_LOOKUP_FIB
};
static enum L3FWD_LOOKUP_MODE lookup_mode;

...

case 'F':
     if (lookup_mode != L3FWD_LOOKUP_DEFAULT) {
         RTE_LOG(ERR, L3FWD, "Only one lookup mode is allowed at a time!");
         return -1;
     }
     lookup_mode = L3FWD_LOOKUP_FIB;
     break;
...

Additionally, i really think this '-E'/'-L'/'-F' business is silly and 
it would be better to have something like "--lookup=exact|lpm|fib", and 
leave the old -E/-L options for backwards compatibility.

-- 
Thanks,
Anatoly