DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas.monjalon@6wind.com>
To: Olivier Matz <olivier.matz@6wind.com>
Cc: "Venkatesan, Venky" <venky.venkatesan@intel.com>,
	"Richardson, Bruce" <bruce.richardson@intel.com>,
	dev@dpdk.org, "Ananyev,
	Konstantin" <konstantin.ananyev@intel.com>,
	"Lu, Wenzhuo" <wenzhuo.lu@intel.com>,
	"Zhang, Helin" <helin.zhang@intel.com>,
	"Wu, Jingjing" <jingjing.wu@intel.com>,
	"adrien.mazarguil@6wind.com" <adrien.mazarguil@6wind.com>,
	"nelio.laranjeiro@6wind.com" <nelio.laranjeiro@6wind.com>,
	"Yigit, Ferruh" <ferruh.yigit@intel.com>
Subject: Re: [dpdk-dev] [PATCH 0/6] get status of Rx and Tx descriptors
Date: Mon, 06 Mar 2017 12:02:34 +0100	[thread overview]
Message-ID: <5158489.C766vy8AX9@xps13> (raw)
In-Reply-To: <20170304214527.330cccde@platinum>

2017-03-04 21:45, Olivier Matz:
> "Venkatesan, Venky" <venky.venkatesan@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.

  reply	other threads:[~2017-03-06 11:02 UTC|newest]

