Re: [dpdk-dev] [PATCH v2 0/3] Support setting TX rate for queue and VF

[Neil Horman <nhorman@tuxdriver.com>]

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
> > 
> > 
>