From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-DB5-obe.outbound.protection.outlook.com (mail-eopbgr40044.outbound.protection.outlook.com [40.107.4.44]) by dpdk.org (Postfix) with ESMTP id AAC321B1FE for ; Wed, 3 Oct 2018 15:55:32 +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=dlJbKBBSz9Vna9Fn5eaiQBCNjbH85lb18guCpgh0iNc=; b=Fk1ofrGb4xBcSVhAowY0zWqZ0FLt9JrtO/O73s/Ue4M1maysLKJfoesDTeg6s36EN8rvoBJYjisv4aj/8Bt/Zc6ZFmHYz9qPCJIWLVKvYyjaOiNfkk/z5ZuMLYNtd63gxWFyBWRSmsG/xZhPTPW4+v3vUNZcHf5PxVjO+V06Iy0= Received: from VI1PR05MB4224.eurprd05.prod.outlook.com (52.133.12.13) by VI1PR05MB1918.eurprd05.prod.outlook.com (10.166.44.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1207.21; Wed, 3 Oct 2018 13:55:29 +0000 Received: from VI1PR05MB4224.eurprd05.prod.outlook.com ([fe80::f911:fe8:927d:400b]) by VI1PR05MB4224.eurprd05.prod.outlook.com ([fe80::f911:fe8:927d:400b%3]) with mapi id 15.20.1207.018; Wed, 3 Oct 2018 13:55:29 +0000 From: Dekel Peled To: Yongseok Koh CC: "dev@dpdk.org" , Shahaf Shuler , Ori Kam Thread-Topic: [PATCH 3/4] net/mlx5: add DV encap L2 and L3 operations Thread-Index: AQHUWuZ81hz3HEl6d0WhwfOEWORtC6UNJfewgABAGNCAACUiQA== Date: Wed, 3 Oct 2018 13:55:28 +0000 Message-ID: References: <1538059845-35896-1-git-send-email-dekelp@mellanox.com> <1538059845-35896-4-git-send-email-dekelp@mellanox.com> <20181003065817.GD10028@mtidpdk.mti.labs.mlnx> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=dekelp@mellanox.com; x-originating-ip: [193.47.165.251] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; VI1PR05MB1918; 6:gwCKlcZAkGJdReUB0bKDy121Mt1ljbPGAHfytG0byApEt8PoGK2XHhsCZmEOUSLUJXwxoNdDySizprMebPXRXTk4+1qqrMOQo+C1dJ7jSBC463z9ZYef+8biwtBNDBh0EBLA0Xa5f/HV04AkyW5x6SvoxwREEn5Tczff33ORmpeOJuTI+M7qxiUeoC+JVYXUlU7s/7rrbZfuMu9GVkmYanAMTlKUnCXNSvCyH3b/FqYqMP7faD57n6eEgbJagax5Ru4QZ63QX8flC32/dMbpshX41wVCBwIv+NNdQjZoF7b20wtesPqF9Z6BXY4sjEc52vQgqHbmbmVG88oRDQN0jLY1MQp4OM/OuLhQqFzQU9+W5U9EvX3sPqqt9PB+YvYj42yl7hv82tkLotbdc0iL6Rh6ruUUn06/wldclexC0if3iR0Be3oaIUeACMHsQfqRV/Qh0i0nndKVVMHgGIaNhQ==; 5:Bj1PeQ3Xem41nhe2X/rEUgf2apkj2ZqhewwXgqWiodXyL6RgG3J0KXTvvlQnjlOayoFpi8Dk7XZOPr+49JX44bSryITZyGzIqVm16QidCSSjlm3Nd16cDyRdpXfu7CBk89fLKaGZN3PgbnIdFGqjGle0H3Kcpdc8YVkzh+LFRr8=; 7:5J7iJxVE/PBI3+EGbx1rWed6jJx/nisOZsxTrIRHHVwVmlIY2WkMlnOhFhJRFhPNXGkvL47zrBhrPV+iwrYU8n8/v+qqvtErHRoRbgLaewV2Kuo/2tBF36eUnFcvkUkrySKEjY0vS2tWGy9Sbakn63eFpZhIBTUTfADQ/V6DjID1h7/b6EGSI6KaORaobbpljUfQyUG9RX1i2JEQVgMFoW2/4WndtHDEZuKwaQecNQotl07zhupKIFYwM4PiYahR x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 4ba30e6c-7406-4cc3-4be4-08d62937e0d4 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600074)(711020)(4618075)(2017052603328)(7153060)(7193020); SRVR:VI1PR05MB1918; x-ms-traffictypediagnostic: VI1PR05MB1918: x-microsoft-antispam-prvs: x-exchange-antispam-report-test: UriScan:; x-ms-exchange-senderadcheck: 1 x-exchange-antispam-report-cfa-test: BCL:0; PCL:0; RULEID:(8211001083)(6040522)(2401047)(5005006)(8121501046)(3231355)(944501410)(52105095)(10201501046)(3002001)(93006095)(93001095)(6055026)(149066)(150057)(6041310)(20161123560045)(20161123564045)(20161123558120)(20161123562045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991048); SRVR:VI1PR05MB1918; BCL:0; PCL:0; RULEID:; SRVR:VI1PR05MB1918; x-forefront-prvs: 0814A2C7A3 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(396003)(39860400002)(346002)(376002)(366004)(13464003)(199004)(189003)(2900100001)(186003)(93886005)(5660300001)(81166006)(86362001)(106356001)(81156014)(105586002)(6436002)(66066001)(9686003)(8676002)(53946003)(33656002)(7736002)(305945005)(53936002)(76176011)(74316002)(2906002)(6116002)(8936002)(6506007)(6636002)(55016002)(25786009)(478600001)(4326008)(7696005)(486006)(53546011)(97736004)(316002)(107886003)(54906003)(6246003)(6862004)(476003)(68736007)(3846002)(11346002)(102836004)(229853002)(14454004)(5250100002)(93156006)(2940100002)(4744004)(446003)(14444005)(71190400001)(26005)(71200400001)(256004)(99286004); DIR:OUT; SFP:1101; SCL:1; SRVR:VI1PR05MB1918; H:VI1PR05MB4224.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; MX:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-microsoft-antispam-message-info: JY7rVnAOtwxOvRJ435AzFdAs+DvV4gclVcipAXeeCzC+THkvHIpq1ZwLdSJ1h/wPONYmKBrVHK+yTBN9nYkuN7mHJU13OdEre00Ho+ouvxsBOaansgQLvjsWAek0yL1DR7qENagEd0XB2ko3iDWeMLw7Dh2EgUKQQ6sUtoGr4EQLLvntD3xSFhAETJgp4So5bPpKalFccl3WaFAxZ37QSFsMicUiY54cThkniVjsPTjLLCbkpczaMS6VWsjBeZVp6Aly+d/3RtZk/3OKVu6TWNpD0b/wjKroOgKrriee4cPIv6x4shsAoKwV8XDwmS5erf69hs3UXlaOobPTiRllWPbGAbrtFGzCrtmF1kwIkNM= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 4ba30e6c-7406-4cc3-4be4-08d62937e0d4 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Oct 2018 13:55:29.0203 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR05MB1918 Subject: Re: [dpdk-dev] [PATCH 3/4] net/mlx5: add DV encap L2 and L3 operations 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: Wed, 03 Oct 2018 13:55:33 -0000 PSB > -----Original Message----- > From: Dekel Peled > Sent: Wednesday, October 3, 2018 2:47 PM > To: Yongseok Koh > Cc: dev@dpdk.org; Shahaf Shuler ; Ori Kam > > Subject: RE: [PATCH 3/4] net/mlx5: add DV encap L2 and L3 operations >=20 > One more issue, PSB. >=20 > > -----Original Message----- > > From: Dekel Peled > > Sent: Wednesday, October 3, 2018 11:35 AM > > To: Yongseok Koh > > Cc: dev@dpdk.org; Shahaf Shuler ; Ori Kam > > > > Subject: RE: [PATCH 3/4] net/mlx5: add DV encap L2 and L3 operations > > > > Thanks, PSB. > > > > > -----Original Message----- > > > From: Yongseok Koh > > > Sent: Wednesday, October 3, 2018 9:58 AM > > > To: Dekel Peled > > > Cc: dev@dpdk.org; Shahaf Shuler ; Ori Kam > > > > > > Subject: Re: [PATCH 3/4] net/mlx5: add DV encap L2 and L3 operations > > > > > > On Thu, Sep 27, 2018 at 05:50:44PM +0300, Dekel Peled wrote: > > > > This patch adds support for Direct Verbs encap operations, L2 and L= 3. > > > > > > > > Signed-off-by: Dekel Peled > > > > --- > > > > drivers/net/mlx5/mlx5_flow_dv.c | 249 > > > > +++++++++++++++++++++++++++++++++++++++- > > > > 1 file changed, 244 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c > > > > b/drivers/net/mlx5/mlx5_flow_dv.c index 1f3fcb8..50925ac 100644 > > > > --- a/drivers/net/mlx5/mlx5_flow_dv.c > > > > +++ b/drivers/net/mlx5/mlx5_flow_dv.c > > > > @@ -85,6 +85,178 @@ > > > > } > > > > > > > > /** > > > > + * Validate the tunnel encap action. > > > > + * > > > > + * @param[in] action_flags > > > > + * holds the actions detected until now. > > > > + * @param[in] action > > > > + * Pointer to the encap action. > > > > + * @param[in] attr > > > > + * Pointer to flow attributes > > > > + * @param[out] error > > > > + * Pointer to error structure. > > > > + * > > > > + * @return > > > > + * 0 on success, a negative errno value otherwise and rte_ernno = is > set. > > > > + */ > > > > +static int > > > > +mlx5_flow_validate_action_tunnel_encap(uint64_t action_flags, > > > > > > A static func in this file should start from flow_dv_. Or, you can > > > move these to mlx5_flow.c as general validation helpers. I prefer the > latter. > > > > Renamed functions to flow_dv_validate_action_tunnel_... > > Left it as static in mlx5_flow_dv.c, since it is relevant for DV only. > > > > > > > > > + const struct rte_flow_action *action, > > > > + const struct rte_flow_attr *attr, > > > > + struct rte_flow_error *error) { > > > > + const struct rte_flow_action_tunnel_encap *tunnel_encap =3D > > > > +action->conf; > > > > + > > > > + if (!tunnel_encap || !(tunnel_encap->buf)) > > > > + return rte_flow_error_set(error, EINVAL, > > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > > + action, > > > > > > Two lines above can be merged? Please check others and merge lines > > > as much as possible. > > > > Done. > > > > > > > > > + "configuration cannot be null"); > > > > + if (action_flags & MLX5_ACTION_DROP) > > > > + return rte_flow_error_set(error, > > > > + ENOTSUP, > > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > > + NULL, > > > > + "can't drop and encap in same > > > flow"); > > > > + if (action_flags & (MLX5_ACTION_TUNNEL_ENCAP | > > > > + MLX5_ACTION_TUNNEL_ENCAP_L3)) > > > > + return rte_flow_error_set(error, > > > > + ENOTSUP, > > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > > + NULL, > > > > + "can't have 2 encap actions in same" > > > > + " flow"); > > > > + if (attr->ingress) > > > > + return rte_flow_error_set(error, ENOTSUP, > > > > + > > > RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > > > > + NULL, > > > > + "encap action not supported for " > > > > + "ingress"); > > > > + return 0; > > > > +} > > > > + > > > > +/** > > > > + * Validate the tunnel encap L3 action > > > > + * > > > > + * @param[in] action_flags > > > > + * holds the actions detected until now. > > > > + * @param[in] action > > > > + * Pointer to the encap action. > > > > + * @param[in] attr > > > > + * Pointer to flow attributes > > > > + * @param[out] error > > > > + * Pointer to error structure. > > > > + * > > > > + * @return > > > > + * 0 on success, a negative errno value otherwise and rte_ernno = is > set. > > > > + */ > > > > +static int > > > > +mlx5_flow_validate_action_tunnel_encap_l3(uint64_t action_flags, > > > > + const struct rte_flow_action *action, > > > > + const struct rte_flow_attr *attr, > > > > + struct rte_flow_error *error) { > > > > + const struct rte_flow_action_tunnel_encap_l3 *tunnel_encap_l3 =3D > > > > + action->conf; > > > > > > Indentation. > > > > > > const struct rte_flow_action_tunnel_encap_l3 *tunnel_encap_l3 =3D > > > action->conf; > > > > Done. > > > > > > > > > + > > > > + if (!tunnel_encap_l3 || !(tunnel_encap_l3->buf)) > > > > + return rte_flow_error_set(error, EINVAL, > > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > > + action, > > > > + "configuration cannot be null"); > > > > + if (action_flags & MLX5_ACTION_DROP) > > > > + return rte_flow_error_set(error, > > > > + ENOTSUP, > > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > > + NULL, > > > > + "can't drop and encap in same > > > flow"); > > > > + if (action_flags & (MLX5_ACTION_TUNNEL_ENCAP | > > > > + MLX5_ACTION_TUNNEL_ENCAP_L3)) > > > > + return rte_flow_error_set(error, > > > > + ENOTSUP, > > > > + RTE_FLOW_ERROR_TYPE_ACTION, > > > > + NULL, > > > > + "can't have 2 encap actions in same" > > > > + " flow"); > > > > + if (attr->ingress) > > > > + return rte_flow_error_set(error, ENOTSUP, > > > > + > > > RTE_FLOW_ERROR_TYPE_ATTR_INGRESS, > > > > + NULL, > > > > + "encap action not supported for " > > > > + "ingress"); > > > > + return 0; > > > > +} > > > > + > > > > +/** > > > > + * Convert encap action to DV specification. > > > > + * > > > > + * @param[in] action > > > > + * Pointer to action structure. > > > > + * @param[in] ctx > > > > + * Verbs context. > > > > + * @param[out] error > > > > + * Pointer to the error structure. > > > > + * > > > > + * @return > > > > + * Pointer to action on success, NULL otherwise and rte_errno is= set. > > > > + */ > > > > +static struct ibv_flow_action * > > > > +mlx5_flow_dv_create_encap(const struct rte_flow_action *action, > > > > > > Change to flow_dv_create_encap() > > > > Done. > > > > > > > > > + struct ibv_context *ctx, > > > > + struct rte_flow_error *error) { > > > > + struct ibv_flow_action *encap_verb =3D NULL; > > > > + const struct rte_flow_action_tunnel_encap *encap_data; > > > > + > > > > + encap_data =3D (const struct rte_flow_action_tunnel_encap > > > > +*)action- > > > >conf; > > > > + encap_verb =3D mlx5_glue- > > > >dv_create_flow_action_packet_reformat(ctx, > > > > + encap_data->size, > > > > + encap_data->size ? encap_data->buf : > > > > + NULL, > > > > + > > > MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TO_L2_TU > > > NNEL, > > > > + MLX5DV_FLOW_TABLE_TYPE_NIC_TX); > > > > > > Indentation. > > > > I'm using very long MLX5DV_... names defined in rdma-core. > > If I use the required indentation I get illegal line length. > > > > > > > > encap_verb =3D mlx5_glue->dv_create_flow_action_packet_reformat > > > (ctx, encap_data->size, > > > encap_data->size ? encap_data->buf : NULL, > > > > > > > MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TO_L2_TUNNEL, > > > MLX5DV_FLOW_TABLE_TYPE_NIC_TX); > > > > > > Please make the same change to others. > > > > > > > + if (!encap_verb) > > > > + rte_flow_error_set(error, EINVAL, > > > RTE_FLOW_ERROR_TYPE_ACTION, > > > > + NULL, "cannot create encap action"); > > > > + return encap_verb; > > > > +} > > > > + > > > > +/** > > > > + * Convert encap L3 action to DV specification. > > > > + * > > > > + * @param[in] action > > > > + * Pointer to action structure. > > > > + * @param[in] ctx > > > > + * Verbs context. > > > > + * @param[out] error > > > > + * Pointer to the error structure. > > > > + * > > > > + * @return > > > > + * Pointer to action on success, NULL otherwise and rte_errno is= set. > > > > + */ > > > > +static struct ibv_flow_action * > > > > +mlx5_flow_dv_create_encap_l3(const struct rte_flow_action > > > > +*action, > > > > > > Change to flow_dv_create_encap_l3() > > > > Done. > > > > > > > > > + struct ibv_context *ctx, > > > > + struct rte_flow_error *error) { > > > > + struct ibv_flow_action *encap_l3_verb =3D NULL; > > > > + const struct rte_flow_action_tunnel_encap_l3 *encap_l3_data; > > > > + > > > > + encap_l3_data =3D > > > > + (const struct rte_flow_action_tunnel_encap_l3 *)action- > > > >conf; > > > > + encap_l3_verb =3D mlx5_glue- > > > >dv_create_flow_action_packet_reformat(ctx, > > > > + encap_l3_data->size, > > > > + encap_l3_data->size ? encap_l3_data->buf : > > > > + NULL, > > > > + > > > MLX5DV_FLOW_ACTION_PACKET_REFORMAT_TYPE_L2_TO_L3_TU > > > NNEL, > > > > + MLX5DV_FLOW_TABLE_TYPE_NIC_TX); > > > > + if (!encap_l3_verb) > > > > + rte_flow_error_set(error, EINVAL, > > > RTE_FLOW_ERROR_TYPE_ACTION, > > > > + NULL, "cannot create encap L3 action"); > > > > + return encap_l3_verb; > > > > +} > > > > + > > > > +/** > > > > * Verify the @p attributes will be correctly understood by the > > > > NIC and > > > store > > > > * them in the @p flow if everything is correct. > > > > * > > > > @@ -337,6 +509,26 @@ > > > > action_flags |=3D MLX5_ACTION_COUNT; > > > > ++actions_n; > > > > break; > > > > + case RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP: > > > > + ret =3D mlx5_flow_validate_action_tunnel_encap > > > > + (action_flags, > > > > + actions, > > > > + attr, > > > > + error); > > > > > > Can be one line. > > > > Done. > > > > > > > > > + if (ret < 0) > > > > + return ret; > > > > + action_flags |=3D MLX5_ACTION_TUNNEL_ENCAP; > > > > + break; > > > > + case RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP_L3: > > > > + ret =3D mlx5_flow_validate_action_tunnel_encap_l3 > > > > + (action_flags, > > > > + actions, > > > > + attr, > > > > + error); > > > > + if (ret < 0) > > > > + return ret; > > > > + action_flags |=3D MLX5_ACTION_TUNNEL_ENCAP_L3; > > > > + break; > > > > default: > > > > return rte_flow_error_set(error, ENOTSUP, > > > > > > > RTE_FLOW_ERROR_TYPE_ACTION, > > > > @@ -1047,10 +1239,19 @@ > > > > * Flow action to translate. > > > > * @param[in, out] dev_flow > > > > * Pointer to the mlx5_flow. > > > > + * @param[in] ctx > > > > + * Verbs context. > > > > + * @param[out] error > > > > + * Pointer to the error structure. > > > > + * > > > > + * @return > > > > + * 0 on success, a negative errno value otherwise and rte_ernno = is > set. > > > > */ > > > > -static void > > > > +static int > > > > flow_dv_create_action(const struct rte_flow_action *action, > > > > - struct mlx5_flow *dev_flow) > > > > + struct mlx5_flow *dev_flow, > > > > + struct ibv_context *ctx, > > > > > > If it is just priv->ctx, it would be better to get dev as an arg and > > > make mlx5_flow_dv_create_encap*(dev, ...) gets priv->ctx from dev. > > > > I considered it during implementation, but preferred to give the > > functions only what they need. > > > > > > > > > + struct rte_flow_error *error) > > > > { > > > > const struct rte_flow_action_queue *queue; > > > > const struct rte_flow_action_rss *rss; @@ -1095,10 +1296,35 @@ > > > > /* Added to array only in apply since we need the QP */ > > > > flow->actions |=3D MLX5_ACTION_RSS; > > > > break; > > > > + case RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP: > > > > + dev_flow->dv.actions[actions_n].type =3D > > > > + MLX5DV_FLOW_ACTION_IBV_FLOW_ACTION; > > > > + dev_flow->dv.actions[actions_n].action =3D > > > > + mlx5_flow_dv_create_encap(action, ctx, error); > > > > + if (!(dev_flow->dv.actions[actions_n].action)) > > > > + return -rte_errno; > > > > + dev_flow->dv.encap_verb =3D > > > > + dev_flow->dv.actions[actions_n].action; > > > > + flow->actions |=3D MLX5_ACTION_TUNNEL_ENCAP; > > > > + actions_n++; > > > > + break; > > > > + case RTE_FLOW_ACTION_TYPE_TUNNEL_ENCAP_L3: > > > > + dev_flow->dv.actions[actions_n].type =3D > > > > + MLX5DV_FLOW_ACTION_IBV_FLOW_ACTION; > > > > + dev_flow->dv.actions[actions_n].action =3D > > > > + mlx5_flow_dv_create_encap_l3(action, ctx, error); > > > > + if (!(dev_flow->dv.actions[actions_n].action)) > > > > + return -rte_errno; > > > > + dev_flow->dv.encap_verb =3D > > > > + dev_flow->dv.actions[actions_n].action; > > > > + flow->actions |=3D MLX5_ACTION_TUNNEL_ENCAP_L3; > > > > + actions_n++; > > > > + break; > > > > default: > > > > break; > > > > } > > > > dev_flow->dv.actions_n =3D actions_n; > > > > + return 0; > > > > } > > > > > > > > static uint32_t matcher_zero[MLX5_ST_SZ_DW(fte_match_param)] =3D { > > > > 0 }; @@ -1247,6 +1473,7 @@ > > > > }; > > > > void *match_value =3D dev_flow->dv.value.buf; > > > > uint8_t inner =3D 0; > > > > + int retval =3D 0; > > > > > > > > if (priority =3D=3D MLX5_FLOW_PRIO_RSVD) > > > > priority =3D priv->config.flow_prio - 1; @@ -1262,9 +1489,14 > > > @@ > > > > matcher.egress =3D attr->egress; > > > > if (flow_dv_matcher_register(dev, &matcher, dev_flow, error)) > > > > return -rte_errno; > > > > - for (; actions->type !=3D RTE_FLOW_ACTION_TYPE_END; actions++) > > > > - flow_dv_create_action(actions, dev_flow); > > > > - return 0; > > > > + for (; actions->type !=3D RTE_FLOW_ACTION_TYPE_END; actions++) { > > > > + retval =3D > > > > + flow_dv_create_action(actions, dev_flow, priv->ctx, error); > > > > + if (retval) > > > > + break; > > > > > > The following looks better matched? > > > > > > if (flow_dv_create_action(dev, actions, dev_flow, error)) > > > return -rte_errno; > > > > > > And leave 'return 0;' as is below. > > > > OK. > > > > > > > > BTW, flow_dv_matcher_register() allocates mlx5_flow_dv_matcher or > > > increase the refcnt. Then, if it fails here, doesn't it need to clean= that up? >=20 > Missed this in the first time. > Moved the create_action loop earlier, right after the create_item loop, a= nd > before the matcher_register call. >=20 Please ignore previous comment. If the create_action fails, the destroy function will clean up the matcher = allocations as well. > > > > > > > + } > > > > + > > > > > > No blank line is allowed. Please fix other occurrences too. > > > > Done. > > > > > > > > > + return retval; > > > > } > > > > > > > > /** > > > > @@ -1447,6 +1679,13 @@ > > > > LIST_REMOVE(dev_flow, next); > > > > if (dev_flow->dv.matcher) > > > > flow_dv_matcher_release(dev, dev_flow- > > dv.matcher); > > > > + > > > > + if (dev_flow->dv.encap_verb) { > > > > + claim_zero(mlx5_glue->destroy_flow_action > > > > + (dev_flow->dv.encap_verb)); > > > > + dev_flow->dv.encap_verb =3D NULL; > > > > + } > > > > + > > > > rte_free(dev_flow); > > > > } > > > > } > > > > -- > > > > 1.8.3.1 > > > >