From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 956FAA32A1 for ; Thu, 24 Oct 2019 11:48:19 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 366301E937; Thu, 24 Oct 2019 11:47:21 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 234C51E935 for ; Thu, 24 Oct 2019 11:47:14 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 Oct 2019 02:47:14 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,224,1569308400"; d="scan'208";a="228452882" Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.10]) ([10.237.221.10]) by fmsmga002.fm.intel.com with ESMTP; 24 Oct 2019 02:47:11 -0700 To: Pavan Nikhilesh Bhagavatula , Andrew Rybchenko , Jerin Jacob Kollanukkaran Cc: "dev@dpdk.org" , Adrien Mazarguil , Thomas Monjalon , Xiaolong Ye , Bruce Richardson References: 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+iQJUBBMBCgA+AhsDAh4BAheABQsJCAcDBRUK CQgLBRYCAwEAFiEE0jZTh0IuwoTjmYHH+TPrQ98TYR8FAl1meboFCQlupOoACgkQ+TPrQ98T YR9ACBAAv2tomhyxY0Tp9Up7mNGLfEdBu/7joB/vIdqMRv63ojkwr9orQq5V16V/25+JEAD0 60cKodBDM6HdUvqLHatS8fooWRueSXHKYwJ3vxyB2tWDyZrLzLI1jxEvunGodoIzUOtum0Ce gPynnfQCelXBja0BwLXJMplM6TY1wXX22ap0ZViC0m714U5U4LQpzjabtFtjT8qOUR6L7hfy YQ72PBuktGb00UR/N5UrR6GqB0x4W41aZBHXfUQnvWIMmmCrRUJX36hOTYBzh+x86ULgg7H2 1499tA4o6rvE13FiGccplBNWCAIroAe/G11rdoN5NBgYVXu++38gTa/MBmIt6zRi6ch15oLA Ln2vHOdqhrgDuxjhMpG2bpNE36DG/V9WWyWdIRlz3NYPCDM/S3anbHlhjStXHOz1uHOnerXM 1jEjcsvmj1vSyYoQMyRcRJmBZLrekvgZeh7nJzbPHxtth8M7AoqiZ/o/BpYU+0xZ+J5/szWZ aYxxmIRu5ejFf+Wn9s5eXNHmyqxBidpCWvcbKYDBnkw2+Y9E5YTpL0mS0dCCOlrO7gca27ux ybtbj84aaW1g0CfIlUnOtHgMCmz6zPXThb+A8H8j3O6qmPoVqT3qnq3Uhy6GOoH8Fdu2Vchh TWiF5yo+pvUagQP6LpslffufSnu+RKAagkj7/RSuZV25Ag0EV9ZMvgEQAKc0Db17xNqtSwEv mfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ESYpV8QWj0xK4YM0dLxnDU2IYxjEshSB1T qAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4AibPtrHuIXWQOBECcVZTTOdZYGAzaYzxiA ONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxDUQljeNvKYt1lZE/gAUUxNLWsYyTT+22/ vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35p iVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVjsM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQ I3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdcq9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYH fVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH71PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZ qw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFBVOQOxCvwRG2QCgcJ/UTn5vlivul+cThi 6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJl Rr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYCGwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNh HwUCXWZ5wAUJB3FgggAKCRD5M+tD3xNhH2O+D/9OEz62YuJQLuIuOfL67eFTIB5/1+0j8Tsu o2psca1PUQ61SZJZOMl6VwNxpdvEaolVdrpnSxUF31kPEvR0Igy8HysQ11pj8AcgH0a9FrvU /8k2Roccd2ZIdpNLkirGFZR7LtRw41Kt1Jg+lafI0efkiHKMT/6D/P1EUp1RxOBNtWGV2hrd 0Yg9ds+VMphHHU69fDH02SwgpvXwG8Qm14Zi5WQ66R4CtTkHuYtA63sS17vMl8fDuTCtvfPF HzvdJLIhDYN3Mm1oMjKLlq4PUdYh68Fiwm+boJoBUFGuregJFlO3hM7uHBDhSEnXQr5mqpPM 6R/7Q5BjAxrwVBisH0yQGjsWlnysRWNfExAE2sRePSl0or9q19ddkRYltl6X4FDUXy2DTXa9 a+Fw4e1EvmcF3PjmTYs9IE3Vc64CRQXkhujcN4ZZh5lvOpU8WgyDxFq7bavFnSS6kx7Tk29/ wNJBp+cf9qsQxLbqhW5kfORuZGecus0TLcmpZEFKKjTJBK9gELRBB/zoN3j41hlEl7uTUXTI JQFLhpsFlEdKLujyvT/aCwP3XWT+B2uZDKrMAElF6ltpTxI53JYi22WO7NH7MR16Fhi4R6vh FHNBOkiAhUpoXRZXaCR6+X4qwA8CwHGqHRBfYFSU/Ulq1ZLR+S3hNj2mbnSx0lBs1eEqe2vh cA== Message-ID: Date: Thu, 24 Oct 2019 10:47:11 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH v12 0/7] ethdev: add new Rx offload 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/22/2019 4:20 PM, Pavan Nikhilesh Bhagavatula wrote: >> -----Original Message----- >> From: Ferruh Yigit >> Sent: Tuesday, October 22, 2019 7:51 PM >> To: Andrew Rybchenko ; Pavan Nikhilesh >> Bhagavatula ; Jerin Jacob Kollanukkaran >> >> Cc: dev@dpdk.org; Adrien Mazarguil ; >> Thomas Monjalon ; Xiaolong Ye >> ; Bruce Richardson >> >> Subject: Re: [dpdk-dev] [PATCH v12 0/7] ethdev: add new Rx >> offload flags >> On 10/22/2019 11:16 AM, Andrew Rybchenko wrote: >>> Hi, >>> >>> @Pavan, see question below. >>> >>> On 10/21/19 6:34 PM, Ferruh Yigit wrote: >>>> On 10/21/2019 4:19 PM, Pavan Nikhilesh Bhagavatula wrote: >>>>> Hi Ferruh, >>>>> >>>>>> -----Original Message----- >>>>>> From: Ferruh Yigit >>>>>> Sent: Monday, October 21, 2019 8:37 PM >>>>>> To: Andrew Rybchenko ; Pavan >> Nikhilesh >>>>>> Bhagavatula ; Jerin Jacob >> Kollanukkaran >>>>>> >>>>>> Cc: dev@dpdk.org; Adrien Mazarguil >> ; >>>>>> Thomas Monjalon ; Xiaolong Ye >>>>>> ; Bruce Richardson >>>>>> >>>>>> Subject: Re: [dpdk-dev] [PATCH v12 0/7] ethdev: add new Rx >>>>>> offload flags >>>>>> On 10/18/2019 11:31 AM, Andrew Rybchenko wrote: >>>>>>> On 10/18/19 12:42 PM, Ferruh Yigit wrote: >>>>>>>> On 10/18/2019 8:32 AM, Andrew Rybchenko wrote: >>>>>>>>> Hi Ferruh, >>>>>>>>> >>>>>>>>> since I've reviewed I'll reply as I understand it. >>>>>>>>> >>>>>>>>> On 10/17/19 8:43 PM, Ferruh Yigit wrote: >>>>>>>>>> On 10/17/2019 1:02 PM, pbhagavatula@marvell.com wrote: >>>>>>>>>>> From: Pavan Nikhilesh >>>>>>>>>>> >>>>>>>>>>> Add new Rx offload flags `DEV_RX_OFFLOAD_RSS_HASH` >> and >>>>>>>>>>> `DEV_RX_OFFLOAD_FLOW_MARK`. These flags can be >> used to >>>>>>>>>>> enable/disable PMD writes to rte_mbuf fields `hash.rss` >> and >>>>>> `hash.fdir.hi` >>>>>>>>>>> and also `ol_flags:PKT_RX_RSS` and >> `ol_flags:PKT_RX_FDIR`. >>>>>>>>>> Hi Pavan, >>>>>>>>>> >>>>>>>>>> Initially sorry for involving late, >>>>>>>>>> >>>>>>>>>> When we expose an interface to the applications, they will >> expect >>>>>> those will be >>>>>>>>>> respected by underlying PMDs. >>>>>>>>>> As far as I can see drivers are updated to report new added >> Rx >>>>>> offload flags as >>>>>>>>>> supported capabilities but drivers are not using those flags at >> all, >>>>>> so >>>>>>>>>> application providing that flag won't really enable/disable >>>>>> anything, I think >>>>>>>>>> this is a problem and it is wrong to lie even for the PMDs J >>>>>>>>> It is required to let applications know that the offload is >> supported. >>>>>>>>> There are a number of cases when an offload cannot be >> disabled, >>>>>>>>> but it does not mean that the offload must not be advertised. >>>>>>>> Can't disable is something else, although I believe that is rare >> case, in >>>>>> this >>>>>>>> case driver can enable/disable the RSS and representing this as >> an >>>>>> offload >>>>>>>> capability. >>>>>>> It is not enabling/disabling the RSS. It is enabling/disabling RSS >> hash >>>>>>> delivery >>>>>>> together with an mbuf. >>>>>> >>>>>> Got it, it is related to the RSS hash delivery. >>>>>> >>>>>>>> But when user want to configure this offload by setting or >> unsetting >>>>>> in offload >>>>>>>> config, driver just ignores it. >>>>>>> When application enables offload, it says that it needs it and >> going to >>>>>> use >>>>>>> (required). When the offload is not enabled, application simply >> don't >>>>>> care. >>>>>>> So, if the information is still provided it does not harm. >>>>>> >>>>>> Not sure if there is no harm, a config option not respected by >>>>>> underlying PMDs >>>>>> silently is a problem I think. >>> >>> Some time ago the idea of offloads which cannot be disabled >>> by PMD was discussed and, if I remember decision correctly, >>> it was decided that it will overcomplicate it. >>> It was discussed when new offload API is introduced. >>> Logging is helpful sometimes, but it is not a panacea. >> >> I remember it has been discussed in the context of CRC, because some >> HW were not >> able to disable CRC STRIP, it has been implemented as I suggested for >> this case, >> when application requests to disable CRC STRIP offload, the PMD print a >> log >> saying it can't be disabled and keep the internal state as offload >> enabled. >> >> Other than this I think all offloads announced as supported by PMD can >> be >> enabled and disabled. >> >>> >>>>>>>>> If driver see benefits from disabling the offload (e.g. avoid >> delivery >>>>>>>>> of RSS hash from NIC to host), it can do it after the patchset. >>>>>>>> Yes but I think this patchset shouldn't ignore that disabling the >>>>>> feature is not >>>>>>>> implemented yet. If those PMDs that has been updated to >> report >>>>>> the HASH >>>>>>>> capability has RSS enabled by default, I suggest adding a check >> for >>>>>> this offload >>>>>>>> in PMD, >>>>>>>> if it is requested to disable (which means not requested for >> enable), >>>>>> print a >>>>>>>> log saying disabling HASH is not supported and set this flag in >> the >>>>>> offload >>>>>>>> configuration to say PMD is configured to calculate the HASH. >>>>>>>> Later PMD maintainers may prefer to replace that error log with >>>>>> actual disable code. >>>>>>> It is possible to do. Of course, it is better to provide real offload >>>>>>> values on get, but >>>>>>> eth_conf is const in rte_eth_dev_configure(), so, we can't >> change it >>>>>> and >>>>>>> it is good. >>>>>>> So, the only way is rte_eth_rx_queue_info_get(). >>>>>>> I guess there is a lot of space for the same improvement for >> other Rx >>>>>>> offloads >>>>>>> in various PMDs. >>>>>> >>>>>> We don't need the update 'eth_conf' parameter of the >>>>>> 'rte_eth_dev_configure()', >>>>>> that is what user requested, but config stored in 'dev->data- >>> dev_conf' >>>>>> which >>>>>> can be updated. >>> >>> What for? As I understand data->dev_conf should not be used >> outside >>> librte_ethdev and drivers. Basically it means that all patched drivers >>> should be updated to log information message if the offload is not >>> requested, but will be enabled anyway and updated >>> data->dev_conf.rxmode.offloads. Please, confirm. >> >> To keep PMD internal state correct. >> >>> >>>>>>> Also I worry that it could be not that trivial to do in all effected >> PMDs. >>>>>> >>>>>> Yes it can be some work, and if this patchset doesn't do it, who >> will do >>>>>> the work? >>> >>> Pavan, will you do or should I care about it if confirmed? > > I will send a v13. > >>> >>>>>>>>>> Specific to `DEV_RX_OFFLOAD_RSS_HASH`, we have already >>>>>> some RSS config >>>>>>>>>> structures and it is part of the 'rte_eth_dev_configure()' API, >>>>>> won't it create >>>>>>>>>> multiple way to do same thing? >>>>>>>>> No, a new offload is responsible for RSS hash delivery from >> NIC to >>>>>> host >>>>>>>>> and fill in in mbuf returned to application on Rx. >>>>>>>> What you have described is already happening without the >> new >>>>>> offload flag and >>>>>>>> this is my concern that we are duplicating it. >>>>>>>> >>>>>>>> >>>>>>>> There is a 'struct rte_eth_rxmode' (under 'struct rte_eth_conf') >>>>>>>> which has 'enum rte_eth_rx_mq_mode mq_mode;' >>>>>>>> >>>>>>>> If "mq_mode == ETH_MQ_RX_NONE" hash calculation is >> disabled, >>>>>> and >>>>>>>> 'mbuf::hash::rss' is not updated. >>>>>>> No-no. It binds RSS distribution and hash delivery. What the new >>>>>>> offload allows to achieve: I want Rx to spread traffic over many >> Rx >>>>>>> queues, but I don't need RSS hash. >>>>>> >>>>>> I see, so RSS configuration will stay same, but driver needs to take >> care >>>>>> the >>>>>> new flags to decide to update or not the mbuf::rss::hash field. >>>>>> >>>>>> I don't know if disabling RSS but calculating hash is supported, if >> not >>>>>> supported that case also should be checked by driver. >>> >>> Yes, it is interesting question what should happen if >> ETH_MQ_RX_NONE with >>> DEV_RX_OFFLOAD_RSS_HASH. It sounds like rte_ethdev should >> check >>> dev_conf.rxmode.mq_mode & ETH_MQ_RX_RSS_FLAG is set when >>> DEV_RX_OFFLOAD_RSS_HASH is requested. Otherwise the request is >> invalid. >> >> +1 >> >>> May be it could be relaxed in the future, but not now. >>> >>>>>>>> (Thanks Bruce to helping finding it out) >>>>>>>> >>>>>>>> >>>>>>>>>> And for the `ol_flags:PKT_RX_RSS` flag, it was already used >> to >>>>>> mark that >>>>>>>>>> 'mbuf::hash::rss' is valid, right? Is there anything new related >> that >>>>>> in the set? >>>>>>>>> As I understand you mean, ol_flags::PKT_RX_RSS_HASH. >>>>>>>>> Yes, the new offload allows say if application needs it or now. >>>>>>>>> Basically it decouples RSS distribution and hash delivery. >>>>>>>> Setting 'ol_flags::PKT_RX_RSS_HASH' and 'mbuf::hash::rss' >> already >>>>>> there and not >>>>>>>> changing. I just want to clarify since this is not clear in the >> commit log. >>>>>>>> >>>>>>>> Only addition is to add a new flag to control PMD to >> enable/disable >>>>>> hash >>>>>>>> calculation (which PMDs ignore in the patch ???) >>>>>>> It is not calculation, but delivery of the value from HW to >> applications. >>>>>> >>>>>> OK >>>>>> >>>>>>> Yes, commit log may/should be improved.> >>>>>>>>>> Specific to the `DEV_RX_OFFLOAD_FLOW_MARK` and >>>>>> `RTE_FLOW_ACTION_FLAG`, they are >>>>>>>>>> rte_flow actions, application can verify and later request >> these >>>>>> actions via >>>>>>>>>> rte_flow APIs. Why we are adding an additional >> RX_OFFLOAD flag >>>>>> for them? >>>>>>>>> The reason is basically the same as above. HW needs to know >> in >>>>>> advance, >>>>>>>>> if application is going to use flow marks and configure Rx >> queue to >>>>>> enable >>>>>>>>> the information delivery. >>>>>>>> What you described is done via 'rte_flow_create()' API, >> application >>>>>> will request >>>>>>>> those actions via API and Rx queue will be configured >> accordingly, >>>>>> this is more >>>>>>>> dynamic approach. Why application need to set this additional >>>>>> configuration flag? >>>>>>> More dynamic approach is definitely better, but it is not always >>>>>> possible. >>>>>>> Some PMDs can't even change MTU dynamically or MTU >> changing >>>>>> requires >>>>>>> restart which is hardly really a dynamic change. Of course, it is >>>>>>> unlikely that >>>>>>> MTU is changed when traffic is running etc, but still possible. >>>>>>> The information about necessity to support flow marks delivery >> may >>>>>>> be required on Rx queue setup and cannot be changed >> dynamically >>>>>> when >>>>>>> Rx queue is running and application would like to add flow rule >> with >>>>>> mark >>>>>>> action. >>>>>> It doesn't need to be changed dynamically, application can call >>>>>> 'rte_flow_validate()' and learn if it can set this action or not. >> Perhaps I >>>>>> am >>>>>> missing something, when it is required to have this as >> configuration >>>>>> option? >>>>>> >>>>>>>> And as above the new RX offload flags ignored by PMDs, hard >> to >>>>>> understand what >>>>>>>> is the intention here. >>>>>>>> >>>>>>>> >>>>>>>> Above usage of flags feels like the intention is adding some >> capability >>>>>>>> information for the PMDs more that adding new offload >>>>>> configuration. >>>>>>>> If so this is bigger/older problem, and instead of abusing the >> offload >>>>>> flags we >>>>>>>> can think of an API to present device capabilities, and move >>>>>> features.ini >>>>>>>> content to the API in long term. >>>>>>> What I really like with these new offload flags for Rx hash and >> flow >>>>>> mark is >>>>>>> that it makes features which provide information in mbuf on Rx >>>>>> consistent: >>>>>>>  - want timestamp? => DEV_RX_OFFLOAD_TIMESTAMP >>>>>>>  - want Rx checksum flags => DEV_RX_OFFLOAD_CHECKSUM >>>>>>>  - want to strip VLAN? => DEV_RX_OFFLOAD_VLAN_STRIP >>>>>>>  - want RSS hash? => DEV_RX_OFFLOAD_RSS_HASH >>>>>>>  - want flow mark support? => >> DEV_RX_OFFLOAD_FLOW_MARK >>>>>>> >>>>>>> Also it perfectly fits dynamic mbuf fields and allows to make RSS >> hash >>>>>>> and flow mark fields dynamic with the new offloads as controls >>>>>> Agree RSS_HASH fits well, my main concern with the patchset is >> driver >>>>>> implementations are missing and just ignored. >>>>>> >>>>> Ignoring driver implementation is intentional as it involves adding a >> branch >>>>> in Rx fastpath function for all drivers and might have -ve effects on >> performance. >>>> Yes it may affect performance. Also it may be too much driver >> specific >>>> implementation. >>>> >>>> That is why I suggest, following: >>>> For the drivers that claim this capability, >>>> - For the case driver updates the mbuf::rss:hash >>>> Check if this offload requested or not, if not print an error and set >> internal >>>> config as this offload enabled >>> >>> I would not say it is an error. At maximum it is warning, but I would >> use >>> just info log level. I think there is nothing critical there. >> >> Any log is OK, only info level my be disabled by default by many drivers, >> to be >> sure this is printed, warning can be better. >> >>> >>>> - For the case driver not updates the mbuf::rss:hash >>>> Check if this offload requested or not, if requested print an error >> and set >>>> internal config as this offload disabled >>> >>> If so, the offload is not advertised and generic checks in rte_ethdev >>> catch it and return error. >> >> You are right, for this case it shouldn't be advertised. >> >> >> Just to double check the understanding of this discussion, is following >> summary >> correct? >> >> A) When RSS enabled >> 1) if RSS_HASH set, put hash value to "mbuf::rss:hash" and set >> PKT_RX_RSS_HASH >> 2) if RSS_HASH unset, don't updated >> "mbuf::rss:hash"/PKT_RX_RSS_HASH >> >> B) When RSS disabled >> 3) if RSS_HASH set, >> i) if HW supports HASH calculation offload independently, enable it >> and put hash value to "mbuf::rss:hash" and set PKT_RX_RSS_HASH >> ii) if HW doesn't support it return error >> 4) if RSS_HASH unset, don't updated >> "mbuf::rss:hash"/PKT_RX_RSS_HASH >> >> >> This may be good for (3) to enable HASH calculation offload >> independent from >> RSS. Not sure if this is supported by HW or this is the motivation of the >> patch. >> >> But for (1), since the hash value is already part of descriptor, this will >> bring >> an additional check, as Pavan mentioned, which is not good. >> >> For (3) is there any intention to implicitly enable RSS, this was my >> understanding and concern to have two different methods to enable >> RSS, >> I guess my understanding was wrong but I want to confirm this. >> >> Currently RSS hash delivery is implied when RSS is enabled. >> > > Correct me if I'm wrong but current agreement is to print a log message when application > tries to disable RSS and it's not supported by underlying PMD. Yes. > So all the PMD which we aren’t currently modifying need to be have a log message. No. If a PMD not announced this capability, it doesn't need to check this flag. From the PMDs that announce this capability, the ones that doesn't support disabling the offload should log this and update the PMD config to represent the actual status. > Also, it applies to FLOW_FLAG and FLOW_MARK. I am not really sure about them, I think we are duplicating the rte_flow API and we shouldn't have them as Rx offload, I would like to hear more comment about this. > > If we are at an agreement I will send v13. > > Thanks, > Pavan. > >>> >>>> Later PMD maintainers may prefer to replace those errors with >> actual >>>> implementation if they want. >>> >>> [snip] >>> >