netdev - Re: [net-next 10/10] net/mlx5e: Add support for PCI relaxed ordering

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 29 Jun 2020 12:32:44 +0300
From:   Aya Levin <ayal@...lanox.com>
To:     Bjorn Helgaas <helgaas@...nel.org>,
        Jakub Kicinski <kuba@...nel.org>
Cc:     Saeed Mahameed <saeedm@...lanox.com>,
        "mkubecek@...e.cz" <mkubecek@...e.cz>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Tariq Toukan <tariqt@...lanox.com>, linux-pci@...r.kernel.org,
        Alexander Duyck <alexander.h.duyck@...ux.intel.com>
Subject: Re: [net-next 10/10] net/mlx5e: Add support for PCI relaxed ordering



On 6/26/2020 11:12 PM, Bjorn Helgaas wrote:
> On Wed, Jun 24, 2020 at 10:22:58AM -0700, Jakub Kicinski wrote:
>> On Wed, 24 Jun 2020 10:34:40 +0300 Aya Levin wrote:
>>>>> I think Michal will rightly complain that this does not belong in
>>>>> private flags any more. As (/if?) ARM deployments take a foothold
>>>>> in DC this will become a common setting for most NICs.
>>>>
>>>> Initially we used pcie_relaxed_ordering_enabled() to
>>>>    programmatically enable this on/off on boot but this seems to
>>>> introduce some degradation on some Intel CPUs since the Intel Faulty
>>>> CPUs list is not up to date. Aya is discussing this with Bjorn.
>>> Adding Bjorn Helgaas
>>
>> I see. Simply using pcie_relaxed_ordering_enabled() and blacklisting
>> bad CPUs seems far nicer from operational perspective. Perhaps Bjorn
>> will chime in. Pushing the validation out to the user is not a great
>> solution IMHO.
> 
> I'm totally lost, but maybe it doesn't matter because it looks like
> David has pulled this series already.
> 
> There probably *should* be a PCI core interface to enable RO, but
> there isn't one today.
> 
> pcie_relaxed_ordering_enabled() doesn't *enable* anything.  All it
> does is tell you whether RO is already enabled.
> 
> This patch ([net-next 10/10] net/mlx5e: Add support for PCI relaxed
> ordering) apparently adds a knob to control RO, but I can't connect
> the dots.  It doesn't touch PCI_EXP_DEVCTL_RELAX_EN, and that symbol
> doesn't occur anywhere in drivers/net except tg3, myri10ge, and niu.
> 
> And this whole series doesn't contain PCI_EXP_DEVCTL_RELAX_EN or
> pcie_relaxed_ordering_enabled().

I wanted to turn on RO on the ETH driver based on 
pcie_relaxed_ordering_enabled().
 From my experiments I see that pcie_relaxed_ordering_enabled() return 
true on Intel(R) Xeon(R) CPU E5-2650 v3 @ 2.30GHz. This CPU is from 
Haswell series which is known to have bug in RO implementation. In this 
case, I expected pcie_relaxed_ordering_enabled() to return false, 
shouldn't it?

In addition, we are worried about future bugs in new CPUs which may 
result in performance degradation while using RO, as long as the 
function pcie_relaxed_ordering_enabled() will return true for these 
CPUs. That's why we thought of adding the feature on our card with 
default off and enable the user to set it.

> 
> I do have a couple emails from Aya, but they didn't include a patch
> and I haven't quite figured out what the question was.
> 
>>>> So until we figure this out, will keep this off by default.
>>>>
>>>> for the private flags we want to keep them for performance analysis as
>>>> we do with all other mlx5 special performance features and flags.