RE: [PATCH v6] net/bonding: another fix to LACP mempool size

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Gaoxiang Liu [mailto:gaoxiangliu0@163.com]
> Sent: Friday, 25 March 2022 14.34
> 
> The following log message may appear after a slave is idle(or nearly
> idle)
> for a few minutes:"PMD: Failed to allocate LACP packet from pool".
> And bond mode 4 negotiation may fail.
> 
> Problem:When bond mode 4 has been chosed and delicated queue has
> not been enable, all mbufs from a slave' private pool(used
> exclusively for transmitting LACPDUs) have been allocated in
> interrupt thread, and are still sitting in the device's tx
> descriptor ring and other cores' mempool caches in fwd thread.
> Thus the interrupt thread can not alloc LACP packet from pool.
> 
> Solution: Ensure that each slave'tx (LACPDU) mempool owns more than
> n-tx-queues * n-tx-descriptor + fwd_core_num *
> per-core-mmempool-flush-threshold mbufs.
> 
> Note that the LACP tx machine fuction is the only code that allocates
> from a slave's private pool. It runs in the context of the interrupt
> thread, and thus it has no mempool cache of its own.
> 
> Signed-off-by: Gaoxiang Liu <liugaoxiang@huawei.com>
> 
> ---
> v2:
> * Fixed compile issues.
> 
> v3:
> * delete duplicate code.
> 
> v4;
> * Fixed some issues.
> 1. total_tx_desc should use +=
> 2. add detailed logs
> 
> v5:
> * Fixed some issues.
> 1. move CACHE_FLUSHTHRESH_MULTIPLIER to rte_eth_bond-8023ad.c
> 2. use RTE_MIN
> 
> v6:
> * add a comment of CACHE_FLUSHTHRESH_MULTIPLIER macro
> ---
>  drivers/net/bonding/rte_eth_bond_8023ad.c | 11 ++++++++---
>  lib/mempool/rte_mempool.c                 |  2 ++
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/bonding/rte_eth_bond_8023ad.c
> b/drivers/net/bonding/rte_eth_bond_8023ad.c
> index ca50583d62..2c39b0d062 100644
> --- a/drivers/net/bonding/rte_eth_bond_8023ad.c
> +++ b/drivers/net/bonding/rte_eth_bond_8023ad.c
> @@ -1050,6 +1050,7 @@ bond_mode_8023ad_activate_slave(struct
> rte_eth_dev *bond_dev,
>  	uint32_t total_tx_desc;
>  	struct bond_tx_queue *bd_tx_q;
>  	uint16_t q_id;
> +	uint32_t cache_size;
> 
>  	/* Given slave mus not be in active list */
>  	RTE_ASSERT(find_slave_by_id(internals->active_slaves,
> @@ -1100,11 +1101,15 @@ bond_mode_8023ad_activate_slave(struct
> rte_eth_dev *bond_dev,
>  		total_tx_desc += bd_tx_q->nb_tx_desc;
>  	}
> 
> +/* BONDING_8023AD_CACHE_FLUSHTHRESH_MULTIPLIER  is the same as
> + * CACHE_FLUSHTHRESH_MULTIPLIER already defined in rte_mempool.c */
> +#define BONDING_8023AD_CACHE_FLUSHTHRESH_MULTIPLIER 1.5
> +
> +	cache_size = RTE_MIN(RTE_MEMPOOL_CACHE_MAX_SIZE, 32);
> +	total_tx_desc += rte_lcore_count() * cache_size *
> BONDING_8023AD_CACHE_FLUSHTHRESH_MULTIPLIER;
>  	snprintf(mem_name, RTE_DIM(mem_name), "slave_port%u_pool",
> slave_id);
>  	port->mbuf_pool = rte_pktmbuf_pool_create(mem_name,
> total_tx_desc,
> -		RTE_MEMPOOL_CACHE_MAX_SIZE >= 32 ?
> -			32 : RTE_MEMPOOL_CACHE_MAX_SIZE,
> -		0, element_size, socket_id);
> +		cache_size, 0, element_size, socket_id);
> 
>  	/* Any memory allocation failure in initialization is critical
> because
>  	 * resources can't be free, so reinitialization is impossible. */
> diff --git a/lib/mempool/rte_mempool.c b/lib/mempool/rte_mempool.c
> index c5a699b1d6..6126067628 100644
> --- a/lib/mempool/rte_mempool.c
> +++ b/lib/mempool/rte_mempool.c
> @@ -55,6 +55,8 @@ static void
>  mempool_event_callback_invoke(enum rte_mempool_event event,
>  			      struct rte_mempool *mp);
> 
> +/* CACHE_FLUSHTHRESH_MULTIPLIER is the same as
> + * BONDING_8023AD_CACHE_FLUSHTHRESH_MULTIPLIER in
> rte_eth_bond_8023ad.c */
>  #define CACHE_FLUSHTHRESH_MULTIPLIER 1.5
>  #define CALC_CACHE_FLUSHTHRESH(c)	\
>  	((typeof(c))((c) * CACHE_FLUSHTHRESH_MULTIPLIER))
> --
> 2.32.0
> 

Acked-by: Morten Brørup <mb@smartsharesystems.com>