From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10069.outbound.protection.outlook.com [40.107.1.69]) by dpdk.org (Postfix) with ESMTP id BCA2F10A3 for ; Mon, 29 Oct 2018 19:03:54 +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=6INg4sVZ6Y+l2LDggM+GqST2m3MX3sGXzGmq7kV3/ZY=; b=lIFUhM/PW5cdPwZ801mN9vYwWKDLDntUdQUI3VqiAabWTv/w3pNfPOMVPNhN7dSvBbQWHgORkSs2S9NLDP9L7LhXL53j8hlTwRMUHA/qrEgDYM6ubZoO/FZs+4d+pxY5GTgtYWCow3Vbf3sNeFgkYoStZWjY4MjrL446EtpdwNA= Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by DB3PR0502MB4026.eurprd05.prod.outlook.com (52.134.72.143) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1273.21; Mon, 29 Oct 2018 18:03:53 +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.027; Mon, 29 Oct 2018 18:03:53 +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/ck6U1ua5ggADMCAA= Date: Mon, 29 Oct 2018 18:03:52 +0000 Message-ID: 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: [69.181.245.183] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB3PR0502MB4026; 6:YfT0dkBN/KVVQ4se5EVt442vw/shKXkV3D2KOh4HVOtUQ+AGqK1B5FQnXylFCh0Qx0M/wmq6vpzEcOoUS36bivlTCFGcdqlWAtnn/8FA7CDE1PwT8bcnbnDyOySATj5heS8EWCvZEsX2bkZeppfd/OdH5bGftxrI9OhitDBnM3Bkf3hfSNkWusRlF2v6oyC01DQq3Z8hQA0oNoaT/avkD41xbzuqsmT6aOIL4GioRVVQCJaIA/JTeKW1m/TeVJGphdrGv5RGQ7x4DRWs/zbAaoHey58tRAvsqAS7qIVJHs2dt3KIZ09aaFOG6g0pWV7avzxW1rdNibTcA85YAjclydRM5FJANJt2OgaOO9AgQlba4CdMg9rFdvSucifC8zg417tjCPoP4KxVwabiC1evbTYoLfPvWPLputbYrRRZ7sIEWeJpaH5gj1V7HWuqmXZL30Ovtqo+OteLaIKwKkX9/g==; 5:av8VjKKHfPsQ/XhnnCoE/gAfLlteFinvla+AZj5X7d7BwbTFNZz22z7/Xx3Cdcjuwrx4pXZ4n/0BZ3wqtxoUm3Veylwbq4mpdTQ2hPqBrTU6XpEHhnwkRulj6H0cvJEnHVDEYPApAVRul45Pm/20ZYPJIt+d9Exne92TOZ5R/o8=; 7:q2L39YgPxQrAlzo1FboXvlNw3/TwMXyZcKJ+kvi7LqIWXzSvO1iUzadO6uioBMqY+BcVzPCPqXfTpflBohq+TNlqudEXmeCGop2Z3eUp1GjyIqS4Pn2+JvbwrYwhL+n4a9GlEfpl/fYegxUf35OLWA== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: fd381a37-9b82-4f07-e7dc-08d63dc8e2fd x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(7020095)(4652040)(8989299)(5600074)(711020)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7153060)(7193020); SRVR:DB3PR0502MB4026; x-ms-traffictypediagnostic: DB3PR0502MB4026: 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)(8121501046)(5005006)(3231382)(944501410)(52105095)(93006095)(93001095)(3002001)(10201501046)(6055026)(148016)(149066)(150057)(6041310)(20161123560045)(20161123564045)(20161123562045)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(201708071742011)(7699051)(76991095); SRVR:DB3PR0502MB4026; BCL:0; PCL:0; RULEID:; SRVR:DB3PR0502MB4026; x-forefront-prvs: 084080FC15 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(136003)(396003)(376002)(39860400002)(346002)(366004)(13464003)(199004)(189003)(51234002)(575784001)(86362001)(83716004)(186003)(8676002)(25786009)(82746002)(81156014)(106356001)(6636002)(6436002)(5660300001)(446003)(6512007)(36756003)(81166006)(6246003)(229853002)(71200400001)(54906003)(71190400001)(37006003)(53936002)(486006)(105586002)(11346002)(476003)(2616005)(2900100001)(316002)(305945005)(33656002)(4326008)(66066001)(478600001)(8936002)(5250100002)(7736002)(6862004)(68736007)(99286004)(6506007)(26005)(53546011)(256004)(102836004)(6486002)(97736004)(14454004)(14444005)(3846002)(76176011)(6116002)(2906002); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB4026; 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: 9h+H6CyiiayK/FNQS/kLLB4d4HgFgL2uMou/poURTD32EojXW+OzMBUsG8748siQWfEkQIUWz9R5pZGVWaqxocKsnIh21JXj+ZBaK8QfhhXVYW7B2PqjNAmEK/z2zryIQPHyc0xtdyU+i59ut8tKh6BKpWFIbOBg0HJsWhpF2IhkxEeYMtSpzJuFLEbjntj9SFeAyIS99OZIvxRqrmlQUny97WlerMUvNdLwIE0p7BI3NkXi3+vOqLoQwAC8ss2u2hdFvm8qn6w9nO43Vs7Dzfj/908GMZ0M5s75RD9Vin1ovjiZsKWgGdxWT1r6K75lHQlWxGeUkT0Tkmy+iph/welCa21QrkjPfC9+rRRlO6E= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: <3B647CA8B145B447967FD58EE7A4BDD5@eurprd05.prod.outlook.com> Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: fd381a37-9b82-4f07-e7dc-08d63dc8e2fd X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Oct 2018 18:03:52.9077 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB4026 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: Mon, 29 Oct 2018 18:03:55 -0000 > 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 in > dv_translate. And then in flow_dv_create_action change the switch case > to ifs. Then, I should ask you a question. Why did you put actions/layers in rte_fl= ow struct in the first place? And _prepare() API even takes pointers of item_f= lags and actions_flags in order to fill in the structs for nothing. Verbs and TC= F 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 case o= f TCF, _apply() doesn't need to know the action_flags and its _translate does= n't even fill in the struct while its _prepare() does in vain. Where's the consistency? What is the definition of the APIs? 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 patch. So, I think _prepare is the right place to fill in the flags. Let me know how you want to change it. Thanks, Yongseok >=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 flag= s >>=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 >=20