Re: [dpdk-dev] [PATCH v2 0/4] Introduce IF proxy library

["Morten Brørup" <mb@smartsharesystems.com>]

Andrzej,

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of David Marchand
> Sent: Wednesday, March 25, 2020 9:08 AM
> 
> Hello Andrzej,
> 
> On Tue, Mar 10, 2020 at 12:11 PM Andrzej Ostruszka
> <aostruszka@marvell.com> wrote:
> >

<snip>

> >
> > What has changed since the RFC
> > ==============================
> >
> > - Platform dependent parts has been separated into a ifpx_platform
> >   structure with callbacks for initialization, getting information
> about
> >   the interface, listening to the changes and closing of the library.
> >   That should allow easier reimplementation.
> >
> > - Notification scheme has been changed - instead of having just
> >   callbacks now event queueing is also available (or a mix of those
> >   two).

Thank you for adding event queueing!

David mentions ABI forward compatibility below. Consider using a dynamically sized generic TLV (type, length, value) message format instead of a big union structure for the events. This would make it easier to extend the list of event types without breaking the ABI.

And I am still strongly opposed to the callback method:
The callbacks are handled as DPDK interrupts, which are running in a non-DPDK thread, i.e. a running callback may be preempted by some other Linux process. This makes it difficult to implement callbacks correctly.
The risk of someone calling a non-thread safe function from a callback is high, e.g. DPDK hash table manipulation (except lookup) is not thread safe.

Your documentation is far too vague about this:
Please note however that the context in which these callbacks are 
called is most probably different from the one in which packets are 
handled and it is application writer responsibility to use proper 
synchronization mechanisms - if they are needed.

You need a big fat WARNING about how difficult the DPDK interrupt thread is to work with. As I described above, it is not "most probably" it is "certainly" a very different kind of context.

Did you check that the functions you use in your example callbacks are all thread safe and non-blocking, so they can safely be called from a non-DPDK thread that may be preempted by a another Linux process?

<snip>

> 
> I can see we end up exposing structures for registering callbacks.
> Did you consider some ways to avoid exposure of those? (thinking of
> ABI maintenance for when this library will elect to non-experimental).
> I can see some canary at the end of an enum, can we do without it?
> 
> Is there a pb with merging ifpx support into the existing l3fwd
> application rather than introduce a new example?
> 
> 
> --
> David Marchand
>