From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10042.outbound.protection.outlook.com [40.107.1.42]) by dpdk.org (Postfix) with ESMTP id E1291F94 for ; Wed, 31 Oct 2018 09:25:44 +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=QWK8+EZ82QsgkFn9Pvb+tws5a0a38EWiY+qGcAsNE3Q=; b=UWBvC99/Dkc5NVRb9ol+vSEkPtn3uolJ+PPUULMWUU/Y/Sowy7eoA4yc2CGEFAAWBBl6Rxwv/a4K4bbVCGKrUuqp7lcX8XOu0c9vqhgXbhehmIEEBootaZThNtDEHbhqSdRkSZRPF+whmqIv7DX0EF7paNNY3wZaFPz3O0wxABg= Received: from AM4PR05MB3425.eurprd05.prod.outlook.com (10.171.187.142) by AM4PR05MB1779.eurprd05.prod.outlook.com (10.165.246.138) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.1273.25; Wed, 31 Oct 2018 08:25:42 +0000 Received: from AM4PR05MB3425.eurprd05.prod.outlook.com ([fe80::61ec:ffec:5ebf:7bd6]) by AM4PR05MB3425.eurprd05.prod.outlook.com ([fe80::61ec:ffec:5ebf:7bd6%3]) with mapi id 15.20.1294.021; Wed, 31 Oct 2018 08:25:42 +0000 From: Ori Kam To: Yongseok Koh CC: Shahaf Shuler , "dev@dpdk.org" Thread-Topic: [PATCH] net/mlx5: fix Direct Verbs getting item and action flags Thread-Index: AQHUbuSH+I7dIxYbCkyDfM3wP2/ck6U1ua5ggADMCACAAnHOEA== Date: Wed, 31 Oct 2018 08:25:42 +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: x-originating-ip: [37.122.157.213] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; AM4PR05MB1779; 6:O/qBgImn8M+f+vMh7MiDjJfQxpxT3yGvDk2E10st5+nUOxNPlNCDY7caD1zAt/RSQaRJvRCD2YIihicwwjGCMwjqDV5zvw/lafRAuKMV2v5SE67+otqogJWY88bvQKDtSGZ0NEsX+pkctxbrx9Ejcg6S8kAxhG9/gPPHveJXxl8ffZ8fSqZdguEisaqk3CWhzguS/QjaJfRoRW92H19oU374ExEkMU9t0nXI0kRC8zTBg3KwO30IrRqx5fl2Dek6jQlMLeBod5yLQzT2Ne7p8T6+s4fBprL+SLYcDbK26qSO6IuiJMFTO5TGlPq+TFXNMiY8ViQ+0rwP+M7Bvsxc8Gm6SX88GgJt/S7yqFZ9kIC5cEU6H9y0So2C7dhLz9DS0mftg7JM/xnN/LLnmfaB/G2RtVj4PpqFP/diZKaUbx5bkCtNrs8DwN/EhrQleddDguoZTR0AuCemOACd1zTKEQ==; 5:dwspPlF0u1dn+nCXQEDmBRSjX8Dynwuoq0kJIfh7K0wki3MS20nupl/a7r5++apegEFz9msBvHGiUXR9CCWKFukC934pF7VjaKeYM3LkmyK3cNpYSRg8BP0X03vfz0cSF/IXLJJF11a3Ct2CFYK34in5UDuNjOPxa2a3pMKu7tc=; 7:Dqjco0w/MpSvAPJdq39twEwMSu6jZX3d6gGtbiySf9qME0iAfb/8753dJCykzYoWNwuCk6mS3ynzLRpuIc+higSRhI+M03Z2sxQEsl6xmflsrlSBaIzjGn1sRa5/rbGspz/nbFAjQfL6c2eav+J5yw== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: e5b810ec-0648-4dc3-7010-08d63f0a72c3 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:AM4PR05MB1779; x-ms-traffictypediagnostic: AM4PR05MB1779: authentication-results: spf=none (sender IP is ) smtp.mailfrom=orika@mellanox.com; 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)(20161123558120)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123564045)(20161123560045)(20161123562045)(201708071742011)(7699051)(76991095); SRVR:AM4PR05MB1779; BCL:0; PCL:0; RULEID:; SRVR:AM4PR05MB1779; x-forefront-prvs: 084285FC5C x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(346002)(136003)(396003)(366004)(376002)(39860400002)(189003)(199004)(51234002)(13464003)(11346002)(68736007)(81156014)(54906003)(105586002)(8676002)(446003)(106356001)(8936002)(81166006)(486006)(74316002)(6116002)(71190400001)(476003)(6436002)(3846002)(55016002)(25786009)(7736002)(305945005)(316002)(6636002)(6862004)(5660300001)(4326008)(71200400001)(186003)(99286004)(5250100002)(66066001)(575784001)(9686003)(6246003)(53936002)(478600001)(14454004)(97736004)(2906002)(229853002)(86362001)(76176011)(7696005)(33656002)(2900100001)(26005)(6506007)(256004)(102836004)(14444005)(53546011); DIR:OUT; SFP:1101; SCL:1; SRVR:AM4PR05MB1779; H:AM4PR05MB3425.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: mSZjMnic0oA0GdasZKCnb05DjV6w/kWoFcHAK/acy9frtESLnEWBF6M8pTiLHlyKagZTmPQNyF+nlnoDaBLjJz5VYfuHAdJIhGFLAGECqIejkqU+yl5s1vqraWbusZqMabyN0S9o/3u+zC+AoEDxxsmKlzEZniJXpYJRFcZjQcq5HKf4seSoEd2JfKP/8ZX1tRXD51q0eBXmfPzrF+rrWY/OySJiokv0Q7y18I707Fx6KXhLap00R+7XoiWP35DDRITPxUGKlnm4vmdp3hdeWLdAVV+CuSE/mzPPRqWS3lnd5lVHA0ahI0PSIgaRdU7ToL/aMZqbEALhMNnIp4+Mux9gkN/tMl7+kMs3bZyV+2M= 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: e5b810ec-0648-4dc3-7010-08d63f0a72c3 X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Oct 2018 08:25:42.6116 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM4PR05MB1779 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: Wed, 31 Oct 2018 08:25:45 -0000 Hi, PSB > -----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 f= lags >=20 > > On Oct 28, 2018, at 11:03 PM, Ori Kam wrote: > > > > Why should DV prepare function return the list of actions? > > > > 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 item= _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 case= of > TCF, _apply() doesn't need to know the action_flags and its _translate do= esn't > even fill in the struct while its _prepare() does in vain. > Where's the consistency? What is the definition of the APIs? According to design prepare function is responsible for allocating the flow= . In case of Verbs the allocation size is dependent on the number of actions, while in Direct Verbs the size is fixed. This is the reason for the differe= nce. If it helps one can add the scan for items and actions. Currently is was no= t needed in case of Direct Verbs maybe it can help for example for Dekel patches wer= e 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 th= e actions based on the real number of actions. >=20 > 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 patc= h. >=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 > > > >> -----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 > >> > >> Flow driver has to provide detected item flags and action flags via > >> flow_drv_prepare(). DV doesn't return flags at all. > >> > >> Fixes: 865a0c15672c ("net/mlx5: add Direct Verbs prepare function") > >> Cc: orika@mellanox.com > >> > >> Signed-off-by: Yongseok Koh > >> --- > >> drivers/net/mlx5/mlx5_flow_dv.c | 115 > >> ++++++++++++++++++++++++++++++++++++---- > >> 1 file changed, 106 insertions(+), 9 deletions(-) > >> > >> 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, > >> } > >> > >> /** > >> + * 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; > >> > >> + 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 > >