Re: [dpdk-dev] [PATCH v7 0/6] Add lock-free ring and mempool handler

[Ola Liljedahl <Ola.Liljedahl@arm.com>]

On Mon, 2019-04-01 at 19:23 +0000, Eads, Gage wrote:
> > 
> > 
> > On Mon, 18 Mar 2019 21:49:44 +0000
> > "Eads, Gage" <gage.eads@intel.com> wrote:
> > 
> > > 
> > > Hi all,
> > > 
> > > Friendly reminder that in order to get this feature into 19.08 (assuming
> > folks also want that :)), the API deprecation notice needs to be merged into
> > 19.05.
> > > 
> > > 
> > > Thanks,
> > > Gage
> > Given the recent API/ABI stability discussion, this is the kind of patch
> > that
> > really needs to be examined and justified.
> Can you point me to the discussion (assuming it was on the ML)? I'm aware of
> Ferruh's changes to the docs, but not the discussion you referenced.
> 
> The lock-free ring functionality itself is a valuable addition to DPDK,
> primarily because it lacks the standard ring's non-preemptive constraint. The
> non-preemptive constraint is important in an application with both high
> priority, performance-sensitive data-plane components and low-priority
> control-plane components. This was important enough to warrant further
> clarification recently[1], and has been a discussion topic for some
> time[2][3].
> 
> The modified API, rte_ring_get_memsize(), was added to allow users to
> initialize rings in separately allocated memory. This function isn't called in
> DPDK's examples/apps/drivers, and a quick google search didn't turn up any
> open source projects that call the function, so I suspect that a majority of
> ring code uses rte_ring_create() instead of rte_ring_get_memsize() +
> rte_ring_init(). So I suspect this interface change will affect a small
> percentage of DPDK users.
> 
> As a straw-man counter-proposal, we could instead introduce a lock-free
> specific function rte_lf_ring_get_memsize() that lock-free ring users would
> call instead of rte_ring_get_memsize(). This would avoid the API modification,
> but:
> - It's awkward to have one rte_lf_ring_* function and otherwise access the
> lock-free ring through rte_ring_* functions.
> - It's also easy to envision a user incorrectly calling rte_ring_get_memsize()
> rather than rte_lf_ring_get_memsize() for a lock-free ring, since otherwise
> rte_ring_* functions are used. DPDK would have no way to detect that the
> allocated memory is too small, and if such a bug occurs it would manifest
> itself as memory corruption.
> - Changing rte_ring_get_memsize() to take a flags argument may be the better
> long-term design, if another flag is introduced that likewise uses a different
> ring size.
> 
> Another approach is to break out the lock-free ring into a fully separate API.
As in the RFC I posted: http://patches.dpdk.org/patch/50095/
Cleaner API, simpler implementation.

 One of the goals of my patchset was to allow applications to switch to lock-free rings with minimal code change; I think the value of the lock-free ring warrants such an approach.

A noble goal but personally I think DPDK API's and implementations are getting
more and more messy and thus difficult to use and difficult to maintain.
Is it so much worse to have separate but structurally equivalent API's?
Yes, blocking vs non-blocking can no longer be selected at runtime (startup
time), I think this is the biggest limitation.

-- Ola


(Unfortunately without hard numbers on the cost or benefit of such a change, these arguments are fairly subjective.)

Thanks,
Gage

[1] https://patches.dpdk.org/patch/43122/
[2] http://mails.dpdk.org/archives/dev/2013-November/000714.html
[3] http://mails.dpdk.org/archives/dev/2014-January/001163.html

-- 
Ola Liljedahl, Networking System Architect, Arm
Phone +46706866373, Skype ola.liljedahl