From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-AM5-obe.outbound.protection.outlook.com (mail-eopbgr30063.outbound.protection.outlook.com [40.107.3.63]) by dpdk.org (Postfix) with ESMTP id EC30E2C2B for ; Wed, 3 Oct 2018 08:58:28 +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=ss6Jr0qPZJv+sSZXNcXHx/oGTYK3KDh2i9Z9zIrkVPk=; b=ZfCB2kzAgf7UNQ7kHI5HbdfOP7zibt/pPOyB3Q412NtuZ7jCQC05fsBn5G93B88iY1SQu093TFAeFJszJeHx2TWZ8+lSo8fqcL9wKcFJZxzjSeRo+2SIR+w7Byd81Kp5rP0cuvt9f9uAvX8VY3RWKISQ2Rut3DDVgkcCpfOpu4k= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB3947.eurprd05.prod.outlook.com (52.134.71.159) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1207.18; Wed, 3 Oct 2018 06:58:27 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::1cb0:661b:ecab:6045]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::1cb0:661b:ecab:6045%2]) with mapi id 15.20.1185.024; Wed, 3 Oct 2018 06:58:27 +0000 From: Yongseok Koh To: Dekel Peled CC: "dev@dpdk.org" , Shahaf Shuler , Ori Kam Thread-Topic: [PATCH 3/4] net/mlx5: add DV encap L2 and L3 operations Thread-Index: AQHUVnHb/3Y5788/wEyRhIVdU53mo6UNIAkA Date: Wed, 3 Oct 2018 06:58:27 +0000 Message-ID: <20181003065817.GD10028@mtidpdk.mti.labs.mlnx> References: <1538059845-35896-1-git-send-email-dekelp@mellanox.com> <1538059845-35896-4-git-send-email-dekelp@mellanox.com> In-Reply-To: <1538059845-35896-4-git-send-email-dekelp@mellanox.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-clientproxiedby: DM6PR08CA0021.namprd08.prod.outlook.com (2603:10b6:5:80::34) To DB3PR0502MB3980.eurprd05.prod.outlook.com (2603:10a6:8:10::27) authentication-results: spf=none (sender IP is ) smtp.mailfrom=yskoh@mellanox.com; x-ms-exchange-messagesentrepresentingtype: 1 x-originating-ip: [209.116.155.178] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB3PR0502MB3947; 6:VfFzdjicv7HNxbNGUFX7qRqCRChOTCs79t++IXr9/tezdMNQQAzaMZ9CQDd1aimptvRHAxusliMgR90xnwVwRvinX/9YA5T+zXJcFOpdKw9OGEd98FLZw4ogM6Up2E19uJlvJh9C+o6NZnrV2VAj8sj6/WDSeKNSfzeFVlmneF6pSiJ7kBw6xDgt8rVn0JzN+yNGn5DP+BQrJHp8+wc5ydPX2wDTKYqfNeniyQnxZ33MrM0BkGJSUz2T1NcXLgmiLLCLHMORuK+TOlfqdrieW4EjTE5MP3vjlsPnPlQRfu103ooSpqB/PXH+nj8GIvMu3sRIn6bPN8W1mQiQ1QxhSPZfdlu660oCfg5pEO3MSk1fG6tvJ6zjhP4nK8IjXgojF2dORcLYE9jcgGMEZSCN2+NmUGzs3XWRFrk8H/by6JfF/1Z33M2I+/e0BzZIeBIwg/Ec6sJlgak9NCzzRgMH/Q==; 5:fqoHTRC3rVbOjbx8B7ni6KewvHsfHCGXO/G8AqstXjwSA6BJ33DcPJ2G0dNWuAd2VyqNRnwvm5GW7JroxpnYI88N2D/idONEpKIMsAfHH3BClvtAd+6Nt+XRIq5EItXWnZvOHuOxHpt3OTYuA5RRdUwAzXoDBFf2kbCYJEXQxa4=; 7:t5N2Qu/f5V919TJO94KWhzrI/VrqxXNJUYJVnbrYJ6QiwcRpxYEC81CfNt1EiyYDlFZQUWzpoLL1amLHPRPQwjtqWOpQyVX9xoftr7dfm0BfuO+ZRmzeE8Ysfp9Vufo7Z5HPDLXo846gd6vEA70locYFjNQG+liUeT/ElMLPqB4UKzMtwHjbw/Kr7xRR0zSDFNqLkqqS/T3DwONWbs8PDJn7KIcHfgAXWoqsE+UhDxkFX7Hjp7/hSr8jCbZZzup6 x-ms-office365-filtering-correlation-id: e4dd1d0b-5cd0-488f-c40b-08d628fd9e32 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:DB3PR0502MB3947; x-ms-traffictypediagnostic: DB3PR0502MB3947: 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)(10201501046)(3231355)(944501410)(52105095)(93006095)(93001095)(3002001)(6055026)(149066)(150057)(6041310)(20161123560045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123558120)(201708071742011)(7699051); SRVR:DB3PR0502MB3947; BCL:0; PCL:0; RULEID:; SRVR:DB3PR0502MB3947; x-forefront-prvs: 0814A2C7A3 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(396003)(39860400002)(376002)(366004)(136003)(189003)(199004)(316002)(105586002)(106356001)(5660300001)(2900100001)(3846002)(66066001)(6116002)(71190400001)(1076002)(8936002)(71200400001)(68736007)(97736004)(5250100002)(86362001)(81156014)(81166006)(2906002)(6636002)(8676002)(33656002)(4744004)(4326008)(107886003)(6436002)(6862004)(99286004)(6486002)(486006)(446003)(11346002)(476003)(53936002)(54906003)(6246003)(9686003)(6512007)(6506007)(102836004)(386003)(14444005)(33896004)(478600001)(256004)(305945005)(52116002)(25786009)(229853002)(186003)(14454004)(7736002)(26005)(76176011); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB3947; H:DB3PR0502MB3980.eurprd05.prod.outlook.com; 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-antispam-message-info: aY2Clc7w4amQ+ILnuNJEEX1Nsx47WQbF2nSvcSeZm7g0MGb+oDyDtfvOUf42I5bfSnqcAlxDc7p1bGKN8fa/pA6/lyvZD2m0GwZT1xJMTQKm5TR4ActPl1qgCn4x25Y7dXV5nPouiMLtxRgQ1E5XbyVogg9hQGLXSZVWCAZIP9MaOPk8XltFbw8NQO20YtP1IvQkXLmyoFybqHagNRohPijRNWB79kUbpyLnZwV4SaUf9DB+EnKSBqA1jWYWtHZMYsemmkJkfQoq6l7xui5K1yrIGLXsrEUFss47nQI+ak8edqwunRLgOv0Wj2E/wznnEMk0HIo3e+ktPwvkJG+dWW1BzSimdxvOMRD8SpGf4xc= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: <96D8A1AC816D8F4BB435823BEB59BF56@eurprd05.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: e4dd1d0b-5cd0-488f-c40b-08d628fd9e32 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Oct 2018 06:58:27.1422 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB3947 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 06:58:29 -0000 On Thu, Sep 27, 2018 at 05:50:44PM +0300, Dekel Peled wrote: > This patch adds support for Direct Verbs encap operations, L2 and L3. >=20 > Signed-off-by: Dekel Peled > --- > drivers/net/mlx5/mlx5_flow_dv.c | 249 ++++++++++++++++++++++++++++++++++= +++++- > 1 file changed, 244 insertions(+), 5 deletions(-) >=20 > 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 @@ > } > =20 > /** > + * 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 the= se to mlx5_flow.c as general validation helpers. I prefer the latter. > + 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->con= f; > + > + 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. > + "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; > + > + 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() > + 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->con= f; > + 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); Indentation. 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() > + 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_TUNNEL, > + 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. > + 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. > + 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; > } > =20 > 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; > =20 > 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. 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? > + } > + No blank line is allowed. Please fix other occurrences too. > + return retval; > } > =20 > /** > @@ -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); > } > } > --=20 > 1.8.3.1 >=20