Re: [dpdk-dev] rte_ring features in use (or not)

[Olivier MATZ <olivier.matz@6wind.com>]

On Wed, 25 Jan 2017 12:14:56 +0000, Bruce Richardson
<bruce.richardson@intel.com> wrote:
> Hi all,
> 
> while looking at the rte_ring code, I'm wondering if we can simplify
> that a bit by removing some of the code it in that may not be used.
> Specifically:
> 
> * Does anyone use the NIC stats functionality for debugging? I've
>   certainly never seen it used, and it's presence makes the rest less
>   readable. Can it be dropped?

What do you call NIC stats? The stats that are enabled with
RTE_LIBRTE_RING_DEBUG?

If yes, I was recently thinking almost the same about mempool stats. The
need to enable stats at compilation makes them less usable. On the
other hand, I feel the mempool/ring stats may be useful, for instance
to check if mbufs are used from mempool cache, and not from common pool.

For mempool, my conclusion was:
- Enabling stats (debug) changes the ABI, because it adds a field in
  the structure, this is bad
- enabling stats is not the same than enabling debug, we should have 2
  different ifdefs
- if statistics don't cost a lot, they should be enabled by default,
  because it's a good debug tool (ex: have a stats for each access to
  common pool)

For the ring, in my opinion, the stats could be fully removed.


> * RTE_RING_PAUSE_REP_COUNT is set to be disabled at build time, and
>   so does anyone actually use this? Can it be dropped?

This option looks like a hack to use the ring in conditions where it
should no be used (preemptable threads). And having a compile-time
option for this kind of stuff is not in vogue ;)


> * Who uses the watermarks feature as is? I know we have a sample app
>   that uses it, but there are better ways I think to achieve the same
>   goal while simplifying the ring implementation. Rather than have a
> set watermark on enqueue, have both enqueue and dequeue functions
> return the number of free or used slots available in the ring (in
> case of enqueue, how many free there are, in case of dequeue, how
> many items are available). Easier to implement and far more useful to
> the app.

+1