[Openframeworkwg] source code for proof of concept framework available

[Hefty, Sean]

Thanks for the feedback - I'll make some updates accordingly.

> I took a few mins of looking, just some random structural comments
> - Public headers need to be cautious of what names they introduce into
>   the global space. If the global name doesn't start with 'fi_' then
>   some underscores are needed eg:
>   51  #define container_of
>   46 typedef uint16_t be16_t;
>   81 typedef unsigned long __attribute__((aligned(4))) packed_ulong;
>   94 #define UMAD_IOCTL_MAGIC        0x1b
>   [and more starting with umad_, maybe fi_ib_umad]

I haven't done anything with the umad stuff that's there.  It's basically copied in from umad.h for kernel ABI support.  I should probably remove it until there's something there that's actually usable.

> - I always hated that sockets exposed the network byte order
>   to apps, IMHO that should be avoided unless it is on a high speed
>   path ..

I'm indifferent myself, but I do care that the interface clearly conveys which order the data is used (for immediate data, rkeys, etc.) 

> - const and type correctness - I didn't check deeply enough
>   but there were a few bare 'char *' in some signatures, might be
>   right, or maybe should be 'const char *' or ' void *'. This needs to be
>   right everywhere or it is a PITA to use from C++

agreed - I have made a first pass over using const, but looking quickly, it looks like there are a couple of places where const is still needed.

> - It is also worth understanding the proper use of restrict and
>   no except.
> - fe_errno.h ... either have sane library
>   specific unique error constants and xlate the kernel syscall errnos
>   to the correct context specific error code,
>   or stick with POSIX errno codes everwhere..

My main issue with errno values are that practically everything gets mapped to EINVAL, and ends up being useless for debugging.  So, I went with an 'extended' errno scheme.  I use errno values where reasonable, but allow for additional values beyond those defined in errno.h.  I also added places for vendor error codes.

>   Did you decide on an consistent 'errors returned in int' or
>   'errors as errno scheme'?

The current code is structured as returning >= 0 on success, and negative error code on failure.  Success is typically 0 or some number of bytes.

> - fi_atomic and fl_arch can probably use gcc intrinsics - introducing the
>   multi-threaded memory model in C++11 caused gcc to gain a full set
>   of memory barrier and atomic builtins for C code.

fi_arch was simply copied from libibverbs -- I haven't done anything with it.  It should be removed or moved internal for provider use only.  fi_atomic is a placeholder for defining atomic operations over the network.

> The downside of having 'ops' pointers neatly organized into
> functionally group'd structures is now you pay a double dereference
> cost at every call site, verbs had only a single dereference.

I agree.

The trade-off is that the ops pointers can reference static structures, which should always be a cache hit.  Verbs places the ops in dynamically allocated memory.  So, we reduce the memory footprint, and can leave ops NULL where entire sets of operations are not supported.

The ops pointers also make it easier to extend specific functionality.

I guess an app could always declare their own function pointer and use it to avoid the double deref.

> Also, the complete loss of static type safety by having 'fid_t' be the
> only argument type seems like a big negative to me..

I have a note to look at changing this, including possibly removing the fid_t data type completely, but that's a significant change, so I've been putting it off.

- Sean