From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50049.outbound.protection.outlook.com [40.107.5.49]) by dpdk.org (Postfix) with ESMTP id 4847E1BE65 for ; Thu, 5 Jul 2018 21:56:26 +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=7/mfKjyqR3s30H+LNqieI5EKtFD+QtvHL+uWwA46XaU=; b=Q2tdRxoazdx4O1Xc+MhcLphitOM0u4l6taIiyyisQbu7pNSGVsUrSh9bifCZL9rXpUlMD+NSADvRi2CJePt6hX5x5Jv8gvtetY5ZhSB/r7FXICbVkV5u8/VUA1T31kUtGTQno0WL42bjhpzyBRyXNnwKQvg1HM+XzU2IEN/D2Dg= Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=yskoh@mellanox.com; Received: from yongseok-MBP.local (209.116.155.178) by HE1PR0501MB2043.eurprd05.prod.outlook.com (2603:10a6:3:35::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.906.25; Thu, 5 Jul 2018 19:56:22 +0000 Date: Thu, 5 Jul 2018 12:56:09 -0700 From: Yongseok Koh To: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Cc: dev@dpdk.org, Adrien Mazarguil Message-ID: <20180705195607.GA47821@yongseok-MBP.local> References: <90a73b5f33e147ffa3a668f5d19410de17f96045.1530111623.git.nelio.laranjeiro@6wind.com> <20180704083418.GA45405@minint-98vp2qg> <20180705084735.rrgqf3v7dpmo5col@laranjeiro-vm.dev.6wind.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20180705084735.rrgqf3v7dpmo5col@laranjeiro-vm.dev.6wind.com> User-Agent: Mutt/1.9.3 (2018-01-21) X-Originating-IP: [209.116.155.178] X-ClientProxiedBy: DM5PR06CA0090.namprd06.prod.outlook.com (2603:10b6:4:3a::31) To HE1PR0501MB2043.eurprd05.prod.outlook.com (2603:10a6:3:35::21) X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 80b7f895-fa42-426c-38eb-08d5e2b1625f X-MS-Office365-Filtering-HT: Tenant X-Microsoft-Antispam: UriScan:; BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989117)(5600053)(711020)(48565401081)(4534165)(4627221)(201703031133081)(201702281549075)(8990107)(2017052603328)(7153060)(7193020); SRVR:HE1PR0501MB2043; X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2043; 3:AOWXUu22KkixwJpsDcPviRsn9JNhF7k/gFJdFFc9Y9FdlJuXRoHyEZkTFtey92MuyY1o/KyQkOORyBQ/jEqyjsqpEWVGdVOMHgOqpv/QuMPbv9Ia5sldQ2DRzwufjaSQtZPPTpTmf4HJaRN9e3fv1DjrJNG/noM9m2hFIH2tnjr2os81v8iPRd7GDbTuWoKw1D0mp3E24cJ712NPPxF/2l4rtlnTS50wcFOCcyS62F3xGtxtOV0NgFvttozco89A; 25:iQnxO21pBs2ofpRan5szjLvxtgBuY1ojDSvBy6QaxYSoW42+PtdnLBQXHjUWaPO27PKB+Y7c6lAt4565mgLx9z1sfHussQMdX09nx+sjgw5gGDJqAFAlrPFVGWEJzmjSRQPG4/kO5Gj0lCMqkrlRlFBZcjq4Ejsk4AUcojadlea1kHnya+N6P0j19ciqffSPbirm+vCD934fBaynJTnUkNBt1+n+XJA+NDir2NseHqyubRuM6whG+xc8g69IiebOFVy+un3iDX7kD3c+Yw3JkcC6V9EuCtGD8G9DF1vAEG2v0796SzyBP0rqbh31SMAQlUIP5kBxXYzMIH15wH8nIQ==; 31:s4cboOuD49xq1u82XwpY75pgpU/Zl8MLPmatSkEJy0BBOxFcJKQ0Me0XUAlorHi0jSbJWvBWOVsVgjR6XVyKfWK/4sihXEz9B/Bvnhxb67G711J/xZ8/vXJ1NECuHd7FpbAowmiut+FJnzTJcNdb5Y146lRSUC95rgRVhCUcaEAhohVoU/xzQmpT467weCOWxw4GQArEdSyTSUTfrDaR37OUPx96Ay992clgWFpH9hg= X-MS-TrafficTypeDiagnostic: HE1PR0501MB2043: X-LD-Processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2043; 20:l2coU+Gcvi89/GN1zThoUjDCVNq1D123GjoHRR21cGzLYYwafHwfZhURxenTOPY+fKRVKnqplgqTMdBfVJnlUA75yzvGjtNqxxrhdgSDn7VcbjVVB3aVUiXoWrFRylr26MGDJ240QjsOhwJIw9+zGrbt6eTqrk04WfMwYREQG27tusvz7j8uRNfsS+eCfXzr0WounCNr1k9ysxcZx/+c6/nElw0qeFw8XbFaetzodJU9z5diH8a+IMQo1GV+mpYRIzq9+T4X4coscg1IC5b3xC75yuzDjOjAQkGIGVQinSSwCAe1V8e0IO7RcocdNWs53L4gqJpJjgLZ3uTx8JVGlpbiaIP73NWfLyJxakj3tBNE/0kvPsDbW11BPwpcotT3JDelIiYe/FvIRz7YoUBynBILYcAfZDJGV3sF7hy9U8n+mi14r4kt8fUu3JNyWTkXX+kpTzk6SyqbrOSIX3SRzE3/ybGzgdvTcYUc9X/Js4rkoR4Usb/VsDKrHtCL7Kwg; 4:24ncuJqu+chXhkH6zIP2urQmglCzojZFt4qH5l1juN1bw/HnZ+mhxCxIwxSF0itMTUOx4LMqpphoheNQ59385brq3taxuxb2XqD1dOTQyXAkAbdLlvVCRE99Bokk9ow/tqEPAzutd59wCT2xk/LybvPQB1FCe70bG6ZEWlzSGO7S+JIMRbnKb/JtZEOTE6Z+UyTKNxPFGeIRcOATq+niui/IBNVdV0+D2NiBRAw0oH8yGfqdFOfl4hJTxrVUNYYcixpSKErhcJKws2CQYZupnX0QiCJSA3C/JHW+NGNZnPtRvY7vBFZVZY0/wEY0IyJd X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(788757137089); X-MS-Exchange-SenderADCheck: 1 X-Exchange-Antispam-Report-CFA-Test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(8121501046)(5005006)(10201501046)(93006095)(93001095)(3002001)(3231254)(944501410)(52105095)(6055026)(149027)(150027)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123558120)(20161123564045)(20161123560045)(20161123562045)(6072148)(201708071742011)(7699016); SRVR:HE1PR0501MB2043; BCL:0; PCL:0; RULEID:; SRVR:HE1PR0501MB2043; X-Forefront-PRVS: 0724FCD4CD X-Forefront-Antispam-Report: SFV:NSPM; SFS:(10009020)(366004)(376002)(136003)(346002)(396003)(39860400002)(189003)(199004)(50466002)(316002)(7736002)(486006)(81166006)(478600001)(81156014)(229853002)(8676002)(86362001)(33656002)(956004)(68736007)(186003)(98436002)(14444005)(476003)(446003)(11346002)(58126008)(8936002)(26005)(93886005)(6916009)(6666003)(5660300001)(16526019)(25786009)(6506007)(106356001)(97736004)(386003)(305945005)(6246003)(1076002)(105586002)(3846002)(53936002)(23756003)(33896004)(9686003)(7696005)(2906002)(76176011)(52116002)(47776003)(55016002)(4326008)(66066001)(2870700001)(6116002)(18370500001)(309714004); DIR:OUT; SFP:1101; SCL:1; SRVR:HE1PR0501MB2043; 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: =?iso-8859-1?Q?1; HE1PR0501MB2043; 23:Ji2Lqd5IrVzWBm/iF9zI09ROljtNF36O3PUpa?= =?iso-8859-1?Q?jI2P2+v9aK2dtXOncEZjzeNtb62mzcr/+3lwMF7xTjc2SAloYq9/R3oxku?= =?iso-8859-1?Q?CEMy7+XYY6ALE5S9YkHQSiYIKd12GOVzvYWF/dhbPTXetj/XrSMegFBV4B?= =?iso-8859-1?Q?Ma8tLg1fDTprJiwwwuH0aPMkaxAPGtGWv6rTxiTCVxiNNLDhm0Rdt7wn7z?= =?iso-8859-1?Q?jU8PUwKtTkvPAePBspOCpmmOsKO3SiXPaqEN9JbCSvtVVoqE+8f8xCX4vl?= =?iso-8859-1?Q?E8E6X/OsUtntO930CJ85TWVEPHls+XqVyjKu7mX+YAzAtMArJ/3mpG8eM2?= =?iso-8859-1?Q?FHKu355PB3u/Ozeujrnnu9YXCNXYbN1kZi/RiY9XYZqBAPDT1doXjsAlNz?= =?iso-8859-1?Q?O89Ulxj0bgyDYIiLvPycgvmrYoQFrTOjEMy0X8nXimQ/RrP/hMPL8zvr1a?= =?iso-8859-1?Q?yP+U2calTC7+nPr939BNmZBevylqYWiobSGEpBjZfDpEld8Stsv5odNd+K?= =?iso-8859-1?Q?kypZlAsa4nnufHCyuYnOYZvugHdGrBpAXqbPpZTq4Ghuv6ZCWtDPdO5Vf8?= =?iso-8859-1?Q?u+AaU2NzG5uC+WGhqLVGxbmqClu9joVKh6xGIJkjcyn1bEG1AYVbrXtGNk?= =?iso-8859-1?Q?CbWEvfzlD310G15YolMzT/I463lT2a1lkMqImJvTg8dapjpxiZJ3z2BSIX?= =?iso-8859-1?Q?dXGZ8GcLOCQvRjh9cdmOxFqDQiXf/yvmboA3+V3bMZAy3yRIeqborwIUpq?= =?iso-8859-1?Q?bYVwwj9Vv6znr4WUg/E8QRIU/Ul9YgQid6tUu1Sf+jA18g9Rn5kWCC9mMJ?= =?iso-8859-1?Q?LlkHQ1kxnUTQxiaWrh+DoXsVcMT8fH7yym/bKJzQwdghK1+FBkxPBm5EHx?= =?iso-8859-1?Q?yEDdVx2W+zkF6+/hWfUBoIU/U1Rr5IazkBXQCwNPqx/MhsEEZ8iB5BwmNm?= =?iso-8859-1?Q?E+SQmwefY5WZER4Wmi4/PudWGWrfVX35Z0LcQckae3LGtcLN0g/B3SHUzz?= =?iso-8859-1?Q?5vHwLz6zQ6iS3Qg16gIbfUmBcs13w+yWlh8DQ4Q/95C3S/Kd2bsQGb232d?= =?iso-8859-1?Q?QMYlxCD46TmNZiiU5DD6rUbLpMK4U7HS5vwHfkcTQCzm0lmv9dMq8ZoJ0Q?= =?iso-8859-1?Q?XuOq2rnPGNCOaiLJ//HHjp0ILyG0LIf6iy5o+IjYrUS16o8Av0HyBEEvb9?= =?iso-8859-1?Q?r3sh9h9RuEEaSLauee1/9z0Pdggjuzgl4zCPOgPN65kt5Y6rFdJRJ0WztO?= =?iso-8859-1?Q?7NsLIv6LHUH5pzWxryo6H3vGR6tR248eLF6MfPfS4FPDQzAIgif3TFJo4O?= =?iso-8859-1?Q?92uObiHxTeb6rOg+vnQcPYUmyko81v18gl7qiIe/Ul1EG/sGkAJrSpxMzB?= =?iso-8859-1?Q?SUS1fe9udBGi5OA8KcVhX1g6Pde+WIFvQKQKBhcSB8b1AxsicZ2ew=3D?= =?iso-8859-1?Q?=3D?= X-Microsoft-Antispam-Message-Info: 7Tz+OUJON/jVavkCTU/ywO0Znl3SlYqR2DgICNokz9pyXnx/BiQyALvzzyR5RjiDkrNz8+Uxv1isYhrjMVWZpEwSOUj8ZlccEaN0/b3Jdi2D6+wW6yNoB++7GkdMmWsl4qsrUf3jY2GkcZTk1P4OBvVuvWs+1fO0DRV0K3iS4/SCUKqE8+WQM8DDCp7dFg6n/9ZEPowGF8vqcH1nBhaoipL28LdFeYIVFAnsbrgP/FKyAuip5vLd+NGeaB2xL/EEX+OdzLvxzIKKYRJRsb0t03FgLyQb70yEbArgNQngIJ5gVPjQeQoenIJ7zkY3NHcP6v66OWzrpc9PZeHrRcgVZzeEbdKZNl6W5T8XzeZwqUw= X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2043; 6:klkFWQStZX7F+iAv9fYYs+L8766g+xO0mOgxn6w263Zm4RhtrMT8odPomFUahFsSwpS5jDHt9cxW6/M9E3RwCaFM/vRLjqY7bYmKxEYLoU4ciH6SNabJ9vyjivO3unI3Ku/X6ia3eudEOWaZL5lXbfwJybONU1t4w36uRJT/yoMCXDmc5FVwRaBp61AVnoWoZ0yfFRdGPwe4AciRgDpvgmSQrc3muZO3JEBQjJxkXS4xhzpl1lsKje28e3f6z5woX+AlYXrK0QwzzY4LMsLpRwFS4NKRIwRZOIwLN9Sos3uRBvmgQFVI7L4ogk5de6CAOzzypVGmyn8HjK+FKvAztGe8ds6PXNendqyU1cVOYv2zK68ireE3kOl5fGLbZ3LN728f8yKX1A97RvMWaN3EIAgh0dD4/c7kpDN0FOAUvd7DgYKdt3GjVxfxoY/H9+Vp3FOvu2+4Yjj0DlRjD9C2Vw==; 5:OZS5oI0hoEfVGkeM68Zpk0uC2JiCdZGfYXFy0W2TIg6+kUkbYgVMPq2kDNq3O+yQbT0A2zmWJMhv1q6P2pFnoOef8D8FJxezQcta5V68/0jLSqN1pu+ylpdIGzW29shn4dMDGnWTujaP/pj3qB9IhrzRkLn6YoxGxz+6Wtt9vvU=; 24:hbXCDldBUWeylKQckuVxx7MdO9xmRVIV3G3fs/P4kG6IbQnNjSugfN3QXVL93zTENxM4Ar54NDxyYuHURj22gd4nKg0gkEqplnNsZx5wwio= SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1; HE1PR0501MB2043; 7:mkpXGZgP/7TxJsb0QIW4cwb+6J7UtCywnPd03M0vCOEithSOeMBRuEtWBG8N1+Sk0GKuOuyKmeF8e/3txnaOx1gGb/gZA5J7DYcwf/ttMYK08JD7psPj3Syw472vyDUBXp6qWqEXNohEavZ1mt1ZRl1LmIcYkLVV0YEer+SZITmFTZoQZD61yyH0fH3YjLBJ+sufKjellUuAndzolKsiHsYb5+0VQFSvlMSJezQPHslB+9O2jy37hptpuGdDJUKN X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2018 19:56:22.0651 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 80b7f895-fa42-426c-38eb-08d5e2b1625f X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: HE1PR0501MB2043 Subject: Re: [dpdk-dev] [PATCH v2 12/20] net/mlx5: add mark/flag flow action 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, 05 Jul 2018 19:56:26 -0000 On Thu, Jul 05, 2018 at 10:47:35AM +0200, Nélio Laranjeiro wrote: > On Wed, Jul 04, 2018 at 01:34:19AM -0700, Yongseok Koh wrote: > > On Wed, Jun 27, 2018 at 05:07:44PM +0200, Nelio Laranjeiro wrote: > > > Signed-off-by: Nelio Laranjeiro > > > --- > > > drivers/net/mlx5/mlx5_flow.c | 209 +++++++++++++++++++++++++++++++++++ > > > 1 file changed, 209 insertions(+) > > > > > > diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c > > > index 57f072c03..a39157533 100644 > > > --- a/drivers/net/mlx5/mlx5_flow.c > > > +++ b/drivers/net/mlx5/mlx5_flow.c > > > @@ -52,6 +52,10 @@ extern const struct eth_dev_ops mlx5_dev_ops_isolate; > > > #define MLX5_FLOW_FATE_DROP (1u << 0) > > > #define MLX5_FLOW_FATE_QUEUE (1u << 1) > > > > > > +/* Modify a packet. */ > > > +#define MLX5_FLOW_MOD_FLAG (1u << 0) > > > +#define MLX5_FLOW_MOD_MARK (1u << 1) > > > + > > > /** Handles information leading to a drop fate. */ > > > struct mlx5_flow_verbs { > > > unsigned int size; /**< Size of the attribute. */ > > > @@ -70,6 +74,8 @@ struct rte_flow { > > > struct rte_flow_attr attributes; /**< User flow attribute. */ > > > uint32_t layers; > > > /**< Bit-fields of present layers see MLX5_FLOW_ITEMS_*. */ > > > + uint32_t modifier; > > > + /**< Bit-fields of present modifier see MLX5_FLOW_MOD_*. */ > > > > Why do you think flag and mark modify a packet? I don't think modifier is an > > appropriate name. > > API terminology: "Actions that modify matching traffic contents or its > properties. This includes adding/removing encapsulation, encryption, > compression and marks." > > > > uint32_t fate; > > > /**< Bit-fields of present fate see MLX5_FLOW_FATE_*. */ > > > struct mlx5_flow_verbs verbs; /* Verbs flow. */ > > > @@ -954,6 +960,12 @@ mlx5_flow_action_drop(const struct rte_flow_action *actions, > > > actions, > > > "multiple fate actions are not" > > > " supported"); > > > + if (flow->modifier & (MLX5_FLOW_MOD_FLAG | MLX5_FLOW_MOD_MARK)) > > > + return rte_flow_error_set(error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > + actions, > > > + "drop is not compatible with" > > > + " flag/mark action"); > > > if (size < flow_size) > > > mlx5_flow_spec_verbs_add(flow, &drop, size); > > > flow->fate |= MLX5_FLOW_FATE_DROP; > > > @@ -1007,6 +1019,144 @@ mlx5_flow_action_queue(struct rte_eth_dev *dev, > > > return 0; > > > } > > > > > > +/** > > > + * Validate action flag provided by the user. > > > + * > > > + * @param actions > > > + * Pointer to flow actions array. > > > + * @param flow > > > + * Pointer to the rte_flow structure. > > > + * @param flow_size > > > + * Size in bytes of the available space for to store the flow information. > > > + * @param error > > > + * Pointer to error structure. > > > + * > > > + * @return > > > + * size in bytes necessary for the conversion, a negative errno value > > > + * otherwise and rte_errno is set. > > > > Like I asked for the previous patches, please be more verbose for function > > description and explanation of args and return value. > > I've update the documentation of all patches it would be strange to see > some with correct comments and some without :) > > > > + */ > > > +static int > > > +mlx5_flow_action_flag(const struct rte_flow_action *actions, > > > + struct rte_flow *flow, const size_t flow_size, > > > + struct rte_flow_error *error) > > > +{ > > > + unsigned int size = sizeof(struct ibv_flow_spec_action_tag); > > > + struct ibv_flow_spec_action_tag tag = { > > > + .type = IBV_FLOW_SPEC_ACTION_TAG, > > > + .size = size, > > > + .tag_id = mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT), > > > + }; > > > + > > > + if (flow->modifier & MLX5_FLOW_MOD_FLAG) > > > + return rte_flow_error_set(error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > + actions, > > > + "flag action already present"); > > > + if (flow->fate & MLX5_FLOW_FATE_DROP) > > > + return rte_flow_error_set(error, ENOTSUP, > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > + actions, > > > + "flag is not compatible with drop" > > > + " action"); > > > + if (flow->modifier & MLX5_FLOW_MOD_MARK) > > > + return 0; > > > + flow->modifier |= MLX5_FLOW_MOD_FLAG; > > > + if (size <= flow_size) > > > + mlx5_flow_spec_verbs_add(flow, &tag, size); > > > + return size; > > > +} > > > + > > > +/** > > > + * Update verbs specification to modify the flag to mark. > > > + * > > > + * @param flow > > > + * Pointer to the rte_flow structure. > > > + * @param mark_id > > > + * Mark identifier to replace the flag. > > > + */ > > > +static void > > > +mlx5_flow_verbs_mark_update(struct rte_flow *flow, uint32_t mark_id) > > > +{ > > > + struct ibv_spec_header *hdr; > > > + int i; > > > + > > > + /* Update Verbs specification. */ > > > + hdr = (struct ibv_spec_header *)flow->verbs.specs; > > > + for (i = 0; i != flow->verbs.attr->num_of_specs; ++i) { > > > > flow->verbs.attr/specs can be null in case of validation call. But you don't > > need to fix it because it is anyway changed and fixed when you add RSS action. > > You are right, but it still need to be fixed, if for some reason a > bisect is used this may break the bug research. > > > > + if (hdr->type == IBV_FLOW_SPEC_ACTION_TAG) { > > > + struct ibv_flow_spec_action_tag *t = > > > + (struct ibv_flow_spec_action_tag *)hdr; > > > + > > > + t->tag_id = mlx5_flow_mark_set(mark_id); > > > + } > > > + hdr = (struct ibv_spec_header *)((uintptr_t)hdr + hdr->size); > > > + } > > > +} > > > + > > > +/** > > > + * Validate action mark provided by the user. > > > + * > > > + * @param actions > > > + * Pointer to flow actions array. > > > + * @param flow > > > + * Pointer to the rte_flow structure. > > > + * @param flow_size[in] > > > + * Size in bytes of the available space for to store the flow information. > > > + * @param error > > > + * Pointer to error structure. > > > + * > > > + * @return > > > + * size in bytes necessary for the conversion, a negative errno value > > > + * otherwise and rte_errno is set. > > > + */ > > > +static int > > > +mlx5_flow_action_mark(const struct rte_flow_action *actions, > > > + struct rte_flow *flow, const size_t flow_size, > > > + struct rte_flow_error *error) > > > +{ > > > + const struct rte_flow_action_mark *mark = actions->conf; > > > + unsigned int size = sizeof(struct ibv_flow_spec_action_tag); > > > + struct ibv_flow_spec_action_tag tag = { > > > + .type = IBV_FLOW_SPEC_ACTION_TAG, > > > + .size = size, > > > + }; > > > + > > > + if (!mark) > > > + return rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > + actions, > > > + "configuration cannot be null"); > > > + if (mark->id >= MLX5_FLOW_MARK_MAX) > > > + return rte_flow_error_set(error, EINVAL, > > > + RTE_FLOW_ERROR_TYPE_ACTION_CONF, > > > + &mark->id, > > > + "mark must be between 0 and" > > > + " 16777199"); > > > > Use %d and (MLX5_FLOW_MARK_MAX - 1), instead of fixed string. > > It needs an snprintf, rte_flow_error_set() does not accept formatting > strings. I think the following would work but never mind. I'm okay with leaving it as is. No need to make a change here. #define STRINGIFY(x) #x #define TOSTRING(x) STRINGIFY(x) "mark must be between 0 and " TOSTRING(MLX5_FLOW_MARK_MAX - 1)); > >[...] > > > +/** > > > + * Mark the Rx queues mark flag if the flow has a mark or flag modifier. > > > + * > > > + * @param dev > > > + * Pointer to Ethernet device. > > > + * @param flow > > > + * Pointer to flow structure. > > > + */ > > > +static void > > > +mlx5_flow_rxq_mark(struct rte_eth_dev *dev, struct rte_flow *flow) > > > +{ > > > + struct priv *priv = dev->data->dev_private; > > > + > > > + (*priv->rxqs)[flow->queue]->mark |= > > > + flow->modifier & (MLX5_FLOW_MOD_FLAG | MLX5_FLOW_MOD_MARK); > > > > This has to be !!(...) as rxq->mark has only 1 bit. But, it is also fixed by > > coming RSS patches. Not sure what's benefit of splitting patches in this way. > > Same answer as above, even if fixed after, it still need a fix here. > > > > +} > > > + > > > /** > > > * Validate a flow supported by the NIC. > > > * > > > @@ -1281,6 +1456,7 @@ mlx5_flow_list_create(struct rte_eth_dev *dev, > > > if (ret < 0) > > > goto error; > > > } > > > + mlx5_flow_rxq_mark(dev, flow); > > > TAILQ_INSERT_TAIL(list, flow, next); > > > return flow; > > > error: > > > @@ -1323,8 +1499,31 @@ static void > > > mlx5_flow_list_destroy(struct rte_eth_dev *dev, struct mlx5_flows *list, > > > struct rte_flow *flow) > > > { > > > + struct priv *priv = dev->data->dev_private; > > > + struct rte_flow *rflow; > > > + const uint32_t mask = MLX5_FLOW_MOD_FLAG & MLX5_FLOW_MOD_MARK; > > > + int mark = 0; > > > + > > > mlx5_flow_fate_remove(dev, flow); > > > TAILQ_REMOVE(list, flow, next); > > > + if (!(flow->modifier & mask)) { > > > + rte_free(flow); > > > + return; > > > + } > > > + /* > > > + * When a flow is removed and this flow has a flag/mark modifier, all > > > + * flows needs to be parse to verify if the Rx queue use by the flow > > > + * still need to track the flag/mark request. > > > + */ > > > > When a flow is created, mlx5_flow_rxq_mark() is called. Is there a specific > > reason for not writing a separate function in order to drop rxq->mark bit? > > > > > + TAILQ_FOREACH(rflow, &priv->flows, next) { > > > + if (!(rflow->modifier & mask)) > > > + continue; > > > + if (flow->queue == rflow->queue) { > > > + mark = 1; > > > + break; > > > + } > > > + } > > > + (*priv->rxqs)[flow->queue]->mark = !!mark; > > > > mark can be either 0 or 1, then !!mark == mark anyway. > > > > > rte_free(flow); > > > } > > > > > > @@ -1358,10 +1557,19 @@ mlx5_flow_list_flush(struct rte_eth_dev *dev, struct mlx5_flows *list) > > > void > > > mlx5_flow_stop(struct rte_eth_dev *dev, struct mlx5_flows *list) > > > { > > > + struct priv *priv = dev->data->dev_private; > > > struct rte_flow *flow; > > > + unsigned int i; > > > + unsigned int idx; > > > > > > TAILQ_FOREACH_REVERSE(flow, list, mlx5_flows, next) > > > mlx5_flow_fate_remove(dev, flow); > > > + for (idx = 0, i = 0; idx != priv->rxqs_n; ++i) { > > > + if (!(*priv->rxqs)[idx]) > > > + continue; > > > + (*priv->rxqs)[idx]->mark = 0; > > > + ++idx; > > > + } > > > > Same question here but looks like this part is being moved to > > mlx5_flow_rxqs_clear() in the future. > > Addressing both question here, for the flow_stop() and flow_destroy() > the process is different, for the stop, the flow remains with the mark > bit set but all queues must me cleared, there is no comparison to make. > As you can see, it don't even get a flow, it directly unset the mask bit > in the Rx queues. > For the destroy the issue is different, several flows may be using the > same Rx queues, if one of them will remains and has a mark, then the > associated queues must keep their mark bit set. > As the process is different, it would end in two distinct functions and > each one used by a single function. > > For the mlx5_flow_rxq_mark(), the situation is different, the same > process is make when a flow is created and the flow are started. I knew the differences but I just wanted to ask if having a separate function can be a viable option, e.g., mlx5_flow_rxq_mark_set() mlx5_flow_rxq_mark_clear() mlx5_flow_rxq_mark_trim() Thanks, Yongseok