From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-AM5-obe.outbound.protection.outlook.com (mail-eopbgr00061.outbound.protection.outlook.com [40.107.0.61]) by dpdk.org (Postfix) with ESMTP id F0BD7A49 for ; Thu, 1 Nov 2018 01:30:51 +0100 (CET) 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=El4GBawDyr3SSvJ/Xgsxjg72HNHSpWtKodr/r3tYgqE=; b=WCgIge9L7Sr8LlGiyvv9Eb61bjUOsxQiB6fmPh5lANhdC0RYq/LTh87c3rI5FQW4pWCnyf7adqLWSo0eoErke15oKK8MzWAKVZ7DXkdNNtVdBGZwq/6C8ksHOrj3hFuOIMeEiX3jJ5Pr+DSjfiMomMRTr/or3BXTG7RDtcz7/jc= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB4091.eurprd05.prod.outlook.com (52.134.68.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1273.26; Thu, 1 Nov 2018 00:30:45 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::f8a1:fcab:94f0:97cc]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::f8a1:fcab:94f0:97cc%4]) with mapi id 15.20.1273.030; Thu, 1 Nov 2018 00:30:45 +0000 From: Yongseok Koh To: Ori Kam CC: Shahaf Shuler , "dev@dpdk.org" Thread-Topic: [PATCH] net/mlx5: fix Direct Verbs getting item and action flags Thread-Index: AQHUbuSH+I7dIxYbCkyDfM3wP2/ck6U1ua5ggADMCACAAnHOEIABHvMA Date: Thu, 1 Nov 2018 00:30:45 +0000 Message-ID: <471C5307-7241-4E38-A5FC-8B24034826CD@mellanox.com> References: <20181028173440.27808-1-yskoh@mellanox.com> 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=yskoh@mellanox.com; x-originating-ip: [209.116.155.178] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB3PR0502MB4091; 6:JAO6F65xW6PzmxsCLSaRZh6uModTHmTtIrtxPDgWkLk8WKFa4nd7DeEAViBQEmEKUKRxm8aQFu77MO0H1ag0gDF+6iuf7BNgzPGdwgf0iE+1idoBjOY25hP4QbolaQM60qUJlXKOX2hhXcr0V0RHFbrBrNmrjUkIQo9fHjV3hm1NH5E4/km4npLqfAm8DRbp6qPfwRGOwulWfKBtFmATQACd/6d2KZY0XIAFm2UoCAuJ2bl5HT495t6r+7+M6dqbp+3ZbTaTo65Wa1o3wBP589a9WuS+02PJeZnnOB5MJJWZtvjFgUS15yaTBUDfhptWVFHm8vI0E8+j7KYzwm5sZiuP1gU2u7H5RQEHAAWtWfX0A8iWqELh7jx1KRNObJELcUtwRzP0QFBpToVkShVke+BMa83cKE/ZwkwW8znMMbqBuG3JsQbTXsBPgUbsm8Qlg3SXS3yShV9GrByZ7iit0g==; 5:wAf0foN2kDHsk+OK19E8X/OExqNeWkGavpWhWaLNPqOK1iRqpqbhZ4yrmSOvYxfZg5fwi0RvKVU7wFw4Oq/3h0l7Vi4vqzQ7SLsKIzsXx37KHWKPWXKkRauVisfORq26jZb7CjOjtHMaXJPYte9/Yv5XkBMRe9w9G48vzb+txac=; 7:iiKRkE94Pk0CvksqvGNOHwY/tLZ3ReGt4sHSbonoShbaeiriIy9ZLvbf4Rs7QjFNx2+Ymn+1bIIAw1yUHwSbV2mKrIcU24NSoKovGb1tXtJjhE5+5/zdM13bmWC4S4k8J0XyQKbJ8CnEIMba2KDVBQ== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: 6956b409-c873-4061-70d0-08d63f914392 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:DB3PR0502MB4091; x-ms-traffictypediagnostic: DB3PR0502MB4091: 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)(3231382)(944501410)(52105095)(3002001)(10201501046)(93006095)(93001095)(6055026)(148016)(149066)(150057)(6041310)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123560045)(20161123564045)(20161123558120)(201708071742011)(7699051)(76991095); SRVR:DB3PR0502MB4091; BCL:0; PCL:0; RULEID:; SRVR:DB3PR0502MB4091; x-forefront-prvs: 0843C17679 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(39860400002)(366004)(376002)(136003)(396003)(13464003)(52314003)(51234002)(199004)(189003)(54906003)(229853002)(105586002)(316002)(106356001)(37006003)(8936002)(26005)(93886005)(66066001)(6512007)(97736004)(53936002)(102836004)(36756003)(5250100002)(6436002)(6506007)(6486002)(53546011)(14444005)(256004)(71200400001)(71190400001)(83716004)(486006)(8676002)(14454004)(186003)(81156014)(81166006)(2900100001)(33656002)(2616005)(82746002)(476003)(11346002)(446003)(5660300001)(4326008)(478600001)(305945005)(6246003)(7736002)(99286004)(76176011)(68736007)(86362001)(575784001)(25786009)(3846002)(6862004)(6116002)(2906002)(6636002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB4091; H:DB3PR0502MB3980.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: 2sHSZzrlnLhI9eRqy1vn9u5MX2kpYwsb1ladIAhdevn5xvHFKX7fk+LSw4M0eD9bGTOnvtZo+LRe3h9chNAu1c84TbtCgf6OeQbvY0va1UZ1ha/cBKhbMJ1Ikr5AvA7ReDSwIFRPc6vZaTTLvF4GMvW2YNhvRCwI1k3y5iWtykUUCUF157tyzYW59S+vld8+htUBYYaNXDBeIlIQfPzhVbSVSE0FRpjY9+5MERjjM8dObX2XT7Zpv38hN5oO8dwM+8iGpu/KEUd8U2BxRdR2PrnSid51hI7apCvjqITcmKeJcNDuLZLy13OlIZ6RrvkITKeIcnuG+Q/NMg3+7uzKkmGx6kJfxcYrL1zxbbyRwgE= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: <729FC5AB10563741AD141ACD682A4698@eurprd05.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 6956b409-c873-4061-70d0-08d63f914392 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Nov 2018 00:30:45.5071 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB4091 Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix Direct Verbs getting item and action flags 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, 01 Nov 2018 00:30:52 -0000 > On Oct 31, 2018, at 1:25 AM, Ori Kam wrote: >=20 > Hi, > PSB >=20 >> -----Original Message----- >> From: Yongseok Koh >> Sent: Monday, October 29, 2018 8:04 PM >> To: Ori Kam >> Cc: Shahaf Shuler ; dev@dpdk.org >> Subject: Re: [PATCH] net/mlx5: fix Direct Verbs getting item and action = flags >>=20 >>> On Oct 28, 2018, at 11:03 PM, Ori Kam wrote: >>>=20 >>> Why should DV prepare function return the list of actions? >>>=20 >>> The only reason I can think of, is if you want to remove the for loop i= n >>> dv_translate. And then in flow_dv_create_action change the switch case >>> to ifs. >>=20 >> Then, I should ask you a question. Why did you put actions/layers in rte= _flow >> struct in the first place? And _prepare() API even takes pointers of ite= m_flags >> and actions_flags in order to fill in the structs for nothing. Verbs and= TCF >> fills in the structs but only DV doesn't. I think you just wanted to use >> flow->actions for _apply() and it is filled in _translate(). But, in cas= e of >> TCF, _apply() doesn't need to know the action_flags and its _translate d= oesn't >> even fill in the struct while its _prepare() does in vain. >> Where's the consistency? What is the definition of the APIs? >=20 > According to design prepare function is responsible for allocating the fl= ow. > In case of Verbs the allocation size is dependent on the number of action= s, > while in Direct Verbs the size is fixed. This is the reason for the diffe= rence. > If it helps one can add the scan for items and actions. Currently is was = not needed > in case of Direct Verbs maybe it can help for example for Dekel patches w= ere to commands > (encap and encap) should sometime be combined in to one command. > Also maybe in future moving to Direct Steering it will be useful to save = the actions based on the > real number of actions. I knew that. That doesn't explain why _prepare() gets item_flags and action_flags to fil= l in, because it isn't used anyway after _prepare() call. And I want to see more consiste= ncy here. "Someone will add it later if needed" sounds bad. Anyway, I'll push more fixes together related to this flag handling because= it breaks tunnel flow badly. I'll also refactor many of code lines in DV and Verbs. I have changed this patch as 'superseded'. Thanks, Yongseok >> If people started to use the fields (because it is there), it won't work= as >> expected. Slava wants to use the flags in _translate() for his vxlan pat= ch. >>=20 >> So, I think _prepare is the right place to fill in the flags. >>=20 >> Let me know how you want to change it. >>=20 >>=20 >> Thanks, >> Yongseok >>=20 >>>=20 >>>> -----Original Message----- >>>> From: Yongseok Koh >>>> Sent: Sunday, October 28, 2018 7:35 PM >>>> To: Shahaf Shuler >>>> Cc: dev@dpdk.org; Yongseok Koh ; Ori Kam >>>> >>>> Subject: [PATCH] net/mlx5: fix Direct Verbs getting item and action fl= ags >>>>=20 >>>> Flow driver has to provide detected item flags and action flags via >>>> flow_drv_prepare(). DV doesn't return flags at all. >>>>=20 >>>> Fixes: 865a0c15672c ("net/mlx5: add Direct Verbs prepare function") >>>> Cc: orika@mellanox.com >>>>=20 >>>> Signed-off-by: Yongseok Koh >>>> --- >>>> drivers/net/mlx5/mlx5_flow_dv.c | 115 >>>> ++++++++++++++++++++++++++++++++++++---- >>>> 1 file changed, 106 insertions(+), 9 deletions(-) >>>>=20 >>>> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c >>>> b/drivers/net/mlx5/mlx5_flow_dv.c >>>> index 8f729f44f8..67c133c2fb 100644 >>>> --- a/drivers/net/mlx5/mlx5_flow_dv.c >>>> +++ b/drivers/net/mlx5/mlx5_flow_dv.c >>>> @@ -354,6 +354,103 @@ flow_dv_validate(struct rte_eth_dev *dev, const >>>> struct rte_flow_attr *attr, >>>> } >>>>=20 >>>> /** >>>> + * Extract item flags and action flags. >>>> + * >>>> + * @param[in] items >>>> + * Pointer to the list of items. >>>> + * @param[in] actions >>>> + * Pointer to the list of actions. >>>> + * @param[out] item_flags >>>> + * Pointer to bit mask of all items detected. >>>> + * @param[out] action_flags >>>> + * Pointer to bit mask of all actions detected. >>>> + */ >>>> +static void >>>> +flow_dv_get_item_action_flags(const struct rte_flow_item items[], >>>> + const struct rte_flow_action actions[], >>>> + uint64_t *item_flags, uint64_t *action_flags) >>>> +{ >>>> + uint64_t detected_items =3D 0; >>>> + uint64_t detected_actions =3D 0; >>>> + int tunnel; >>>> + >>>> + for (; items->type !=3D RTE_FLOW_ITEM_TYPE_END; items++) { >>>> + tunnel =3D !!(detected_items & MLX5_FLOW_LAYER_TUNNEL); >>>> + switch (items->type) { >>>> + case RTE_FLOW_ITEM_TYPE_ETH: >>>> + detected_items |=3D tunnel ? >>>> MLX5_FLOW_LAYER_INNER_L2 : >>>> + >>>> MLX5_FLOW_LAYER_OUTER_L2; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_VLAN: >>>> + detected_items |=3D tunnel ? >>>> MLX5_FLOW_LAYER_INNER_VLAN : >>>> + >>>> MLX5_FLOW_LAYER_OUTER_VLAN; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_IPV4: >>>> + detected_items |=3D tunnel ? >>>> + MLX5_FLOW_LAYER_INNER_L3_IPV4 >>>> : >>>> + >>>> MLX5_FLOW_LAYER_OUTER_L3_IPV4; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_IPV6: >>>> + detected_items |=3D tunnel ? >>>> + MLX5_FLOW_LAYER_INNER_L3_IPV6 >>>> : >>>> + >>>> MLX5_FLOW_LAYER_OUTER_L3_IPV6; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_TCP: >>>> + detected_items |=3D tunnel ? >>>> + MLX5_FLOW_LAYER_INNER_L4_TCP : >>>> + >>>> MLX5_FLOW_LAYER_OUTER_L4_TCP; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_UDP: >>>> + detected_items |=3D tunnel ? >>>> + MLX5_FLOW_LAYER_INNER_L4_UDP >>>> : >>>> + >>>> MLX5_FLOW_LAYER_OUTER_L4_UDP; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_GRE: >>>> + case RTE_FLOW_ITEM_TYPE_NVGRE: >>>> + detected_items |=3D MLX5_FLOW_LAYER_GRE; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_VXLAN: >>>> + detected_items |=3D MLX5_FLOW_LAYER_VXLAN; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_VXLAN_GPE: >>>> + detected_items |=3D MLX5_FLOW_LAYER_VXLAN_GPE; >>>> + break; >>>> + case RTE_FLOW_ITEM_TYPE_META: >>>> + detected_items |=3D MLX5_FLOW_ITEM_METADATA; >>>> + break; >>>> + default: >>>> + break; >>>> + } >>>> + } >>>> + for (; actions->type !=3D RTE_FLOW_ACTION_TYPE_END; actions++) { >>>> + switch (actions->type) { >>>> + case RTE_FLOW_ACTION_TYPE_FLAG: >>>> + detected_actions |=3D MLX5_FLOW_ACTION_FLAG; >>>> + break; >>>> + case RTE_FLOW_ACTION_TYPE_MARK: >>>> + detected_actions |=3D MLX5_FLOW_ACTION_MARK; >>>> + break; >>>> + case RTE_FLOW_ACTION_TYPE_DROP: >>>> + detected_actions |=3D MLX5_FLOW_ACTION_DROP; >>>> + break; >>>> + case RTE_FLOW_ACTION_TYPE_QUEUE: >>>> + detected_actions |=3D MLX5_FLOW_ACTION_QUEUE; >>>> + break; >>>> + case RTE_FLOW_ACTION_TYPE_RSS: >>>> + detected_actions |=3D MLX5_FLOW_ACTION_RSS; >>>> + break; >>>> + case RTE_FLOW_ACTION_TYPE_COUNT: >>>> + detected_actions |=3D MLX5_FLOW_ACTION_COUNT; >>>> + break; >>>> + default: >>>> + break; >>>> + } >>>> + } >>>> + *item_flags =3D detected_items; >>>> + *action_flags =3D detected_actions; >>>> +} >>>> + >>>> +/** >>>> * Internal preparation function. Allocates the DV flow size, >>>> * this size is constant. >>>> * >>>> @@ -376,15 +473,15 @@ flow_dv_validate(struct rte_eth_dev *dev, const >>>> struct rte_flow_attr *attr, >>>> */ >>>> static struct mlx5_flow * >>>> flow_dv_prepare(const struct rte_flow_attr *attr __rte_unused, >>>> - const struct rte_flow_item items[] __rte_unused, >>>> - const struct rte_flow_action actions[] __rte_unused, >>>> - uint64_t *item_flags __rte_unused, >>>> - uint64_t *action_flags __rte_unused, >>>> + const struct rte_flow_item items[], >>>> + const struct rte_flow_action actions[], >>>> + uint64_t *item_flags, uint64_t *action_flags, >>>> struct rte_flow_error *error) >>>> { >>>> uint32_t size =3D sizeof(struct mlx5_flow); >>>> struct mlx5_flow *flow; >>>>=20 >>>> + flow_dv_get_item_action_flags(items, actions, item_flags, >>>> action_flags); >>>> flow =3D rte_calloc(__func__, 1, size, 0); >>>> if (!flow) { >>>> rte_flow_error_set(error, ENOMEM, >>>> @@ -1067,7 +1164,7 @@ flow_dv_create_action(const struct >> rte_flow_action >>>> *action, >>>> dev_flow->dv.actions[actions_n].tag_value =3D >>>> mlx5_flow_mark_set(MLX5_FLOW_MARK_DEFAULT); >>>> actions_n++; >>>> - flow->actions |=3D MLX5_FLOW_ACTION_FLAG; >>>> + assert(flow->actions & MLX5_FLOW_ACTION_FLAG); >>>> break; >>>> case RTE_FLOW_ACTION_TYPE_MARK: >>>> dev_flow->dv.actions[actions_n].type =3D >>>> MLX5DV_FLOW_ACTION_TAG; >>>> @@ -1075,18 +1172,18 @@ flow_dv_create_action(const struct >>>> rte_flow_action *action, >>>> mlx5_flow_mark_set >>>> (((const struct rte_flow_action_mark *) >>>> (action->conf))->id); >>>> - flow->actions |=3D MLX5_FLOW_ACTION_MARK; >>>> + assert(flow->actions & MLX5_FLOW_ACTION_MARK); >>>> actions_n++; >>>> break; >>>> case RTE_FLOW_ACTION_TYPE_DROP: >>>> dev_flow->dv.actions[actions_n].type =3D >>>> MLX5DV_FLOW_ACTION_DROP; >>>> - flow->actions |=3D MLX5_FLOW_ACTION_DROP; >>>> + assert(flow->actions & MLX5_FLOW_ACTION_DROP); >>>> break; >>>> case RTE_FLOW_ACTION_TYPE_QUEUE: >>>> queue =3D action->conf; >>>> flow->rss.queue_num =3D 1; >>>> (*flow->queue)[0] =3D queue->index; >>>> - flow->actions |=3D MLX5_FLOW_ACTION_QUEUE; >>>> + assert(flow->actions & MLX5_FLOW_ACTION_QUEUE); >>>> break; >>>> case RTE_FLOW_ACTION_TYPE_RSS: >>>> rss =3D action->conf; >>>> @@ -1098,7 +1195,7 @@ flow_dv_create_action(const struct >> rte_flow_action >>>> *action, >>>> flow->rss.types =3D rss->types; >>>> flow->rss.level =3D rss->level; >>>> /* Added to array only in apply since we need the QP */ >>>> - flow->actions |=3D MLX5_FLOW_ACTION_RSS; >>>> + assert(flow->actions & MLX5_FLOW_ACTION_RSS); >>>> break; >>>> default: >>>> break; >>>> -- >>>> 2.11.0