Thread overview: 72+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-24  9:54 [dpdk-dev] [RFC 0/9] get Rx and Tx used descriptors Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 1/9] ethdev: clarify api comments of rx queue count Olivier Matz
2016-11-24 10:52   ` Ferruh Yigit
2016-11-24 11:13     ` Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 2/9] ethdev: move queue id check in generic layer Olivier Matz
2016-11-24 10:59   ` Ferruh Yigit
2016-11-24 13:05     ` Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 3/9] ethdev: add handler for Tx queue descriptor count Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 4/9] net/ixgbe: optimize Rx " Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 5/9] net/ixgbe: add handler for Tx " Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 6/9] net/igb: optimize rx " Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 7/9] net/igb: add handler for tx " Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 8/9] net/e1000: optimize rx " Olivier Matz
2016-11-24  9:54 ` [dpdk-dev] [RFC 9/9] net/e1000: add handler for tx " Olivier Matz
2017-01-13 16:44 ` [dpdk-dev] [RFC 0/9] get Rx and Tx used descriptors Olivier Matz
2017-01-13 17:32   ` Richardson, Bruce
2017-01-17  8:24     ` Olivier Matz
2017-01-17 13:56       ` Bruce Richardson
2017-03-01 17:19 ` [dpdk-dev] [PATCH 0/6] get status of Rx and Tx descriptors Olivier Matz
2017-03-01 17:19   ` [dpdk-dev] [PATCH 1/6] ethdev: add descriptor status API Olivier Matz
2017-03-01 18:22     ` Andrew Rybchenko
2017-03-02 13:57       ` Olivier Matz
2017-03-02 14:19         ` Andrew Rybchenko
2017-03-02 14:54           ` Olivier Matz
2017-03-02 15:05             ` Andrew Rybchenko
2017-03-02 15:14               ` Olivier Matz
2017-03-01 17:19   ` [dpdk-dev] [PATCH 2/6] net/ixgbe: implement " Olivier Matz
2017-03-01 17:19   ` [dpdk-dev] [PATCH 3/6] net/e1000: implement descriptor status API (igb) Olivier Matz
2017-03-02  1:28     ` Lu, Wenzhuo
2017-03-02 13:58       ` Olivier Matz
2017-03-01 17:19   ` [dpdk-dev] [PATCH 4/6] net/e1000: implement descriptor status API (em) Olivier Matz
2017-03-02  1:22     ` Lu, Wenzhuo
2017-03-02 14:46       ` Olivier Matz
2017-03-03  1:15         ` Lu, Wenzhuo
2017-03-01 17:19   ` [dpdk-dev] [PATCH 5/6] net/mlx5: implement descriptor status API Olivier Matz
2017-03-02  7:56     ` Nélio Laranjeiro
2017-03-01 17:19   ` [dpdk-dev] [PATCH 6/6] net/i40e: " Olivier Matz
2017-03-01 18:02   ` [dpdk-dev] [PATCH 0/6] get status of Rx and Tx descriptors Andrew Rybchenko
2017-03-02 13:40     ` Olivier Matz
2017-03-06 10:41       ` Thomas Monjalon
2017-03-01 18:07   ` Stephen Hemminger
2017-03-02 13:43     ` Olivier Matz
2017-03-06 10:41       ` Thomas Monjalon
2017-03-02 15:32   ` Bruce Richardson
2017-03-02 16:14     ` Olivier Matz
2017-03-03 16:18       ` Venkatesan, Venky
2017-03-03 16:45         ` Olivier Matz
2017-03-03 18:46           ` Venkatesan, Venky
2017-03-04 20:45             ` Olivier Matz
2017-03-06 11:02               ` Thomas Monjalon [this message]
2017-03-07 15:59   ` [dpdk-dev] [PATCH v2 " Olivier Matz
2017-03-07 15:59     ` [dpdk-dev] [PATCH v2 1/6] ethdev: add descriptor status API Olivier Matz
2017-03-09 11:49       ` Andrew Rybchenko
2017-03-21  8:32       ` Yang, Qiming
2017-03-24 12:49         ` Olivier Matz
2017-03-27  1:28           ` Yang, Qiming
2017-03-07 15:59     ` [dpdk-dev] [PATCH v2 2/6] net/ixgbe: implement " Olivier Matz
2017-03-07 15:59     ` [dpdk-dev] [PATCH v2 3/6] net/e1000: implement descriptor status API (igb) Olivier Matz
2017-03-08  1:17       ` Lu, Wenzhuo
2017-03-07 15:59     ` [dpdk-dev] [PATCH v2 4/6] net/e1000: implement descriptor status API (em) Olivier Matz
2017-03-07 15:59     ` [dpdk-dev] [PATCH v2 5/6] net/mlx5: implement descriptor status API Olivier Matz
2017-03-07 15:59     ` [dpdk-dev] [PATCH v2 6/6] net/i40e: " Olivier Matz
2017-03-08  1:17       ` Lu, Wenzhuo
2017-03-29  8:36     ` [dpdk-dev] [PATCH v3 0/6] get status of Rx and Tx descriptors Olivier Matz
2017-03-29  8:36       ` [dpdk-dev] [PATCH v3 1/6] ethdev: add descriptor status API Olivier Matz
2017-03-29  8:36       ` [dpdk-dev] [PATCH v3 2/6] net/ixgbe: implement " Olivier Matz
2017-03-29  8:36       ` [dpdk-dev] [PATCH v3 3/6] net/e1000: implement descriptor status API (igb) Olivier Matz
2017-03-29  8:36       ` [dpdk-dev] [PATCH v3 4/6] net/e1000: implement descriptor status API (em) Olivier Matz
2017-03-29  8:36       ` [dpdk-dev] [PATCH v3 5/6] net/mlx5: implement descriptor status API Olivier Matz
2017-03-29  8:36       ` [dpdk-dev] [PATCH v3 6/6] net/i40e: " Olivier Matz
2017-03-30 13:30       ` [dpdk-dev] [PATCH v3 0/6] get status of Rx and Tx descriptors Thomas Monjalon
2017-04-19 15:50         ` Ferruh Yigit

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5158489.C766vy8AX9@xps13 \
    --to=thomas.monjalon@6wind.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=helin.zhang@intel.com \
    --cc=jingjing.wu@intel.com \
    --cc=konstantin.ananyev@intel.com \
    --cc=nelio.laranjeiro@6wind.com \
    --cc=olivier.matz@6wind.com \
    --cc=venky.venkatesan@intel.com \
    --cc=wenzhuo.lu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).