Dev plugins support upstream submit 01 branch

[ncarrier]

Hello,
Here is a patch set which implements plugins support for libiio.

The main goal is to allow implementing libiio backends outside of libiio, in plugin form.

The base idea is that a given directory is scanned for .so files, whose name match a given pattern.
These .so are dlopened and have access to some functions from the internal libiio API (from iio-private.h and debug.h).
For the particular case of backend implementation, two functions are provided, allowing plugins to register context factories, iio_context_factory_register() and iio_context_factory_unregister().
A factory allows the creation of an iio_context, parametrized with key/value string properties.
The local and network plugins are modified to register factories and their context creation function wrappers in context.c are modified to use these factories.

A public API function is added, to allow the access to an arbitrary backend implementation: iio_create_context().

The main drawback I see of this patch set is that it may break libiio on windows. Since I have absolutely no knowledge of this platform, I'm not competent for fixing that issue. I hope some more knowledgeable people will be able to do it. Maybe a conditional compilation approach, disabling the feature in non-dlopen capable platform would be a correct first attempt.

Having the possibility to create custom backends would allow a lot of possibilities, for example :

• record sensors/actuators accesses at runtime without altering the normal functioning of the upper software (and without using a network daemon)
• inject fake data from either a record file or a simulated environment
• access via libiio, sensors not using the kernel iio framework, i.e. add iio support in userland.
• enable any of the previous possibility, at runtime, with no recompilation

I am open to any suggestion or remark.

[pcercuei]

Hi Nicolas,

Thanks for this contribution, this is really appreciated. Having support for plug-ins is definitely something that we want.

However I don't think that this implementation is optimal, for the following reasons:

• as you mentionned, it will break on Windows. The "constructor" and "destructor" attributes are only supported by GCC and Visual Studio won't accept that.
• it adds some complexity (iio_init_plugins, registering the factory properties),
• it makes incremental updates impossible, as the plugins include "iio_private.h", while the internals of libiio can change from one minor revision to the other.

At some point I was toying with a USB backend (via libusb) which gave me ideas about how to handle external plugins while keeping all the complexity in the private part of the library; I will explain this different approach.

Let's define this structure, in a public file iio-backend.h:

struct iio_backend {
    int init(...);
    void exit(void);
    const char * (*get_xml)(void);
    struct iio_backend_ops *ops;
};

The idea behind this, is that you only need to specify the structure of the context that you want to create as a XML string. A new built-in backend named e.g. "dynamic", created with iio_create_context(name, ...) would then lookup for the shared library that matches the given name, locate the "iio_backend" structure, call the "init" function with the additional parameters, then call get_xml( ) to retrieve a XML representation of the context wanted by the plugin. And just like the network backend already does, it would create a iio_context structure from that XML string using the XML backend and piggy-back the "ops" pointer.

This would allow an external plug-in to create a new IIO context structure without accessing the internals of libiio, which is much more robust vs. updates.
Besides, I think that most of the iio_backend_ops callbacks of the plugin could be implemented only using the public API (at least that was the case on my USB backend).

Another advantage of this method, is that you can save a pointer to the loaded plugin in the context's pdata, and free it from the destroy( ) callback. This avoids having to call any other function to load / unload the plugin, and it removes all the complexity of the "factory".

It is also less intrusive, in the sense that all the complexity stays in one place (e.g. dynamic.c), and we could just disable this new "dynamic" meta-backend for Windows builds.

Any thoughts about this approach?

[ncarrier]

Hello,

Having support for plug-ins is definitely something that we want.

This is a super good news for us, we really need it too.

Portability issues

The "constructor" and "destructor" attributes are only supported by GCC and Visual Studio won't accept that.

It seems possible to do the same with VS. But I'm no competent to implement it. The main portability issue is IMHO the plug-in loading mechanism.
You propose a lazy loading of the plug-in at context creation request, this implies that a plugin is a backend implementation. My approach doesn't enforce this kind of coupling.

Factory

it adds some complexity (iio_init_plugins, registering the factory properties)
iio_create_context(name, ...) [...] call the "init" function with the additional parameters

These points can be summarized by : parametrize context creation with an ellipsis VS key value properties
In fact the "init" function is a factory, that is a way to create "objects" and to fine tune this creation.
Personally, I favour naming things explicitly. For the same reason, I'd rather avoid the ellipsis which removes the explicit naming of the parameters.
The factory registration is optional, that is, it could be removed from the local and network backends. In this case they would lose the compatibility with the iio_create_context() facility, which is not a big concern in itself. What's more, a plugin not implementing a backend will not require registering a factory.

