From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id B5BEE5A92 for ; Fri, 13 Mar 2015 12:28:28 +0100 (CET) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga101.fm.intel.com with ESMTP; 13 Mar 2015 04:28:27 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.11,394,1422950400"; d="scan'208";a="679621417" Received: from irsmsx110.ger.corp.intel.com ([163.33.3.25]) by fmsmga001.fm.intel.com with ESMTP; 13 Mar 2015 04:28:27 -0700 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.117]) by irsmsx110.ger.corp.intel.com ([169.254.15.236]) with mapi id 14.03.0195.001; Fri, 13 Mar 2015 11:28:26 +0000 From: "Ananyev, Konstantin" To: Olivier MATZ , Vlad Zolotarov , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v6 3/3] ixgbe: Add LRO support Thread-Index: AQHQXW070HG1yhPCSUOGsmfe4cNABZ0aRExA Date: Fri, 13 Mar 2015 11:28:25 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258213F5EC4@irsmsx105.ger.corp.intel.com> References: <1425928037-28732-1-git-send-email-vladz@cloudius-systems.com> <1425928037-28732-4-git-send-email-vladz@cloudius-systems.com> <2601191342CEEE43887BDE71AB977258213F5039@irsmsx105.ger.corp.intel.com> <54FEF011.6010205@cloudius-systems.com> <2601191342CEEE43887BDE71AB977258213F5475@irsmsx105.ger.corp.intel.com> <54FF63CB.4040506@cloudius-systems.com> <2601191342CEEE43887BDE71AB977258213F589B@irsmsx105.ger.corp.intel.com> <5500734A.7060800@cloudius-systems.com> <5502A8E8.3010004@6wind.com> In-Reply-To: <5502A8E8.3010004@6wind.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6 3/3] ixgbe: Add LRO support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 13 Mar 2015 11:28:29 -0000 Hi Olivier, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz@6wind.com] > Sent: Friday, March 13, 2015 9:08 AM > To: Vlad Zolotarov; Ananyev, Konstantin; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 3/3] ixgbe: Add LRO support >=20 > Hi Vlad, >=20 > On 03/11/2015 05:54 PM, Vlad Zolotarov wrote: > >>>> About the existing RX/TX functions and PPC support: > >>>> Note that all of them were created before PPC support for DPDK was > >>>> introduced. > >>>> At that moment only IA was supported. > >>>> That's why in some places where you would expect to see 'mb()' there > >>>> are 'volatile' and/or ' rte_compiler_barrier' instead. > >>>> Why all that places wasn't updated when PPC support was added - > >>>> that's another question. > >>>> From my understanding - with current implementation some of DPDK > >>>> PMDs RX/TX functions and rte_ring wouldn't work correctly > >>> on PPC. > >>>> So, I suppose we need to decide for ourselves - do we really want to > >>>> support PPC and other architectures with non-IA memory > >>> model or not? > >>>> If not, then I think we don't need any mb()s inside recv_pkts_lro() > >>>> - just rte_compiler_barrier seems enough, and no point to > >>> complain about > >>>> it in comments. > >>>> If yes - then why to introduce a new function with a known potential > >>>> bug? > >>> In order to introduce a new function with the proper implementation o= r > >>> to fix any other places with the similar weakness I would need a prop= er > >>> tools like a proper platform-dependent barrier-macros similar to > >>> smp_Xmb() Linux macros that reduce to a compiler barrier where > >>> appropriate or to a proper memory fence where needed. > >> I understand that. > >> Let's add new macro for that: rte_smp_Xmb() or something, > >> so it would be just rte_compiler_barrier() for x86 and a proper mb() > >> for PPC. > > > > There was an idea to use the C11 built-in memory barriers. I suggest we > > open a separate discussion about that and add these and the appropriate > > fixes in a separate series. There are quite a few places to fix anyway, > > which are currently broken on PPC so this patch doesn't make things any > > worse. However adding a new memory barrier doesn't belong to an LRO > > functionality and thus to this series. >=20 > This is an interesting discussion. Just for reference, I submitted a > patch on this topic but it was probably too early as only Intel > architecture was supported at that time. >=20 > See http://dpdk.org/ml/archives/dev/2014-May/002597.html I do remember that conversation :) At that moment, as nothing except IA wasn't supported, I feel it was not ne= eded. Though now, if we do want to support PPC and other architectures with weak = memory model, I think we do need to introduce some platform dependent set of Xmb() macros= . See http://dpdk.org/ml/archives/dev/2014-October/006729.html Actually while thinking about it once again: Is there any good use for rte_compiler_barrier() for PPC memory model? I can't think about any. So I wonder can't we just make for PPC: #define rte_compiler_barrier rte_mb While keeping it as it is for IA. Would save us from searching/replacing though all the code. Konstantin >=20 > Regards, > Olivier