From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id 7D7CEA00E6 for ; Wed, 20 Mar 2019 11:45:30 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2C2745323; Wed, 20 Mar 2019 11:45:30 +0100 (CET) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id EFAEA4F91 for ; Wed, 20 Mar 2019 11:45:27 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 20 Mar 2019 03:45:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,248,1549958400"; d="scan'208";a="127012038" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.46]) ([10.237.221.46]) by orsmga008.jf.intel.com with ESMTP; 20 Mar 2019 03:45:23 -0700 To: Thomas Monjalon Cc: Hyong Youb Kim , "John Daley (johndale)" , Andrew Rybchenko , Qi Zhang , "dev@dpdk.org" , Shahaf Shuler , Jerin Jacob , David Marchand , Maxime Coquelin , Konstantin Ananyev , Hemant Agrawal , Stephen Hemminger , gaetan.rivet@6wind.com References: <20190305055659.3095-1-hyonkim@cisco.com> <1715472.XOf6rlv5YZ@xps> <3429576.gmczXWLcQl@xps> From: Ferruh Yigit Openpgp: preference=signencrypt Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata= mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ +iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9 GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb +dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1 1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJVBBMBAgA/AhsDBgsJCAcDAgYVCAIJCgsE FgIDAQIeAQIXgBYhBNI2U4dCLsKE45mBx/kz60PfE2EfBQJbughWBQkHwjOGAAoJEPkz60Pf E2Eft84QAIbKWqhgqRfoiw/BbXbA1+qm2o4UgkCRQ0yJgt9QsnbpOmPKydHH0ixCliNz1J8e mRXCkMini1bTpnzp7spOjQGLeAFkNFz6BMq8YF2mVWbGEDE9WgnAxZdi0eLY7ZQnHbE6AxKL SXmpe9INb6z3ztseFt7mqje/W/6DWYIMnH3Yz9KzxujFWDcq8UCAvPkxVQXLTMpauhFgYeEx Nub5HbvhxTfUkapLwRQsSd/HbywzqZ3s/bbYMjj5JO3tgMiM9g9HOjv1G2f1dQjHi5YQiTZl 1eIIqQ3pTic6ROaiZqNmQFXPsoOOFfXF8nN2zg8kl/sSdoXWHhama5hbwwtl1vdaygQYlmdK H2ueiFh/UvT3WG3waNv2eZiEbHV8Rk52Xyn2w1G90lV0fYC6Ket1Xjoch7kjwbx793Kz/RfQ rmBY8/S4DTGn3oq3dMdQY+b6+7VMUeLMMh2CXYO9ErkOq+qNTD1IY+cBAkXnaDbQfz0zbste ZGWH74FAZ9nCpDOqbRTrBL42aMGhfOWEyeA1x7+hl6JZfabBWAuf4nnCXuorKHzBXTrf7u7p fXsKQClWRW77PF1VmzrtKNVSytQAmlCWApQIw20AarFipXmVdIjHmJPU611WoyxZPb4JTOxx 5cv9B+nr/RIB+v5dcStyHCCwO1be7nBDdCgd4F6kTQPLuQINBFfWTL4BEACnNA29e8TarUsB L5n6eLZHXcFvVwNLVlirWOClHXf44o2KnN3ww+eBEmKVfEFo9MSuGDNHS8Zw1NiGMYxLIUgd U6gGrVVs/VrQWL82pbMk6jCj98N+BXIri+6K1z+AImz7ax7iF1kDgRAnFWU0znWWBgM2mM8Y gDjcxfXk4sCKnvf6Gjo08Ey5zmqx7dekAKU2EEp8Q1EJY3jbymLdZWRP4AFFMTS1rGMk0/tt v71NBg1GobCcbNfn9chK/jhqxYhAJqq86RdJQkt3/9x1U1Oq0vXCt4JVVHmkxePtUiuWTTt+ aYlUAsKYZsWvncExvw77x2ArYDmaK0yfjh37wp0lY7DOJHFxoyT8tyWZlLci/VMRG2Ja33xj 0CN4C1yBg+QDeV3QFxQo42iA/ykdXPUR3ezmsND3XKvVLTC4DNb3V/EZQ7jBj64+bEK0VW4G B31VP00ApNQvSoczsIOAKdk97RNbpmPw6q10ILIB+9T1xbnFYzshzGF17oC0/GENIHATx8vZ masOZoDiOZQpeneLgnFE9JfzhLTxv6wNZcc/HLXRQVTkDsQr8ERtkAoHCf1E5+b5Yr7pfnE4 YuhET746o25S53ELUYPIs49qoJsEJL34/oexMfPGyPIlrbufiNyty5jc/1MRwUlhJlJ5IOHy ZUa+6CLR7GdImusFkPJUJwARAQABiQI8BBgBAgAmAhsMFiEE0jZTh0IuwoTjmYHH+TPrQ98T YR8FAlu6CHAFCQXE7zIACgkQ+TPrQ98TYR9nXxAAqNBgkYNyGuWUuy0GwDQCbu3iiMyH1+D7 llafPcK4NYy1Z4AYuVwC9nmLaoj+ozdqS3ncRo57ncRsKEJC46nDJJZYZ5LSJVn63Y3NBF86 lxQAgjj2oyZEwaLKtKbAFsXL43jv1pUGgSvWwYtDwHITXXFQto9rZEuUDRFSx4sg9OR+Q6/6 LY+nQQ3OdHlBkflzYMPcWgDcvcTAO6yasLEUf7UcYoSWTyMYjLB4QuNlXzTswzGVMssJF/vo V8lD1eqqaSUWG3STF6GVLQOr1NLvN5+kUBiEStHFxBpgSCvYY9sNV8FS6N24CAWMBl+10W+D 2h1yiiP5dOdPcBDYKsgqDD91/sP0WdyMJkwdQJtD49f9f+lYloxHnSAxMleOpyscg1pldw+i mPaUY1bmIknLhhkqfMmjywQOXpac5LRMibAAYkcB8v7y3kwELnt8mhqqZy6LUsqcWygNbH/W K3GGt5tRpeIXeJ25x8gg5EBQ0Jnvp/IbBYQfPLtXH0Myq2QuAhk/1q2yEIbVjS+7iowEZNyE 56K63WBJxsJPB2mvmLgn98GqB4G6GufP1ndS0XDti/2K0o8rep9xoY/JDGi0n0L0tk9BHyoP Y7kaEpu7UyY3nVdRLe5H1/MnFG8hdJ97WqnPS0buYZlrbTV0nRFL/NI2VABl18vEEXvNQiO+ vM8= Message-ID: <09e2672e-c94f-5f7d-84e3-343a7be7452e@intel.com> Date: Wed, 20 Mar 2019 10:45:22 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.3 MIME-Version: 1.0 In-Reply-To: <3429576.gmczXWLcQl@xps> Content-Type: text/plain; charset="UTF-8" Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v2] net/enic: add private API to set ingress VLAN rewrite mode 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Message-ID: <20190320104522.sIsknsywioFzZ4dzCzi5mPEHrRSSrY1x4QZVP6EveGQ@z> On 3/19/2019 8:30 PM, Thomas Monjalon wrote: > 19/03/2019 19:00, Ferruh Yigit: >> On 3/19/2019 5:36 PM, Thomas Monjalon wrote: >>> 19/03/2019 18:29, Ferruh Yigit: >>>> On 3/14/2019 10:04 PM, Thomas Monjalon wrote: >>>>> 14/03/2019 03:58, Hyong Youb Kim: >>>>>> On Wed, Mar 13, 2019 at 10:29:53PM +0100, Thomas Monjalon wrote: >>>>>>> 13/03/2019 22:11, John Daley (johndale): >>>>>>>> From: Thomas Monjalon >>>>>>>>> 13/03/2019 19:32, Ferruh Yigit: >>>>>>>>>> On 3/5/2019 7:11 AM, Hyong Youb Kim wrote: >>>>>>>>>>> The driver currently has a devarg to set the rewrite mode during >>>>>>>>>>> init. Some apps want to programatically set it after running >>>>>>>>>>> rte_eal_init() and finding that ports are VIC. Add a private >>>>>>>>>>> function to support such applications. >>>>>>>>>> >>>>>>>>>> It is not good idea to have PMD specific APIs (although we already have >>>>>>>>> some). >>>>>>>>>> >>>>>>>>>> Specific to this case, as far as I can see it is to pass a config >>>>>>>>>> value and do the action related to it, what would you think having a >>>>>>>>>> generic key/value set/get API in ethdev for this? Similar to rawdev >>>>>>>>> get_attr/set_attr [1]? >>>>>>>>>> >>>>>>>>>> My concern is it may turn into something like ioctl with many things >>>>>>>>>> pushed to it, and cause possible duplication ... >>>>>>>>> >>>>>>>>> Yes, it is clearly ioctl style. >>>>>>>>> >>>>>>>>> Please could you explain more what is the rewrite mode? >>>>>>>>> Does it apply to the port or the queue? >>>>>>>>> >>>>>>>> It applies to a port. By default the Cisco VIC VLAN tags every packet on ingress even if they were untagged coming in on the wire. They are tagged with VLAN 0 or a VLAN id programmed into the NIC depending on the configuration. Its part of the original design, to maintain priority bits, ancient history. >>>>>>>> >>>>>>>> Some apps don't like this (VPP) or take a slower path (OVS). Hyong added a ig-vlan-rewrite=untag devarg to disable this (leave untagged/default vlan packets untagged) during rte_eal_init and this is helpful for OVS, but VPP likes to set the rewrite mode after rte_eal_init() and finding the ports are VIC ports. So that is the reasoning behind the private API call. >>>>>>> >>>>>>> It looks like an application will always set this flag or never. >>>>>>> So I don't see the need for an API function. >>>>>>> Why VPP prefers set this flag later? >>>>>>> Would it be better to have some driver-specific flags, no matter the ports? >>>>>> >>>>>> As is, there seem to be two ways apps deal with NIC-specific tweaks/quirks. >>>>>> >>>>>> 1. Leave everything to the user. >>>>>> >>>>>> Let the human user specify NIC-specific settings (e.g. devarg, >>>>>> not-so-standard MTU/MRU, offloads with not-so-uniform behavior). The >>>>>> app simply passes these to DPDK and does nothing else. Devargs are >>>>>> passed to rte_eal_init. Other settings are applied during the >>>>>> configure phase after rte_eal_init. >>>>>> >>>>>> For example, OVS seems to go for a smallest common denominator that >>>>>> works with most NICs out of the box. Otherwiese, it kinda falls into >>>>>> this camp. >>>>>> >>>>>> For a problematic NIC that needs user intervention, troubleshooting >>>>>> goes like this :-) >>>>>> - Install app. >>>>>> - Run with settings that worked on a previous machine. >>>>>> - Some features suddenly do not work. >>>>>> - Google search this and that ("why this does not work on this server?"). >>>>>> - Contact vendors. >>>>>> - Find out this NIC has unexpected behavior. >>>>>> - Set devarg, tweak MTU/MRU, ... ("Oh need to set this for .."). >>>>>> - Now the features work. >>>>>> >>>>>> 2. Hide ugly tweaks from the user. >>>>>> >>>>>> VPP falls into this camp. The user specifies BDFs in the config (no >>>>>> devargs). The app calls rte_eal_init(BDFs), iterates through the >>>>>> discovered ports, applies whatever NIC-specific settings necessary >>>>>> during the configure phase (i.e. do this for vendor A NIC, do that for >>>>>> vendor B NIC), and then start the ports. >>>>>> >>>>>> The ingress vlan rewrite mode is devarg now, so is not usable in this >>>>>> model. One way around it is a private API. Driver specific flags >>>>>> during the configure phase would also work fine. Though, enic might be >>>>>> the only user of those flags. >>>>> >>>>> I think DPDK needs some driver configuration. >>>>> Currently the config is done per device with devargs. >>>>> The next devargs format allow this: >>>>> driver=enic,rewrite=on >>>>> and it can be passed to rte_eal_init(). >>>>> >>>>> We did not progress on the implementation of this format in recent months, >>>>> but you are welcome to help! >>>>> Instead of passing devargs in the whitelist/blacklist options, >>>>> we should introduce a new option, like --dev. >>>> >>>> But it will be still devarg in new implementation. >>> >>> With the new syntax, no need to specify a device. >>> We can match a driver or multiple drivers sharing the same property. >>> >>>> I guess for this use case, there is a need to pass this information from an API. >>>> Options can be: >>>> 1- PMD specific API >>>> 2- Extend ethdev dev_ops for each usecase >>>> 3- Have a generic API, as suggested above >>>> 4- Extend configure to accept flags >>>> >>>> I don't see a winner in above list, each has some problems. Any comment on how >>>> to proceed? >>> >>> I don't see a problem with the devargs approach. >> >> Devargs either passed via command line to DPDK application, or parameter to >> hotplug APIs. > > The application can pass whatever it wants to EAL. This means changing current device probe logic completely, right. Instead of probing everything on start, probe nothing and application add devices via eal (hotplug) API calls with providing devargs. I have no issue with this picture, only it doesn't look soon. > In the case described above, the application wants to enable > a mode of the driver for all its devices. > That's why I think the right solution is a driver option, > which can be achieved with the new devargs syntax. > >> If someone wants to use regular probe without any command line argument, and >> later configure the device via an API, can devargs be used? > > This is a scenario different of what is asked above. > In the case of a specific configuration of one device, > we have three choices. > These are your suggestions, with my comments: > 1- PMD specific API > 2- Extend ethdev dev_ops for each usecase > (3- Have a generic API) = choice 2 > (4- Extend configure to accept flags) = choice 1 > This is a choice 3: > - no support of exotic features Not sure if this is a real option for a vendor, HWs has exotic features and they want to enable it, I believe SW should not be the blocker for this. Also I definitely agree that exotic features should not break main/common usage, make it hard to use or make it confusing/complex etc. Until we have a better solution I guess we need to continue with private APIs.