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

["Eads, Gage" <gage.eads@intel.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?

No -- I think both are valid options, with their own tradeoffs.

> Yes, blocking vs non-blocking can no longer be selected at runtime (startup
> time), I think this is the biggest limitation.

Run-time selection with the LF flag means we can easily re-use a large amount of pre-existing ring code -- e.g. the way the ring tests are re-purposed for lock-free rings in this patchset.

The implementation may not be as easily maintained, but we don't have to maintain LF-specific tests/benchmarks/etc.

Maintainers/tech leads, do y'all have a preference?

Thanks,
Gage