[dpdk-dev] [PATCH 0/6] get status of Rx and Tx descriptors
thomas.monjalon at 6wind.com
Mon Mar 6 12:02:34 CET 2017
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
> > > > 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.
> 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.
More information about the dev