From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50050.outbound.protection.outlook.com [40.107.5.50]) by dpdk.org (Postfix) with ESMTP id 4886C4C74 for ; Mon, 5 Nov 2018 07:43:53 +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=GxbgOqAGM/PGIRXK5xcEaQjIeSA8fM+7ML6EtJOq8sw=; b=SP7rBmKkzBtuVdKrjXDzfd9BnTKxWOtCgH+cQBiaDpb1d/Mcwnz0Z6/LNWVdkROM0iQrlbQQ8RHcRY5FQVlPdXMfzvJobMN8fDjIifzLWAiAH6O7FFv5vjWIGabo8NjrTWzwYfAzAHpaN8pT9uOeok9F58wx15cGwivzAG+79vs= 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.1294.26; Mon, 5 Nov 2018 06:43:51 +0000 Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::58e7:97d8:f9c1:4323]) by DB3PR0502MB3980.eurprd05.prod.outlook.com ([fe80::58e7:97d8:f9c1:4323%3]) with mapi id 15.20.1294.028; Mon, 5 Nov 2018 06:43:51 +0000 From: Yongseok Koh To: Ori Kam CC: Shahaf Shuler , "dev@dpdk.org" Thread-Topic: [PATCH 2/3] net/mlx5: fix Direct Verbs flow tunnel Thread-Index: AQHUcvAvOimJ0t337kOQbAoxlFooYaU/STIAgADeHQCAAI7SgIAACdAA Date: Mon, 5 Nov 2018 06:43:50 +0000 Message-ID: <8950F407-7CF3-45D4-8A07-E970FD0702D8@mellanox.com> References: <20181102210801.28370-1-yskoh@mellanox.com> <20181102210801.28370-2-yskoh@mellanox.com> <20181105053732.GE15737@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=yskoh@mellanox.com; x-originating-ip: [69.181.245.183] x-ms-publictraffictype: Email x-microsoft-exchange-diagnostics: 1; DB3PR0502MB3947; 6:vP0kyId5aeOBIdjsj9Ys1/CCKVic0RMlicRDUOEl5M9WKAJ2BlIT3IXeroQFWZMjyPpGO5mVZNSS94FudDWXrqgWkp7vfp0O28PUJNKd7Ba5Ix+PhLiwjh3MSXCYItmW7UHcFXrO5HWjBidEgQpPN1sGXPih4wxL86vDE9EUVAfPPEwNHJSxLhCuxpoOtPQ92N8FYnuedR7igreHSYmAaVQnAqRhYwfZoMKq0nv0Ph7Ncw7nqykwfmGIhCtuJFzXavoeth5wtXe58AHTynX92RZC8HlRuFO+GcX0SQhVOnRP3PL91oQisChD5FsrjtT+YHdP2tz5D8RnKyEcRgUij68eAejYTL/0txLdnzotdc6cRcxCFXT4H+JM1l/GsedTYalaTooBl4dVJePfldFWiB5pj//4+bTipx5u3+IB9JPOfIXdQLa4fTD2Lr17avPjPXOZcnCexuKDOR5AJ0z3wg==; 5:xiQFNjyXfjq1WXUBzYZQsjn7R9R1v/L3FfsAyNgrqmIOZSoiLX7eT653zm8Kskn25J8325965MbHgTzl4rFJbXlOVzTUSrEAQr+h4Pc2BbXke0jMxJYhnloVComIls+ySbkS33aN/PWcB5fxGAduciv0nCHBSWblMZLJkpfy/DQ=; 7:p2UJ1/qGhDaOkDigqPU8TyPrb5hon24YxvGiaAp0/KOPiNe+ARBtMUOtQB+S6Ji5/YNCVMzucYJLZpfYHZelfrMSf9nY5h3fMaobPYUTb+G91Gsx3hSC6g/9YBa7kF5g50JVD5VATn7RINMgXJuHNw== x-ms-exchange-antispam-srfa-diagnostics: SOS; x-ms-office365-filtering-correlation-id: a33b029f-f29f-4c1b-e01d-08d642ea0c02 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)(3231382)(944501410)(52105095)(3002001)(93006095)(93001095)(6055026)(148016)(149066)(150057)(6041310)(20161123560045)(20161123564045)(201703131423095)(201702281528075)(20161123555045)(201703061421075)(201703061406153)(20161123562045)(20161123558120)(201708071742011)(7699051)(76991095); SRVR:DB3PR0502MB3947; BCL:0; PCL:0; RULEID:; SRVR:DB3PR0502MB3947; x-forefront-prvs: 08476BC6EF x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(366004)(136003)(376002)(346002)(39860400002)(396003)(13464003)(199004)(189003)(2616005)(86362001)(36756003)(106356001)(76176011)(105586002)(486006)(186003)(446003)(11346002)(53546011)(99286004)(6506007)(7736002)(102836004)(305945005)(256004)(14444005)(4326008)(6862004)(25786009)(26005)(82746002)(93886005)(6116002)(3846002)(316002)(37006003)(54906003)(33656002)(71200400001)(6636002)(2906002)(83716004)(71190400001)(478600001)(53936002)(476003)(2900100001)(6512007)(66066001)(6246003)(5660300001)(14454004)(6436002)(6486002)(229853002)(68736007)(81156014)(8676002)(81166006)(8936002)(97736004); DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB3947; 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: 2eaLdJoCzoYqCXRim9Xa7X5m2C6Fl4zzrbj3bbOV2+NFl5RYTEd8xOCJ6X2Wfiqg7qCgOjY4YvAIqEbAFnbaK8gLGbGTpHxfdHESiarqjcve1iCbhH1zVYsh47IgvdyeUL6MyWGdp+h3Cph43wVTW84c5nSk43VITZfxyf45yMatQKcBmxzz/ooeQdmDu07aUR6otrAC4aZ2lv7Bii4kn7kI8Nx/0j9vzctakQJ7Ishka6mmo8j/MKRftGfPlpp6JyG9nd1oJypYVG60RhD28xt62fsthtFxHwnGO7W/tD/qV4d8INFb23Gx9FnmN/Ljrr7crWhZPmsoxZwf2dbezVTYD2FTPxCmYt6mtkVhicU= spamdiagnosticoutput: 1:99 spamdiagnosticmetadata: NSPM Content-Type: text/plain; charset="us-ascii" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: a33b029f-f29f-4c1b-e01d-08d642ea0c02 X-MS-Exchange-CrossTenant-originalarrivaltime: 05 Nov 2018 06:43:50.9236 (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 2/3] net/mlx5: fix Direct Verbs flow tunnel 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, 05 Nov 2018 06:43:53 -0000 > On Nov 4, 2018, at 10:08 PM, Ori Kam wrote: >=20 >=20 >=20 >> -----Original Message----- >> From: Yongseok Koh >> Sent: Monday, November 5, 2018 7:38 AM >> To: Ori Kam >> Cc: Shahaf Shuler ; dev@dpdk.org >> Subject: Re: [PATCH 2/3] net/mlx5: fix Direct Verbs flow tunnel >>=20 >> On Sun, Nov 04, 2018 at 01:22:34AM -0700, Ori Kam wrote: >>>=20 >>>> -----Original Message----- >>>> From: Yongseok Koh >>>> Sent: Friday, November 2, 2018 11:08 PM >>>> To: Shahaf Shuler >>>> Cc: dev@dpdk.org; Yongseok Koh ; Ori Kam >>>> >>>> Subject: [PATCH 2/3] net/mlx5: fix Direct Verbs flow tunnel >>>>=20 >>>> 1) Fix layer parsing >>>> In translation of tunneled flows, dev_flow->layers must not be used to >>>> check tunneled layer as it contains all the layers parsed from >>>> flow_drv_prepare(). Checking tunneled layer is needed to distinguish >>>> between outer and inner item. This should be based on dynamic parsing. >> With >>>> dev_flow->layers on a tunneled flow, items will always be interpreted = as >>>> inner as dev_flow->layer already has all the items. Dynamic parsing >>>> (item_flags) is added as there's no such code. >>>>=20 >>>> 2) Refactoring code >>>> - flow_dv_create_item() and flow_dv_create_action() are merged into >>>> flow_dv_translate() for consistency with Verbs and *_validate(). >>>=20 >>> I don't like the idea of combining 2 distinct functions into one. >>> I think a function should be as short as possible and do only one thing= , >>> if there is no good reason why two functions should be combined they sh= ould >> not >>> be combined. >>> If you want to align both the Direct Verbs and Verbs I think we can sp= lit the >> Verbs >>> code. >>=20 >> Look at the other lengthy switch-case clauses in validate/prepare/transl= ate in >> each driver. This DV translate is the only exception. I'd rather like to= ask >> why. I didn't like the lengthy function from the beginning but you wante= d to >> keep it. Of course, I considered to split the Verbs one but that's the r= eason >> why I chose to merge DV code. If we feel this lengthy func is really com= plex and >> gets error prone, then we can refactor all the code at once later. Or, I= still >> prefer the graph approach. That would be simpler. >>=20 >=20 > I agree with you that all functions should have been split( No excuse als= o kept the basic > structure as it was), specific in this one I had extra reason to make the= split since > creation of items also need adding matcher so it was different. > In any case I agree that we should have consistency with other functions = so ether we change > them all or just this one. I think do to time let's do what you suggested= and change only this one > or just leave it as is.=20 > Regarding the graph approach I think we should wait to see if the current= approach is good > enough and in next releases maybe switch to the graph approach. Thanks for understanding. Will push v2 once the unit test is done. Yongseok,=