> Zero-copy access to mempool caches is beneficial for PMD performance, and > must be provided by the mempool library to fix [Bug 1052] without a > performance regression.
LGTM in general, thank you for working on it. Few comments below. > > [Bug 1052]: https://bugs.dpdk.org/show_bug.cgi?id=1052 > > v2: > * Fix checkpatch warnings. > * Fix missing registration of trace points. > * The functions are inline, so they don't go into the map file. > v1 changes from the RFC: > * Removed run-time parameter checks. (Honnappa) > This is a hot fast path function; requiring correct application > behaviour, i.e. function parameters must be valid. > * Added RTE_ASSERT for parameters instead. RTE_ASSERT(n <= RTE_MEMPOOL_CACHE_MAX_SIZE); I think it is too excessive. Just: if (n <= RTE_MEMPOOL_CACHE_MAX_SIZE) return NULL; seems much more convenient for the users here and more close to other mempool/ring API behavior. In terms of performance - I don’t think one extra comparison here would really count. I also think would be really good to add: add zc_(get|put)_bulk_start(), zc_(get|put)_bulk_finish(). Where _start would check/fill the cache and return the pointer, while _finsih would updathe cache->len. Similar to what we have for rte_ring _peek_ API. That would allow to extend this API usage - let say inside PMDs it could be used not only for MBUF_FAST_FREE case, but for generic TX code path (one that have to call rte_mbuf_prefree()) also. > Code for this is only generated if built with RTE_ENABLE_ASSERT. > * Removed fallback when 'cache' parameter is not set. (Honnappa) > * Chose the simple get function; i.e. do not move the existing objects in > the cache to the top of the new stack, just leave them at the bottom. > * Renamed the functions. Other suggestions are welcome, of course. ;-) > * Updated the function descriptions. > * Added the functions to trace_fp and version.map. Would be great to add some test-cases in app/test to cover this new API.
