On 6/16/26 10:08 AM, Ales Musil wrote:
> 
> 
> On Tue, Jun 16, 2026 at 10:02 AM Dumitru Ceara <[email protected]
> <mailto:[email protected]>> wrote:
> 
>     On 6/15/26 2:34 PM, Dumitru Ceara wrote:
>     > On 6/11/26 8:03 PM, Ilya Maximets wrote:
>     >> On 6/9/26 3:23 PM, Ales Musil wrote:
>     >>>
>     >>>
>     >>> On Tue, Jun 9, 2026 at 2:20 PM Ilya Maximets <[email protected]
>     <mailto:[email protected]> <mailto:[email protected]
>     <mailto:[email protected]>>> wrote:
>     >>>
>     >>>     On 6/9/26 12:47 PM, Ales Musil via dev wrote:
>     >>>     > Add a Single-Producer, Single-Consumer lock-free ring buffer
>     >>>     > to lib/ for use as a building block in removing pinctrl_mutex
>     >>>     > contention from packet-in processing.
>     >>>     >
>     >>>     > The ring buffer uses a pre-allocated array with atomic read
>     >>>     > and write indices for synchronization.  Data is copied in
>     >>>     > and out by value (memcpy), making it well suited for small,
>     >>>     > fixed-size structs.  Capacity must be a power of two; the
>     >>>     > implementation uses unsigned 32-bit wraparound arithmetic
>     >>>     > for correct index handling.
>     >>>     >
>     >>>     > Provide SPSC_RING_INIT() for compile-time capacity checking
>     >>>     > and SPSC_RING_FOR_EACH_POP() for idiomatic drain loops.
>     >>>     >
>     >>>     > Include unit tests covering basic push/pop, FIFO ordering,
>     >>>     > full ring behavior, index wraparound, the FOR_EACH_POP
>     >>>     > macro, and multi-field struct elements.
>     >>>
>     >>>     Hi, Ales.  Thanks for the patch!  Not a full review, but I
>     wonder why
>     >>>     a new data structure is needed here.  Is the existing mpsc-
>     queue not
>     >>>     sufficient?  It is a bit more general, but serves the same
>     purpose more
>     >>>     or less.
>     >>>
>     >>>     Best regards, Ilya Maximets.
>     >>>
>     >>>
>     >>> Hi Ilya,
>     >>>
>     >>> thank you for the comment, I didn't feel like introducing another
>     >>> struct that would work with the mpsc-queue node as it didn't
>     really fit
>     >>> into the existing ones, in the end. On the micro optimaztion side,
>     >>> mpsc-queue uses a linked list, while the spsc has an array buffer so
>     >>> it should be better for cache performance, it probably doesn't
>     matter that
>     >>> much here.
>     >>
>     >> Following up on the off-list conversation, both mpsc and spsc
>     have their
>     >> advantages and disadvantages:
>     >
>     > Hi Ilya, Ales,
>     >
>     > Thanks for summarizing this!
>     >
>     >>
>     >> 1. Code change to use existing mpsc would likely be smaller than
>     adding
>     >>    a whole new data structure.
>     >>
>     >> 2. mpsc will be a little slower on memory accesses due to linked
>     list nature.
>     >>
>     >> 3. mpsc will not have a problem where the messages get dropped due to
>     >>    limited space in the queue.  Current hmap limits the number of
>     updates,
>     >>    but it handles duplicates, while the queue does not, so
>     limiting the
>     >>    queue to the same number of elements as in hmap may cause
>     higher drop
>     >>    rate for the messages overall in case there are many duplicate
>     events.
>     >>
>     >
>     > Out of the issues listed here I'd worry the most about this one.
>     >
>     > We do risk dropping more valid "arp reply" messages in favor of
>     "dup arp
>     > replies".  Would it make sense to use two maps?  I'm not sure I
>     like the
>     > "unbounded" nature of mpsc queues either.
>     >
> 
>     Thinking some more about it: two maps would require additional memory
>     allocation and potentially copying (if we use spsc).
> 
>     Also, the scenario we're talking about here is when the main thread is
>     really slow in draining the queue.
> 
>     Even today, if the main thread is slow, we'll hit the current hmap limit
>     and start dropping stuff.
> 
>     We'll probably not be worse than we are today if we use the spsc queue.
>     Maybe to be more conservative we should just increase the spsc queue
>     size to something larger than MAX_MAC_BINDINGS (e.g., 4x).  What do you
>     think?
> 
> 
> Increasing the size of the queue was something I was thinking about
> initially.
> And yeah it might be enough to just increase it. I can post the next
> revision
> with the capacity increased.
> 

OK, that sounds good to me.  I'll be looking for the next revision, thanks!

> 
>     >> 4. spsc will allocate memory upfront potentially consuming more
>     than the
>     >>    on-demand nature of entries in mpsc.  Though it will consume
>     less at
>     >>    max capacity, likely.
>     >>
>     >>>
>     >>> I'm probably fine with rewriting it using mpsc-queue if that
>     feels like a better
>     >>> fit.
>     >> Given the pros and cons above, I do not have a strong opinion on
>     which
>     >> one is better for this use case.  So, I'll leave this to
>     maintainers to
>     >> decide.
>     >>
>     >> Best regards, Ilya Maximets.
> 
>     Regards,
>     Dumitru
> 
> 
> Thanks,
> Ales 

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to