From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 0A23FD4E0; Wed, 11 Jan 2017 19:07:27 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP; 11 Jan 2017 10:07:26 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,346,1477983600"; d="scan'208";a="921428134" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.237.220.38]) ([10.237.220.38]) by orsmga003.jf.intel.com with ESMTP; 11 Jan 2017 10:07:25 -0800 To: "Iremonger, Bernard" , "Lu, Wenzhuo" , "dev@dpdk.org" References: <1484102853-53205-1-git-send-email-wenzhuo.lu@intel.com> <8CEF83825BEC744B83065625E567D7C224D1CB14@IRSMSX108.ger.corp.intel.com> <8CEF83825BEC744B83065625E567D7C224D1CCE7@IRSMSX108.ger.corp.intel.com> <8CEF83825BEC744B83065625E567D7C224D1CD35@IRSMSX108.ger.corp.intel.com> <727ff859-107b-74c3-1110-7ed8bdbc70f6@intel.com> <8CEF83825BEC744B83065625E567D7C224D1CDBD@IRSMSX108.ger.corp.intel.com> Cc: "Wu, Jingjing" , "stable@dpdk.org" From: Ferruh Yigit Message-ID: <838b982b-8bda-bb1c-bd7e-782438fcaa91@intel.com> Date: Wed, 11 Jan 2017 18:07:25 +0000 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: <8CEF83825BEC744B83065625E567D7C224D1CDBD@IRSMSX108.ger.corp.intel.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH] app/testpmd: fix ixgbe private API calling 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, 11 Jan 2017 18:07:28 -0000 On 1/11/2017 4:29 PM, Iremonger, Bernard wrote: > Hi Ferruh, > >> -----Original Message----- >> From: Yigit, Ferruh >> Sent: Wednesday, January 11, 2017 4:19 PM >> To: Iremonger, Bernard ; Lu, Wenzhuo >> ; dev@dpdk.org >> Cc: Wu, Jingjing ; stable@dpdk.org >> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix ixgbe >> private API calling >> >> On 1/11/2017 3:47 PM, Iremonger, Bernard wrote: >>> Hi Ferruh, >>> >>>> -----Original Message----- >>>> From: Yigit, Ferruh >>>> Sent: Wednesday, January 11, 2017 3:27 PM >>>> To: Iremonger, Bernard ; Lu, Wenzhuo >>>> ; dev@dpdk.org >>>> Cc: Wu, Jingjing ; stable@dpdk.org >>>> Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] app/testpmd: fix ixgbe >>>> private API calling >>>> >>>> On 1/11/2017 3:20 PM, Iremonger, Bernard wrote: >>>>> Hi Wenzhuo, >>>>> >>>>> >>>>>>> Subject: [dpdk-dev] [PATCH] app/testpmd: fix ixgbe private API >>>>>>> calling >>>>>>> >>>>>>> Some ixgbe private APIs are added to expose ixgbe specific functions. >>>>>>> When they're used by testpmd, there's no check for if the NICs are >>>> ixgbe. >>>>>>> Other NICs also have chance to call these APIs. >>>>>>> This patch add the check and the feedback print. >>>>>> >>>>>> I am not sure that testpmd is the right place to do this. >>>>>> The rte_pmd_ixgbe_* functions are public API's which can be called >>>>>> by other applications. >>>>>> The checks should be in the rte_pmd_ixgbe_* API's >>>>> >>>>> It is useful to handle the return code -ENOTSUP in testpmd. >>>>> >>>> >>>> Makes sense, and I think it is good idea to add them in your patch, >>>> since it introduces returning -ENOTSUP, would you mind sending a new >>>> version of your patch with this update? >>>> So we can drop this patch completely. >>>> >>>> Thanks, >>>> ferruh >>>> >>> I don't think this patch should be dropped. >>> Testpmd is already handling -EINVAL and -ENODEV. >>> It makes sense for it to handle -ENOTSUP for the cases in the patch. >> >> This patch adds driver check [1] before ixgbe APIs, since now that check is >> done within ixgbe APIs by your patch. Why do we need this patch at all? >> >> [1] >> if (strstr(dev_info.driver_name, "ixgbe") != NULL) > > I think our lines have got slightly crossed. > This patch is doing two things, the check [1] above which is not needed now (after my ixgbe patch). > Secondly it is handling the ret code of the rte_pmd_ixgbe_* API's, I think this is useful. I was thinking returning -ENOTSUP introduced for your patch, but it seems this is not true for all functions, some is already returning -ENOTSUP, so testpmd should cover them. I believe intention of this patch is the first item you have mentioned, but can be converted to second one too. Wenzhuo, When do you have time, can you convert this patch to one that covers "-ENOTSUP" error messages for ixgbe APIs please? If you don't have time, please let me know I can do the patch? Thanks, ferruh > > Regards, > > Bernard. > > >