Dependency on the libiio internals

I agree that it is not a good thing to depend on the internals of libiio. I think there is a need to identify a clear part of libiio's internals which should be extracted to a libiio-plugins included (or linked by) plugin implementors. These would become part of a public API, which should be maintained, just for plugins.
Then all is a matter of knowing what to put in that library. I think libiio should provide some facilities to plugins implementors. The approach I followed, was to allow the local backend to be extracted as a plugin and export what it needed for it's implementation.
This resulted in the "Export the API functions needed by plugins" commit. These functions are a good candidate for being put in a libiio-plugins.h, I think.
My particular need is to implement a backend which behaves exactly like a local backend, but which can be configured, in some cases at runtime, to provide additional services, like for example, recording raw sensor data, or simulating a failure.

About the xml

I think that the xml part is orthogonal to the backend context creation. If I understand well, it's role is to create the "devices tree structure" behind the backend. Backend implementors should be free to create this structure in the way they want. XML not being the right solution for every use cases.
I think the XML feature should take place in libiio-plugins, as a service which plug-ins could make use of, if it fitted their needs.

iio_init_plugins and iio_cleanup_plugins

I made an error exposing these two and they must be removed: they are not needed. They can just be constructor/destructor functions like the plugins initialization functions. I have just tested it and it works fine. I feared an "order of execution hazard", but in fact, there is none.

Plugin de-registration

save a pointer to the loaded plugin in the context's pdata

Again it implies plugin == backend implementation, which is an unneeded restriction. The destructor can take care of cleanup and is called directly by the dlclose() call issued by the plugins subsystem.

To sum up

The struct iio_backend is a good approach, but has the default of coupling not so related functionalities, which can be separated at a really low cost, that is :

• parametrized backend instantiation
• plugin registration and cleanup
• plugin implementation services (xml)
• storage of the backend ops

In my humble opinion...

I think we should :

1. extract both local and network backends as plugins
   thus decoupling libiio-core and plugins implementations
2. allow to disable plugins at compile time (which would make local and network plugins built-in)
3. implement destructor / constructor for VS
4. implement module loading for VS
5. hide iio_init_plugins and iio_cleanup_plugins and use them as constructor/destructor
6. create a proper API for plugin implementation (including the xml part)

I can work on items 1, 2 and 5 on my own. I am not competent for 3 and 4 I don't even have access to a windows pc... I can work on 6, but it will need some more discussion to design the API and choose which inner functions to move to it.

[pcercuei]

Hi,

Let me clear up some points:

The problem with accessing the internals of libiio is not only about the visibility of the functions. If the plug-in has visibility on the internal structures, e.g. the iio_context structure, then a compiled plug-in will become incompatible with the core library as soon as the iio_context structure is modified in the core. This means that there would be a strong dependency between the version of the core and the plug-ins. In that case, there is not much interest in decoupling the core and the current backends...

About the XML, it is a 1:1 representation of the IIO context. It only contains public information about the context, that the application could otherwise retrieve with the public API functions when using any backend. This is a requirement for the remote feature to work. This has two implications:

• it is always possible to recreate the structure of the IIO context from the XML string,
• you can't create a IIO context whose structure cannot be represented with a XML string.

Concretely, the local backend could very well generate a XML string representing the context, and have all the internal structures created via the XML backend (just like the network backend does). It would just store its private data in its iio_context_pdata and iio_device_pdata structures, hidden from the core. The advantage of using a XML string to create the IIO context, is that it greatly reduces the use of internal functions and/or structures, so it should be the preferred way.

A final thought (for now) about the loading of the plug-ins: I think it is a really bad idea to automatically load all the plug-ins from a given directory and allow them to call a constructor. This is a huge security hole...

[ncarrier]

Hello,
Sorry for the delay in the answer, I had some days off.

The problem with accessing the internals of libiio [...] there is not much interest in decoupling the core and the current backends...

IMHO it is just a matter of how much you expose of the libiio to the plugin implementors. It is hard to settle, as providing too little will reduce the interest of the plugin interface but providing too much will provoke the problems you lay stress on wisely.

About the XML [...] so it should be the preferred way.

I'd rather not have forced the use of XML, but in the end, I'm fine with it.

A final thought (for now) about the loading of the plug-ins [...] This is a huge security hole...

Here I really don't understand. I proposed loading plugins from a well controlled location, written in the library at build time. Why would this be a security hole ? If one isn't able to control what goes to this location, why would he/she be able to control the location where the libiio.so is ? Then one should be able to replace/hack it...

