From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR01-DB5-obe.outbound.protection.outlook.com (mail-db5eur01on0062.outbound.protection.outlook.com [104.47.2.62]) by dpdk.org (Postfix) with ESMTP id 3BABE1B3A0 for ; Thu, 12 Jul 2018 20:25:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=4C4Yvu38PjE8Nx+bwnk7eaz6NgJbRCYK9Wf9XTntxtI=; b=uPzNRKHGMDSkD/2TTW9cCNKiYQQPCsp6FY67v7AVe+T7/fq63HQpqYKGY72quGNMqFurWkqKPaLxh0+ZDwz3hjPyh54+2rjaRs52RZiN18a3ovzcu0VyYgcEDz+cMv9sHRGBygc9nITmvud+J9TccvWm+3y89LvBWia1j4GZN7o= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=yskoh@mellanox.com; Received: from yongseok-MBP.local (209.116.155.178) by AM5PR0501MB2034.eurprd05.prod.outlook.com (2603:10a6:203:1a::20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.952.18; Thu, 12 Jul 2018 18:25:38 +0000 Date: Thu, 12 Jul 2018 11:25:27 -0700 From: Yongseok Koh To: Adrien Mazarguil Cc: Shahaf Shuler , Nelio Laranjeiro , dev@dpdk.org Message-ID: <20180712182526.GA73570@yongseok-MBP.local> References: <20180627173355.4718-1-adrien.mazarguil@6wind.com> <20180627173355.4718-3-adrien.mazarguil@6wind.com> <20180712005917.GD69686@yongseok-MBP.local> <20180712104646.GT5211@6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180712104646.GT5211@6wind.com> User-Agent: Mutt/1.9.3 (2018-01-21) X-Originating-IP: [209.116.155.178] X-ClientProxiedBy: BN6PR03CA0018.namprd03.prod.outlook.com (2603:10b6:404:23::28) To AM5PR0501MB2034.eurprd05.prod.outlook.com (2603:10a6:203:1a::20) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: e1a8da8f-3b04-4368-5723-08d5e824deb1 X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989117)(5600053)(711020)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(48565401081)(2017052603328)(7153060)(7193020); SRVR:AM5PR0501MB2034; X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2034; 3:nBQsRfX33dJ3DuIFvzmG4P26I19tVA7Mz6cUNWffPA4hqnIzAViJJVAG3a2UZ5gmRgot//IeUMD6B1FxB0qqEuhoUBHyweS14oN4v3M8GeIhOGINrXJlD4VidILEnHIy+nOzJX9eDE+0OTRp64vu5qeMGDkiAdobXQU1zpmbuvSaRR9wLuTYW7nS4H+drQ5Y3JrCIK+w0Kk6uGBu1uBEikisvASe18aVj4dbwH6r8YvOmh8OvBPz0HHlBOm9YIQo; 25:qphKdV8kZVTrwQBZ+wvM19rSjJ6wjCiepBo5nzO/AmOp32WyTTyygbMipqrVh5G37Q5vshlaMiHi/84OifCaRq1mwEhehIKWeqM1Y2CtdyaU+/M/zdL3EG9WzwFYlYf76XunGUzAEcwLHWG5XrSyvt73eStXYHuyO81Xw1DlalhhZKwg+xNsp+j3YvVLDgMxJTzpW8Fs4HwlVjL35Rj6205PadBB7cd4DDE3dI3JAWq+J3i6PlSxPFMEU1H5KU80iKy1d7wIfqk+gAWan0Tn4QwgRs1/XEHbbAwJ69ZJ8BCT1dqM2ql4r9BbBtL4hqbLiGX64CpZJ5jVn1Jfnt2psw==; 31:ExpX7rgkMaNyctdOnRwx9LsPETi/XjyUM+w5+u2cNPymhv13cjVY3yfAfwqlCSN1+mtrxK024PtTZYTko4MYhEf+DBTZRkdLaJQcsejIrD0MooTTpTc44yhZtH4SmgNUt/ywEnakx3AqGE1A2SKUTT23QcdCH24fWvkQH9URPB/fBuz2/hqLWlbziBDPDzhd8HczgX/gmEugSxF09J6xzTakc547HMYOaGSrY66tGbY= X-MS-TrafficTypeDiagnostic: AM5PR0501MB2034: X-LD-Processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2034; 20:tblJ7byKP1SacePbzp+eMWaV00DwREUgRl6AyfT0ROrOCimYk2rO/JuQjjeVSnVIf8jJhu4Cc5J84ZIQYmq8U9osgeyB2WOI0kMxbTJTB46D7Vv7hJel9TDQexHl05VW9pTZvrb8D4hrjYhYPpBkiyq1ECnFashxXyOtLak+0QVNXb1x4FSmZcHbKw8EQZn6nX/nf4wZHhYTcwLs39zOcAc0TckYzraY/T9kwkCUo64OuSTschDQ0zrqEwa3zbHOyWXENaN5uaBslXqhtOnCJNCPXk64j6yyCDcjTE7Fvnr73AfKlWY+gNolCaJL0L033JNZgAwNWO/97FCP/azMNlmaW2D+oEudSVVymQaRk7BXN5/qoNBZG3fI2Pk3CmNq5F2gjL8DMqy87BFXKZ+9PvSohavBYFyx5BlB2AcYRLRXWSM92oP5CxxJ8mNlP14DhObYnWkVeqVvRrQZe5Khn8m098K6DoitoTvxhjluvO/zokLcKMA2KJzq6WCCmrUt; 4:Q0NaUBSu6pq9rSLe6qdHnOGcW17CuyipcM8u3oM1Vepg0WyJHVFya2zlTY2O0hZndMQhYRaldRO/gNYQw98WyQ4HL2zuefEm/bvnlS2+GogPogNJnv9rlVAntWQVTI4QaUoU4+eTvE8QA5ZWjXbiFomBvTYvgFH9JOnd8+gcb8HpLhoLfJWNtbG6EaTYyffORnQbe2Hzzj6RsEAXz3vsUCq7ushAWe45axPwQqBnwemCQSKTsKjw+2LSbnppfdWVE9oMGcfBMrKvEGie4U3Z4RpsdRefWjDpFl0Odi9lJ0fVmOHsvY5ro5JCj40vA53H X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(211171220733660); X-MS-Exchange-SenderADCheck: 1 X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(93006095)(93001095)(3002001)(3231311)(944501410)(52105095)(10201501046)(6055026)(149027)(150027)(6041310)(20161123558120)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(6072148)(201708071742011)(7699016); SRVR:AM5PR0501MB2034; BCL:0; PCL:0; RULEID:; SRVR:AM5PR0501MB2034; X-Forefront-PRVS: 0731AA2DE6 X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(136003)(346002)(376002)(396003)(366004)(39860400002)(189003)(199004)(105586002)(81156014)(33896004)(6916009)(305945005)(54906003)(386003)(6506007)(476003)(486006)(446003)(106356001)(956004)(11346002)(316002)(8676002)(86362001)(8936002)(66066001)(478600001)(2906002)(16586007)(6666003)(575784001)(186003)(26005)(76176011)(7736002)(16526019)(47776003)(7696005)(52116002)(14444005)(5660300001)(58126008)(50466002)(68736007)(81166006)(97736004)(93886005)(98436002)(4326008)(9686003)(33656002)(23726003)(55016002)(1076002)(3846002)(53936002)(6246003)(6116002)(229853002)(25786009)(18370500001); DIR:OUT; SFP:1101; SCL:1; SRVR:AM5PR0501MB2034; H:yongseok-MBP.local; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; Received-SPF: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) X-Microsoft-Exchange-Diagnostics: =?us-ascii?Q?1; AM5PR0501MB2034; 23:6pbH8dbXK7ggYoLKL2sfGRy1p1bKRpX1mVC5GEm?= =?us-ascii?Q?PlABEtXLLjZeGd/MxKIApPz7ul0K/qYh5TCVeUjMcnwXV5FL1z7W0/5uhRwb?= =?us-ascii?Q?ea0XWQOvM0pTsQ1EocGyKPlXlk5T0e30m0Un+NV/FFIW1t14TR0L1DAQ1XuR?= =?us-ascii?Q?JdSuJVDpwG+z455CmpvmOP7mgg/nZlpA7uPMNNGjWWGAr3fk81vZLxKfzWts?= =?us-ascii?Q?+Jw23Hm5YBJCQ9RWCp3NdN+J3rKldLWKXpdwNUKK5wbx+M0enU5myA8jeA2G?= =?us-ascii?Q?u581T2Ovs83sxRMskx0RhCZvpDIYrNuuTzjC7fNqF2VhCKPQczfZYKsezt1B?= =?us-ascii?Q?rdgDJ06cKdYt+GszfcmGJfbNmO0Vkyulnk9pSJNnFRnzt7rVsXMX2Hq8JTjs?= =?us-ascii?Q?ZphFJ02qPZ7YRrj659Bz7l7HE2802ZmbPWv4dyoNsStl5DOGPWJcB3rsFqmy?= =?us-ascii?Q?ktGpqGhP8Q3/CcHhaGT3S7I+HvXF1VfT8rQ4LD9RRTJtFLGA29v3cs0cenMH?= =?us-ascii?Q?zDn968yAHJfI2iM/c4//ag6hBbwV+l64VNu9CknPpSrUAOh1LzA9IIRyhg2n?= =?us-ascii?Q?h97PbgDJt9/4OBVMT5dPUf1MYxoz0pw3Wny18HjKe6CTu1XXtFlegCsrVgnK?= =?us-ascii?Q?7PGMQJeXPlQWvx3ds2iYFnck14bDfoFEd5Aq8qSBtl0li73W+HWA0LcFhn3X?= =?us-ascii?Q?04/k55FwZ3BiQtivGj63yi1JtUghXQCSzYne60XXQdZ0mb38Z0xKrnKYt9Jx?= =?us-ascii?Q?Ur5Z0xKtx6SZ/JnnVoPYIzvZLufcP7KV7WwgvyH7dLH29/HrZ2Z3WaoeFEIV?= =?us-ascii?Q?37cU7NXumPwTtMkYjWygqAx6sAZ2TUTNDwr5/AmX/jBzZQkHRs1l9TB6dzqU?= =?us-ascii?Q?+gMHDW8pgfKW6jGgF44oj9aKXAeMZH6OSQ6YeVngkLcsn+BtGnwYUGnS6EyM?= =?us-ascii?Q?EdB42h7OHXrFYvsnN33xEqCG2thtYO3gnl2L1OATI6deZ2AIi35X/HixLu8y?= =?us-ascii?Q?dcohj/4X4TY4/7AmQp8SIFog88U7CBtcceUOXBV8PgQfuDq6USfRthzd5yCd?= =?us-ascii?Q?EgVoZQMCq2rcw4yAnkEIqHafw4Gz0WD10NCkpGBaJAIPEZgx0YZWZBtokEK8?= =?us-ascii?Q?RYUSB6Ke+iNYYhhGuPpz+PP6Pg3zGp+HcwiA5LA24IsMj7+D4In7aVf3Y2d/?= =?us-ascii?Q?5ImNT/f8SAH5DrhOyoX9QXrO2HcbOSzsn2T2knv3nJqmYlZXUzu21pnR/PpY?= =?us-ascii?Q?iV/HAEdJpeNdJwtGZdP2opRvRLlJqhZsLN2fsBF91ePd4AKHxtSuuiHQBjOT?= =?us-ascii?Q?QuAa1HVnJenbBwm2Fu/imcdmRLKyQYGU9a8JU1tOdjbuDEykBfROPkDcZAvZ?= =?us-ascii?Q?M6MkOVg=3D=3D?= X-Microsoft-Antispam-Message-Info: H4yLs87fRtL/+e+UcBzi0l51Lg52PhXz8C8B/CX4z2x6N+0YIAKIEGLCxDoE3et+Wz7iOVKMcsfF5Anp3vvmyYnqe43h7B+HXYKSCcFA7d60x0beTuYma90XXQCRSkJF6EfdvRyGb6X5UGN4LPbbDXKwLKPiBkJnWjljurGXYMAqF9qNqNqMhWVKTOL1ebOAkI2iVSO6hDIbiqXhlJxj+fz9o7bcTcp7vUfeoHLWLUdKVkWyUP6wUP4qE/RNLQfDJGk3cn3jGw4ge5OZP69nMXnn902t6syZ2mKPRTiJF0kUZ6+zsQLvF9N7R1o7Si/GG+pLkmPjVlYt1Yx5TbdHW1NxftsGNIDlkEGPgxMk3Ds= X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2034; 6:/+B7XCpUG8Dv+KJlVOBmNRcZLmA4RRnlkowOJLbZBNbvZubiPROeuhrhT1qaV1LHnA5Zz1qG9fgqnlm9iqcO2x4m5K37v9+iB9/c5xoe4ojcEfRjCpM7FDgXjermDSOLKT2LGNFUqGw5ImsQhVzxO+kC3lEwKMQTdkTQ+bEMsS8eQpYGE5s9BUAm9d3raMdDBOeveo1dvdQN8kLbngC39SQdu/4fqBi9Zwg7/jy5njf3Zb/7587POFTgGCEXCze9PochutDAmjmGaCSM8p+kSxNzkn8gHVDfgtUZ6TQGZQWds+sZmrR6cZuqBMrVBsbdFkw/rD3PMrronBJDSSpWFrOU9BPxh5psA/wnHCXfc5S4vGxku3HGfbwA4t85BW6870VHWXZ1wrIno19/2bObj7RY4Xa6T2gIvVjsTLb4Z/PJdou3GiP43pRHHuRre2aN82URrws5T6ClItGliTiSGw==; 5:B9XoJagaLn4FXcJnhZ2uI2Crmw+g6EqnWYBgqzH1tRopUfVbBEOALWISJHs+i8TGJRPhFjUjjE5ykpLE8sywueFSY6T8CGyU8tQ7qqpZuBMEd14OD0v78/QaUerNRvlonv/Rk+874m12ysDgEbOQRQJLFKNbGK8y+dxqbE5fG+I=; 24:PqW8/bTDiu+75RHvlCGijCCcu32I+Ulv3PRLiBMu1aSCnLsKsfiUjE33RtS3Y2pvUm33lVC1aCQWNRZHEwtl36ntKMjZeCZzKm8s9gQvEjU= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; AM5PR0501MB2034; 7:wwKY9yNB63+JPW3CSA6pOcl0XZFxzThpcMcxXzefXoqqgKg4+vETQwG6qtTqhGhCL2M73rAwe2dREw/bi2qr0IQOl72P4GQU70uY/KnkuLdge43i0F9OeDCo75jf4vKXkN7VJneCzlPZuCuFnipDxUQYqjNB8PoYdnXd+8b0yQqCKB3Rv2tqXP8SD41RBrz0u3wWm/H+PfzGjakWEJNiMBueXAWSklVysWSd2OMVjHiuEFBuJYH/KINAKRKhHm0v X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 12 Jul 2018 18:25:38.5886 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: e1a8da8f-3b04-4368-5723-08d5e824deb1 X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM5PR0501MB2034 Subject: Re: [dpdk-dev] [PATCH 2/6] net/mlx5: add framework for switch flow rules X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 12 Jul 2018 18:25:42 -0000 On Thu, Jul 12, 2018 at 12:46:46PM +0200, Adrien Mazarguil wrote: > On Wed, Jul 11, 2018 at 05:59:18PM -0700, Yongseok Koh wrote: > > On Wed, Jun 27, 2018 at 08:08:12PM +0200, Adrien Mazarguil wrote: > > > Because mlx5 switch flow rules are configured through Netlink (TC > > > interface) and have little in common with Verbs, this patch adds a separate > > > parser function to handle them. > > > > > > - mlx5_nl_flow_transpose() converts a rte_flow rule to its TC equivalent > > > and stores the result in a buffer. > > > > > > - mlx5_nl_flow_brand() gives a unique handle to a flow rule buffer. > > > > > > - mlx5_nl_flow_create() instantiates a flow rule on the device based on > > > such a buffer. > > > > > > - mlx5_nl_flow_destroy() performs the reverse operation. > > > > > > These functions are called by the existing implementation when encountering > > > flow rules which must be offloaded to the switch (currently relying on the > > > transfer attribute). > > > > > > Signed-off-by: Adrien Mazarguil > > > Signed-off-by: Nelio Laranjeiro > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c > > > index 9241855be..93b245991 100644 > > > --- a/drivers/net/mlx5/mlx5_flow.c > > > +++ b/drivers/net/mlx5/mlx5_flow.c > > > @@ -4,6 +4,7 @@ > > > */ > > > > > > #include > > > +#include > > > #include > > > #include > > > > > > @@ -271,6 +272,7 @@ struct rte_flow { > > > /**< Store tunnel packet type data to store in Rx queue. */ > > > uint8_t key[40]; /**< RSS hash key. */ > > > uint16_t (*queue)[]; /**< Destination queues to redirect traffic to. */ > > > + void *nl_flow; /**< Netlink flow buffer if relevant. */ > > > }; > > > > > > static const struct rte_flow_ops mlx5_flow_ops = { > > > @@ -2403,6 +2405,106 @@ mlx5_flow_actions(struct rte_eth_dev *dev, > > > } > > > > > > /** > > > + * Validate flow rule and fill flow structure accordingly. > > > + * > > > + * @param dev > > > + * Pointer to Ethernet device. > > > + * @param[out] flow > > > + * Pointer to flow structure. > > > + * @param flow_size > > > + * Size of allocated space for @p flow. > > > + * @param[in] attr > > > + * Flow rule attributes. > > > + * @param[in] pattern > > > + * Pattern specification (list terminated by the END pattern item). > > > + * @param[in] actions > > > + * Associated actions (list terminated by the END action). > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. > > > + * > > > + * @return > > > + * A positive value representing the size of the flow object in bytes > > > + * regardless of @p flow_size on success, a negative errno value otherwise > > > + * and rte_errno is set. > > > + */ > > > +static int > > > +mlx5_flow_merge_switch(struct rte_eth_dev *dev, > > > + struct rte_flow *flow, > > > + size_t flow_size, > > > + const struct rte_flow_attr *attr, > > > + const struct rte_flow_item pattern[], > > > + const struct rte_flow_action actions[], > > > + struct rte_flow_error *error) > > > +{ > > > + struct priv *priv = dev->data->dev_private; > > > + unsigned int n = mlx5_domain_to_port_id(priv->domain_id, NULL, 0); > > > + uint16_t port_list[!n + n]; > > > + struct mlx5_nl_flow_ptoi ptoi[!n + n + 1]; > > > + size_t off = RTE_ALIGN_CEIL(sizeof(*flow), alignof(max_align_t)); > > > + unsigned int i; > > > + unsigned int own = 0; > > > + int ret; > > > + > > > + /* At least one port is needed when no switch domain is present. */ > > > + if (!n) { > > > + n = 1; > > > + port_list[0] = dev->data->port_id; > > > + } else { > > > + n = mlx5_domain_to_port_id(priv->domain_id, port_list, n); > > > + if (n > RTE_DIM(port_list)) > > > + n = RTE_DIM(port_list); > > > + } > > > + for (i = 0; i != n; ++i) { > > > + struct rte_eth_dev_info dev_info; > > > + > > > + rte_eth_dev_info_get(port_list[i], &dev_info); > > > + if (port_list[i] == dev->data->port_id) > > > + own = i; > > > + ptoi[i].port_id = port_list[i]; > > > + ptoi[i].ifindex = dev_info.if_index; > > > + } > > > + /* Ensure first entry of ptoi[] is the current device. */ > > > + if (own) { > > > + ptoi[n] = ptoi[0]; > > > + ptoi[0] = ptoi[own]; > > > + ptoi[own] = ptoi[n]; > > > + } > > > + /* An entry with zero ifindex terminates ptoi[]. */ > > > + ptoi[n].port_id = 0; > > > + ptoi[n].ifindex = 0; > > > + if (flow_size < off) > > > + flow_size = 0; > > > + ret = mlx5_nl_flow_transpose((uint8_t *)flow + off, > > > + flow_size ? flow_size - off : 0, > > > + ptoi, attr, pattern, actions, error); > > > + if (ret < 0) > > > + return ret; > > > > So, there's an assumption that the buffer allocated outside of this API is > > enough to include all the messages in mlx5_nl_flow_transpose(), right? If > > flow_size isn't enough, buf_tmp will be used and _transpose() doesn't return > > error but required size. Sounds confusing, may need to make a change or to have > > clearer documentation. > > Well, isn't it already documented? Besides these are the usual snprintf() > semantics used everywhere in these files, I think this was a major topic of > discussion with Nelio on the flow rework series :) > > buf_tmp[] is internal to mlx5_nl_flow_transpose() and used as a fallback to > complete a pass when input buffer is not large enough (including > zero-sized). Having a valid buffer is a constraint imposed by libmnl, > because we badly want to know how much space will be needed assuming the > flow rule was successfully processed. > > Without libmnl, the helpers it provides would have been written in a way > that doesn't require buf_tmp[]. However libmnl is just too convenient to > pass up, hence this compromise. > > (just to remind onlookers, we want to allocate the minimum amount of memory > we possibly can for resources needed by each flow rule, and do so through a > single allocation, end goal being to support millions of flow rules while > wasting as little memory as possible.) > > > > > diff --git a/drivers/net/mlx5/mlx5_nl_flow.c b/drivers/net/mlx5/mlx5_nl_flow.c > > > index 7a8683b03..1fc62fb0a 100644 > > > --- a/drivers/net/mlx5/mlx5_nl_flow.c > > > +++ b/drivers/net/mlx5/mlx5_nl_flow.c > > > @@ -5,7 +5,9 @@ > > > > > > #include > > > #include > > > +#include > > > #include > > > +#include > > > #include > > > #include > > > #include > > > @@ -14,11 +16,248 @@ > > > #include > > > #include > > > > > > +#include > > > #include > > > #include > > > > > > #include "mlx5.h" > > > > > > +/** Parser state definitions for mlx5_nl_flow_trans[]. */ > > > +enum mlx5_nl_flow_trans { > > > + INVALID, > > > + BACK, > > > + ATTR, > > > + PATTERN, > > > + ITEM_VOID, > > > + ACTIONS, > > > + ACTION_VOID, > > > + END, > > > +}; > > > + > > > +#define TRANS(...) (const enum mlx5_nl_flow_trans []){ __VA_ARGS__, INVALID, } > > > + > > > +#define PATTERN_COMMON \ > > > + ITEM_VOID, ACTIONS > > > +#define ACTIONS_COMMON \ > > > + ACTION_VOID, END > > > + > > > +/** Parser state transitions used by mlx5_nl_flow_transpose(). */ > > > +static const enum mlx5_nl_flow_trans *const mlx5_nl_flow_trans[] = { > > > + [INVALID] = NULL, > > > + [BACK] = NULL, > > > + [ATTR] = TRANS(PATTERN), > > > + [PATTERN] = TRANS(PATTERN_COMMON), > > > + [ITEM_VOID] = TRANS(BACK), > > > + [ACTIONS] = TRANS(ACTIONS_COMMON), > > > + [ACTION_VOID] = TRANS(BACK), > > > + [END] = NULL, > > > +}; > > > + > > > +/** > > > + * Transpose flow rule description to rtnetlink message. > > > + * > > > + * This function transposes a flow rule description to a traffic control > > > + * (TC) filter creation message ready to be sent over Netlink. > > > + * > > > + * Target interface is specified as the first entry of the @p ptoi table. > > > + * Subsequent entries enable this function to resolve other DPDK port IDs > > > + * found in the flow rule. > > > + * > > > + * @param[out] buf > > > + * Output message buffer. May be NULL when @p size is 0. > > > + * @param size > > > + * Size of @p buf. Message may be truncated if not large enough. > > > + * @param[in] ptoi > > > + * DPDK port ID to network interface index translation table. This table > > > + * is terminated by an entry with a zero ifindex value. > > > + * @param[in] attr > > > + * Flow rule attributes. > > > + * @param[in] pattern > > > + * Pattern specification. > > > + * @param[in] actions > > > + * Associated actions. > > > + * @param[out] error > > > + * Perform verbose error reporting if not NULL. > > > + * > > > + * @return > > > + * A positive value representing the exact size of the message in bytes > > > + * regardless of the @p size parameter on success, a negative errno value > > > + * otherwise and rte_errno is set. > > > + */ > > > +int > > > +mlx5_nl_flow_transpose(void *buf, > > > + size_t size, > > > + const struct mlx5_nl_flow_ptoi *ptoi, > > > + const struct rte_flow_attr *attr, > > > + const struct rte_flow_item *pattern, > > > + const struct rte_flow_action *actions, > > > + struct rte_flow_error *error) > > > +{ > > > + alignas(struct nlmsghdr) > > > + uint8_t buf_tmp[MNL_SOCKET_BUFFER_SIZE]; > > > + const struct rte_flow_item *item; > > > + const struct rte_flow_action *action; > > > + unsigned int n; > > > + struct nlattr *na_flower; > > > + struct nlattr *na_flower_act; > > > + const enum mlx5_nl_flow_trans *trans; > > > + const enum mlx5_nl_flow_trans *back; > > > + > > > + if (!size) > > > + goto error_nobufs; > > > +init: > > > + item = pattern; > > > + action = actions; > > > + n = 0; > > > + na_flower = NULL; > > > + na_flower_act = NULL; > > > + trans = TRANS(ATTR); > > > + back = trans; > > > +trans: > > > + switch (trans[n++]) { > > > + struct nlmsghdr *nlh; > > > + struct tcmsg *tcm; > > > + > > > + case INVALID: > > > + if (item->type) > > > + return rte_flow_error_set > > > + (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ITEM, > > > + item, "unsupported pattern item combination"); > > > + else if (action->type) > > > + return rte_flow_error_set > > > + (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_ACTION, > > > + action, "unsupported action combination"); > > > + return rte_flow_error_set > > > + (error, ENOTSUP, RTE_FLOW_ERROR_TYPE_UNSPECIFIED, NULL, > > > + "flow rule lacks some kind of fate action"); > > > + case BACK: > > > + trans = back; > > > + n = 0; > > > + goto trans; > > > + case ATTR: > > > + /* > > > + * Supported attributes: no groups, some priorities and > > > + * ingress only. Don't care about transfer as it is the > > > + * caller's problem. > > > + */ > > > + if (attr->group) > > > + return rte_flow_error_set > > > + (error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ATTR_GROUP, > > > + attr, "groups are not supported"); > > > + if (attr->priority > 0xfffe) > > > + return rte_flow_error_set > > > + (error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ATTR_PRIORITY, > > > + attr, "lowest priority level is 0xfffe"); > > > + if (!attr->ingress) > > > + return rte_flow_error_set > > > + (error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > > > + attr, "only ingress is supported"); > > > + if (attr->egress) > > > + return rte_flow_error_set > > > + (error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > > > + attr, "egress is not supported"); > > > + if (size < mnl_nlmsg_size(sizeof(*tcm))) > > > + goto error_nobufs; > > > + nlh = mnl_nlmsg_put_header(buf); > > > + nlh->nlmsg_type = 0; > > > + nlh->nlmsg_flags = 0; > > > + nlh->nlmsg_seq = 0; > > > + tcm = mnl_nlmsg_put_extra_header(nlh, sizeof(*tcm)); > > > + tcm->tcm_family = AF_UNSPEC; > > > + tcm->tcm_ifindex = ptoi[0].ifindex; > > > + /* > > > + * Let kernel pick a handle by default. A predictable handle > > > + * can be set by the caller on the resulting buffer through > > > + * mlx5_nl_flow_brand(). > > > + */ > > > + tcm->tcm_handle = 0; > > > + tcm->tcm_parent = TC_H_MAKE(TC_H_INGRESS, TC_H_MIN_INGRESS); > > > + /* > > > + * Priority cannot be zero to prevent the kernel from > > > + * picking one automatically. > > > + */ > > > + tcm->tcm_info = TC_H_MAKE((attr->priority + 1) << 16, > > > + RTE_BE16(ETH_P_ALL)); > > > + break; > > > + case PATTERN: > > > + if (!mnl_attr_put_strz_check(buf, size, TCA_KIND, "flower")) > > > + goto error_nobufs; > > > + na_flower = mnl_attr_nest_start_check(buf, size, TCA_OPTIONS); > > > + if (!na_flower) > > > + goto error_nobufs; > > > + if (!mnl_attr_put_u32_check(buf, size, TCA_FLOWER_FLAGS, > > > + TCA_CLS_FLAGS_SKIP_SW)) > > > + goto error_nobufs; > > > + break; > > > + case ITEM_VOID: > > > + if (item->type != RTE_FLOW_ITEM_TYPE_VOID) > > > + goto trans; > > > + ++item; > > > + break; > > > + case ACTIONS: > > > + if (item->type != RTE_FLOW_ITEM_TYPE_END) > > > + goto trans; > > > + assert(na_flower); > > > + assert(!na_flower_act); > > > + na_flower_act = > > > + mnl_attr_nest_start_check(buf, size, TCA_FLOWER_ACT); > > > + if (!na_flower_act) > > > + goto error_nobufs; > > > + break; > > > + case ACTION_VOID: > > > + if (action->type != RTE_FLOW_ACTION_TYPE_VOID) > > > + goto trans; > > > + ++action; > > > + break; > > > + case END: > > > + if (item->type != RTE_FLOW_ITEM_TYPE_END || > > > + action->type != RTE_FLOW_ACTION_TYPE_END) > > > + goto trans; > > > + if (na_flower_act) > > > + mnl_attr_nest_end(buf, na_flower_act); > > > + if (na_flower) > > > + mnl_attr_nest_end(buf, na_flower); > > > + nlh = buf; > > > + return nlh->nlmsg_len; > > > + } > > > + back = trans; > > > + trans = mlx5_nl_flow_trans[trans[n - 1]]; > > > + n = 0; > > > + goto trans; > > > +error_nobufs: > > > + if (buf != buf_tmp) { > > > + buf = buf_tmp; > > > + size = sizeof(buf_tmp); > > > + goto init; > > > + } > > > > Continuing my comment above. > > This part is unclear. It looks to me that this func does: > > > > 1) if size is zero, consider it as a testing call to know the amount of memory > > required. > > Yeah, in fact this one is a shortcut to speed up this specific scenario as > it happens all the time in the two-pass use case. You can lump it with 2). > > > 2) if size isn't zero but not enough, it stops writing to buf and start over to > > return the amount of memory required instead of returning error. > > 3) if size isn't zero and enough, it fills in buf. > > > > Do I correctly understand? > > Yes. Another minor note for 2), the returned buffer is also filled up to the > point of failure (mimics snprintf()). > > Perhaps the following snippet can better summarize the envisioned approach: > > int ret = snprintf(NULL, 0, "something", ...); > > if (ret < 0) { > goto court; > } else { > char buf[ret]; > > snprintf(buf, sizeof(buf), "something", ...); /* Guaranteed. */ > [...] > } I know you and Nelio mimicked snprintf() but as _merge() isn't a public API for users but an internal API. I didn't think it should necessarily be like that. I hoped to have it used either for testing (knowing size) or real translation - 1) and 3). And no possibility for 2), then 2) would've been handled by assert(). I beleive this could've made the code simpler. However, as I already acked Nelio's patchset, agreed on the idea and Nelio already documented the behavior, Acked-by: Yongseok Koh Thanks