From: Konstantin Ananyev <konstantin.ananyev@huawei.com>
To: Gagandeep Singh <g.singh@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: RE: [v3 3/3] examples/l3fwd: enhance valid ports checking
Date: Wed, 20 Nov 2024 11:10:21 +0000 [thread overview]
Message-ID: <66021a79793049649c75f7a83f600433@huawei.com> (raw)
In-Reply-To: <20241120040516.2836371-4-g.singh@nxp.com>
> 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.singh@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;
> +}
prev parent reply other threads:[~2024-11-20 11:10 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-15 10:14 [PATCH 1/3] examples/l3fwd: support single route file Gagandeep Singh
2024-07-15 10:14 ` [PATCH 2/3] examples/l3fwd: fix return value on rules add Gagandeep Singh
2024-07-16 6:55 ` Hemant Agrawal
2024-07-15 10:14 ` [PATCH 3/3] examples/l3fwd: fix maximum acceptable port ID in routes Gagandeep Singh
2024-07-17 10:17 ` Konstantin Ananyev
2024-07-18 6:30 ` Gagandeep Singh
2024-07-18 10:01 ` Konstantin Ananyev
2024-07-22 3:28 ` Gagandeep Singh
2024-07-22 4:27 ` Gagandeep Singh
2024-07-23 16:22 ` Konstantin Ananyev
2024-07-24 8:02 ` Konstantin Ananyev
2024-08-02 10:13 ` Gagandeep Singh
2024-08-06 3:41 ` [v2 0/3] L3fwd changes Gagandeep Singh
2024-08-06 3:41 ` [v2 1/3] examples/l3fwd: support single route file Gagandeep Singh
2024-08-06 3:41 ` [v2 2/3] examples/l3fwd: fix return value on rules add Gagandeep Singh
2024-08-06 3:41 ` [v2 3/3] examples/l3fwd: enhance valid ports checking Gagandeep Singh
2024-11-13 19:39 ` Stephen Hemminger
2024-11-19 17:13 ` Thomas Monjalon
2024-11-20 4:07 ` Gagandeep Singh
2024-11-20 4:05 ` [v3 0/3] L3fwd changes Gagandeep Singh
2024-11-20 4:05 ` [v3 1/3] examples/l3fwd: support single route file Gagandeep Singh
2024-11-20 4:05 ` [v3 2/3] examples/l3fwd: fix return value on rules add Gagandeep Singh
2024-11-20 4:05 ` [v3 3/3] examples/l3fwd: enhance valid ports checking Gagandeep Singh
2024-11-20 11:10 ` Konstantin Ananyev [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=66021a79793049649c75f7a83f600433@huawei.com \
--to=konstantin.ananyev@huawei.com \
--cc=dev@dpdk.org \
--cc=g.singh@nxp.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).