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

在 2021/12/21 23:31, Sanford, Robert 写道:

Hi Connor,

On 12/20/21, 9:03 PM, "Min Hu (Connor)" [off-list ref] wrote:

quoted

Hi, Sanford,

quoted

quoted

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.)

quoted

quoted

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.

quoted

quoted

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

Thanks Sanford, I see. It is characteristics of ring queues which is
different with common queue, like buffers.

quoted

quoted

quoted

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