[RFC BlueZ 0/2] mesh: Deliver mesh packets over datagram socket

[RFC BlueZ 0/2] mesh: Deliver mesh packets over datagram socket

[Michał Lowas-Rzechonek]

This patchset adds another D-Bus API for attaching applications, called
AttachFD.

When application uses that API, it receives one end of a datagram socket
pair, which it's supposed to recv() from in order to receive mesh
packets, instead of wating for *MessageReceived() calls over D-Bus.

This significantly reduces system load for high traffic environment
(e.g. an application that subscribes to a large number of publications
in a big network).

Message delivery is one way only: application is still supposed to call
*Send methods via D-Bus, although the socket pair is bidirectional, so
it would be possible to add sending as well.

Michał Lowas-Rzechonek (1):
  mesh: Implement AttachFD method

Przemysław Fierek (1):
  mesh: Add documentation for AttachFD

 doc/mesh-api.txt |  40 +++++++++++++++
 mesh/mesh.c      |  12 ++++-
 mesh/model.c     | 126 ++++++++++++++++++++++++++++++++++++++++++++++-
 mesh/node.c      |  83 ++++++++++++++++++++++++++++++-
 mesh/node.h      |   4 +-
 5 files changed, 259 insertions(+), 6 deletions(-)

-- 
2.20.1

[RFC BlueZ 1/2] mesh: Add documentation for AttachFD

[Michał Lowas-Rzechonek]

From: Przemysław Fierek <przemyslaw.fierek@silvair.com>

---
 doc/mesh-api.txt | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/doc/mesh-api.txt b/doc/mesh-api.txt
index 6d905cf48..cd2ffd2a8 100644
--- a/doc/mesh-api.txt
+++ b/doc/mesh-api.txt
@@ -118,6 +118,46 @@ Methods:
 			org.bluez.mesh.Error.AlreadyExists,
 			org.bluez.mesh.Error.Failed
 
+    object node, array{byte, array{(uint16, dict)}} configuration, fd
+            AttachFD(object app_root, uint64 token)
+
+        This is an alternative version of the Attach() method. This method
+        acquires file descriptor for message reception.
+
+        Method behaviour and usage is similiar to the Attach() method.
+        The difference between those two method is that after call AttachFD()
+        all received messages will be passed via file descriptor,
+        instead of daemon calling MessageReceived() and
+        DevKeyMessageReceived() methods.
+
+
+        The data format used for the exchange:
+
+            Message types:
+                0 - Device Key Message
+                1 - Application Key Message
+
+            struct {
+                uint8_t element;
+                uint8_t src;
+
+                uint8_t type;
+                union {
+                    struct {
+                        uint8_t net_idx;
+                        bool remote;
+                    } dev; // only meaningful when type is 0
+
+                    struct {
+                        uint8_t app_idx;
+                        uint16_t dst;
+                        uint8_t label[16]; // only meaningful when dst is virtual
+                    } app; // only meaningful when type is 1
+                }
+
+                uint8_t data[];
+            };
+
 	void Leave(uint64 token)
 
 		This removes the configuration information about the mesh node
-- 
2.20.1

[RFC BlueZ 2/2] mesh: Implement AttachFD method

[Michał Lowas-Rzechonek]

---
 mesh/mesh.c  |  12 ++++-
 mesh/model.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 mesh/node.c  |  83 ++++++++++++++++++++++++++++++++-
 mesh/node.h  |   4 +-
 4 files changed, 219 insertions(+), 6 deletions(-)

