[PATCH DRAFT RFC] upoe: tiny daemon for PoE devices

[Bjørn Mork]

[ CCing robimarko as the author and maintainer of the tps23861 driver ]

Rafał Miłecki <zajec5 at gmail.com> writes:

> From: Rafał Miłecki <rafal at milecki.pl>
>
> This is a tiny app that reads list of PoE devices and initializes them
> using built-in drivers. PoE status can be read using ubus.
>
> Signed-off-by: Rafał Miłecki <rafal at milecki.pl>
> ---
> This is a *** DRAFT *** for how we could handle PoE devices in OpenWrt.
> Please feel free to comment if this EARLY & INCOMPLETE design looks sane
> enough.
> ---
>  CMakeLists.txt |   9 ++++
>  dummy.c        |  24 +++++++++
>  upoe.c         | 144 +++++++++++++++++++++++++++++++++++++++++++++++++
>  upoe.h         |  36 +++++++++++++
>  4 files changed, 213 insertions(+)
>  create mode 100644 CMakeLists.txt
>  create mode 100644 dummy.c
>  create mode 100644 upoe.c
>  create mode 100644 upoe.h
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> new file mode 100644
> index 0000000..6474663
> --- /dev/null
> +++ b/CMakeLists.txt
> @@ -0,0 +1,9 @@
> +cmake_minimum_required(VERSION 2.6)
> +
> +project(upoe)
> +set(CMAKE_C_FLAGS "-std=c99 -D_GNU_SOURCE -Wall")
> +
> +add_executable(upoe upoe.c dummy.c)
> +target_link_libraries(upoe ubox ubus uci)
> +
> +install(TARGETS upoe RUNTIME DESTINATION bin)
> diff --git a/dummy.c b/dummy.c
> new file mode 100644
> index 0000000..de0abfb
> --- /dev/null
> +++ b/dummy.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "upoe.h"
> +
> +static int upoe_dummy_probe_device(struct upoe *upoe, struct uci_element *e)
> +{
> +	struct device *dev = calloc(1, sizeof(*dev));
> +
> +	dev->drv = &dummy;
> +	upoe_device_register(upoe, dev);
> +
> +	return 0;
> +}
> +
> +static int upoe_dummy_get_status(struct upoe *upoe, struct device *dev)
> +{
> +	return 0;
> +}
> +
> +struct driver dummy = {
> +	.type = "dummy",
> +	.probe_device = upoe_dummy_probe_device,
> +	.get_status = upoe_dummy_get_status,
> +}; 
> diff --git a/upoe.c b/upoe.c
> new file mode 100644
> index 0000000..3dfa834
> --- /dev/null
> +++ b/upoe.c
> @@ -0,0 +1,144 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "upoe.h"
> +
> +/**************************************************
> + * Device
> + **************************************************/
> +
> +static int upoe_device_status(struct ubus_context *ctx,
> +			      struct ubus_object *obj,
> +			      struct ubus_request_data *req,
> +			      const char *method,
> +			      struct blob_attr *msg)
> +{
> +	struct upoe *upoe = container_of(ctx, struct upoe, ubus);
> +	struct device *dev = container_of(obj, struct device, ubus_obj);
> +	struct blob_buf b = { };
> +	//void *c;
> +
> +	blob_buf_init(&b, 0);
> +
> +	dev->drv->get_status(upoe, dev);
> +	blobmsg_add_string(&b, "foo", "bar");
> +
> +	ubus_send_reply(ctx, req, b.head);
> +
> +	blob_buf_free(&b);
> +
> +	return 0;
> +}
> +
> +static const struct ubus_method upoe_ubus_device_methods[] = {
> +	UBUS_METHOD_NOARG("status", upoe_device_status),
> +};
> +
> +static struct ubus_object_type upoe_ubus_device_object_type =
> +	UBUS_OBJECT_TYPE("upoe-device", upoe_ubus_device_methods);
> +
> +int upoe_device_register(struct upoe *upoe, struct device *dev)
> +{
> +        struct ubus_object *obj = &dev->ubus_obj;
> +	char *name = NULL;
> +	int err;
> +
> +	if (asprintf(&name, "upoe.%s", "dummy0") == -1)
> +		return -ENOMEM;
> +
> +	obj->name = name;
> +	obj->type = &upoe_ubus_device_object_type;
> +	obj->methods = upoe_ubus_device_methods;
> +	obj->n_methods = ARRAY_SIZE(upoe_ubus_device_methods);
> +
> +	err = ubus_add_object(&upoe->ubus, obj);
> +	if (err) {
> +		fprintf(stderr, "Failed to add ubus object\n");
> +		free(name);
> +	}
> +
> +	return err;
> +}
> +
> +/**************************************************
> + * main()
> + **************************************************/
> +
> +static struct driver *drivers[] = {
> +	&dummy,
> +};
> +
> +static int upoe_probe_devices(struct upoe *upoe) {
> +	struct uci_package *p;
> +	struct uci_element *e;
> +
> +	uci_load(upoe->uci, "upoe", &p);
> +	if (!p) {
> +		fprintf(stderr, "Failed to load upoe config\n");
> +		return -ENOENT;
> +	}
> +
> +	uci_foreach_element(&p->sections, e) {
> +		struct uci_section *s;
> +		const char *type;
> +		int i;
> +
> +		s = uci_to_section(e);
> +
> +		if (strcmp(s->type, "device"))
> +			continue;
> +
> +		type = uci_lookup_option_string(upoe->uci, s, "type");
> +		if (!type)
> +			continue;
> +
> +		for (i = 0; i < ARRAY_SIZE(drivers); i++) {
> +			struct driver *drv = drivers[i];
> +
> +			if (!strcmp(drv->type, type)) {
> +				drv->probe_device(upoe, e);
> +				break;
> +			}
> +		}
> +	}
> +
> +	uci_unload(upoe->uci, p);
> +
> +	return 0;
> +}
> +
> +int main(int argc, char **argv) {
> +	struct upoe *upoe = calloc(1, sizeof(*upoe));
> +	int err;
> +
> +	if (!upoe) {
> +		err = -ENOMEM;
> +		goto err_out;
> +	}
> +
> +	upoe->uci = uci_alloc_context();
> +	if (!upoe->uci) {
> +		err = -ENOMEM;
> +		goto err_free_upoe;
> +	}
> +
> +	if (ubus_connect_ctx(&upoe->ubus, NULL)) {
> +		fprintf(stderr, "Failed to connect to ubus\n");
> +		err = -ENXIO;
> +		goto err_uci_free;
> +	}
> +
> +	upoe_probe_devices(upoe);
> +
> +	uloop_init();
> +	ubus_add_uloop(&upoe->ubus);
> +	uloop_run();
> +	uloop_done();
> +
> +	ubus_shutdown(&upoe->ubus);
> +err_uci_free:
> +	uci_free_context(upoe->uci);
> +err_free_upoe:
> +	free(upoe);
> +err_out:
> +	return err;
> +}
> diff --git a/upoe.h b/upoe.h
> new file mode 100644
> index 0000000..c3fd18d
> --- /dev/null
> +++ b/upoe.h
> @@ -0,0 +1,36 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#ifndef UPOE_H_
> +#define UPOE_H_
> +
> +#include <uci.h>
> +#include <libubus.h>
> +
> +struct upoe;
> +struct device;
> +
> +struct driver {
> +	const char		*type;
> +	int			(*probe_device)(struct upoe *upoe, struct uci_element *e);
> +	int			(*get_status)(struct upoe *upoe, struct device *dev);
> +};
> +
> +struct device {
> +	struct driver		*drv;
> +	struct ubus_object	ubus_obj;
> +	void			(*get_status)(struct device *dev);
> +
> +	struct device		*next;
> +};
> +
> +struct upoe {
> +	struct uci_context	*uci;
> +	struct ubus_context	ubus;
> +	struct device		*devices;
> +};
> +
> +int upoe_device_register(struct upoe *upoe, struct device *dev);
> +
> +struct driver dummy;
> +
> +#endif


I don't have strong opinions either way here.  I am pretty sure we'll
have to redesign this at some point no matter how much we try to think
about it right now.  We just don't have enough experience with different
hardware interfaces and use cases to make a proper design.  At least, I
don't have...

Hoping Robert will chime in, obviously having thought about this when
designing the tps23861 hwmon interface.

This is diffult.  Just take a look at the existing PoE MIBs. The IETF
experts should have solved this problem for us a long time ago, defining
the statistiscs and settings required for any PoE managment.  But RFC3621
was never sufficient, so every vendor made their own PoE MIBs.

Do we want this daemon to be a low-level or high-level interface?  Will
this be the interface used by some future lldp plugin, implementing the
full layer2 802.3at/bt stuff?  If so, then we should define the suitable
per-port hooks for that.  Maybe just use the lldp TLVs directly, or
almost directy?

Or is this just a high-level thing for UCI/LUCI only?  Then I guess the
above example just needs a per-port enable, and it would be fine.

Sorry, I just have plenty of questions and no answers.  Not very
helpful.  Will shut up now.



Bjørn