Hi Olivier, > -----Original Message----- > From: Olivier MATZ [mailto:olivier.matz at 6wind.com] > Sent: Friday, March 13, 2015 9:08 AM > To: Vlad Zolotarov; Ananyev, Konstantin; dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 3/3] ixgbe: Add LRO support > > Hi Vlad, > > 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 or > >>> to fix any other places with the similar weakness I would need a proper > >>> 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. > > 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. > > 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 needed. 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 > > Regards, > Olivier