The most important for me is to provide a plugin support which will allow us to do what we need. So even if I don't completely agree with your point of view, I am currently trying to work on an implementation based on your proposal.

I will come back to you when I'll have questions.

[pcercuei]

Hi,

If you control the directory where the plug-ins are located, and don't allow a regular user to load plug-ins from a different directory then there is indeed no security problem.

I will wait for your new implementation then.

Thanks for your work!

[ncarrier]

Hello,
So here comes the first question ^^
A libiio plugin must know at least the iio_backend structure in order to provide an instance. But if a plugin as access to struct iio_backend, then it must also have access to struct iio_backend_ops because of the ops field it has to fill. I suspect also that each callback from the iio_backend_ops should have access to the structures they are passed as arguments. At least in the current implementation, local and backend networks do access their fields in their callbacks implementations.

So what concretely should we expose to a plugin ?

[pcercuei]

Hi,

The struct iio_backend_ops should be visible to the plug-ins, that's certain.

The callbacks, on the other hand, should resort to use the public API as much as possible. I don't think that it would be hard to do; most of the fields of the iio_* structures defined in "iio-private.h" can already be retrieved using API functions. Some fields cannot (e.g. device->ctx or channel->dev have no equivalent API function), but it would make sense to have API functions for those as well.

For the iio_[context|device|channel]_pdata, we could just have public accessors à la iio_device_get_data / iio_device_set_data, not part of the public API and defined in a separate header (e.g. iio-backend.h) reserved for the backend implementations.

From my experience, the network backend could very well be using the public API of libiio (with the few additions listed above); the main reason why it currently doesn't, is because it is faster to type chn->id than iio_channel_get_id(chn) :)

The local backend uses the internals much more as it does all the work of creating the IIO context from what it sees in sysfs. It would be possible though to simplify it enough so that it only uses API functions and backend-specific functions, by making it generate XML from what it sees in sysfs, and use the XML backend to create all the structures. The XML backend, in that case, would be the only one built into the library and the only one with a full visibility on the internals.

I can work on de-coupling the local backend from the core of the library once we have a well-defined API; for now I think we can keep it built-in. Having the network backend decoupled, on the other hand, seems a good way to test the robustness of the plug-in implementation.

Paul

[ncarrier]

Ok,
Then I think I am going to start by doing the decoupling work on the network backend. This will help me identify the needed API.
Do usable automated tests exist allowing me to check for regressions ?

[pcercuei]

There are no automated tests - I just use the static analyzer of clang to identify the most obvious mistakes.

[rofferom]

I will continue the work of Nicolas. From what I understand of the discussion, I have tried to write a patch following your implementation suggestion Paul.

A small preview is available at Parrot-Developers/libiio@1a7c2a6 (the commit has been done after commits of pull request #17).

Here are the main steps of my work :

• Create iio-backend.h, with struct iio_backend_ops, struct iio_backend, and some tool functions that can be used by backend implementations
• Add the API struct iio_context * iio_create_context(const char *name) to open a dynamic backend (using dlopen())

I have still question about this draft :

• I don't know where to store the handle returned by dlopen()
• A backend implementation may need to get some parameters at creation, what is the expected way to do this in libiio ?
• Should I try to update network backend to use iio-backend.h, or can it be done later ?

I will continue to work on this patch (I need to check if it fits our needs, and then improve it), but I would like to have a feedback from you, as I'm not sure of the implementation to provide.

[pcercuei]

Hi,

Inside dynamic.c, you actually include "iio-private.h", so you have access to the internal structures, unlike the external plugins. This means that you don't actually need functions like "iio_context_set_ops( )", you can just do "ctx->ops = ops;".

About where to store the handle:
the iio_context_pdata structure is private to the "dynamic" backend, and should not be accessible by the plugin. This doesn't prevent you from having an opaque pointer to a "iio_backend_pdata", with accessors, that you store inside the iio_context_pdata structure, and that your plugin can use freely to store its data. Then your "dynamic" backend (not the plugin) owns the "iio_context_pdata" structure, and that's where you can store the handle.

About parameters passing: I have a a preference on using a variadic function "iio_create_context(name, ...)", where the additional parameters are passed to the plugin's init() function, but I am fine with alternatives if they are simple enough.

You don't have to update the network backend, it can certainly be done later.

[rofferom]

Ok, I should work to store the handle in this way.

I will implement a variadic function, I have no specific suggestion for this point.

I will work on this way. Once I manage to implement dynamic backends for our needs, I will send you an update for this pull request.

Thanks !

[pcercuei]

Closing this, the "dev" branch has support for plugins.