[dpdk-dev] [PATCH 0/6] get status of Rx and Tx descriptors

[Thomas Monjalon]

2017-03-04 21:45, Olivier Matz:
> "Venkatesan, Venky" <venky.venkatesan at intel.com> wrote:
> > From: Olivier Matz
> > > On Fri, 3 Mar 2017 16:18:52 +0000, "Venkatesan, Venky" wrote:  
> > > > From: Olivier Matz
> > > > > On Thu, 2 Mar 2017 15:32:15 +0000, Bruce Richardson wrote:  
> > > > > > On Wed, Mar 01, 2017 at 06:19:06PM +0100, Olivier Matz wrote:  
> > > > > > > The usage of these functions can be:
> > > > > > > - on Rx, anticipate that the cpu is not fast enough to process
> > > > > > >   all incoming packets, and take dispositions to solve the
> > > > > > >   problem (add more cpus, drop specific packets, ...)
> > > > > > > - on Tx, detect that the link is overloaded, and take dispositions
> > > > > > >   to solve the problem (notify flow control, drop specific
> > > > > > >   packets)
> > > > > > >  
[...]
> > > > > > Are these really needed for real applications? I suspect our
> > > > > > trivial l3fwd power example can be made to work ok without them.

OK, please remove the use of such old API in the example.

[...]
> So, the penalty, in the worst case (burst of 32, 100c/pkt) is ~6%.
> Given the information it provides, it is acceptable to me.

Any penalty is acceptable, given it is not mandatory to call these functions.

> Note we are talking here about an optional API, that would only impact
> people that use it.

Yes, it just brings more information and can be used for some debug measures.

[...]
> Also, changing the Rx burst function is much more likely to be refused
> than adding an optional API.

Yes, changing Rx/Tx API is not really an option and does not bring so much
benefits.

[...]
> > > > So, NAK. My suggestion would be to go back to the older API.  
> > > 
> > > I don't understand the reason of your nack.
> > > The old API is there (for Rx it works the same), and it is illustrated in an
> > > example. Since your arguments also applies to the old API, so why are you
> > > saying we should keep the older API?
> > >   
> > 
> > I am not a fan of the old API either. In hindsight, it was a mistake
> > (which we didn't catch in time). As Bruce suggested, the example
> > should be reworked to work without the API, and deprecate it.

Agreed to deprecate the old API.
However, there is no relation with this new optional API.

> Before deprecating an API, I think we should check if people are using
> it and if it can really be replaced. I think there are many things that
> could be deprecated before this one.

Yes we can discuss a lot of things but let's focus on this one :)

> > > For Tx, I want to know if I have enough room to send my packets before
> > > doing it. There is no API yet to do that.
> > 
> > Yes. This could be a lightweight API if it returned a count (txq->nb_tx_free) instead of actually touching the ring, which is what I have a problem with. If the implementation changes to that, that may be okay to do. The Tx API has more merit than the Rx API, but not coded around an offset.
> 
> Returning txq->nb_tx_free does not work because it is a software view,
> which becomes wrong as soon as the hardware has send the packets.
> Example:
> 1. Send many packets at very high rate, the tx ring becomes full
> 2. wait that packets are transmitted
> 3. read nb_tx_free, it returns 0, which is not what I want
> 
> So in my case there is a also a need for a Tx descriptor status API.
> 
> Thomas, you are the maintainer of ethdev API, do you have an opinion?

You show some benefits and it does not hurt any existing API.
So we cannot reject such a feature, even if its best use is for debug
or specific applications.
I think the concern here was the fear of seeing this called in some
benchmark applications. You just have to highlight in the API doc that
there are some performance penalties.