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 C2B3345616; Mon, 22 Jul 2024 05:28:41 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 4D33F402DD; Mon, 22 Jul 2024 05:28:41 +0200 (CEST) Received: from AM0PR83CU005.outbound.protection.outlook.com (mail-westeuropeazon11010052.outbound.protection.outlook.com [52.101.69.52]) by mails.dpdk.org (Postfix) with ESMTP id B6791402DA; Mon, 22 Jul 2024 05:28:39 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector10001; d=microsoft.com; cv=none; b=d3Qf1H8vv6Lz+IGvn/NPhS+TQu2OZpxywwhYl0gEf4pDtDsSdtNbavEBalCHH/UeOsohV2n1/KFqFlvddlFPJw2JKvDZNnSGQgyr+UnjATI3BgQIR4PVPoUttiVztVHw3qxGaMGr2yEOIlwF+Xqc+j7GUFZbriQiRGcBwcxpNzESXPNCReqxdGvI9piR28efNtuCn7gux5b8wJvfDJ2EkB5OgHxbiVQ9RbfecXxSJAtI6pNbHOkhHp4l1ZLETAB13Cxk53X2ITLURLWNRKyNIc0RLwS3STruMUuKP9t66T9YBQ4R+eLDQjer9J/vwB/Qy6WjsoJQn6Yx6tdTBKDZXA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector10001; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=983vigcpMxPIErjuUZgtuTJRgIgsPyAgfKTxt7IKyJE=; b=A91xjv2PRsRUGS4MvJovW6MIUBqqzezr6TBQeEcrgTgYRwkuSY+xDumRbG3W+AMiMUXL5WaRWMfs/hjT2L1JgR9ztlLL9tPguC/evISfxaJxm9EcYnbYrFraWimQxXrp+jHSTVhmi7AZCQ9hq3ppH4iRDL2UrVRFiwx6q6Lxpdh62VrrG9djs3uYxgLGYeHl6Pisx+4cNdWCBmTPIDCPDEBDd5rSyYXaKieLeK7UptCQ750ns+O9esYLN4mgNRDAtLMdZ0wEVhpHNQ5TuysVkOAW+jseVPxj7tjeUviAaR1cE6wCg4vRkXUewfk6OQcvDlsZEZJUiyFdM1ViIJWQ4Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com; dkim=pass header.d=nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=983vigcpMxPIErjuUZgtuTJRgIgsPyAgfKTxt7IKyJE=; b=qdLAfxz335nVInNzyoB2FMpV8rS1cbsaMqo3OyLNv3yM6xkMRoOlPcEPjjrIZz882jWBb4/CZ2vyJStbYM26d4fkOYY5zUUFT53Zt+d4sD6SFSCss4BdXmACEbOrHHo9IFWi5H8/11aeP7IqeMtcbr/Cscv87W2+Z0+zpvRRIRc= Received: from AS8SPR01MB0024.eurprd04.prod.outlook.com (2603:10a6:20b:3d0::24) by DB9PR04MB8461.eurprd04.prod.outlook.com (2603:10a6:10:2cf::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7762.23; Mon, 22 Jul 2024 03:28:37 +0000 Received: from AS8SPR01MB0024.eurprd04.prod.outlook.com ([fe80::c634:479e:8f8a:a325]) by AS8SPR01MB0024.eurprd04.prod.outlook.com ([fe80::c634:479e:8f8a:a325%7]) with mapi id 15.20.7784.017; Mon, 22 Jul 2024 03:28:36 +0000 From: Gagandeep Singh To: Konstantin Ananyev , "dev@dpdk.org" , Konstantin Ananyev , Sean Morrissey CC: "stable@dpdk.org" Subject: RE: [PATCH 3/3] examples/l3fwd: fix maximum acceptable port ID in routes Thread-Topic: [PATCH 3/3] examples/l3fwd: fix maximum acceptable port ID in routes Thread-Index: AQHa1p/wVRBmr3j9f0St0JrbTClx37H6t9YAgAFKzOCAAEMlgIAF2LrQ Date: Mon, 22 Jul 2024 03:28:36 +0000 Message-ID: References: <20240715101458.645014-1-g.singh@nxp.com> <20240715101458.645014-3-g.singh@nxp.com> <370cf325ad02427cbca5a37756da4c35@huawei.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=nxp.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: AS8SPR01MB0024:EE_|DB9PR04MB8461:EE_ x-ms-office365-filtering-correlation-id: 78c97b05-7324-494a-f22d-08dca9fe5f86 x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; ARA:13230040|366016|1800799024|376014|38070700018; x-microsoft-antispam-message-info: =?us-ascii?Q?B3Wh4NaWy/945uFK3yxuhAzbKwrKaKpqzYqpZ1GwIObNE/r0SHzHxMQgSwVn?= =?us-ascii?Q?Ab5OA+jzChhA9nKxWojyPkEM3ZTqNJD4d89EHOlFPEpcmk5iZZ1Lr+xlXleS?= =?us-ascii?Q?NV/uY3ipaTZ7eSTzeV1TaiY+xkR15wwX5tEJodZxX/WDuW30yyG8YGQXKbtG?= =?us-ascii?Q?aSmVOxGwAD5wvQcZVOp4JfmwuDUGcvMgFfPC9qlN0EGkIFt/oDmJcYXDRShZ?= =?us-ascii?Q?a1oNtpKH2BIkba/ifzSZHuf6xN/YPz33aL7iWF4zm8rA2xhtza15eRzbJNuB?= =?us-ascii?Q?O3owT5HlE+EbeHe2Y2ct2sj1Fk8uvdTjG2cjNKLTSROpPHB81CIuh7pmfRNb?= =?us-ascii?Q?rGjd+bAF2BPQHSOAUUaCx3TYsNuvC5EEM7d1uAwnOswu38oJ2F/tHzxhH8Lm?= =?us-ascii?Q?df/M4zEdNQN4x/0CWEAlKS71eCWJEoPAkD6oJrxTo4+I7Y82HrR1yTC44OjW?= =?us-ascii?Q?DKGrhA8r8hkVvFGGNC2ZMwOy24vGxQCEUGWySlxZwuUOhha3wh2hoJqsD7kP?= =?us-ascii?Q?EdpNelkuU+wO4rrJ/NMHnCTGvbsGiT7IiPz3gHrfsnFKITgQah3LWHp38ffQ?= =?us-ascii?Q?SVQ4npoCyh8ypS2jrjXa4uKchLe2I9ItXz1NqrAlFS9N/dO5KqpF+15dsN6A?= =?us-ascii?Q?mHjZxJEuVjk3XsN3ry+YwK4kJ+JO8gsxUULNVnm7ZRHSjDig6AnwsEpWmehc?= =?us-ascii?Q?Okz5BCRFZV20t8gs8eaj6VCI42LzwV3f1Qvjfa77yEIzAzW4slljo4NW0v/B?= =?us-ascii?Q?5nJSANKa9GVVlLIlxzVtTMmQpysT45ga+3MJb9IaOVLJKSgCArgMFi9C7nr7?= =?us-ascii?Q?ov1NF4X4KbcGopQwq0Sl9lyPEiddCqqqhj7GhvYn0otIgP3M7Dv3Qb+Ub7Cd?= =?us-ascii?Q?L8JYp7IXD4TBnnOUjPJKtc8mC1+tSufVp7saLqBit+dTRL5txWmX7B82pRuV?= =?us-ascii?Q?vTHdGaBTZ6frhCsxDwLH/BP8/x/fv+hR4jYqIiIoDBtIRW15HxTpU+XJ/oOt?= =?us-ascii?Q?GR2XXlBYko+RFuQtORzCSh83JKrKwAv4Is5NPSvvrIJPWDPgrtJ0bjfBfu7S?= =?us-ascii?Q?YFDITP2twygmfbaZvGj3p8akyLLZ6CN7Dyd8pr6M1UFsz3nx8uE72Yo4hgnD?= =?us-ascii?Q?UoOl+xeIBSUJluXvoTkn6cSHg7QEyKsKA9qMe76s8wb2A7L8KI7qkzC99ywG?= =?us-ascii?Q?jbS0lAZk3z8ANvN2cbbN40Gtl+/DZc23fugQRe97Jbb8RCZJL6pGphKUg7sL?= =?us-ascii?Q?f3wzSXviiTCVCYMCyadu9mpVVMtcpjDcc3rkinEXgRie0NE+2zIwhPqk+mYi?= =?us-ascii?Q?f/Fmr1Re5UhGjw15muNkSkCx+vY2Lqjeuqh5zHiRJPa3uFAyA0eONeqts7rT?= =?us-ascii?Q?ZTQMSKDxQdn+WXipfXMs+aFo3lXYuFUaXJQ6LCZNqk5tgW64YA=3D=3D?= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AS8SPR01MB0024.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(13230040)(366016)(1800799024)(376014)(38070700018); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?RKhJtKYFSJ57IGqirWjYChJ77fAcYrnCRZE10BbSNCBdBZ82vXGx7AAdlTTN?= =?us-ascii?Q?NdO1S/7yqCffAkrgQJxOb3fh8rGLNUBQYNB1PLiSVBmc6/Ux15mPKuqsL2uP?= =?us-ascii?Q?wLuf2LxJwSJ/5JhYQa8CCHw01OXrQYsgJk4l1KxDhbsubjPWZ/6zNP7OPdkX?= =?us-ascii?Q?VUMMVv+kmQrDlVwnH44KSMNZij9uEE9+yGjGpeKr3bzb00LKOUu8axBZB00+?= =?us-ascii?Q?rp3x1enmyUTxNJezKxN4dpjl9dfyMAJDq++L0tPkcPPCSzd+ld8X6irb5hgS?= =?us-ascii?Q?yE4AkIBK4951FRBhmielh9E+JW8ev8zpH7yr5Z8QczRjdP12ccZlPqrsiSu7?= =?us-ascii?Q?27dDbzgCHWuiBhs8FQIMEZVVfNpuXe+/wAWq8mc6ocv5ft99HL1ed2rNMlrP?= =?us-ascii?Q?BUwt82E3xzYCoAVIKai2cquYgOzEbhGL7mF7FL96uJzsGqlOVqTZ9uXQGDlM?= =?us-ascii?Q?rGzklJquH8GcmTzhvAJyqq42NIViQne8l9jRSUNSQ35oXpm6gjRsEiMyuviM?= =?us-ascii?Q?ozDyUbDUjeA7ePASxM4Sor2m5/OX6eCaznLU9ZyHe1jBYMAU+VrJ68qt/uBj?= =?us-ascii?Q?u5/D9G7FOoSAchSM7teP8WIv2N0tpVQ8J/DgcaS19750WN/M4n2ana6xpVuE?= =?us-ascii?Q?Rb5YQV+l1gOBoU874kUi8hbeMmKm4rn+sDDZYL54vloWJekRSlBKhMIYDPvY?= =?us-ascii?Q?73cJyIcheAhYpLNg16SivtLMm1embxFvRxxGP9V0TrPVZvKcl+mGahxeZMHH?= =?us-ascii?Q?kvGi8LdGC2ZzXsMZVsQqy29dsvwNk7+QGvIkqiQhquFWu5lp0iHPXNJNpNa+?= =?us-ascii?Q?1+U/1OSAzbDE14rCcHFi4dEhj3prvlx45CNihcdMbeJYHD5fWXShxjnQPIQn?= =?us-ascii?Q?8RVDouNWUoGLB7WTHE6g80Ws/aEZ0ywzcKXN56+tzcIIO/NzvSGqS000HNmG?= =?us-ascii?Q?jM7STtkZjdBA5ZCEQgFEz0b+0TyCARAqUDFkuTZ9k+UlVTBsQHk41fJwv3Y5?= =?us-ascii?Q?BgZowbJYj4CvGV0TkZ/DS+61GswEyNbcyRrWsak+Hjv/kRKRzSj1z6XltHmg?= =?us-ascii?Q?qlqMn28mvvw+/O/IuCbm6vGfjXJ8G4mtFyykioKPTakUHoIiWnktA+A+0gPu?= =?us-ascii?Q?fCAe57FVSPxU7xAZdsfEgvPnBQM79FP1Y59eUg6eMeg04/3bcbJW4J/BYyYB?= =?us-ascii?Q?lQtdF0/qxT6xqnz/pmaWdjLb6kDohzGpSVcmM2haT87qw3AbSsllmHv7eb2L?= =?us-ascii?Q?975XCV3XL/ltkXssPBtD5IvkWWoaMbIN0t9aUEQVU8U9MM3KlBG7iiadefEa?= =?us-ascii?Q?ood96sT0xzhgUlnAmu7oDmHrAysz2NhdtsEHE0x3cvSAKNZ8aVXkBcMsrx4k?= =?us-ascii?Q?9igtHquAzNl7XioKeidPBhUX1LSTl4tNPrL5Oz7FE1OyUkuB0EtE7goxCJ+x?= =?us-ascii?Q?5+9VZCcpoUQ5z6W5yM1F9X72Zr214/dapUd7dzOdyG8Usr1RI2ZDrSmZR3rS?= =?us-ascii?Q?pd93H/4gsRx25GgHqijXaKA+V/BPn5o/Y7lDPgcSh9jMxwRbrfP3bpohRlCU?= =?us-ascii?Q?Gyy6B0kaqJ+ddYEYbkQ=3D?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: AS8SPR01MB0024.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 78c97b05-7324-494a-f22d-08dca9fe5f86 X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jul 2024 03:28:36.8857 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: xqPpZrSMpgPo9r7dRG28uRSAswsB1nFsFJLwUO4rZU1b8FoOIc0gduKiiIsKC43k X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB9PR04MB8461 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 Hi, > -----Original Message----- > From: Konstantin Ananyev > Sent: Thursday, July 18, 2024 3:32 PM > To: Gagandeep Singh ; dev@dpdk.org; Konstantin Ananyev > ; Sean Morrissey > > Cc: stable@dpdk.org > Subject: RE: [PATCH 3/3] examples/l3fwd: fix maximum acceptable port ID i= n > routes >=20 >=20 >=20 > > > > Application is accepting routes for port ID up to UINT8_MAX for > > > > LPM amd EM routes on parsing the given rule file, but only up to > > > > 32 ports can be enabled as per the variable enabled_port_mask > > > > which is defined as uint32_t. > > > > > > > > This patch restricts the rules parsing code to accept routes for > > > > port ID up to 31 only to avoid any unnecessary maintenance of > > > > rules which will never be used. > > > > > > If we want to add this extra check, probably better to do it in setup= _lpm(). > > > Where we already check that port is enabled, and If not, then this > > > route rule will be skipped: > > > > > > /* populate the LPM table */ > > > for (i =3D 0; i < route_num_v4; i++) { > > > struct in_addr in; > > > > > > /* skip unused ports */ > > > if ((1 << route_base_v4[i].if_out & > > > enabled_port_mask) =3D=3D 0) > > > continue; > > > > > > Same for EM. > > I am trying to update the check for MAX if_out value in rules config fi= le parsing > which will be before setup_lpm(). > > The reason is, restricting and adding only those rules which can be > > used by the application while populating the route_base_v4/v6 at first > > step and avoid unnecessary memory allocation for local variables to sto= re more > not required rules. >=20 > Hmm... but why it is a problem? Not really a problem, Just trying to optimize wherever it Is possible. >=20 > > > > > ((1 << route_base_v4[i].if_out & > > > enabled_port_mask) > > By looking into this check, it seems restriction to maximum 31 port ID > > while parsing rule file becomes more valid as this check can pass due > > to overflow in case value of route_base_v4[i].if_out Is 31+. >=20 > Agree, I think we need both, and it probably need to be in setup_lpm(). > Something like: >=20 > if (route_base_v4[i].if_out >=3D sizeof(enabled_port_mask) * CHAR_BIT || > ((1 << route_base_v4[i].if_out & enabled_port_mask) =3D=3D 0) { > /* print some error message here*/ > rte_exiit(...); /* or return an error */ } >=20 Yes, I can change it to this. > > > > > Another question here - why we just silently skip the rule with inval= id port? > > In read_config_files_lpm() we are calling the rte_exit in case port ID = is 31+. > > In setup_lpm, skipping the rules for the ports which are not enabled > > and not giving error, I guess probably because of ease of use. > > e.g. user has only single ipv4_routes config file with route rules for > > port ID 0,1,2,3,4 and want to use same file for multiple test cases > > like 1. when only port 0 enabled 2. when only port 0 and 1 enabled and > > so on. > > In this case, user can avoid to have separate route files for each of t= he test case. >=20 > The problem as I see it - we are not consistent here. > In some cases we just silently skip rules with invalid (or disabled) port= numbers, in > other cases we generate an error and exit. > For me it would be better, if we follow one simple policy (abort with err= or) here > for all cases. Ok, I will add the rte_exit if route port is invalid or not enabled.=20 With this change onwards It will be assumed user will add only those routes With port IDs which are valid and enabled in the application. >=20 > > > > > Probably need to fail with error... that what ACL code-path does. > > > > > > > Fixes: e7e6dd643092 ("examples/l3fwd: support config file for EM") > > > > Fixes: 52def963fc1c ("examples/l3fwd: support config file for > > > > LPM/FIB") > > > > Cc: sean.morrissey@intel.com > > > > Cc: stable@dpdk.org > > > > > > > > Signed-off-by: Gagandeep Singh > > > > --- > > > > examples/l3fwd/em_route_parse.c | 6 ++++-- > > > > examples/l3fwd/lpm_route_parse.c | 6 ++++-- > > > > 2 files changed, 8 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/examples/l3fwd/em_route_parse.c > > > > b/examples/l3fwd/em_route_parse.c index 8b534de5f1..65c71cd1ba > > > > 100644 > > > > --- a/examples/l3fwd/em_route_parse.c > > > > +++ b/examples/l3fwd/em_route_parse.c > > > > @@ -65,7 +65,8 @@ em_parse_v6_rule(char *str, struct em_rule *v) > > > > /* protocol. */ > > > > GET_CB_FIELD(in[CB_FLD_PROTO], v->v6_key.proto, 0, UINT8_MAX, 0); > > > > /* out interface. */ > > > > - GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, UINT8_MAX, 0); > > > > + GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, > > > > + (sizeof(enabled_port_mask) * CHAR_BIT) - 1, 0); > > > > > > > > return 0; > > > > } > > > > @@ -102,7 +103,8 @@ em_parse_v4_rule(char *str, struct em_rule *v) > > > > /* protocol. */ > > > > GET_CB_FIELD(in[CB_FLD_PROTO], v->v4_key.proto, 0, UINT8_MAX, 0); > > > > /* out interface. */ > > > > - GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, UINT8_MAX, 0); > > > > + GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, > > > > + (sizeof(enabled_port_mask) * CHAR_BIT) - 1, 0); > > > > > > > > return 0; > > > > } > > > > diff --git a/examples/l3fwd/lpm_route_parse.c > > > > b/examples/l3fwd/lpm_route_parse.c > > > > index f27b66e838..357c12d9fe 100644 > > > > --- a/examples/l3fwd/lpm_route_parse.c > > > > +++ b/examples/l3fwd/lpm_route_parse.c > > > > @@ -110,7 +110,8 @@ lpm_parse_v6_rule(char *str, struct > > > > lpm_route_rule > > > > *v) > > > > > > > > rc =3D lpm_parse_v6_net(in[CB_FLD_DST_ADDR], v->ip_32, &v->depth)= ; > > > > > > > > - GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, UINT8_MAX, 0); > > > > + GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, > > > > + (sizeof(enabled_port_mask) * CHAR_BIT) - 1, 0); > > > > > > > > return rc; > > > > } > > > > @@ -132,7 +133,8 @@ lpm_parse_v4_rule(char *str, struct > > > > lpm_route_rule > > > > *v) > > > > > > > > rc =3D parse_ipv4_addr_mask(in[CB_FLD_DST_ADDR], &v->ip, > > > > &v->depth); > > > > > > > > - GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, UINT8_MAX, 0); > > > > + GET_CB_FIELD(in[CB_FLD_IF_OUT], v->if_out, 0, > > > > + (sizeof(enabled_port_mask) * CHAR_BIT) - 1, 0); > > > > > > > > return rc; > > > > } > > > > -- > > > > 2.25.1 > > > > Gagan