Date:   Mon, 26 Dec 2016 08:33:43 +0000
From:   maowenan <maowenan@...wei.com>
To:     maowenan <maowenan@...wei.com>,
        Alexander Duyck <alexander.duyck@...il.com>
CC:     Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "weiyongjun (A)" <weiyongjun1@...wei.com>,
        Dingtianhong <dingtianhong@...wei.com>,
        "Wangzhou (B)" <wangzhou1@...ilicon.com>
Subject: RE: [PATCH] ethtool: add one ethtool option to set relax ordering
 mode



> -----Original Message-----
> From: maowenan
> Sent: Saturday, December 24, 2016 4:30 PM
> To: 'Alexander Duyck'
> Cc: Jeff Kirsher; Stephen Hemminger; netdev@...r.kernel.org; weiyongjun (A);
> Dingtianhong; Wangzhou (B)
> Subject: RE: [PATCH] ethtool: add one ethtool option to set relax ordering mode
> 
> 
> 
> > -----Original Message-----
> > From: Alexander Duyck [mailto:alexander.duyck@...il.com]
> > Sent: Friday, December 23, 2016 11:43 PM
> > To: maowenan
> > Cc: Jeff Kirsher; Stephen Hemminger; netdev@...r.kernel.org;
> > weiyongjun (A); Dingtianhong
> > Subject: Re: [PATCH] ethtool: add one ethtool option to set relax
> > ordering mode
> >
> > On Thu, Dec 22, 2016 at 10:14 PM, maowenan <maowenan@...wei.com>
> > wrote:
> > >
> > >
> > >> -----Original Message-----
> > >> From: Jeff Kirsher [mailto:jeffrey.t.kirsher@...el.com]
> > >> Sent: Friday, December 23, 2016 9:07 AM
> > >> To: maowenan; Alexander Duyck
> > >> Cc: Stephen Hemminger; netdev@...r.kernel.org; weiyongjun (A);
> > >> Dingtianhong
> > >> Subject: Re: [PATCH] ethtool: add one ethtool option to set relax
> > >> ordering mode
> > >>
> > >> On Fri, 2016-12-23 at 00:40 +0000, maowenan wrote:
> > >> > > -----Original Message-----
> > >> > > From: Alexander Duyck [mailto:alexander.duyck@...il.com]
> > >> > > Sent: Thursday, December 22, 2016 11:54 PM
> > >> > > To: maowenan
> > >> > > Cc: Stephen Hemminger; netdev@...r.kernel.org;
> > jeffrey.t.kirsher@...el.
> > >> > > com;
> > >> > > weiyongjun (A); Dingtianhong
> > >> > > Subject: Re: [PATCH] ethtool: add one ethtool option to set
> > >> > > relax ordering mode
> > >> > >
> > >> > > On Wed, Dec 21, 2016 at 5:39 PM, maowenan
> > <maowenan@...wei.com>
> > >> > > wrote:
> > >> > > >
> > >> > > >
> > >> > > > > -----Original Message-----
> > >> > > > > From: Stephen Hemminger [mailto:stephen@...workplumber.org]
> > >> > > > > Sent: Thursday, December 22, 2016 9:28 AM
> > >> > > > > To: maowenan
> > >> > > > > Cc: netdev@...r.kernel.org; jeffrey.t.kirsher@...el.com
> > >> > > > > Subject: Re: [PATCH] ethtool: add one ethtool option to set
> > >> > > > > relax ordering mode
> > >> > > > >
> > >> > > > > On Thu, 8 Dec 2016 14:51:38 +0800 Mao Wenan
> > >> > > > > <maowenan@...wei.com> wrote:
> > >> > > > >
> > >> > > > > > This patch provides one way to set/unset IXGBE NIC TX and
> > >> > > > > > RX relax ordering mode, which can be set by ethtool.
> > >> > > > > > Relax ordering is one mode of 82599 NIC, to enable this
> > >> > > > > > mode can enhance the performance for some cpu architecure.
> > >> > > > >
> > >> > > > > Then it should be done by CPU architecture specific quirks
> > >> > > > > (preferably in PCI
> > >> > > > > layer) so that all users get the option without having to
> > >> > > > > do manual
> > >> > >
> > >> > > intervention.
> > >> > > > >
> > >> > > > > > example:
> > >> > > > > > ethtool -s enp1s0f0 relaxorder off ethtool -s enp1s0f0
> > >> > > > > > relaxorder on
> > >> > > > >
> > >> > > > > Doing it via ethtool is a developer API (for testing) not
> > >> > > > > something that makes sense in production.
> > >> > > >
> > >> > > >
> > >> > > > This feature is not mandatory for all users, acturally relax
> > >> > > > ordering default configuration of 82599 is 'disable', So this
> > >> > > > patch gives one way to
> > >> > >
> > >> > > enable relax ordering to be selected in some performance condition.
> > >> > >
> > >> > > That isn't quite true.  The default for Sparc systems is to
> > >> > > have it enabled.
> > >> > >
> > >> > > Really this is something that is platform specific.  I agree
> > >> > > with Stephen that it would work better if this was handled as a
> > >> > > series of platform specific quirks handled at something like
> > >> > > the PCI layer rather than be a switch the user can toggle on and off.
> > >> > >
> > >> > > With that being said there are changes being made that should
> > >> > > help to improve the situation.  Specifically I am looking at
> > >> > > adding support for the DMA_ATTR_WEAK_ORDERING which may also
> > >> > > allow us to identify cases where you might be able to specify
> > >> > > the DMA behavior via the DMA mapping instead of having to make
> > >> > > the final decision in the device itself.
> > >> > >
> > >> > > - Alex
> > >> >
> > >> > Yes, Sparc is a special case. From the NIC driver point of view,
> > >> > It is no need for some ARCHs to do particular operation and
> > >> > compiling branch, ethtool is a flexible method for user to make
> > >> > decision whether
> > >> > on|off this feature.
> > >> > I think Jeff as maintainer of 82599 has some comments about this.
> > >>
> > >> My original comment/objection was that you attempted to do this
> > >> change as a module parameter to the ixgbe driver, where I directed
> > >> you to use ethtool so that other drivers could benefit from the
> > >> ability to enable/disable relaxed ordering.  As far as how it gets
> > >> implemented in ethtool or PCI layer, makes little difference to me,
> > >> I only had issues with the driver specific module parameter
> > >> implementation,
> > which is not acceptable.
> > >
> > >
> > > Thank you Jeff and Alex.
> > > And then I have gone through mail thread about "i40e: enable PCIe
> > > relax ordering for SPARC", It only works for SPARC, any other ARCH
> > > who wants to enable DMA_ATTR_WEAK_ORDERING feature, should define
> > > the
> > new macro, recompile the driver module.
> > >
> > > Because of the above reasons, we implement in ethtool to give the
> > > final user a convenient way to on|off special feature, no need
> > > define new macro, easy to extend the new features, and also good
> > > benefit for other
> > driver as Jeff referred.
> > >
> >
> > I think the point is we shouldn't base the decision on user input.
> > The fact is the PCIe device control register should have a bit that
> > indicates if the device is allowed to enable relaxed ordering or not.
> > If we can guarantee that the bit is set in all the cases where it
> > should be set, and cleared in all the cases where it should not then
> > we could use something like that to determine if the device is
> > supposed to enable relaxed ordering instead of trying to make the decision
> ourselves.
> >
> > - Alex
> 
> ok. We are focusing on the register.
> And yes, to enable relax ordering for 82599 should be set by one or more bits of
> Rx/TX DCA Control Register, these bits should be set in many cpu architectures,
> such as arm64, sparc, and so on, and should be cleared in other ARCHs.
> By the way, how do you enable SPARC macro, how and where to define this
> compiling macro when user one to enable relax ordering under SPARC system?
> #ifndef CONFIG_SPARC
> 
> 


Hi, Alex,
Have you already sent out the patches about DMA_ATTR_WEAK_ORDERING?
We want to get you how to enable DMA_ATTR_WEAK_ORDERING by PCIe layer,
and we can refer to that.

Powered by blists - more mailing lists