diff --git a/mesh/mesh.c b/mesh/mesh.c
index 4abae4d92..f43d00b7c 100644
--- a/mesh/mesh.c
+++ b/mesh/mesh.c
@@ -612,14 +612,18 @@ static void attach_ready_cb(void *user_data, int status, struct mesh_node *node)
 {
 	struct l_dbus_message *reply;
 	struct l_dbus_message *pending_msg;
+	const char *method;
 
 	pending_msg = l_queue_remove_if(pending_queue, simple_match, user_data);
 	if (!pending_msg)
 		return;
 
+	method = l_dbus_message_get_member(pending_msg);
+
 	if (status == MESH_ERROR_NONE) {
 		reply = l_dbus_message_new_method_return(pending_msg);
-		node_build_attach_reply(node, reply);
+		node_build_attach_reply(node, reply,
+						!strcmp(method, "AttachFD"));
 	} else
 		reply = dbus_error(pending_msg, status, "Attach failed");
 
@@ -635,7 +639,7 @@ static struct l_dbus_message *attach_call(struct l_dbus *dbus,
 	const char *app_path, *sender;
 	struct l_dbus_message *pending_msg;
 
-	l_debug("Attach");
+	l_debug("%s", l_dbus_message_get_member(msg));
 
 	if (!l_dbus_message_get_arguments(msg, "ot", &app_path, &token))
 		return dbus_error(msg, MESH_ERROR_INVALID_ARGS, NULL);
@@ -846,6 +850,10 @@ static void setup_network_interface(struct l_dbus_interface *iface)
 					"oa(ya(qa{sv}))", "ot", "node",
 					"configuration", "app", "token");
 
+	l_dbus_interface_method(iface, "AttachFD", 0, attach_call,
+					"oa(ya(qa{sv}))h", "ot", "node",
+					"configuration", "fd", "app", "token");
+
 	l_dbus_interface_method(iface, "Leave", 0, leave_call, "", "t",
 								"token");
 
diff --git a/mesh/model.c b/mesh/model.c
index 5ed95afac..0bef11e36 100644
--- a/mesh/model.c
+++ b/mesh/model.c
@@ -21,6 +21,8 @@
 #include <config.h>
 #endif
 
+#include <sys/types.h>
+#include <sys/socket.h>
 #include <sys/time.h>
 #include <ell/ell.h>
 
@@ -44,6 +46,33 @@
 
 #define VIRTUAL_BASE			0x10000
 
+enum fd_msg_type {
+	DEV_KEY_MSG = 0,
+	APP_KEY_MSG = 1,
+};
+
+struct fd_msg {
+
+	uint8_t element;
+	uint16_t src;
+
+	enum fd_msg_type type :8;
+	union {
+		struct {
+			uint16_t net_idx;
+			uint8_t remote;
+		} dev;
+
+		struct {
+			uint16_t app_idx;
+			uint16_t dst;
+			uint8_t label[16];
+		} app;
+	};
+
+	uint8_t data[];
+} __attribute__((packed));
+
 struct mesh_model {
 	const struct mesh_model_ops *cbs;
 	void *user_data;
@@ -782,7 +811,50 @@ static int add_sub(struct mesh_net *net, struct mesh_model *mod,
 	return MESH_STATUS_SUCCESS;
 }
 
-static void send_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
+static struct fd_msg *fd_msg_new(uint8_t ele_idx, uint16_t src, uint16_t size, const uint8_t *data, enum fd_msg_type type)
+{
+	size_t msg_len = sizeof(struct fd_msg) + size;
+	struct fd_msg *msg = l_malloc(msg_len);
+
+	msg->element = ele_idx;
+	msg->src = src;
+	msg->type = type;
+
+	memcpy(msg->data, data, size);
+
+	return msg;
+}
+
+static void fd_msg_send(struct l_io *io, struct fd_msg *msg, size_t size)
+{
+	struct iovec iov = {
+		.iov_base = msg,
+		.iov_len = sizeof(struct fd_msg) + size,
+	};
+	struct msghdr hdr = {
+		.msg_iov = &iov,
+		.msg_iovlen = 1,
+	};
+
+	(void)sendmsg(l_io_get_fd(io), &hdr, MSG_NOSIGNAL);
+
+	l_free(msg);
+}
+
+static void send_fd_dev_key_msg_rcvd(struct l_io *io, uint8_t ele_idx,
+				     uint16_t src, uint16_t app_idx,
+				     uint16_t net_idx, uint16_t size,
+				     const uint8_t *data)
+{
+	struct fd_msg *msg = fd_msg_new(ele_idx, src, size, data, DEV_KEY_MSG);
+
+	msg->dev.net_idx = net_idx;
+	msg->dev.remote = (app_idx != APP_IDX_DEV_LOCAL);
+
+	fd_msg_send(io, msg, size);
+}
+
+static void send_dbus_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
 					uint16_t src, uint16_t app_idx,
 					uint16_t net_idx, uint16_t size,
 					const uint8_t *data)
@@ -818,7 +890,40 @@ static void send_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
 	l_dbus_send(dbus, msg);
 }
 
