From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1DE9645D54; Wed, 20 Nov 2024 12:10:26 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id AFF0D42F55; Wed, 20 Nov 2024 12:10:25 +0100 (CET) Received: from frasgout.his.huawei.com (frasgout.his.huawei.com [185.176.79.56]) by mails.dpdk.org (Postfix) with ESMTP id 5921840E36 for ; Wed, 20 Nov 2024 12:10:23 +0100 (CET) Received: from mail.maildlp.com (unknown [172.18.186.216]) by frasgout.his.huawei.com (SkyGuard) with ESMTP id 4XtdrP2ssdz6K92y; Wed, 20 Nov 2024 19:08:05 +0800 (CST) Received: from frapeml500007.china.huawei.com (unknown [7.182.85.172]) by mail.maildlp.com (Postfix) with ESMTPS id 1AC13140119; Wed, 20 Nov 2024 19:10:22 +0800 (CST) Received: from frapeml500007.china.huawei.com (7.182.85.172) by frapeml500007.china.huawei.com (7.182.85.172) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.39; Wed, 20 Nov 2024 12:10:21 +0100 Received: from frapeml500007.china.huawei.com ([7.182.85.172]) by frapeml500007.china.huawei.com ([7.182.85.172]) with mapi id 15.01.2507.039; Wed, 20 Nov 2024 12:10:21 +0100 From: Konstantin Ananyev To: Gagandeep Singh , "dev@dpdk.org" Subject: RE: [v3 3/3] examples/l3fwd: enhance valid ports checking Thread-Topic: [v3 3/3] examples/l3fwd: enhance valid ports checking Thread-Index: AQHbOwGMQFu9cq5uNkqlSrR1wjLEOLLAAzig Date: Wed, 20 Nov 2024 11:10:21 +0000 Message-ID: <66021a79793049649c75f7a83f600433@huawei.com> References: <20240806034120.3165295-1-g.singh@nxp.com> <20241120040516.2836371-1-g.singh@nxp.com> <20241120040516.2836371-4-g.singh@nxp.com> In-Reply-To: <20241120040516.2836371-4-g.singh@nxp.com> Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.206.138.73] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org > The current port ID validation logic in the routes add code has > two issues: >=20 > - It can pass if port ID in route is 31+. > - It silently skips rules with disabled or invalid > port IDs >=20 > 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 >=20 > Signed-off-by: Gagandeep Singh > --- > 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(-) >=20 > diff --git a/examples/l3fwd/em_route_parse.c b/examples/l3fwd/em_route_pa= rse.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" >=20 > -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; >=20 > 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 >=20 > +/* Select Longest-Prefix, Exact match, Forwarding Information Base or Ac= cess 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_E= THPORTS]; > /* mask of enabled ports */ > extern uint32_t enabled_port_mask; >=20 > +/* 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); >=20 > +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; >=20 > - if ((1 << em_route_base_v4[i].if_out & > - enabled_port_mask) =3D=3D 0) > - continue; > + ret =3D 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; > + } >=20 > entry =3D &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; >=20 > - if ((1 << em_route_base_v6[i].if_out & > - enabled_port_mask) =3D=3D 0) > - continue; > + ret =3D 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; > + } >=20 > entry =3D &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 =3D 0; i < route_num_v4; i++) { > struct in_addr in; >=20 > - /* Skip unused ports. */ > - if ((1 << route_base_v4[i].if_out & > - enabled_port_mask) =3D=3D 0) > - continue; > + /* Check for valid ports */ > + ret =3D 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; > + } >=20 > ret =3D 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 =3D 0; i < route_num_v6; i++) { >=20 > - /* Skip unused ports. */ > - if ((1 << route_base_v6[i].if_out & > - enabled_port_mask) =3D=3D 0) > - continue; > + /* Check for valid ports. */ > + ret =3D 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; > + } >=20 > ret =3D 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 =3D 0; i < route_num_v4; i++) { > struct in_addr in; >=20 > - /* skip unused ports */ > - if ((1 << route_base_v4[i].if_out & > - enabled_port_mask) =3D=3D 0) > - continue; > + /* Check for valid ports */ > + ret =3D 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; > + } >=20 > ret =3D 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 =3D 0; i < route_num_v6; i++) { >=20 > - /* skip unused ports */ > - if ((1 << route_base_v6[i].if_out & > - enabled_port_mask) =3D=3D 0) > - continue; > + /* Check for valid ports */ > + ret =3D 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; > + } >=20 > ret =3D 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 { >=20 > 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; >=20 > 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 =3D MEMPOOL_CACHE_SIZE; > /**< Ports set in promiscuous mode off by default. */ > static int promiscuous_on; >=20 > -/* Select Longest-Prefix, Exact match, Forwarding Information Base or Ac= cess 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; >=20 > /* Global variables. */ > @@ -92,7 +84,8 @@ xmm_t val_eth[RTE_MAX_ETHPORTS]; >=20 > /* 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. */ >=20 > @@ -271,6 +264,43 @@ l3fwd_set_alg(const char *optarg) > parm_config.alg =3D parse_acl_alg(optarg); > } >=20 > +/* 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 =3D (sizeof(enabled_port_mask) * CHAR_BIT); > + > + if (mode =3D=3D L3FWD_LOOKUP_LPM || > + mode =3D=3D L3FWD_LOOKUP_FIB) { > + if (is_ipv4) { > + if (route_base_v4[i].if_out >=3D max_port_count || > + ((1 << route_base_v4[i].if_out & > + enabled_port_mask) =3D=3D 0)) > + return -1; > + } else { > + if (route_base_v6[i].if_out >=3D max_port_count || > + ((1 << route_base_v6[i].if_out & > + enabled_port_mask) =3D=3D 0)) > + return -1; > + } > + } else if (mode =3D=3D L3FWD_LOOKUP_EM) { > + if (is_ipv4) { > + if (em_route_base_v4[i].if_out >=3D max_port_count || > + ((1 << em_route_base_v4[i].if_out & > + enabled_port_mask) =3D=3D 0)) > + return -1; > + } else { > + if (em_route_base_v6[i].if_out >=3D max_port_count || > + ((1 << em_route_base_v6[i].if_out & > + enabled_port_mask) =3D=3D 0)) > + return -1; > + } > + } Stupid q - why we don't have any checks for ACL mode?=20 > + return 0; > +}