From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 5CB16BD21 for ; Wed, 26 Oct 2016 17:54:41 +0200 (CEST) Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga102.jf.intel.com with ESMTP; 26 Oct 2016 08:54:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,551,1473145200"; d="scan'208";a="23952440" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.210.150]) by fmsmga005.fm.intel.com with SMTP; 26 Oct 2016 08:54:37 -0700 Received: by (sSMTP sendmail emulation); Wed, 26 Oct 2016 16:54:37 +0100 Date: Wed, 26 Oct 2016 16:54:37 +0100 From: Bruce Richardson To: Thomas Monjalon Message-ID: <20161026155437.GD42064@bricha3-MOBL3.ger.corp.intel.com> References: <1476850306-2141-1-git-send-email-rasesh.mody@qlogic.com> <1476850306-2141-19-git-send-email-rasesh.mody@qlogic.com> <4491648.SeJM0cAZEU@xps13> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4491648.SeJM0cAZEU@xps13> Organization: Intel Research and =?iso-8859-1?Q?De=ACvel?= =?iso-8859-1?Q?opment?= Ireland Ltd. User-Agent: Mutt/1.7.1 (2016-10-04) Cc: dev@dpdk.org, Dept-EngDPDKDev@qlogic.com, ferruh.yigit@intel.com Subject: Re: [dpdk-dev] [PATCH v4 18/32] net/qede: add missing 100G link speed capability X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 26 Oct 2016 15:54:41 -0000 On Wed, Oct 26, 2016 at 05:41:58PM +0200, Thomas Monjalon wrote: > 2016-10-18 21:11, Rasesh Mody: > > From: Harish Patil > > > > This patch fixes the missing 100G link speed advertisement > > when the 100G support was initially added. > > > > Fixes: 2af14ca79c0a ("net/qede: support 100G") > > > > Signed-off-by: Harish Patil > [...] > > [Features] > > +Speed capabilities = Y > > This feature should be checked only when it is fully implemented, > i.e. when you return the real capabilities of the device. > > > --- a/drivers/net/qede/qede_ethdev.c > > +++ b/drivers/net/qede/qede_ethdev.c > > @@ -599,7 +599,8 @@ qede_dev_info_get(struct rte_eth_dev *eth_dev, > > DEV_TX_OFFLOAD_UDP_CKSUM | > > DEV_TX_OFFLOAD_TCP_CKSUM); > > > > - dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G; > > + dev_info->speed_capa = ETH_LINK_SPEED_25G | ETH_LINK_SPEED_40G | > > + ETH_LINK_SPEED_100G; > > } > > It is only faking the capabilities at driver-level. > You should check if the underlying device is able to achieve 100G > before advertising this flag to the application. > > I suggest to update this patch to remove the doc update. > The contract is to fill it only when the code is fixed. > By the way, we must call every other drivers to properly implement > this feature. I agree that devices should only advertise speeds they support and the doc should reflect this ability (or lack of this ability) in a driver. Regards, /Bruce