DPDK patches and discussions
 help / color / mirror / Atom feed
From: David Marchand <david.marchand@6wind.com>
To: "Zhang, Helin" <helin.zhang@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] i40e: Steps and required configurations of how to achieve the best performance!
Date: Thu, 18 Sep 2014 10:57:47 +0200	[thread overview]
Message-ID: <CALwxeUus7snQAXymgPW4wP2hbgPUX2huJVC_byq+aMsyQk4uGw@mail.gmail.com> (raw)
In-Reply-To: <F35DEAC7BCE34641BA9FAC6BCA4A12E70A793C19@SHSMSX104.ccr.corp.intel.com>

Hello Helin,

On Thu, Sep 18, 2014 at 4:39 AM, Zhang, Helin <helin.zhang@intel.com> wrote:

>  Hi David
>
>
>
> *From:* David Marchand [mailto:david.marchand@6wind.com]
> *Sent:* Wednesday, September 17, 2014 10:03 PM
> *To:* Zhang, Helin
> *Cc:* dev@dpdk.org
> *Subject:* Re: [dpdk-dev] i40e: Steps and required configurations of how
> to achieve the best performance!
>
>
>
> On Wed, Sep 17, 2014 at 10:50 AM, Zhang, Helin <helin.zhang@intel.com>
> wrote:
>
>  For the ‘extended tag’, it was defined in PCIe spec, but actually not
> all BIOS implements it. Enabling it in BIOS or at runtime are two choices
> of doing the same thing. I don’t think it can be configured per PCI device
> in BIOS, so we don’t need to do that per PCI device in DPDK. Right?
> Actually we don’t want to touch PCIe settings in DPDK code, that’s why we
> want to let BIOS config as it is by default. If no better choice, we can do
> it in DPDK by changing configurations.
>
>
>
> - Ok, then if we can make a runtime decision (at dpdk level), there is no
> need for bios configuration and there is no need for a build option.
>
> Why don't we get rid of this option ?
>
>
>
> [Helin] Initially, we want to do that for BIOS, if specific BIOS does not
> implement it. That way it needs to be initialized once during
> initialization for each PCI device. Sure, that might not be the best
> option, but it is the easiest way. For Linux end users, the best option
> could be using ‘setpci’ command. It can enable ‘extended_tag’ per PCI
> device.
>

I am not sure I can see how easy it is since you are forcing this in a
build option.
Anyway, all this knowledge should be in the documentation and not in an
obscure build option that looks to be useless in the end.

The more I look at this, the more I think we did not have good enough
argument for this change in eal / igb_uio yet.

We have something that gives "better performance" on "some server" with
"some bios".



>
> As far as the per-device runtime configuration is concerned, I want to
> make sure this pci configuration will not break other "igb_uio" pci devices.
>
> If Intel can tell for sure this won't break other devices, then fine, we
> can go and enable this for all "igb_uio" pci devices.
>
>
>
> [Helin] It is in PCIe specification, and enable it can provide better
> performance generally. But I cannot confirm that it would not break any
> other devices, as I don’t validate all devices. If you really concern it,
> ‘setpci’ can be the best option for you. We can add a script for that later.
>

Why not a script, but documentation is important too: I would say that we
need an explicit list of platforms and nics which support this.



>
> - By the way, there is also the CONFIG_MAX_READ_REQUEST_SIZE option that
> seems to be disabled (or at least its value 0 seems to tell so).
>
> What is its purpose ?
>
>
>
> [Helin] Yes, it was added for performance tuning long long ago. But now it
> seems contribute nothing or too few for the performance number, so I just
> skip it. The default value does nothing on PCIe registers, just keep it as
> is.
>

Not so long ago to dpdk.org (somewhere around 1.7.0 ...).
If this code had no use for "so long", why did it end up on dpdk.org ?
Why should we keep it ?


Thanks.

-- 
David Marchand

  reply	other threads:[~2014-09-18  8:52 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-17  4:18 Zhang, Helin
2014-09-17  8:33 ` David Marchand
2014-09-17  8:50   ` Zhang, Helin
2014-09-17 14:03     ` David Marchand
2014-09-18  2:39       ` Zhang, Helin
2014-09-18  8:57         ` David Marchand [this message]
2014-09-19  3:43           ` Zhang, Helin
2014-10-15  9:41             ` Thomas Monjalon
2014-10-16  0:43               ` Zhang, Helin
2015-02-09 12:12                 ` David Marchand
2015-02-10  0:27                   ` Zhang, Helin

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=CALwxeUus7snQAXymgPW4wP2hbgPUX2huJVC_byq+aMsyQk4uGw@mail.gmail.com \
    --to=david.marchand@6wind.com \
    --cc=dev@dpdk.org \
    --cc=helin.zhang@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).