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

[Costin Lupu <costin.lupu@xxxxxxxxx>]

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