From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp.tuxdriver.com (charlotte.tuxdriver.com [70.61.120.58]) by dpdk.org (Postfix) with ESMTP id BC66758E4 for ; Thu, 5 Jun 2014 13:01:38 +0200 (CEST) Received: from hmsreliant.think-freely.org ([2001:470:8:a08:7aac:c0ff:fec2:933b] helo=localhost) by smtp.tuxdriver.com with esmtpsa (TLSv1:AES128-SHA:128) (Exim 4.63) (envelope-from ) id 1WsVQX-0007JA-N6; Thu, 05 Jun 2014 07:01:51 -0400 Date: Thu, 5 Jun 2014 07:01:44 -0400 From: Neil Horman To: "Ouyang, Changchun" Message-ID: <20140605110144.GA20841@hmsreliant.think-freely.org> References: <1401090331-18455-1-git-send-email-changchun.ouyang@intel.com> <20140526225257.GB15911@neilslaptop.think-freely.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) X-Spam-Score: -2.9 (--) X-Spam-Status: No Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF 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: Thu, 05 Jun 2014 11:01:39 -0000 On Thu, Jun 05, 2014 at 03:11:46AM +0000, Ouyang, Changchun wrote: > Hi, Neil > > " but you're implementaiton requires that it be re-implemented for each PMD " > > [Changchun]: Different PMD(corresponding diff NIC) has different register to set. > It makes sense to me has different implementation to support limit tx rate. > > " Why not just export max tx rates from the PMD and write a generic queuing libarary to do rate limitation for any PMD?" > > [Changchun]: Just export max tx rate is not enough for customer. > I think if we can leverage HW feature to do it, why need a more complicated lib to do so? > > Thanks > Changchun > sorry, I reread my initial comment and see I wasn't being at all clear. You actually do have a nice generic layer for applications to call through in rte_ethdev, which is good. What you don't have is a backoff mechanism. That is to say the return of ENOSUP in rte_eth_dev_set_queue_rate_limit means that your queuing library doesnt' work for any device that doesn't support hardware rate limiting (implying none of the virtual devs get to implement this). Don't you think you should implement a generic software queue rate limit layer for the myrriad of devices that cant' do what ixgbe can? Regards Neil > -----Original Message----- > From: Neil Horman [mailto:nhorman@tuxdriver.com] > Sent: Tuesday, May 27, 2014 6:53 AM > To: Ouyang, Changchun > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF > > On Mon, May 26, 2014 at 03:45:28PM +0800, Ouyang Changchun wrote: > > This patch v2 fixes some errors and warnings reported by checkpatch.pl. > > > > This patch series also contain the 3 items: > > 1. Add API to support setting TX rate for a queue or a VF. > > 2. Implement the functionality of setting TX rate for queue or VF in IXGBE PMD. > > 3. Add commands in testpmd to test the functionality of setting TX rate for queue or VF. > > > > Ouyang Changchun (3): > > Add API to support set TX rate for a queue and VF. > > Implement the functionality of setting TX rate for queue or VF in > > IXGBE PMD. > > Add commands in testpmd to test the functionality of setting TX rate > > for queue or VF. > > > > app/test-pmd/cmdline.c | 159 +++++++++++++++++++++++++++++++++++- > > app/test-pmd/config.c | 47 +++++++++++ > > app/test-pmd/testpmd.h | 3 + > > lib/librte_ether/rte_ethdev.c | 71 ++++++++++++++++ > > lib/librte_ether/rte_ethdev.h | 51 ++++++++++++ > > lib/librte_pmd_ixgbe/ixgbe_ethdev.c | 122 +++++++++++++++++++++++++++ > > lib/librte_pmd_ixgbe/ixgbe_ethdev.h | 13 ++- > > 7 files changed, 462 insertions(+), 4 deletions(-) > > > This seems a bit backwards. queue rate limiting is rather a generic function, that doesn't really need to know any details about the hardware, save for its maximum tx rate, but you're implementaiton requires that it be re-implemented for each PMD. Why not just export max tx rates from the PMD and write a generic queuing libarary to do rate limitation for any PMD? > > Neil > > > -- > > 1.9.0 > > > > >