> -----Original Message----- > From: Morten Brørup <m...@smartsharesystems.com> > Sent: Friday, March 25, 2022 21:14 > To: Gaoxiang Liu <gaoxiangl...@163.com>; ch...@att.com; humi...@huawei.com > Cc: dev@dpdk.org; liugaoxi...@huawei.com; olivier.m...@6wind.com; > andrew.rybche...@oktetlabs.ru > Subject: RE: [PATCH v5] net/bonding: another fix to LACP mempool size > > +CC mempool maintainers > > > From: Gaoxiang Liu [mailto:gaoxiangl...@163.com] > > Sent: Friday, 25 March 2022 14.02 > > > > 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 <liugaoxi...@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 > > --- > > drivers/net/bonding/rte_eth_bond_8023ad.c | 11 ++++++++---
> > > > +/* 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 > > Very important comment. Thank you! > > May I suggest that a similar comment is added to the rte_mempool.c file, so if > CACHE_FLUSHTHRESH_MULTIPLIER is changed there, we don't forget to change the > copy-pasted code in the > rte_eth_bond_8023ad.c file too. It has previously been discussed changing it > from 1.5 to 2 for > symmetry reasons. Then, introduce some kind of public API macro, like RTE_MEMPOOL_CACHE_MAX_FLUSHTHRESH_MULTIPLIER as RTE_MEMPOOL_CACHE_MAX_SIZE does ? So that when calling mempool create API, it can do other kind of calculation, like RTE_MIN(user's new flush multiper, RTE_MEMPOOL_CACHE_MAX_FLUSHTHRESH_MULTIPLIER). Just a suggestion, so that no need add strange comments like BONDING_8023AD in an lib. > > > + > > + 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. */ > > -- > > 2.32.0 > >