-static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
+static void send_dev_key_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
+				       uint16_t src, uint16_t app_idx,
+				       uint16_t net_idx, uint16_t size,
+				       const uint8_t *data)
+{
+	struct l_io *io = node_get_fd_io(node);
+
+	if (io)
+		send_fd_dev_key_msg_rcvd(io, ele_idx, src, app_idx, net_idx,
+					 size, data);
+	else
+		send_dbus_dev_key_msg_rcvd(node, ele_idx, src, app_idx, net_idx,
+					   size, data);
+}
+
+static void send_fd_msg_rcvd(struct l_io *io, uint8_t ele_idx,
+			     uint16_t src, uint16_t dst,
+			     const struct mesh_virtual *virt,
+			     uint16_t app_idx,
+			     uint16_t size, const uint8_t *data)
+{
+	struct fd_msg *msg = fd_msg_new(ele_idx, src, size, data, APP_KEY_MSG);
+
+	msg->app.app_idx = app_idx;
+	msg->app.dst = dst;
+
+	if (virt)
+		memcpy(msg->app.label, virt, sizeof(msg->app.label));
+
+	fd_msg_send(io, msg, size);
+}
+
+
+static void send_dbus_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
 					uint16_t src, uint16_t dst,
 					const struct mesh_virtual *virt,
 					uint16_t app_idx,
@@ -863,6 +968,22 @@ static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
 	l_dbus_send(dbus, msg);
 }
 
+static void send_msg_rcvd(struct mesh_node *node, uint8_t ele_idx,
+			  uint16_t src, uint16_t dst,
+			  const struct mesh_virtual *virt,
+			  uint16_t app_idx,
+			  uint16_t size, const uint8_t *data)
+{
+	struct l_io *io = node_get_fd_io(node);
+
+	if (io)
+		send_fd_msg_rcvd(io, ele_idx, src, dst, virt, app_idx,
+				 size, data);
+	else
+		send_dbus_msg_rcvd(node, ele_idx, src, dst, virt, app_idx,
+				   size, data);
+}
+
 bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
 			uint32_t seq, uint32_t iv_index,
 			uint16_t net_idx, uint16_t src, uint16_t dst,
@@ -986,6 +1107,7 @@ bool mesh_model_rx(struct mesh_node *node, bool szmict, uint32_t seq0,
 				send_msg_rcvd(node, i, src, dst, decrypt_virt,
 						forward.app_idx, forward.size,
 						forward.data);
+
 			else if (decrypt_idx == APP_IDX_DEV_REMOTE ||
 				 decrypt_idx == APP_IDX_DEV_LOCAL)
 				send_dev_key_msg_rcvd(node, i, src, decrypt_idx,
diff --git a/mesh/node.c b/mesh/node.c
index 6140fdf9f..e00bed785 100644
--- a/mesh/node.c
+++ b/mesh/node.c
@@ -26,6 +26,8 @@
 #include <limits.h>
 #include <stdio.h>
 #include <sys/time.h>
+#include <sys/socket.h>
+#include <unistd.h>
 
 #include <ell/ell.h>
 
@@ -105,6 +107,7 @@ struct mesh_node {
 	uint8_t proxy;
 	uint8_t friend;
 	uint8_t beacon;
+	struct l_io *fd_io;
 };
 
 struct node_import {
@@ -260,6 +263,7 @@ static void set_defaults(struct mesh_node *node)
 							MESH_MODE_UNSUPPORTED;
 	node->ttl = TTL_MASK;
 	node->seq_number = DEFAULT_SEQUENCE_NUMBER;
+	node->fd_io = NULL;
 }
 
 static struct mesh_node *node_new(const uint8_t uuid[16])
@@ -341,6 +345,13 @@ static void free_node_resources(void *data)
 	mesh_agent_remove(node->agent);
 	mesh_config_release(node->cfg);
 	mesh_net_free(node->net);
+
+	if (node->fd_io)
+	{
+		l_io_destroy(node->fd_io);
+		node->fd_io = NULL;
+	}
+
 	l_free(node->storage_dir);
 	l_free(node);
 }
@@ -745,6 +756,11 @@ uint16_t node_get_crpl(struct mesh_node *node)
 	return node->comp.crpl;
 }
 
+struct l_io *node_get_fd_io(struct mesh_node *node)
+{
+	return node->fd_io;
+}
+
 uint8_t node_relay_mode_get(struct mesh_node *node, uint8_t *count,
 							uint16_t *interval)
 {
@@ -1650,6 +1666,53 @@ static void send_managed_objects_request(const char *destination,
 					req, l_free, DEFAULT_DBUS_TIMEOUT);
 }
 
