netdev - RE: [PATCH net-next 2/2] add one config to select relax order mode in intel NIC's Kconfig

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] [day] [month] [year] [list]

Message-ID: <F95AC9340317A84688A5F0DF0246F3F201ED835A@szxemi502-mbx.china.huawei.com>
Date:   Sat, 4 Feb 2017 01:36:25 +0000
From:   maowenan <maowenan@...wei.com>
To:     Alexander Duyck <alexander.duyck@...il.com>
CC:     Netdev <netdev@...r.kernel.org>,
        Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Dingtianhong <dingtianhong@...wei.com>
Subject: RE: [PATCH net-next 2/2] add one config to select relax order mode
 in intel NIC's Kconfig



> -----Original Message-----
> From: Alexander Duyck [mailto:alexander.duyck@...il.com]
> Sent: Saturday, February 04, 2017 12:43 AM
> To: maowenan
> Cc: Netdev; Jeff Kirsher
> Subject: Re: [PATCH net-next 2/2] add one config to select relax order mode in
> intel NIC's Kconfig
> 
> On Fri, Feb 3, 2017 at 1:30 AM, Mao Wenan <maowenan@...wei.com> wrote:
> > This patch allows one to enable relax order mode in intel NIC's
> > Kconfig. CONFIG_ARCH_WANT_RELAX_ORDER is a common macro for some
> CPU
> > architecture to use relax order mode in NIC's source codes.
> > CONFIG_ARCH_WANT_RELAX_ORDER can be defined in arch/xxx/Kconfig,
> such
> > as sparc system exists in arch/sparc/Kconfig, but not all of arm64
> > systems can use relax order mode, so it can't be defined in
> > arch/arm64/Kconfig. Therefore PCI_RELAX_ORDER in NIC's Kconfig provide
> > one way to define macro CONFIG_ARCH_WANT_RELAX_ORDER.
> >
> > Signed-off-by: Mao Wenan <maowenan@...wei.com>
> > ---
> >  drivers/net/ethernet/intel/Kconfig | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/Kconfig
> > b/drivers/net/ethernet/intel/Kconfig
> > index 1349b45..b366722 100644
> > --- a/drivers/net/ethernet/intel/Kconfig
> > +++ b/drivers/net/ethernet/intel/Kconfig
> > @@ -275,4 +275,19 @@ config FM10K
> >           To compile this driver as a module, choose M here. The module
> >           will be called fm10k.  MSI-X interrupt support is required
> >
> > +config PCI_RELAX_ORDER
> > +        bool "PCI relax order mode support"
> > +        default n
> > +        select ARCH_WANT_RELAX_ORDER
> > +        ---help---
> > +          This allows one to enable relax order mode in driver.
> > +          CONFIG_ARCH_WANT_RELAX_ORDER is a common macro for
> some
> > +          CPU architecture to use relax order mode in NIC's source codes.
> > +          CONFIG_ARCH_WANT_RELAX_ORDER can be defined in
> arch/xxx/Kconfig,
> > +          such as sparc system exists in arch/sparc/Kconfig, but not all
> > +          of arm64 systems can use relax order mode, so it can't be
> defined
> > +          in arch/arm64/Kconfig. Therefore PCI_RELAX_ORDER provide
> one way
> > +          to define macro CONFIG_ARCH_WANT_RELAX_ORDER. Say Y
> here if you
> > +          want to enable relax order.
> > +
> >  endif # NET_VENDOR_INTEL
> 
> 
> You can't be dropping configuration options like this in here.  The
> ARCH_WANT_RELAX_ORDER should be selected by the architecture as a
> def_bool, not by the user via a kconfig option.  In addition this option has no
> business in the Intel wired LAN directory as this impacts all architectures.
> 
> This is something that would be an architecture specific PCI option and could
> impact other PCI devices beyond what is just in networking.
> If you are wanting to target the arm64 architecture you should probably drop
> this in the /arch/arm64/Kconfig, then that way anyone familiar with the arm64
> hardware can chime in if enabling relaxed ordering causes any known issues.
> 
> - Alex


I have tried to drop configuration to arch/arm64/Kconfig, but I found there will
be side effect as not all of arm64 systems really need relax order, so I put it in intel
Kconfig because it is a feature of PCI device and configured by NIC's register.
By the way, do you agree the first patch of this series, shall I repost it individually? 
[PATCH net-next 1/2] ixgbevf and 82598 relax order mode support