Re: [PATCH v6 3/3] ixgbe: Add LRO support

[Olivier MATZ <olivier.matz-pdR9zngts4EAvxtiuMwx3w@public.gmane.org>]

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

Regards,
Olivier