+static void fd_io_hup(struct l_io *io, void *user_data)
+{
+	struct mesh_node *node = user_data;
+
+	node->fd_io = NULL;
+
+	l_io_destroy(io);
+}
+
+static struct l_io *fd_io_new(struct mesh_node *node, int *fd)
+{
+	struct l_io *io;
+	int fds[2];
+
+	if (socketpair(AF_LOCAL, SOCK_DGRAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
+		       0, fds) < 0)
+	{
+		return NULL;
+	}
+
+	io = l_io_new(fds[0]);
+	if (!io)
+	{
+		goto fail;
+	}
+
+	l_io_set_close_on_destroy(io, true);
+
+	if (!l_io_set_disconnect_handler(io, fd_io_hup, node, NULL))
+	{
+		goto fail;
+	}
+
+	*fd = fds[1];
+
+	return io;
+
+fail:
+	if (io)
+		l_io_destroy(io);
+
+	close(fds[0]);
+	close(fds[1]);
+
+	return NULL;
+}
+
 /* Establish relationship between application and mesh node */
 void node_attach(const char *app_root, const char *sender, uint64_t token,
 					node_ready_func_t cb, void *user_data)
@@ -1770,8 +1833,16 @@ static void build_element_config(void *a, void *b)
 	l_dbus_message_builder_leave_struct(builder);
 }
 
