> The current port ID validation logic in the routes add code has
> two issues:
>
> - It can pass if port ID in route is 31+.
> - It silently skips rules with disabled or invalid
> port IDs
>
> This patch is:
> - Improving the enabled port IDs check logic.
> - Introducing a user option, "exit_on_failure", to control
> the behavior when attempting to add rules for disabled or
> invalid port IDs (either exit or skip)
> - Creating a port ID validation function for use across
> various setup functions
>
> Signed-off-by: Gagandeep Singh <g.si...@nxp.com>
> ---
> examples/l3fwd/em_route_parse.c | 4 +--
> examples/l3fwd/l3fwd.h | 16 +++++++++
> examples/l3fwd/l3fwd_em.c | 22 +++++++++----
> examples/l3fwd/l3fwd_fib.c | 26 ++++++++++-----
> examples/l3fwd/l3fwd_lpm.c | 26 ++++++++++-----
> examples/l3fwd/l3fwd_route.h | 2 ++
> examples/l3fwd/main.c | 58 ++++++++++++++++++++++++++++-----
> 7 files changed, 121 insertions(+), 33 deletions(-)
>
> diff --git a/examples/l3fwd/em_route_parse.c b/examples/l3fwd/em_route_parse.c
> index 8b534de5f1..b80442d7b8 100644
> --- a/examples/l3fwd/em_route_parse.c
> +++ b/examples/l3fwd/em_route_parse.c
> @@ -10,8 +10,8 @@
> #include "l3fwd.h"
> #include "l3fwd_route.h"
>
> -static struct em_rule *em_route_base_v4;
> -static struct em_rule *em_route_base_v6;
> +struct em_rule *em_route_base_v4;
> +struct em_rule *em_route_base_v6;
>
> enum {
> CB_FLD_DST_ADDR,
> diff --git a/examples/l3fwd/l3fwd.h b/examples/l3fwd/l3fwd.h
> index 0cce3406ee..bcbaf4d143 100644
> --- a/examples/l3fwd/l3fwd.h
> +++ b/examples/l3fwd/l3fwd.h
> @@ -57,6 +57,15 @@
> #define L3FWD_HASH_ENTRIES (1024*1024*1)
> #endif
>
> +/* Select Longest-Prefix, Exact match, Forwarding Information Base or Access
> Control. */
> +enum L3FWD_LOOKUP_MODE {
> + L3FWD_LOOKUP_DEFAULT,
> + L3FWD_LOOKUP_LPM,
> + L3FWD_LOOKUP_EM,
> + L3FWD_LOOKUP_FIB,
> + L3FWD_LOOKUP_ACL
> +};
> +
> struct parm_cfg {
> const char *rule_ipv4_name;
> const char *rule_ipv6_name;
> @@ -102,6 +111,9 @@ extern struct rte_ether_addr
> ports_eth_addr[RTE_MAX_ETHPORTS];
> /* mask of enabled ports */
> extern uint32_t enabled_port_mask;
>
> +/* Skip or exit on invalid route */
> +extern bool exit_on_failure;
> +
> /* Used only in exact match mode. */
> extern int ipv6; /**< ipv6 is false by default. */
> extern uint32_t hash_entry_number;
> @@ -222,6 +234,10 @@ init_mem(uint16_t portid, unsigned int nb_mbuf);
> int config_port_max_pkt_len(struct rte_eth_conf *conf,
> struct rte_eth_dev_info *dev_info);
>
> +int
> +l3fwd_validate_routes_port(enum L3FWD_LOOKUP_MODE mode, uint32_t i,
> + bool is_ipv4);
> +
> /* Function pointers for ACL, LPM, EM or FIB functionality. */
> void
> setup_acl(const int socketid);
> diff --git a/examples/l3fwd/l3fwd_em.c b/examples/l3fwd/l3fwd_em.c
> index da9c45e3a4..9b482c3c4e 100644
> --- a/examples/l3fwd/l3fwd_em.c
> +++ b/examples/l3fwd/l3fwd_em.c
> @@ -384,9 +384,14 @@ populate_ipv4_flow_into_table(const struct rte_hash *h)
> struct in_addr src;
> struct in_addr dst;
>
> - if ((1 << em_route_base_v4[i].if_out &
> - enabled_port_mask) == 0)
> - continue;
> + ret = l3fwd_validate_routes_port(L3FWD_LOOKUP_EM, i, true);
> + if (ret) {
> + if (exit_on_failure)
> + rte_exit(EXIT_FAILURE, "IDX: %d: Port ID %d in
> IPv4 rule is not"
> + " enabled\n", i,
> em_route_base_v4[i].if_out);
> + else
> + continue;
> + }
>
> entry = &em_route_base_v4[i];
> convert_ipv4_5tuple(&(entry->v4_key), &newkey);
> @@ -436,9 +441,14 @@ populate_ipv6_flow_into_table(const struct rte_hash *h)
> struct em_rule *entry;
> union ipv6_5tuple_host newkey;
>
> - if ((1 << em_route_base_v6[i].if_out &
> - enabled_port_mask) == 0)
> - continue;
> + ret = l3fwd_validate_routes_port(L3FWD_LOOKUP_EM, i, false);
> + if (ret) {
> + if (exit_on_failure)
> + rte_exit(EXIT_FAILURE, "IDX %d: Port ID %d
> given in IPv6 rule is not"
> + " enabled\n", i,
> em_route_base_v6[i].if_out);
> + else
> + continue;
> + }
>
> entry = &em_route_base_v6[i];
> convert_ipv6_5tuple(&(entry->v6_key), &newkey);
> diff --git a/examples/l3fwd/l3fwd_fib.c b/examples/l3fwd/l3fwd_fib.c
> index 82f1739df7..fa399d8c25 100644
> --- a/examples/l3fwd/l3fwd_fib.c
> +++ b/examples/l3fwd/l3fwd_fib.c
> @@ -667,10 +667,15 @@ setup_fib(const int socketid)
> for (i = 0; i < route_num_v4; i++) {
> struct in_addr in;
>
> - /* Skip unused ports. */
> - if ((1 << route_base_v4[i].if_out &
> - enabled_port_mask) == 0)
> - continue;
> + /* Check for valid ports */
> + ret = l3fwd_validate_routes_port(L3FWD_LOOKUP_FIB, i, true);
> + if (ret) {
> + if (exit_on_failure)
> + rte_exit(EXIT_FAILURE, "IDX %d: Port ID %d in
> IPv4 rule is not"
> + " enabled\n", i,
> route_base_v4[i].if_out);
> + else
> + continue;
> + }
>
> ret = rte_eth_dev_info_get(route_base_v4[i].if_out, &dev_info);
> if (ret < 0)
> @@ -725,10 +730,15 @@ setup_fib(const int socketid)
> /* Populate the fib IPv6 table. */
> for (i = 0; i < route_num_v6; i++) {
>
> - /* Skip unused ports. */
> - if ((1 << route_base_v6[i].if_out &
> - enabled_port_mask) == 0)
> - continue;
> + /* Check for valid ports. */
> + ret = l3fwd_validate_routes_port(L3FWD_LOOKUP_FIB, i, false);
> + if (ret) {
> + if (exit_on_failure)
> + rte_exit(EXIT_FAILURE, "IDX %d: Port ID %d
> given in IPv6 rule is not"
> + " enabled\n", i,
> route_base_v6[i].if_out);
> + else
> + continue;
> + }
>
> ret = rte_eth_dev_info_get(route_base_v6[i].if_out, &dev_info);
> if (ret < 0)
> diff --git a/examples/l3fwd/l3fwd_lpm.c b/examples/l3fwd/l3fwd_lpm.c
> index fec0aeb79c..e3bf2007db 100644
> --- a/examples/l3fwd/l3fwd_lpm.c
> +++ b/examples/l3fwd/l3fwd_lpm.c
> @@ -583,10 +583,15 @@ setup_lpm(const int socketid)
> for (i = 0; i < route_num_v4; i++) {
> struct in_addr in;
>
> - /* skip unused ports */
> - if ((1 << route_base_v4[i].if_out &
> - enabled_port_mask) == 0)
> - continue;
> + /* Check for valid ports */
> + ret = l3fwd_validate_routes_port(L3FWD_LOOKUP_LPM, i, true);
> + if (ret) {
> + if (exit_on_failure)
> + rte_exit(EXIT_FAILURE, "IDX: %d: Port ID %d in
> IPv4 rule is not"
> + " enabled\n", i, route_base_v4[i].if_out);
> + else
> + continue;
> + }
>
> ret = rte_eth_dev_info_get(route_base_v4[i].if_out, &dev_info);
> if (ret < 0)
> @@ -630,10 +635,15 @@ setup_lpm(const int socketid)
> /* populate the LPM table */
> for (i = 0; i < route_num_v6; i++) {
>
> - /* skip unused ports */
> - if ((1 << route_base_v6[i].if_out &
> - enabled_port_mask) == 0)
> - continue;
> + /* Check for valid ports */
> + ret = l3fwd_validate_routes_port(L3FWD_LOOKUP_LPM, i, false);
> + if (ret) {
> + if (exit_on_failure)
> + rte_exit(EXIT_FAILURE, "IDX %d Port ID %d given
> in IPv6 rule is not"
> + " enabled\n", i,
> route_base_v6[i].if_out);
> + else
> + continue;
> + }
>
> ret = rte_eth_dev_info_get(route_base_v6[i].if_out, &dev_info);
> if (ret < 0)
> diff --git a/examples/l3fwd/l3fwd_route.h b/examples/l3fwd/l3fwd_route.h
> index 62263c3540..98254deeeb 100644
> --- a/examples/l3fwd/l3fwd_route.h
> +++ b/examples/l3fwd/l3fwd_route.h
> @@ -81,6 +81,8 @@ struct em_rule {
>
> extern struct lpm_route_rule *route_base_v4;
> extern struct lpm_route_rule *route_base_v6;
> +extern struct em_rule *em_route_base_v4;
> +extern struct em_rule *em_route_base_v6;
> extern int route_num_v4;
> extern int route_num_v6;
>
> diff --git a/examples/l3fwd/main.c b/examples/l3fwd/main.c
> index 994b7dd8e5..b3b5be4076 100644
> --- a/examples/l3fwd/main.c
> +++ b/examples/l3fwd/main.c
> @@ -63,14 +63,6 @@ uint32_t mb_mempool_cache_size = MEMPOOL_CACHE_SIZE;
> /**< Ports set in promiscuous mode off by default. */
> static int promiscuous_on;
>
> -/* Select Longest-Prefix, Exact match, Forwarding Information Base or Access
> Control. */
> -enum L3FWD_LOOKUP_MODE {
> - L3FWD_LOOKUP_DEFAULT,
> - L3FWD_LOOKUP_LPM,
> - L3FWD_LOOKUP_EM,
> - L3FWD_LOOKUP_FIB,
> - L3FWD_LOOKUP_ACL
> -};
> static enum L3FWD_LOOKUP_MODE lookup_mode;
>
> /* Global variables. */
> @@ -92,7 +84,8 @@ xmm_t val_eth[RTE_MAX_ETHPORTS];
>
> /* mask of enabled ports */
> uint32_t enabled_port_mask;
> -
> +bool exit_on_failure; /**< Skip the route rule with invalid or */
> + /**< disabled port ID */
> /* Used only in exact match mode. */
> int ipv6; /**< ipv6 is false by default. */
>
> @@ -271,6 +264,43 @@ l3fwd_set_alg(const char *optarg)
> parm_config.alg = parse_acl_alg(optarg);
> }
>
> +/* This function will work only after read_config_files step */
> +int
> +l3fwd_validate_routes_port(enum L3FWD_LOOKUP_MODE mode, uint32_t i,
> + bool is_ipv4)
> +{
> + uint32_t max_port_count = (sizeof(enabled_port_mask) * CHAR_BIT);
> +
> + if (mode == L3FWD_LOOKUP_LPM ||
> + mode == L3FWD_LOOKUP_FIB) {
> + if (is_ipv4) {
> + if (route_base_v4[i].if_out >= max_port_count ||
> + ((1 << route_base_v4[i].if_out &
> + enabled_port_mask) == 0))
> + return -1;
> + } else {
> + if (route_base_v6[i].if_out >= max_port_count ||
> + ((1 << route_base_v6[i].if_out &
> + enabled_port_mask) == 0))
> + return -1;
> + }
> + } else if (mode == L3FWD_LOOKUP_EM) {
> + if (is_ipv4) {
> + if (em_route_base_v4[i].if_out >= max_port_count ||
> + ((1 << em_route_base_v4[i].if_out &
> + enabled_port_mask) == 0))
> + return -1;
> + } else {
> + if (em_route_base_v6[i].if_out >= max_port_count ||
> + ((1 << em_route_base_v6[i].if_out &
> + enabled_port_mask) == 0))
> + return -1;
> + }
> + }
Stupid q - why we don't have any checks for ACL mode?
> + return 0;
> +}
