Re: [PATCH 3/7] net/bonding: change mbuf pool and ring allocation

["Sanford, Robert" <rsanford@akamai.com>]

Hi Connor,

On 12/20/21, 9:03 PM, "Min Hu (Connor)" <humin29@huawei.com> wrote:

> Hi, Sanford,

> > There is *NO* benefit for the consumer thread (interrupt thread
> > executing tx_machine()) to have caches on per-slave LACPDU pools.
> > The interrupt thread is a control thread, i.e., a non-EAL thread.
> > Its lcore_id is LCORE_ID_ANY, so it has no "default cache" in any
> > mempool.
> Well, sorry, I forgot that interrupt thread is non-EAL thread.

No problem. (I added a temporary rte_log statement in tx_machine
to make sure lcore_id == LCORE_ID_ANY.)

> > There is little or no benefit for active data-plane threads to have
> > caches on per-slave LACPDU pools, because on each pool, the producer
> > thread puts back, at most, one mbuf per second. There is not much
> > contention with the consumer (interrupt thread).
> > 
> > I contend that caches are not necessary for these private LACPDU
> I agree with you.

Thanks.

> > I believe there is a mistake in the ring comments (in 3 places).
> > It would be better if they replace "free" with "full":
> > "... to differentiate a *full* ring from an empty ring."
> > 
> Well, I still can not understand it. I think the ring size is N, it
> should store N items, why "N - 1" items.?
> Hope for your description, thanks.

Here is an excellent article that describes ring buffers, empty vs full, N-1, etc.
https://embedjournal.com/implementing-circular-buffer-embedded-c/#the-full-vs-empty-problem


> >> To fix the bug, how about just setting the flags "RING_F_EXACT_SZ"
> > 
> > Yes, this is a good idea. I will look for examples or test code that
> > use this flag.
> Yes, if fixed, ILGM.

I will use RING_F_EXACT_SZ flag in the next version of the patchset. I did not know about that flag.
	rte_ring_create(... N_PKTS ... RING_F_EXACT_SZ)
... is equivalent to, and looks cleaner than ...
	rte_ring_create(... rte_align32pow2(N_PKTS + 1) ... 0)

I plan to create a separate patchset to update the comments in rte_ring.h,
re RING_F_EXACT_SZ and "free" vs "full".

--
Regards,
Robert Sanford