Re: [UNIKRAFT PATCH 0/5] Introduce Ring Buffer Implementation

["Jung, Alexander" <a.jung@xxxxxxxxxxxxxxx>]

Hi Costin,

Your argument is sound.  I'll be sure to push the ring buffer implementation 
into its own microlibrary on the v2 once it has been reviewed.

Thanks,

Alexander


On 21.07.20, 09:30, "Minios-devel on behalf of Costin Lupu" 
<minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of costin.lupu@xxxxxxxxx> 
wrote:

    Hi Alexander,

    For both ukalloc and uksched we used an OOP approach:
    - ukalloc is a base class extended by ukallocbuddy lib
    - uksched is a base class extended by ukschedcoop lib

    Following the same approach it's clear that an mp_ring lib, let's say,
    would be a lib of its own. Regarding your comment that mbox is another
    message passing implementation, that's true but it should be a lib of
    its own on the same level as ring.

    Another thing - what if I want to add my own mbox implementation but I
    want to use your ring implementation? In that case, along with my mbox
    lib I would also have to enable ukmpi (for your ring) which would also
    contain the mbox legacy implementation. And viceversa if I implement a
    new ring and use the mbox legacy implementation.

    The safest design principle in this case would be a classic one (as
    always): a lib should do just one thing and one thing only.


    Cheers,
    Costin


    On 7/21/20 12:52 AM, Jung, Alexander wrote:
    > Hi Costin,
    > 
    > For this version, I simply chose to introduce it into the ukmpi library 
since 
    > it adopts a similar trait to mbox, which is the current inhabitant of the 
    > microlibrary.  Mbox is another "simple message passing" implementation 
and, for 
    > me, along with the offline conversation I had with Simon (cc'd), it made 
sense 
    > to include it here as an additional provision of the microlibrary itself. 
 
    > 
    > I think your argument is fair in that it can exist as its own library: 
simply 
    > as `ukring`.  At this point, it is simple a decision w.r.t. whether the 
library 
    > constitutes creating enough additional options that would degrade the 
quality 
    > of ukmpi itself.  Particulary, you refer to additional options which will 
be 
    > brought out of the implementation in the future.  For now, I do not see 
    > additional options which would surface themselves at present in the 
KConfig 
    > option menu.  The decision is ultimately whether these options arise from 
    > implementation-specific requirements, in the case of, for instance, 
mp_ring.{c,
    > h} from FreeBSD which is the ring buffer implementation for the 
networking 
    > stack (that it is to say, "would this be part of `ukring` or it's own 
library?")
    > 
    > One thing to note is that the ring implementation, along with mbox, are 
not 
    > POSIX, and so mbox and ring's "exportsyms" do not share similar method 
    > standardization.  This, along with the fact that they are 
Unikraft-centric/-core 
    > and namespaced functions mean as internal communication mechanism its 
placement 
    > is somewhat arbitrary.
    > 
    > What do you think?
    > 
    > Thanks,
    > 
    > Alexander
    > 
    > 
    > On 20.07.20, 22:55, "Minios-devel on behalf of Costin Lupu" 
<minios-devel-bounces@xxxxxxxxxxxxxxxxxxxx on behalf of costin.lupu@xxxxxxxxx> 
wrote:
    > 
    >     Hi Alexander,
    > 
    >     I have a curiosity. Why don't you put this ring implementation in a
    >     library of its own, e.g. ukring or smth? I thought that library
    >     granularity was one of the strong points of Unikraft. Besides, this 
ring
    >     implementation may be subject to more configuration options in the
    >     future which would complicate the ukmpi configuration space.
    > 
    >     Cheers,
    >     Costin
    > 
    >     On 7/20/20 7:40 PM, Alexander Jung wrote:
    >     > From: Alexander Jung <alexander.jung@xxxxxxxxx>
    >     > 
    >     > This series introduces a port of FreeBSD's buf_ring.{h,c} 
implementation for use
    >     > within Unikraft.  This simple ring buffer can be used for message 
passing in
    >     > queues, for example within a AF_UNIX socket implementation between 
two 
    >     > threads.  The implementation is therefore thread safe and provides 
a generic 
    >     > data field which is initialized to the desired ring buffer length.
    >     > 
    >     > The implementation is provided within ukmpi as a new option, 
LIBUKMPI_RING, 
    >     > which exposes the following new methods:
    >     > 
    >     >  - uk_ring_alloc
    >     >  - uk_ring_free
    >     >  - uk_ring_enqueue
    >     >  - uk_ring_dequeue
    >     >  - uk_ring_dequeue_single
    >     >  - uk_ring_advance_single
    >     >  - uk_ring_putback_single
    >     >  - uk_ring_peek
    >     >  - uk_ring_peek_clear_single
    >     >  - uk_ring_full
    >     >  - uk_ring_empty
    >     >  - uk_ring_count
    >     > 
    >     > The port is currently limited, with no support for reading atomic 
values from
    >     > ARM{32,64} CPU registers (described in the comments inline).  As a 
result, use 
    >     > of the ring buffer implementation is untested on ARM.
    >     > 
    >     > Alexander Jung (5):
    >     >   lib/ukmpi: Introduce simple ring interface KConfig option
    >     >   lib/ukmpi: Initial port of FreeBSD's buf_ring.h
    >     >   lib/ukmpi: Provide ring buffer allocation and free methods
    >     >   lib/ukmpi: Include ring buffer into Unikraft build process
    >     >   lib/ukmpi: Export the global ring buffer (de)init methods
    >     > 
    >     >  lib/ukmpi/Config.uk         |  23 ++-
    >     >  lib/ukmpi/Makefile.uk       |   1 +
    >     >  lib/ukmpi/exportsyms.uk     |   3 +
    >     >  lib/ukmpi/include/uk/ring.h | 474 
++++++++++++++++++++++++++++++++++++++++++++
    >     >  lib/ukmpi/ring.c            |  87 ++++++++
    >     >  5 files changed, 587 insertions(+), 1 deletion(-)
    >     >  create mode 100644 lib/ukmpi/include/uk/ring.h
    >     >  create mode 100644 lib/ukmpi/ring.c
    >     > 
    >