[dpdk-dev] [PATCH v1 5/5] ixgbe: Add LRO support
Stephen Hemminger
stephen at networkplumber.org
Wed Mar 4 01:33:59 CET 2015
On Tue, 3 Mar 2015 21:48:43 +0200
Vlad Zolotarov <vladz at cloudius-systems.com> wrote:
> + next_desc:
> + /*
> + * The code in this whole file uses the volatile pointer to
> + * ensure the read ordering of the status and the rest of the
> + * descriptor fields (on the compiler level only!!!). This is so
> + * UGLY - why not to just use the compiler barrier instead? DPDK
> + * even has the rte_compiler_barrier() for that.
> + *
> + * But most importantly this is just wrong because this doesn't
> + * ensure memory ordering in a general case at all. For
> + * instance, DPDK is supposed to work on Power CPUs where
> + * compiler barrier may just not be enough!
> + *
> + * I tried to write only this function properly to have a
> + * starting point (as a part of an LRO/RSC series) but the
> + * compiler cursed at me when I tried to cast away the
> + * "volatile" from rx_ring (yes, it's volatile too!!!). So, I'm
> + * keeping it the way it is for now.
> + *
> + * The code in this file is broken in so many other places and
> + * will just not work on a big endian CPU anyway therefore the
> + * lines below will have to be revisited together with the rest
> + * of the ixgbe PMD.
> + *
> + * TODO:
> + * - Get rid of "volatile" crap and let the compiler do its
> + * job.
> + * - Use the proper memory barrier (rte_rmb()) to ensure the
> + * memory ordering below.
This comment screams "this is broken".
Why not get proper architecture independent barriers in DPDK first.
More information about the dev
mailing list