|SUCCESS| [v7] net/idpf: update for new mempool cache algorithm

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/164750

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-02

commit sha: 20260601183621.252920-1-mb at smartsharesystems.com
Author: Morten Brorup <mb at smartsharesystems.com>
Subject: [PATCH v7] net/idpf: update for new mempool cache algorithm

This patch updates the idpf driver for changes in the mempool cache
algorithm, modifying how the mempool cache is backfilled in the AVX512
code path.

> diff --git a/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c b/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c
> index 8db4c64106..5788a009ab 100644
> --- a/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c
> +++ b/drivers/net/intel/idpf/idpf_common_rxtx_avx512.c

[ ... ]

> @@ -148,15 +148,31 @@ idpf_singleq_rearm(struct idpf_rx_queue *rxq)
>  	/* Can this be satisfied from the cache? */
>  	if (cache->len < IDPF_RXQ_REARM_THRESH) {
>  		/* No. Backfill the cache first, and then fill from it */
> -		uint32_t req = IDPF_RXQ_REARM_THRESH + (cache->size -
> -							cache->len);
>  
> -		/* How many do we require i.e. number to fill the cache + the request */
> +		/* Backfill would exceed the cache bounce buffer limit? */
> +		__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
> +		if (unlikely(cache->size / 2 < IDPF_RXQ_REARM_THRESH)) {
> +			idpf_singleq_rearm_common(rxq);
> +			return;
> +		}
> +
> +		/*
> +		 * Backfill the cache from the backend;
> +		 * move up the hot objects in the cache to the top half of the cache,
> +		 * and fetch (size / 2) objects to the bottom of the cache.
> +		 */
> +		__rte_assume(cache->len < cache->size / 2);
> +		rte_memcpy(&cache->objs[cache->size / 2], &cache->objs[0],
> +				sizeof(void *) * cache->len);

Does this rte_memcpy() have potential for buffer overflow when cache->len
is close to cache->size / 2? The destination starts at cache->size / 2
and copies cache->len objects. If cache->len approaches cache->size / 2,
this could write beyond the cache->objs[] array bounds.

>  		int ret = rte_mempool_ops_dequeue_bulk
> -				(rxq->mp, &cache->objs[cache->len], req);
> +				(rxq->mp, &cache->objs[0], cache->size / 2);
>  		if (ret == 0) {
> -			cache->len += req;
> +			cache->len += cache->size / 2;

Does this code handle the case where cache->len + cache->size / 2
exceeds cache->size? This could result in cache->len being set to a
value larger than cache->size, which may violate mempool cache
invariants.

>  		} else {
> +			/*
> +			 * No further action is required for roll back, as the objects moved
> +			 * in the cache were actually copied, and the cache remains intact.
> +			 */

The comment mentions "roll back" and "cache remains intact", but can this
create a scenario where objects in cache->objs[] are duplicated after the
rte_memcpy() but before the failed dequeue? Does this violate any
assumptions about object uniqueness within the cache?

[ ... ]

> @@ -565,15 +581,31 @@ idpf_splitq_rearm(struct idpf_rx_queue *rx_bufq)
>  	/* Can this be satisfied from the cache? */
>  	if (cache->len < IDPF_RXQ_REARM_THRESH) {
>  		/* No. Backfill the cache first, and then fill from it */
> -		uint32_t req = IDPF_RXQ_REARM_THRESH + (cache->size -
> -							cache->len);
>  
> -		/* How many do we require i.e. number to fill the cache + the request */
> +		/* Backfill would exceed the cache bounce buffer limit? */
> +		__rte_assume(cache->size / 2 <= RTE_MEMPOOL_CACHE_MAX_SIZE / 2);
> +		if (unlikely(cache->size / 2 < IDPF_RXQ_REARM_THRESH)) {
> +			idpf_splitq_rearm_common(rx_bufq);
> +			return;
> +		}
> +
> +		/*
> +		 * Backfill the cache from the backend;
> +		 * move up the hot objects in the cache to the top half of the cache,
> +		 * and fetch (size / 2) objects to the bottom of the cache.
> +		 */
> +		__rte_assume(cache->len < cache->size / 2);
> +		rte_memcpy(&cache->objs[cache->size / 2], &cache->objs[0],
> +				sizeof(void *) * cache->len);

Same question as above for idpf_splitq_rearm(): does this rte_memcpy()
have potential for buffer overflow when cache->len is close to
cache->size / 2?

>  		int ret = rte_mempool_ops_dequeue_bulk
> -				(rx_bufq->mp, &cache->objs[cache->len], req);
> +				(rx_bufq->mp, &cache->objs[0], cache->size / 2);
>  		if (ret == 0) {
> -			cache->len += req;
> +			cache->len += cache->size / 2;

Same question as above: does this code handle the case where cache->len +
cache->size / 2 exceeds cache->size?

>  		} else {
> +			/*
> +			 * No further action is required for roll back, as the objects moved
> +			 * in the cache were actually copied, and the cache remains intact.
> +			 */

Same question as above: can this create a scenario where objects in
cache->objs[] are duplicated after the rte_memcpy() but before the
failed dequeue?