+static void append_fd(struct l_dbus_message_builder *builder, ...)
+{
+	va_list args;
+	va_start(args, builder);
+	l_dbus_message_builder_append_from_valist(builder, "h", args);
+	va_end(args);
+}
+
 void node_build_attach_reply(struct mesh_node *node,
-						struct l_dbus_message *reply)
+				struct l_dbus_message *reply, bool use_fd)
 {
 	struct l_dbus_message_builder *builder;
 
@@ -1784,6 +1855,16 @@ void node_build_attach_reply(struct mesh_node *node,
 	l_dbus_message_builder_enter_array(builder, "(ya(qa{sv}))");
 	l_queue_foreach(node->elements, build_element_config, builder);
 	l_dbus_message_builder_leave_array(builder);
+
+	if (use_fd)
+	{
+		int fd = -1;
+		node->fd_io = fd_io_new(node, &fd);
+		append_fd(builder, fd);
+
+		close(fd);
+	}
+
 	l_dbus_message_builder_finalize(builder);
 	l_dbus_message_builder_destroy(builder);
 }
diff --git a/mesh/node.h b/mesh/node.h
index e26d410c8..6d0696824 100644
--- a/mesh/node.h
+++ b/mesh/node.h
@@ -23,6 +23,7 @@ struct mesh_io;
 struct mesh_agent;
 struct mesh_config;
 struct mesh_config_node;
+struct l_io;
 
 typedef void (*node_ready_func_t) (void *user_data, int status,
 							struct mesh_node *node);
@@ -77,13 +78,14 @@ uint8_t node_friend_mode_get(struct mesh_node *node);
 const char *node_get_element_path(struct mesh_node *node, uint8_t ele_idx);
 const char *node_get_owner(struct mesh_node *node);
 const char *node_get_app_path(struct mesh_node *node);
+struct l_io *node_get_fd_io(struct mesh_node *node);
 bool node_add_pending_local(struct mesh_node *node, void *info);
 void node_attach_io_all(struct mesh_io *io);
 void node_attach_io(struct mesh_node *node, struct mesh_io *io);
 void node_attach(const char *app_root, const char *sender, uint64_t token,
 					node_ready_func_t cb, void *user_data);
 void node_build_attach_reply(struct mesh_node *node,
-						struct l_dbus_message *reply);
+				struct l_dbus_message *reply, bool use_fd);
 void node_create(const char *app_root, const char *sender, const uint8_t *uuid,
 					node_ready_func_t cb, void *user_data);
 void node_import(const char *app_root, const char *sender, const uint8_t *uuid,
-- 
2.20.1

Re: [RFC BlueZ 0/2] mesh: Deliver mesh packets over datagram socket

[Gix, Brian]

Hi Michał,

On Tue, 2020-06-16 at 14:27 +0200, Michał Lowas-Rzechonek wrote:
> This patchset adds another D-Bus API for attaching applications, called
> AttachFD.

We have talked a little about this, but I am personally opposed to
switching the daemon--> App message delivery system to socket based.
This has the feeling of something that has been developed as the
result of a stress test to see how much data can be pushed through
the system as fast as possible, which should not be a common mesh use
case.

I also worry about the increase in system socket resorces...
Currently the daemon consumes 1 socket for App <--> daemon
communication (the one to the dbus daemon), and each App currently
being supported uses one more (again, to dbus daemon) And all
MUXing, marshaling and unmarshaling of messages is handled by
DBUS...   the very reason for it's existance. While creating a
new socket between App and daemon would make the messages flow
faster, it comes at the cost of re-inventing MUXing, marshaling,
unmarshaling plus all the additional sockets. A larger code
footprint, and a *much* larger resource footprint.

This just feels to me like the kind of customization that is fine
for a vendor to do for a specialized high-flow mesh, but not
something that we want to impose on everyone who uses Mesh in the
open source community.

> When application uses that API, it receives one end of a datagram socket
> pair, which it's supposed to recv() from in order to receive mesh
> packets, instead of wating for *MessageReceived() calls over D-Bus.
> 
> This significantly reduces system load for high traffic environment
> (e.g. an application that subscribes to a large number of publications
> in a big network).
> 
> Message delivery is one way only: application is still supposed to call
> *Send methods via D-Bus, although the socket pair is bidirectional, so
> it would be possible to add sending as well.
> 
> Michał Lowas-Rzechonek (1):
>   mesh: Implement AttachFD method
> 
> Przemysław Fierek (1):
>   mesh: Add documentation for AttachFD
> 
>  doc/mesh-api.txt |  40 +++++++++++++++
>  mesh/mesh.c      |  12 ++++-
>  mesh/model.c     | 126 ++++++++++++++++++++++++++++++++++++++++++++++-
>  mesh/node.c      |  83 ++++++++++++++++++++++++++++++-
>  mesh/node.h      |   4 +-
>  5 files changed, 259 insertions(+), 6 deletions(-)
> 

Re: [RFC BlueZ 0/2] mesh: Deliver mesh packets over datagram socket

[michal.lowas-rzechonek]

Brian,

On 06/16, Gix, Brian wrote:
> This has the feeling of something that has been developed as the
> result of a stress test to see how much data can be pushed through
> the system as fast as possible, which should not be a common mesh use
> case.

Well, not really. We came up with this mechanism because of performance
issues with *real world* networks on embedded platforms. And by "real
world" I mean a single network of 200+ nodes.

We need a way to monitor the nodes in such networks, and potentially
much bigger ones too - mesh addess space is 32k+ nodes, and we already
have runnin installations with ~1k nodes, all of which are publishing
sensor data.

Without this, bluetooth-meshd is *unusable*.

> I also worry about the increase in system socket resorces...
> Currently the daemon consumes 1 socket for App <--> daemon
> communication (the one to the dbus daemon), and each App currently
> being supported uses one more (again, to dbus daemon) And all
> MUXing, marshaling and unmarshaling of messages is handled by
> DBUS...   the very reason for it's existance. While creating a
> new socket between App and daemon would make the messages flow
> faster, it comes at the cost of re-inventing MUXing, marshaling,
> unmarshaling plus all the additional sockets. A larger code
> footprint, and a *much* larger resource footprint.

As you can see in the patch, serialization format is very simple (*much*
simpler than D-Bus marshalling).

As for MUXing, I don't really understand what you mean? A node can have
only one application attached to it, so the daemon knows exactly which
socket to use. Same for the application.

About file descriptors: this patch targets primarily an embedded
platform. While I've seen issues with number of open files on busy
network servers, I don't think an IoT device is going to face such
problems.

As for the "resource footprint", it's a tradeoff between open files vs.
CPU. By using D-Bus calls to deliver network packets, you need to:

 - copy the packet from the mesh daemon to dbus daemon
 - have the daemon look up the application by D-Bus address
 - have the daemon deserialize the message header and check permissions
 - copy the packet from dbus daemon to application

This requires an additional copy and an additional context switch, and a
potentially costly permission check (I've mentioned AppArmor in the
previous mail).

So this delivery method in fact consumes *more* resources.

> This just feels to me like the kind of customization that is fine
> for a vendor to do for a specialized high-flow mesh, but not
> something that we want to impose on everyone who uses Mesh in the
> open source community.

I don't want to "impose" anything... the API is entirely optional and
the "regular" Attach() works as it used to, and might be perfectly fine
for simpler applications.

Note that GATT API does the same thing wrt. AquireNotify() call.

-- 
Michał Lowas-Rzechonek <michal.lowas-rzechonek@silvair.com>
Silvair http://silvair.com
Jasnogórska 44, 31-358 Krakow, POLAND