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

["Min Hu (Connor)" <humin29@huawei.com>]

在 2021/12/21 23:31, Sanford, Robert 写道:
> 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
> 
Thanks Sanford, I see. It is characteristics of ring queues which is
different with common queue, like buffers.
> 
>>>> 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
> 
>