lttng-dev.lists.lttng.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH lttng-tools v2 01/20] Fix: Use platform-independant types in sessiond to consumerd communication
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 02/20] Clean-up: Switch enum fields in lttcomm_consumer_msg Yannick Lamarre
                   ` (18 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

From: Jérémie Galarneau <jeremie.galarneau@efficios.com>

The session daemon to consumer daemon is passin around the
lttcomm_consumer_msg structure which contains a relayd_sock
structure.

This structure, in turn, contains a lttcomm_relayd_sock structure which
contains an lttcomm_sock structure.

If you're still following, this lttcomm_sock structure has an
"ops" member which is a pointer. Obviously, this pointer does not have
the same size between a 64-bit session daemon and a 32-bit consumer
which causes communication issues when adding a relayd socket (the
LTTng version is erronously interpreted as 0.2).

This fix introduces "serialized" versions of the aforementioned
structures which only use fixed-size members.

Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
1) Removed mention of big endian from commit message since it's obsolete.
2) Removed mention of big endian in comments since it's obsolete.

 src/bin/lttng-sessiond/consumer.c            |   5 +-
 src/common/kernel-consumer/kernel-consumer.c |  11 +-
 src/common/sessiond-comm/sessiond-comm.c     | 186 +++++++++++++++++++++++++++
 src/common/sessiond-comm/sessiond-comm.h     | 110 +++++++++++++++-
 src/common/ust-consumer/ust-consumer.c       |  11 +-
 tests/regression/tools/live/Makefile.am      |   2 +-
 tests/unit/Makefile.am                       |   6 +-
 7 files changed, 322 insertions(+), 9 deletions(-)

diff --git a/src/bin/lttng-sessiond/consumer.c b/src/bin/lttng-sessiond/consumer.c
index 12db236d..8e917b9f 100644
--- a/src/bin/lttng-sessiond/consumer.c
+++ b/src/bin/lttng-sessiond/consumer.c
@@ -1095,7 +1095,10 @@ int consumer_send_relayd_socket(struct consumer_socket *consumer_sock,
 	msg.u.relayd_sock.net_index = consumer->net_seq_index;
 	msg.u.relayd_sock.type = type;
 	msg.u.relayd_sock.session_id = session_id;
-	memcpy(&msg.u.relayd_sock.sock, rsock, sizeof(msg.u.relayd_sock.sock));
+	ret = lttcomm_relayd_sock_serialize(&msg.u.relayd_sock.sock, rsock);
+	if (ret) {
+		goto error;
+	}
 
 	DBG3("Sending relayd sock info to consumer on %d", *consumer_sock->fd_ptr);
 	ret = consumer_send_msg(consumer_sock, &msg);
diff --git a/src/common/kernel-consumer/kernel-consumer.c b/src/common/kernel-consumer/kernel-consumer.c
index 627cd2a8..16e3b588 100644
--- a/src/common/kernel-consumer/kernel-consumer.c
+++ b/src/common/kernel-consumer/kernel-consumer.c
@@ -454,10 +454,19 @@ int lttng_kconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
 	switch (msg.cmd_type) {
 	case LTTNG_CONSUMER_ADD_RELAYD_SOCKET:
 	{
+		struct lttcomm_relayd_sock relayd_sock;
+
+		ret = lttcomm_relayd_sock_deserialize(&relayd_sock,
+				&msg.u.relayd_sock.sock);
+		if (ret) {
+			/* Received an invalid relayd_sock. */
+			goto error_fatal;
+		}
+
 		/* Session daemon status message are handled in the following call. */
 		consumer_add_relayd_socket(msg.u.relayd_sock.net_index,
 				msg.u.relayd_sock.type, ctx, sock, consumer_sockpoll,
-				&msg.u.relayd_sock.sock, msg.u.relayd_sock.session_id,
+				&relayd_sock, msg.u.relayd_sock.session_id,
 				msg.u.relayd_sock.relayd_session_id);
 		goto end_nosignal;
 	}
diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c
index 0067be16..8aee3cd1 100644
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -27,6 +27,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <endian.h>
 
 #include <common/common.h>
 
@@ -75,6 +76,191 @@ static const char *lttcomm_readable_code[] = {
 
 static unsigned long network_timeout;
 
+LTTNG_HIDDEN
+int sockaddr_in_serialize(struct sockaddr_in_serialized *dst,
+		const struct sockaddr_in *src)
+{
+	assert(src && dst);
+	dst->sin_family = (uint32_t) src->sin_family;
+	dst->sin_port = (uint16_t) src->sin_port;
+	dst->sin_addr.s_addr = src->sin_addr.s_addr;
+	return 0;
+}
+
+LTTNG_HIDDEN
+int sockaddr_in_deserialize(struct sockaddr_in *dst,
+		const struct sockaddr_in_serialized *src)
+{
+	assert(src && dst);
+	dst->sin_family = (sa_family_t) src->sin_family;
+	dst->sin_port = (in_port_t) src->sin_port;
+	dst->sin_addr.s_addr = src->sin_addr.s_addr;
+	return 0;
+}
+
+LTTNG_HIDDEN
+int sockaddr_in6_serialize(struct sockaddr_in6_serialized *dst,
+		const struct sockaddr_in6 *src)
+{
+	assert(src && dst);
+
+	dst->sin6_family = (uint32_t) src->sin6_family;
+	dst->sin6_port = (uint16_t) src->sin6_port;
+	dst->sin6_flowinfo = src->sin6_flowinfo;
+	memcpy(&dst->sin6_addr._s6_addr, src->sin6_addr.s6_addr,
+			sizeof(dst->sin6_addr._s6_addr));
+	dst->sin6_scope_id = src->sin6_scope_id;
+	return 0;
+}
+
+LTTNG_HIDDEN
+int sockaddr_in6_deserialize(struct sockaddr_in6 *dst,
+		const struct sockaddr_in6_serialized *src)
+{
+	assert(src && dst);
+
+	dst->sin6_family = (sa_family_t) src->sin6_family;
+	dst->sin6_port = (in_port_t) src->sin6_port;
+	dst->sin6_flowinfo = src->sin6_flowinfo;
+	memcpy(&dst->sin6_addr.s6_addr, src->sin6_addr._s6_addr,
+	       sizeof(dst->sin6_addr.s6_addr));
+	dst->sin6_scope_id = src->sin6_scope_id;
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sockaddr_serialize(struct lttcomm_sockaddr_serialized *dst,
+		const struct lttcomm_sockaddr *src)
+{
+	int ret = 0;
+
+	assert(src && dst);
+
+	dst->type = (uint32_t) src->type;
+
+	switch (src->type) {
+	case LTTCOMM_INET:
+	{
+		sockaddr_in_serialize(&dst->addr.sin,
+				&src->addr.sin);
+		break;
+	}
+	case LTTCOMM_INET6:
+	{
+		sockaddr_in6_serialize(&dst->addr.sin6,
+				&src->addr.sin6);
+		break;
+	}
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sockaddr_deserialize(struct lttcomm_sockaddr *dst,
+		const struct lttcomm_sockaddr_serialized *src)
+{
+	int ret = 0;
+
+	assert(src && dst);
+
+	dst->type = (enum lttcomm_sock_domain) src->type;
+
+	switch (dst->type) {
+	case LTTCOMM_INET:
+	{
+		sockaddr_in_deserialize(&dst->addr.sin,
+				&src->addr.sin);
+		break;
+	}
+	case LTTCOMM_INET6:
+	{
+		sockaddr_in6_deserialize(&dst->addr.sin6,
+				&src->addr.sin6);
+		break;
+	}
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sock_serialize(struct lttcomm_sock_serialized *dst,
+		const struct lttcomm_sock *src)
+{
+	int ret;
+
+	assert(src && dst);
+
+	dst->fd = src->fd;
+	if (src->proto != LTTCOMM_SOCK_UDP &&
+		src->proto != LTTCOMM_SOCK_TCP) {
+		/* Code flow error. */
+		assert(0);
+	}
+	dst->proto = (uint32_t) src->proto;
+	ret = lttcomm_sockaddr_serialize(&dst->sockaddr, &src->sockaddr);
+
+	return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_sock_deserialize(struct lttcomm_sock *dst,
+		const struct lttcomm_sock_serialized *src)
+{
+	int ret;
+
+	assert(src && dst);
+
+	dst->fd = src->fd;
+	dst->proto = (enum lttcomm_sock_proto) src->proto;
+	if (dst->proto != LTTCOMM_SOCK_UDP &&
+		dst->proto != LTTCOMM_SOCK_TCP) {
+		ret = -EINVAL;
+		goto end;
+	}
+	dst->ops = NULL;
+	ret = lttcomm_sockaddr_deserialize(&dst->sockaddr, &src->sockaddr);
+
+end:
+	return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_relayd_sock_serialize(struct lttcomm_relayd_sock_serialized *dst,
+		const struct lttcomm_relayd_sock *src)
+{
+	int ret;
+
+	assert(src && dst);
+	dst->major = src->major;
+	dst->minor = src->minor;
+	ret = lttcomm_sock_serialize(&dst->sock, &src->sock);
+
+	return ret;
+}
+
+LTTNG_HIDDEN
+int lttcomm_relayd_sock_deserialize(
+		struct lttcomm_relayd_sock *dst,
+		const struct lttcomm_relayd_sock_serialized *src)
+{
+	int ret;
+
+	assert(src && dst);
+	dst->major = src->major;
+	dst->minor = src->minor;
+	ret = lttcomm_sock_deserialize(&dst->sock, &src->sock);
+
+	return ret;
+}
+
 /*
  * Return ptr to string representing a human readable error code from the
  * lttcomm_return_code enum.
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 836c3af6..3c815dea 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -2,6 +2,7 @@
  * Copyright (C) 2011 - David Goulet <david.goulet@polymtl.ca>
  *                      Julien Desfossez <julien.desfossez@polymtl.ca>
  *                      Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
+ *                      Jérémie Galarneau <jeremie.galarneau@efficios.com>
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License, version 2 only,
@@ -210,31 +211,136 @@ struct lttcomm_metadata_request_msg {
 	uint64_t key; /* Metadata channel key. */
 } LTTNG_PACKED;
 
+/* For internal use only */
 struct lttcomm_sockaddr {
 	enum lttcomm_sock_domain type;
 	union {
 		struct sockaddr_in sin;
 		struct sockaddr_in6 sin6;
 	} addr;
+};
+
+/*
+ * Serialized version of the sockaddr_in system structure (may only be used
+ * for communication).
+ * Fields are fixed-size and packed.
+ */
+struct sockaddr_in_serialized {
+	uint32_t sin_family;
+	uint16_t sin_port;
+	struct in_addr_serialized {
+		uint32_t s_addr;
+	} LTTNG_PACKED sin_addr;
+} LTTNG_PACKED;
+
+extern
+int sockaddr_in_serialize(struct sockaddr_in_serialized *dst,
+		const struct sockaddr_in *src);
+extern
+int sockaddr_in_deserialize(struct sockaddr_in *dst,
+		const struct sockaddr_in_serialized *src);
+
+/*
+ * Serialized version of the sockaddr_in6 system structure (may only be used
+ * for communication).
+ * Fields are fixed-size and packed.
+ */
+struct sockaddr_in6_serialized {
+	uint32_t sin6_family;
+	uint16_t sin6_port;
+	uint32_t sin6_flowinfo;
+	struct in6_addr_serialized {
+		/*
+		 * Prefixing with "_" since s6_addr is a "DEFINE"
+		 * which clashes with this.
+		 */
+		uint8_t _s6_addr[16];
+	} LTTNG_PACKED sin6_addr;
+	uint32_t sin6_scope_id;
+} LTTNG_PACKED;
+
+extern
+int sockaddr_in6_serialize(struct sockaddr_in6_serialized *dst,
+		const struct sockaddr_in6 *src);
+extern
+int sockaddr_in6_deserialize(struct sockaddr_in6 *dst,
+		const struct sockaddr_in6_serialized *src);
+
+/*
+ * Serialized version of struct lttcomm_sockaddr (may be used for
+ * communication only).
+ * The struct and its members are packed and its fields are fixed-size.
+ */
+struct lttcomm_sockaddr_serialized {
+	uint32_t type; /* Maps to enum lttcomm_sock_domain */
+	union {
+		struct sockaddr_in_serialized sin;
+		struct sockaddr_in6_serialized sin6;
+	} addr;
 } LTTNG_PACKED;
 
+extern
+int sockaddr_in6_serialize(struct sockaddr_in6_serialized *dst,
+		const struct sockaddr_in6 *src);
+extern
+int sockaddr_in6_deserialize(struct sockaddr_in6 *dst,
+		const struct sockaddr_in6_serialized *src);
+
+/* For internal use only */
 struct lttcomm_sock {
 	int32_t fd;
 	enum lttcomm_sock_proto proto;
 	struct lttcomm_sockaddr sockaddr;
 	const struct lttcomm_proto_ops *ops;
+};
+
+/*
+ * Serialized version of struct lttcomm_sock (may be used for
+ * communication only).
+ * Fields are fixed-size. Structure is packed.
+ */
+struct lttcomm_sock_serialized {
+	int32_t fd;
+	uint32_t proto; /* Maps to enum lttcomm_sock_proto */
+	struct lttcomm_sockaddr_serialized sockaddr;
 } LTTNG_PACKED;
 
+extern
+int lttcomm_sock_serialize(struct lttcomm_sock_serialized *dst,
+		const struct lttcomm_sock *src);
+extern
+int lttcomm_sock_deserialize(struct lttcomm_sock *dst,
+		const struct lttcomm_sock_serialized *src);
+
 /*
  * Relayd sock. Adds the protocol version to use for the communications with
- * the relayd.
+ * the relayd. Internal use only.
  */
 struct lttcomm_relayd_sock {
 	struct lttcomm_sock sock;
 	uint32_t major;
 	uint32_t minor;
+};
+
+/*
+ * Serialized version of struct lttcomm_relayd_sock (may be used for
+ * communications only).
+ * Fields are fixed-size. Structure is packed.
+ */
+struct lttcomm_relayd_sock_serialized {
+	struct lttcomm_sock_serialized sock;
+	uint32_t major;
+	uint32_t minor;
 } LTTNG_PACKED;
 
+extern
+int lttcomm_relayd_sock_serialize(struct lttcomm_relayd_sock_serialized *dst,
+		const struct lttcomm_relayd_sock *src);
+extern
+int lttcomm_relayd_sock_deserialize(
+		struct lttcomm_relayd_sock *dst,
+		const struct lttcomm_relayd_sock_serialized *src);
+
 struct lttcomm_net_family {
 	int family;
 	int (*create) (struct lttcomm_sock *sock, int type, int proto);
@@ -499,7 +605,7 @@ struct lttcomm_consumer_msg {
 			uint64_t net_index;
 			enum lttng_stream_type type;
 			/* Open socket to the relayd */
-			struct lttcomm_relayd_sock sock;
+			struct lttcomm_relayd_sock_serialized sock;
 			/* Tracing session id associated to the relayd. */
 			uint64_t session_id;
 			/* Relayd session id, only used with control socket. */
diff --git a/src/common/ust-consumer/ust-consumer.c b/src/common/ust-consumer/ust-consumer.c
index 94b761cb..06f858c3 100644
--- a/src/common/ust-consumer/ust-consumer.c
+++ b/src/common/ust-consumer/ust-consumer.c
@@ -1354,10 +1354,19 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx,
 	switch (msg.cmd_type) {
 	case LTTNG_CONSUMER_ADD_RELAYD_SOCKET:
 	{
+		struct lttcomm_relayd_sock relayd_sock;
+
+		ret = lttcomm_relayd_sock_deserialize(&relayd_sock,
+				&msg.u.relayd_sock.sock);
+		if (ret) {
+			/* Received an invalid relayd_sock. */
+			goto error_fatal;
+		}
+
 		/* Session daemon status message are handled in the following call. */
 		consumer_add_relayd_socket(msg.u.relayd_sock.net_index,
 				msg.u.relayd_sock.type, ctx, sock, consumer_sockpoll,
-				&msg.u.relayd_sock.sock, msg.u.relayd_sock.session_id,
+				&relayd_sock, msg.u.relayd_sock.session_id,
 				msg.u.relayd_sock.relayd_session_id);
 		goto end_nosignal;
 	}
diff --git a/tests/regression/tools/live/Makefile.am b/tests/regression/tools/live/Makefile.am
index 7a26b42c..857463e6 100644
--- a/tests/regression/tools/live/Makefile.am
+++ b/tests/regression/tools/live/Makefile.am
@@ -20,7 +20,7 @@ EXTRA_DIST += test_ust test_ust_tracefile_count test_lttng_ust
 endif
 
 live_test_SOURCES = live_test.c
-live_test_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+live_test_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
 		$(LIBHASHTABLE) $(LIBHEALTH) $(DL_LIBS) -lrt
 live_test_LDADD += $(LIVE) \
 		$(top_builddir)/src/lib/lttng-ctl/liblttng-ctl.la
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 5f485f00..26989fa0 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -88,7 +88,7 @@ SESSIOND_OBJS += $(top_builddir)/src/bin/lttng-sessiond/trace-ust.$(OBJEXT) \
 endif
 
 test_session_SOURCES = test_session.c
-test_session_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+test_session_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
 		     $(LIBHASHTABLE) $(DL_LIBS) -lrt -lurcu-common -lurcu \
 		     $(KMOD_LIBS) \
 		     $(top_builddir)/src/lib/lttng-ctl/liblttng-ctl.la \
@@ -108,7 +108,7 @@ endif
 # UST data structures unit test
 if HAVE_LIBLTTNG_UST_CTL
 test_ust_data_SOURCES = test_ust_data.c
-test_ust_data_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+test_ust_data_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
 		      $(LIBHASHTABLE) $(DL_LIBS) -lrt -lurcu-common -lurcu \
 		      -llttng-ust-ctl \
 		      $(KMOD_LIBS) \
@@ -131,7 +131,7 @@ KERN_DATA_TRACE=$(top_builddir)/src/bin/lttng-sessiond/trace-kernel.$(OBJEXT) \
 		$(LIBLTTNG_CTL)
 
 test_kernel_data_SOURCES = test_kernel_data.c
-test_kernel_data_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBRELAYD) $(LIBSESSIOND_COMM) \
+test_kernel_data_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBRELAYD) \
 			 $(LIBHASHTABLE) $(DL_LIBS) -lrt
 test_kernel_data_LDADD += $(KERN_DATA_TRACE)
 
-- 
2.11.0

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 02/20] Clean-up: Switch enum fields in lttcomm_consumer_msg
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 01/20] Fix: Use platform-independant types in sessiond to consumerd communication Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 03/20] Clean-up: Switch enum fields in lttcomm_consumer_status_msg Yannick Lamarre
                   ` (17 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Size of an enum type is not guaranteed by the C spec, which is why the
field's tpye is replaced to uin32_t.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
 src/common/sessiond-comm/sessiond-comm.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 3c815dea..47ade871 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -576,7 +576,7 @@ struct lttcomm_consumer_msg {
 			uint32_t nb_init_streams;
 			char name[LTTNG_SYMBOL_NAME_LEN];
 			/* Use splice or mmap to consume this fd */
-			enum lttng_event_output output;
+			uint32_t output; /* enum lttng_event_output */
 			int type; /* Per cpu or metadata. */
 			uint64_t tracefile_size; /* bytes */
 			uint32_t tracefile_count; /* number of tracefiles */
@@ -603,7 +603,7 @@ struct lttcomm_consumer_msg {
 		} LTTNG_PACKED stream;	/* Only used by Kernel. */
 		struct {
 			uint64_t net_index;
-			enum lttng_stream_type type;
+			uint32_t type; /* enum lttng_stream_type */
 			/* Open socket to the relayd */
 			struct lttcomm_relayd_sock_serialized sock;
 			/* Tracing session id associated to the relayd. */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 03/20] Clean-up: Switch enum fields in lttcomm_consumer_status_msg
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 01/20] Fix: Use platform-independant types in sessiond to consumerd communication Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 02/20] Clean-up: Switch enum fields in lttcomm_consumer_msg Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 04/20] Clean-up: Switch enum fields in lttcomm_consumer_status_channel Yannick Lamarre
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Size of an enum type is not guaranteed by the C spec, which is why the
field's tpye is replaced to uin32_t.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
 src/common/sessiond-comm/sessiond-comm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 47ade871..c6ec8eed 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -768,7 +768,7 @@ struct lttcomm_consumer_channel_monitor_msg {
  * Status message returned to the sessiond after a received command.
  */
 struct lttcomm_consumer_status_msg {
-	enum lttcomm_return_code ret_code;
+	uint32_t ret_code; /* enum lttcomm_return_code */
 } LTTNG_PACKED;
 
 struct lttcomm_consumer_status_channel {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 04/20] Clean-up: Switch enum fields in lttcomm_consumer_status_channel
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (2 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 03/20] Clean-up: Switch enum fields in lttcomm_consumer_status_msg Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 05/20] Clean-up: Remove unused structure Yannick Lamarre
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Size of an enum type is not guaranteed by the C spec, which is why the
field's tpye is replaced to uin32_t.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
 src/common/sessiond-comm/sessiond-comm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index c6ec8eed..84d87808 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -772,7 +772,7 @@ struct lttcomm_consumer_status_msg {
 } LTTNG_PACKED;
 
 struct lttcomm_consumer_status_channel {
-	enum lttcomm_return_code ret_code;
+	uint32_t ret_code; /* enum lttcomm_return_code */
 	uint64_t key;
 	unsigned int stream_count;
 } LTTNG_PACKED;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 05/20] Clean-up: Remove unused structure
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (3 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 04/20] Clean-up: Switch enum fields in lttcomm_consumer_status_channel Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 06/20] Clean-up: Removed deprecated field Yannick Lamarre
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
 src/common/sessiond-comm/sessiond-comm.h | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 84d87808..f9f6f97b 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -782,21 +782,6 @@ struct lttcomm_consumer_status_channel {
 #include <lttng/ust-abi.h>
 
 /*
- * Data structure for the commands sent from sessiond to UST.
- */
-struct lttcomm_ust_msg {
-	uint32_t handle;
-	uint32_t cmd;
-	union {
-		struct lttng_ust_channel channel;
-		struct lttng_ust_stream stream;
-		struct lttng_ust_event event;
-		struct lttng_ust_context context;
-		struct lttng_ust_tracer_version version;
-	} u;
-} LTTNG_PACKED;
-
-/*
  * Data structure for the response from UST to the session daemon.
  * cmd_type is sent back in the reply for validation.
  */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 06/20] Clean-up: Removed deprecated field
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (4 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 05/20] Clean-up: Remove unused structure Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 07/20] Document inter-proccesses communication protocols requirements Yannick Lamarre
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

The command using this field was dropped in 2.9, but the field was
left in the union.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
 src/common/sessiond-comm/sessiond-comm.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index f9f6f97b..ac59d770 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -421,7 +421,6 @@ struct lttcomm_session_msg {
 		struct {
 			char channel_name[LTTNG_SYMBOL_NAME_LEN];
 		} LTTNG_PACKED list;
-		struct lttng_calibrate calibrate;
 		/* Used by the set_consumer_url and used by create_session also call */
 		struct {
 			/* Number of lttng_uri following */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 07/20] Document inter-proccesses communication protocols requirements
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (5 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 06/20] Clean-up: Removed deprecated field Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 08/20] Add serialized versions of lttng_channel structs Yannick Lamarre
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

The introduction of serialization changes previously established
requirements for lttng-tools.
This document drafts the guidelines to follow in future communication
protocol development.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Unsure how this document should be structured and what specific information
should be kept here or even if it's the right place.
It might be interesting to move this to lttng-doc since it is likely to also
document communication protocols between consumers (ust and kernel modules)
and the sessiond.

 doc/comm-requirements.txt | 9 +++++++++
 1 file changed, 9 insertions(+)
 create mode 100644 doc/comm-requirements.txt

diff --git a/doc/comm-requirements.txt b/doc/comm-requirements.txt
new file mode 100644
index 00000000..7f62260d
--- /dev/null
+++ b/doc/comm-requirements.txt
@@ -0,0 +1,9 @@
+LTTng Communication Protocols' Requirements
+Yannick Lamarre, April 2019
+
+This document describes the requirements for the development of communication
+protocols.
+The integration of serialization officially breaks from the implicit
+requirement to keep backward compatibility by removing padding fields from
+serialized structures. This forces users to upgrade in a lock-step fashion.
+
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 08/20] Add serialized versions of lttng_channel structs
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (6 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 07/20] Document inter-proccesses communication protocols requirements Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 09/20] Add serdes functions for lttng_channel Yannick Lamarre
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Serialized versions of lttng_channel and lttng_channel_extended are
packed structures to be used in communication protocols for consistent
sizes across platforms. The serialized versions are stripped of pointers
and padding.

Pointers are removed since their size can vary across processes on
platforms supporting multiple pointer sizes (32bits vs 64bits) and this
creates mismatches in structure sizes.
Padding is also removed since it defeats the purpose of a packed struct
and is not used to extend the protocols.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Reworded commit message to better justify removal of pointer and padding
fields.

 include/lttng/channel-internal.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/lttng/channel-internal.h b/include/lttng/channel-internal.h
index 030b4701..c956c19d 100644
--- a/include/lttng/channel-internal.h
+++ b/include/lttng/channel-internal.h
@@ -20,11 +20,41 @@
 
 #include <common/macros.h>
 
+struct lttng_channel_attr_serialized {
+	int overwrite;                      /* -1: session default, 1: overwrite, 0: discard */
+	uint64_t subbuf_size;               /* bytes, power of 2 */
+	uint64_t num_subbuf;                /* power of 2 */
+	unsigned int switch_timer_interval; /* usec */
+	unsigned int read_timer_interval;   /* usec */
+	uint32_t output; /* enum lttng_event_output */
+	/* LTTng 2.1 padding limit */
+	uint64_t tracefile_size;            /* bytes */
+	uint64_t tracefile_count;           /* number of tracefiles */
+	/* LTTng 2.3 padding limit */
+	unsigned int live_timer_interval;   /* usec */
+	/* LTTng 2.7 padding limit */
+
+} LTTNG_PACKED;
+
+struct lttng_channel_serialized {
+	char name[LTTNG_SYMBOL_NAME_LEN];
+	uint32_t enabled;
+	struct lttng_channel_attr_serialized attr;
+
+} LTTNG_PACKED;
+
 struct lttng_channel_extended {
 	uint64_t discarded_events;
 	uint64_t lost_packets;
 	uint64_t monitor_timer_interval;
 	int64_t blocking_timeout;
+};
+
+struct lttng_channel_extended_serialized {
+	uint64_t discarded_events;
+	uint64_t lost_packets;
+	uint64_t monitor_timer_interval;
+	int64_t blocking_timeout;
 } LTTNG_PACKED;
 
 #endif /* LTTNG_CHANNEL_INTERNAL_H */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 09/20] Add serdes functions for lttng_channel
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (7 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 08/20] Add serialized versions of lttng_channel structs Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 10/20] Integrate serialized communication " Yannick Lamarre
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Since those structs are only transferred across unix sockets, endianness
is kept in host order.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
1) Fixed spelling errors
2) Added missing break
   (This code was copied from XXX hence it would be missing from there too)
3) Fixed coding style with declaration (moved before asserts) and
   initialisation (moved after assert)

 include/lttng/channel-internal.h         |   5 ++
 src/common/sessiond-comm/sessiond-comm.c | 114 +++++++++++++++++++++++++++++++
 2 files changed, 119 insertions(+)

diff --git a/include/lttng/channel-internal.h b/include/lttng/channel-internal.h
index c956c19d..db963f13 100644
--- a/include/lttng/channel-internal.h
+++ b/include/lttng/channel-internal.h
@@ -57,4 +57,9 @@ struct lttng_channel_extended_serialized {
 	int64_t blocking_timeout;
 } LTTNG_PACKED;
 
+int lttng_channel_serialize(struct lttng_channel_serialized *dst, const struct lttng_channel *src);
+int lttng_channel_deserialize(struct lttng_channel *dst, const struct lttng_channel_serialized *src);
+int lttng_channel_extended_deserialize(struct lttng_channel_extended *dst, const struct lttng_channel_extended_serialized *src);
+int lttng_channel_extended_serialize(struct lttng_channel_extended_serialized *dst, const struct lttng_channel_extended *src);
+int init_serialized_extended_channel(struct lttng_domain *domain, struct lttng_channel_extended_serialized *extended);
 #endif /* LTTNG_CHANNEL_INTERNAL_H */
diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c
index 8aee3cd1..f1f0ebdf 100644
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -76,6 +76,120 @@ static const char *lttcomm_readable_code[] = {
 
 static unsigned long network_timeout;
 
+int init_serialized_extended_channel(struct lttng_domain *domain, struct
+		lttng_channel_extended_serialized *extended)
+{
+	assert(domain && extended);
+	switch (domain->type) {
+	case LTTNG_DOMAIN_KERNEL:
+		extended->monitor_timer_interval =
+			DEFAULT_KERNEL_CHANNEL_MONITOR_TIMER;
+		extended->blocking_timeout =
+			DEFAULT_KERNEL_CHANNEL_BLOCKING_TIMEOUT;
+		break;
+	case LTTNG_DOMAIN_UST:
+		switch (domain->buf_type) {
+		case LTTNG_BUFFER_PER_UID:
+			extended->monitor_timer_interval =
+				DEFAULT_UST_UID_CHANNEL_MONITOR_TIMER;
+			extended->blocking_timeout =
+				DEFAULT_UST_UID_CHANNEL_BLOCKING_TIMEOUT;
+			break;
+		case LTTNG_BUFFER_PER_PID:
+		default:
+			extended->monitor_timer_interval =
+				DEFAULT_UST_PID_CHANNEL_MONITOR_TIMER;
+			extended->blocking_timeout =
+				DEFAULT_UST_PID_CHANNEL_BLOCKING_TIMEOUT;
+			break;
+		}
+		break;
+	default:
+		/* Default behavior: set to 0. */
+		extended->monitor_timer_interval = 0;
+		extended->blocking_timeout = 0;
+		break;
+	}
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_channel_extended_serialize(struct lttng_channel_extended_serialized *dst,
+		const struct lttng_channel_extended *src)
+{
+	assert(src && dst);
+	dst->discarded_events = src->discarded_events;
+	dst->lost_packets = src->lost_packets;
+	dst->monitor_timer_interval = src->monitor_timer_interval;
+	dst->blocking_timeout = src->blocking_timeout;
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_channel_extended_deserialize(struct lttng_channel_extended *dst,
+		const struct lttng_channel_extended_serialized *src)
+{
+	assert(src && dst);
+	dst->discarded_events = src->discarded_events;
+	dst->lost_packets = src->lost_packets;
+	dst->monitor_timer_interval = src->monitor_timer_interval;
+	dst->blocking_timeout = src->blocking_timeout;
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_channel_serialize(struct lttng_channel_serialized *dst,
+		const struct lttng_channel *src)
+{
+	struct lttng_channel_attr_serialized *dst_attr;
+	const struct lttng_channel_attr *src_attr;
+
+	assert(src && dst);
+	dst_attr = &dst->attr;
+	src_attr = &src->attr;
+
+	dst_attr->overwrite = src_attr->overwrite;
+	dst_attr->subbuf_size = src_attr->subbuf_size;
+	dst_attr->num_subbuf = src_attr->num_subbuf;
+	dst_attr->switch_timer_interval = src_attr->switch_timer_interval;
+	dst_attr->read_timer_interval = src_attr->read_timer_interval;
+	dst_attr->output = (uint32_t) src_attr->output;
+	dst_attr->tracefile_size = src_attr->tracefile_size;
+	dst_attr->tracefile_count = src_attr->tracefile_count;
+	dst_attr->live_timer_interval = src_attr->live_timer_interval;
+
+	dst->enabled = src->enabled;
+	memcpy(dst->name, src->name, sizeof(dst->name));
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_channel_deserialize(struct lttng_channel *dst,
+		const struct lttng_channel_serialized *src)
+{
+	struct lttng_channel_attr *dst_attr;
+	const struct lttng_channel_attr_serialized *src_attr;
+
+	assert(src && dst);
+	dst_attr = &dst->attr;
+	src_attr = &src->attr;
+
+	dst_attr->overwrite = src_attr->overwrite;
+	dst_attr->subbuf_size = src_attr->subbuf_size;
+	dst_attr->num_subbuf = src_attr->num_subbuf;
+	dst_attr->switch_timer_interval = src_attr->switch_timer_interval;
+	dst_attr->read_timer_interval = src_attr->read_timer_interval;
+	dst_attr->output = (enum lttng_event_output) src_attr->output;
+	dst_attr->tracefile_size = src_attr->tracefile_size;
+	dst_attr->tracefile_count = src_attr->tracefile_count;
+	dst_attr->live_timer_interval = src_attr->live_timer_interval;
+
+	dst->enabled = src->enabled;
+	memcpy(dst->name, src->name, sizeof(dst->name));
+	return 0;
+}
+
 LTTNG_HIDDEN
 int sockaddr_in_serialize(struct sockaddr_in_serialized *dst,
 		const struct sockaddr_in *src)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 10/20] Integrate serialized communication for lttng_channel
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (8 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 09/20] Add serdes functions for lttng_channel Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 11/20] Add serialized versions of lttng_event_context structs Yannick Lamarre
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

lttng-ctl and lttng-sessiond use serialized communication for
messages using lttng_channel.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Reworded commit title to better identify changes in patch.

 src/bin/lttng-sessiond/client.c          | 10 +++++++---
 src/common/sessiond-comm/sessiond-comm.h |  4 ++--
 src/lib/lttng-ctl/lttng-ctl.c            | 24 ++++--------------------
 3 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
index f3002f91..614f19c7 100644
--- a/src/bin/lttng-sessiond/client.c
+++ b/src/bin/lttng-sessiond/client.c
@@ -1184,10 +1184,14 @@ error_add_context:
 	}
 	case LTTNG_ENABLE_CHANNEL:
 	{
-		cmd_ctx->lsm->u.channel.chan.attr.extended.ptr =
-				(struct lttng_channel_extended *) &cmd_ctx->lsm->u.channel.extended;
+		struct lttng_channel channel;
+		struct lttng_channel_extended extended;
+
+		lttng_channel_extended_deserialize(&extended, &cmd_ctx->lsm->u.channel.extended);
+		lttng_channel_deserialize(&channel, &cmd_ctx->lsm->u.channel.chan);
+		channel.attr.extended.ptr = &extended;
 		ret = cmd_enable_channel(cmd_ctx->session, &cmd_ctx->lsm->domain,
-				&cmd_ctx->lsm->u.channel.chan,
+				&channel,
 				kernel_poll_pipe[1]);
 		break;
 	}
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index ac59d770..f87b9098 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -402,9 +402,9 @@ struct lttcomm_session_msg {
 		} LTTNG_PACKED disable;
 		/* Create channel */
 		struct {
-			struct lttng_channel chan LTTNG_PACKED;
+			struct lttng_channel_serialized chan;
 			/* struct lttng_channel_extended is already packed. */
-			struct lttng_channel_extended extended;
+			struct lttng_channel_extended_serialized extended;
 		} LTTNG_PACKED channel;
 		/* Context */
 		struct {
diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index f50ca306..15f66034 100644
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -1525,34 +1525,18 @@ int lttng_enable_channel(struct lttng_handle *handle,
 	}
 
 	memset(&lsm, 0, sizeof(lsm));
-	memcpy(&lsm.u.channel.chan, in_chan, sizeof(lsm.u.channel.chan));
-	lsm.u.channel.chan.attr.extended.ptr = NULL;
-
 	if (!in_chan->attr.extended.ptr) {
-		struct lttng_channel *channel;
-		struct lttng_channel_extended *extended;
-
-		channel = lttng_channel_create(&handle->domain);
-		if (!channel) {
-			return -LTTNG_ERR_NOMEM;
-		}
-
-		/*
-		 * Create a new channel in order to use default extended
-		 * attribute values.
-		 */
-		extended = (struct lttng_channel_extended *)
-				channel->attr.extended.ptr;
-		memcpy(&lsm.u.channel.extended, extended, sizeof(*extended));
-		lttng_channel_destroy(channel);
+		init_serialized_extended_channel(&handle->domain, &lsm.u.channel.extended);
 	} else {
 		struct lttng_channel_extended *extended;
 
 		extended = (struct lttng_channel_extended *)
 				in_chan->attr.extended.ptr;
-		memcpy(&lsm.u.channel.extended, extended, sizeof(*extended));
+		lttng_channel_extended_serialize(&lsm.u.channel.extended, extended);
 	}
 
+	lttng_channel_serialize(&lsm.u.channel.chan, in_chan);
+
 	/*
 	 * Verify that the amount of memory required to create the requested
 	 * buffer is available on the system at the moment.
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 11/20] Add serialized versions of lttng_event_context structs
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (9 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 10/20] Integrate serialized communication " Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 12/20] Add serdes functions for lttng_event_context Yannick Lamarre
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Serialized versions of lttng_event_context and lttng_event_perf_counter_ctx are
packed structures to be used in communication protocols for consistent
sizes across platforms. The serialized versions are stripped of pointers
and padding.

Pointers are removed since their size can vary across processes on
platforms supporting multiple pointer sizes (32bits vs 64bits) and this
creates mismatches in structure sizes.
Padding is also removed since it defeats the purpose of a packed struct
and is not used to extend the protocols.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Reworded commit message to better justify removal of pointer and padding
fields.

 include/lttng/event-internal.h | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/include/lttng/event-internal.h b/include/lttng/event-internal.h
index f8130e3b..25a88448 100644
--- a/include/lttng/event-internal.h
+++ b/include/lttng/event-internal.h
@@ -27,6 +27,17 @@
 
 struct lttng_userspace_probe_location;
 
+struct lttng_event_perf_counter_ctx_serialized {
+	uint32_t type;
+	uint64_t config;
+	char name[LTTNG_SYMBOL_NAME_LEN];
+} LTTNG_PACKED;
+
+struct lttng_event_context_serialized {
+	uint32_t ctx; /* enum lttng_event_context_type */
+	struct lttng_event_perf_counter_ctx_serialized perf_counter;
+} LTTNG_PACKED;
+
 struct lttng_event_extended {
 	/*
 	 * exclusions and filter_expression are only set when the lttng_event
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 12/20] Add serdes functions for lttng_event_context
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (10 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 11/20] Add serialized versions of lttng_event_context structs Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 13/20] Integrate serialized communication " Yannick Lamarre
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Since those structs are only transferred across unix sockets, endianness
is kept in host order.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Fixed spelling errors

 src/common/sessiond-comm/sessiond-comm.c | 42 ++++++++++++++++++++++++++++++++
 src/common/sessiond-comm/sessiond-comm.h |  1 +
 2 files changed, 43 insertions(+)

diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c
index f1f0ebdf..26b2d66d 100644
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -76,6 +76,48 @@ static const char *lttcomm_readable_code[] = {
 
 static unsigned long network_timeout;
 
+LTTNG_HIDDEN
+int lttng_event_perf_counter_ctx_serialize(struct lttng_event_perf_counter_ctx_serialized *dst,
+		const struct lttng_event_perf_counter_ctx *src)
+{
+	assert(src && dst);
+	dst->type = src->type;
+	dst->config = src->config;
+	memcpy(&dst->name, &src->name, LTTNG_SYMBOL_NAME_LEN);
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_perf_counter_ctx_deserialize(struct lttng_event_perf_counter_ctx *dst,
+		const struct lttng_event_perf_counter_ctx_serialized *src)
+{
+	assert(src && dst);
+	dst->type = src->type;
+	dst->config = src->config;
+	memcpy(&dst->name, &src->name, LTTNG_SYMBOL_NAME_LEN);
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_context_serialize(struct lttng_event_context_serialized *dst,
+		const struct lttng_event_context *src)
+{
+	assert(src && dst);
+	dst->ctx = (uint32_t) src->ctx;
+	lttng_event_perf_counter_ctx_serialize(&dst->perf_counter, &src->u.perf_counter);
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_context_deserialize(struct lttng_event_context *dst,
+		const struct lttng_event_context_serialized *src)
+{
+	assert(src && dst);
+	dst->ctx = (enum lttng_event_context_type) src->ctx;
+	lttng_event_perf_counter_ctx_deserialize(&dst->u.perf_counter, &src->perf_counter);
+	return 0;
+}
+
 int init_serialized_extended_channel(struct lttng_domain *domain, struct
 		lttng_channel_extended_serialized *extended)
 {
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index f87b9098..3455a4ea 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -31,6 +31,7 @@
 #include <lttng/snapshot-internal.h>
 #include <lttng/save-internal.h>
 #include <lttng/channel-internal.h>
+#include <lttng/event-internal.h>
 #include <lttng/trigger/trigger-internal.h>
 #include <lttng/rotate-internal.h>
 #include <common/compat/socket.h>
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 13/20] Integrate serialized communication for lttng_event_context
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (11 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 12/20] Add serdes functions for lttng_event_context Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 14/20] Add serialized versions of lttng_event structs Yannick Lamarre
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

lttng-ctl and lttng-sessiond use serialized communication for
messages using lttng_event_context.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Fixed coding style issue by adding an empty line between variable declaration
and code.

 include/lttng/event-internal.h           |  4 ++++
 src/bin/lttng-sessiond/client.c          | 17 ++++++++++-------
 src/common/sessiond-comm/sessiond-comm.h |  2 +-
 src/lib/lttng-ctl/lttng-ctl.c            | 12 +-----------
 4 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/include/lttng/event-internal.h b/include/lttng/event-internal.h
index 25a88448..09b4d232 100644
--- a/include/lttng/event-internal.h
+++ b/include/lttng/event-internal.h
@@ -57,4 +57,8 @@ struct lttng_event_extended {
 LTTNG_HIDDEN
 struct lttng_event *lttng_event_copy(const struct lttng_event *event);
 
+int lttng_event_context_serialize(struct lttng_event_context_serialized *dst, const struct lttng_event_context *src);
+int lttng_event_context_deserialize(struct lttng_event_context *dst, const struct lttng_event_context_serialized *src);
+int lttng_event_perf_counter_ctx_serialize(struct lttng_event_perf_counter_ctx_serialized *dst, const struct lttng_event_perf_counter_ctx *src);
+int lttng_event_perf_counter_ctx_deserialize(struct lttng_event_perf_counter_ctx *dst, const struct lttng_event_perf_counter_ctx_serialized *src);
 #endif /* LTTNG_EVENT_INTERNAL_H */
diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
index 614f19c7..45035418 100644
--- a/src/bin/lttng-sessiond/client.c
+++ b/src/bin/lttng-sessiond/client.c
@@ -1071,6 +1071,9 @@ skip_domain:
 	switch (cmd_ctx->lsm->cmd_type) {
 	case LTTNG_ADD_CONTEXT:
 	{
+		struct lttng_event_context evt_ctx;
+
+		lttng_event_context_deserialize(&evt_ctx, &cmd_ctx->lsm->u.context.ctx);
 		/*
 		 * An LTTNG_ADD_CONTEXT command might have a supplementary
 		 * payload if the context being added is an application context.
@@ -1097,7 +1100,7 @@ skip_domain:
 				ret = -LTTNG_ERR_NOMEM;
 				goto error;
 			}
-			cmd_ctx->lsm->u.context.ctx.u.app_ctx.provider_name =
+			evt_ctx.u.app_ctx.provider_name =
 					provider_name;
 
 			context_name = zmalloc(context_name_len + 1);
@@ -1105,7 +1108,7 @@ skip_domain:
 				ret = -LTTNG_ERR_NOMEM;
 				goto error_add_context;
 			}
-			cmd_ctx->lsm->u.context.ctx.u.app_ctx.ctx_name =
+			evt_ctx.u.app_ctx.ctx_name =
 					context_name;
 
 			ret = lttcomm_recv_unix_sock(sock, provider_name,
@@ -1128,14 +1131,14 @@ skip_domain:
 		ret = cmd_add_context(cmd_ctx->session,
 				cmd_ctx->lsm->domain.type,
 				cmd_ctx->lsm->u.context.channel_name,
-				&cmd_ctx->lsm->u.context.ctx,
+				&evt_ctx,
 				kernel_poll_pipe[1]);
 
-		cmd_ctx->lsm->u.context.ctx.u.app_ctx.provider_name = NULL;
-		cmd_ctx->lsm->u.context.ctx.u.app_ctx.ctx_name = NULL;
+		evt_ctx.u.app_ctx.provider_name = NULL;
+		evt_ctx.u.app_ctx.ctx_name = NULL;
 error_add_context:
-		free(cmd_ctx->lsm->u.context.ctx.u.app_ctx.provider_name);
-		free(cmd_ctx->lsm->u.context.ctx.u.app_ctx.ctx_name);
+		free(evt_ctx.u.app_ctx.provider_name);
+		free(evt_ctx.u.app_ctx.ctx_name);
 		if (ret < 0) {
 			goto error;
 		}
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 3455a4ea..955c35a8 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -410,7 +410,7 @@ struct lttcomm_session_msg {
 		/* Context */
 		struct {
 			char channel_name[LTTNG_SYMBOL_NAME_LEN];
-			struct lttng_event_context ctx LTTNG_PACKED;
+			struct lttng_event_context_serialized ctx;
 			uint32_t provider_name_len;
 			uint32_t context_name_len;
 		} LTTNG_PACKED context;
diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index 15f66034..f8ea6ca7 100644
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -821,17 +821,7 @@ int lttng_add_context(struct lttng_handle *handle,
 		memcpy(buf, provider_name, provider_len);
 		memcpy(buf + provider_len, ctx_name, ctx_len);
 	}
-	memcpy(&lsm.u.context.ctx, ctx, sizeof(struct lttng_event_context));
-
-	if (ctx->ctx == LTTNG_EVENT_CONTEXT_APP_CONTEXT) {
-		/*
-		 * Don't leak application addresses to the sessiond.
-		 * This is only necessary when ctx is for an app ctx otherwise
-		 * the values inside the union (type & config) are overwritten.
-		 */
-		lsm.u.context.ctx.u.app_ctx.provider_name = NULL;
-		lsm.u.context.ctx.u.app_ctx.ctx_name = NULL;
-	}
+	lttng_event_context_serialize(&lsm.u.context.ctx, ctx);
 
 	ret = lttng_ctl_ask_sessiond_varlen_no_cmd_header(&lsm, buf, len, NULL);
 end:
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 14/20] Add serialized versions of lttng_event structs
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (12 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 13/20] Integrate serialized communication " Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 15/20] Add serdes functions for lttng_event Yannick Lamarre
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Serialized versions of lttng_event, lttng_event_function_attr and
lttng_event_probe_attr are packed structures to be used in communication
protocols for consistent sizes across platforms. The serialized versions are
stripped of pointers and padding.

Pointers are removed since their size can vary across processes on
platforms supporting multiple pointer sizes (32bits vs 64bits) and this
creates mismatches in structure sizes.
Padding is also removed since it defeats the purpose of a packed struct
and is not used to extend the protocols.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Reworded commit message to better justify removal of pointer and padding
fields.

 include/lttng/event-internal.h | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/include/lttng/event-internal.h b/include/lttng/event-internal.h
index 09b4d232..a43007de 100644
--- a/include/lttng/event-internal.h
+++ b/include/lttng/event-internal.h
@@ -27,6 +27,44 @@
 
 struct lttng_userspace_probe_location;
 
+struct lttng_event_probe_attr_serialized {
+	uint64_t addr;
+
+	uint64_t offset;
+	char symbol_name[LTTNG_SYMBOL_NAME_LEN];
+} LTTNG_PACKED;
+
+struct lttng_event_function_attr_serialized {
+	char symbol_name[LTTNG_SYMBOL_NAME_LEN];
+} LTTNG_PACKED;
+
+struct lttng_event_serialized {
+	uint32_t type; /* enum lttng_event_type */
+
+	char name[LTTNG_SYMBOL_NAME_LEN];
+
+	uint32_t loglevel_type; /* enum lttng_loglevel_type */
+
+	int loglevel;
+
+	int32_t enabled;	/* Does not apply: -1 */
+
+	pid_t pid;
+
+	unsigned char filter;	/* filter enabled ? */
+
+	unsigned char exclusion; /* exclusions added ? */
+
+	/* Event flag, from 2.6 and above. */
+	uint32_t flags; /* enum lttng_event_flag */
+
+	/* Per event type configuration */
+	union {
+		struct lttng_event_probe_attr_serialized probe;
+		struct lttng_event_function_attr_serialized ftrace;
+	} attr;
+} LTTNG_PACKED;
+
 struct lttng_event_perf_counter_ctx_serialized {
 	uint32_t type;
 	uint64_t config;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 15/20] Add serdes functions for lttng_event
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (13 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 14/20] Add serialized versions of lttng_event structs Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 16/20] Integrate serialized communication " Yannick Lamarre
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Since those structs are only transferred across unix sockets, endianness
is kept in host order.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Fixed spelling errors

 include/lttng/event-internal.h           |   6 ++
 src/common/sessiond-comm/sessiond-comm.c | 118 +++++++++++++++++++++++++++++++
 2 files changed, 124 insertions(+)

diff --git a/include/lttng/event-internal.h b/include/lttng/event-internal.h
index a43007de..ec261af4 100644
--- a/include/lttng/event-internal.h
+++ b/include/lttng/event-internal.h
@@ -95,6 +95,12 @@ struct lttng_event_extended {
 LTTNG_HIDDEN
 struct lttng_event *lttng_event_copy(const struct lttng_event *event);
 
+int lttng_event_probe_attr_serialize(struct lttng_event_serialized *dst, const struct lttng_event *src);
+int lttng_event_function_attr_serialize(struct lttng_event_serialized *dst, const struct lttng_event *src);
+int lttng_event_no_attr_serialize(struct lttng_event_serialized *dst, const struct lttng_event *src);
+int lttng_event_probe_attr_deserialize(struct lttng_event *dst, const struct lttng_event_serialized *src);
+int lttng_event_function_attr_deserialize(struct lttng_event *dst, const struct lttng_event_serialized *src);
+int lttng_event_no_attr_deserialize(struct lttng_event *dst, const struct lttng_event_serialized *src);
 int lttng_event_context_serialize(struct lttng_event_context_serialized *dst, const struct lttng_event_context *src);
 int lttng_event_context_deserialize(struct lttng_event_context *dst, const struct lttng_event_context_serialized *src);
 int lttng_event_perf_counter_ctx_serialize(struct lttng_event_perf_counter_ctx_serialized *dst, const struct lttng_event_perf_counter_ctx *src);
diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c
index 26b2d66d..01e564af 100644
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -77,6 +77,124 @@ static const char *lttcomm_readable_code[] = {
 static unsigned long network_timeout;
 
 LTTNG_HIDDEN
+int lttng_event_common_serialize(struct lttng_event_serialized *dst,
+		const struct lttng_event *src)
+{
+	assert(src && dst);
+	dst->type = (uint32_t) src->type;
+
+	memcpy(dst->name, src->name, LTTNG_SYMBOL_NAME_LEN);
+
+	dst->loglevel_type = (uint32_t) src->loglevel_type;
+	dst->loglevel = src->loglevel;
+	dst->enabled = src->enabled;
+	dst->pid = src->pid;
+	dst->filter = src->filter;
+	dst->exclusion = src->exclusion;
+	dst->flags = (uint32_t) src->flags;
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_common_deserialize(struct lttng_event *dst,
+		const struct lttng_event_serialized *src)
+{
+	assert(src && dst);
+	dst->type = (enum lttng_event_type) src->type;
+
+	memcpy(dst->name, src->name, LTTNG_SYMBOL_NAME_LEN);
+
+	dst->loglevel_type = (enum lttng_loglevel_type) src->loglevel_type;
+	dst->loglevel = src->loglevel;
+	dst->enabled = src->enabled;
+	dst->pid = src->pid;
+	dst->filter = src->filter;
+	dst->exclusion = src->exclusion;
+	dst->flags = (enum lttng_event_flag) src->flags;
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_probe_attr_serialize(struct lttng_event_serialized *dst,
+		const struct lttng_event *src)
+{
+	assert(src && dst);
+	lttng_event_common_serialize(dst, src);
+
+	dst->attr.probe.addr = src->attr.probe.addr;
+	dst->attr.probe.offset = src->attr.probe.offset;
+
+	memcpy(dst->attr.probe.symbol_name, src->attr.probe.symbol_name, LTTNG_SYMBOL_NAME_LEN);
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_probe_attr_deserialize(struct lttng_event *dst,
+		const struct lttng_event_serialized *src)
+{
+	assert(src && dst);
+	lttng_event_common_deserialize(dst, src);
+
+	dst->attr.probe.addr = src->attr.probe.addr;
+	dst->attr.probe.offset = src->attr.probe.offset;
+
+	memcpy(dst->attr.probe.symbol_name, src->attr.probe.symbol_name, LTTNG_SYMBOL_NAME_LEN);
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_function_attr_serialize(struct lttng_event_serialized *dst,
+		const struct lttng_event *src)
+{
+	assert(src && dst);
+	lttng_event_common_serialize(dst, src);
+
+	memcpy(dst->attr.ftrace.symbol_name, src->attr.ftrace.symbol_name, LTTNG_SYMBOL_NAME_LEN);
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_function_attr_deserialize(struct lttng_event *dst,
+		const struct lttng_event_serialized *src)
+{
+	assert(src && dst);
+	lttng_event_common_deserialize(dst, src);
+
+	memcpy(dst->attr.ftrace.symbol_name, src->attr.ftrace.symbol_name, LTTNG_SYMBOL_NAME_LEN);
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_no_attr_serialize(struct lttng_event_serialized *dst,
+		const struct lttng_event *src)
+{
+	assert(src && dst);
+	lttng_event_common_serialize(dst, src);
+
+	memset(&dst->attr, 0, sizeof(dst->attr));
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_event_no_attr_deserialize(struct lttng_event *dst,
+		const struct lttng_event_serialized *src)
+{
+	assert(src && dst);
+	lttng_event_common_deserialize(dst, src);
+
+	memset(&dst->attr, 0, sizeof(dst->attr));
+
+	return 0;
+}
+
+LTTNG_HIDDEN
 int lttng_event_perf_counter_ctx_serialize(struct lttng_event_perf_counter_ctx_serialized *dst,
 		const struct lttng_event_perf_counter_ctx *src)
 {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 16/20] Integrate serialized communication for lttng_event
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (14 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 15/20] Add serdes functions for lttng_event Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 17/20] Add serialized versions of lttng_snapshot_output structs Yannick Lamarre
                   ` (3 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

lttng-ctl and lttng-sessiond use serialized communication for
messages using lttng_event.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
1) Reworded commit title
2) Removed empty line prior variable declaration
3) Changed the awkward nested conditions by switch statements

 src/bin/lttng-sessiond/client.c          | 38 ++++++++++++++++++++++++++++----
 src/common/sessiond-comm/sessiond-comm.h |  4 ++--
 src/lib/lttng-ctl/lttng-ctl.c            | 21 ++++++++++++++----
 3 files changed, 53 insertions(+), 10 deletions(-)

diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
index 45035418..3fb69cd9 100644
--- a/src/bin/lttng-sessiond/client.c
+++ b/src/bin/lttng-sessiond/client.c
@@ -1152,7 +1152,7 @@ error_add_context:
 	}
 	case LTTNG_DISABLE_EVENT:
 	{
-
+		struct lttng_event event;
 		/*
 		 * FIXME: handle filter; for now we just receive the filter's
 		 * bytecode along with the filter expression which are sent by
@@ -1179,10 +1179,24 @@ error_add_context:
 				count -= (size_t) ret;
 			}
 		}
-		/* FIXME: passing packed structure to non-packed pointer */
+		lttng_event_no_attr_deserialize(&event, &cmd_ctx->lsm->u.disable.event);
+		if (cmd_ctx->lsm->domain.type == LTTNG_DOMAIN_KERNEL) {
+			switch (event.type) {
+			case LTTNG_EVENT_PROBE:
+			case LTTNG_EVENT_FUNCTION:
+			case LTTNG_EVENT_USERSPACE_PROBE:
+				lttng_event_probe_attr_deserialize(&event, &cmd_ctx->lsm->u.disable.event);
+				break;
+			case LTTNG_EVENT_FUNCTION_ENTRY:
+				lttng_event_function_attr_deserialize(&event, &cmd_ctx->lsm->u.disable.event);
+				break;
+			default:
+				break;
+			}
+		}
 		ret = cmd_disable_event(cmd_ctx->session, cmd_ctx->lsm->domain.type,
 				cmd_ctx->lsm->u.disable.channel_name,
-				&cmd_ctx->lsm->u.disable.event);
+				&event);
 		break;
 	}
 	case LTTNG_ENABLE_CHANNEL:
@@ -1214,6 +1228,7 @@ error_add_context:
 	}
 	case LTTNG_ENABLE_EVENT:
 	{
+		struct lttng_event event;
 		struct lttng_event *ev = NULL;
 		struct lttng_event_exclusion *exclusion = NULL;
 		struct lttng_filter_bytecode *bytecode = NULL;
@@ -1316,7 +1331,22 @@ error_add_context:
 			}
 		}
 
-		ev = lttng_event_copy(&cmd_ctx->lsm->u.enable.event);
+		lttng_event_no_attr_deserialize(&event, &cmd_ctx->lsm->u.disable.event);
+		if (cmd_ctx->lsm->domain.type == LTTNG_DOMAIN_KERNEL) {
+			switch (event.type) {
+			case LTTNG_EVENT_PROBE:
+			case LTTNG_EVENT_FUNCTION:
+			case LTTNG_EVENT_USERSPACE_PROBE:
+				lttng_event_probe_attr_deserialize(&event, &cmd_ctx->lsm->u.disable.event);
+				break;
+			case LTTNG_EVENT_FUNCTION_ENTRY:
+				lttng_event_function_attr_deserialize(&event, &cmd_ctx->lsm->u.disable.event);
+				break;
+			default:
+				break;
+			}
+		}
+		ev = lttng_event_copy(&event);
 		if (!ev) {
 			DBG("Failed to copy event: %s",
 					cmd_ctx->lsm->u.enable.event.name);
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 955c35a8..21535eeb 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -370,7 +370,7 @@ struct lttcomm_session_msg {
 		/* Event data */
 		struct {
 			char channel_name[LTTNG_SYMBOL_NAME_LEN];
-			struct lttng_event event LTTNG_PACKED;
+			struct lttng_event_serialized event;
 			/* Length of following filter expression. */
 			uint32_t expression_len;
 			/* Length of following bytecode for filter. */
@@ -389,7 +389,7 @@ struct lttcomm_session_msg {
 		} LTTNG_PACKED enable;
 		struct {
 			char channel_name[LTTNG_SYMBOL_NAME_LEN];
-			struct lttng_event event LTTNG_PACKED;
+			struct lttng_event_serialized event;
 			/* Length of following filter expression. */
 			uint32_t expression_len;
 			/* Length of following bytecode for filter. */
diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c
index f8ea6ca7..d4696954 100644
--- a/src/lib/lttng-ctl/lttng-ctl.c
+++ b/src/lib/lttng-ctl/lttng-ctl.c
@@ -1114,8 +1114,22 @@ int lttng_enable_event_with_exclusions(struct lttng_handle *handle,
 	}
 
 	lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
-	/* FIXME: copying non-packed struct to packed struct. */
-	memcpy(&lsm.u.enable.event, ev, sizeof(lsm.u.enable.event));
+
+	lttng_event_no_attr_serialize(&lsm.u.enable.event, ev);
+	if (handle->domain.type == LTTNG_DOMAIN_KERNEL) {
+		switch (ev->type) {
+		case LTTNG_EVENT_PROBE:
+		case LTTNG_EVENT_FUNCTION:
+		case LTTNG_EVENT_USERSPACE_PROBE:
+			lttng_event_probe_attr_serialize(&lsm.u.enable.event, ev);
+			break;
+		case LTTNG_EVENT_FUNCTION_ENTRY:
+			lttng_event_function_attr_serialize(&lsm.u.enable.event, ev);
+			break;
+		default:
+			break;
+		}
+	}
 
 	lttng_ctl_copy_string(lsm.session.name, handle->session_name,
 			sizeof(lsm.session.name));
@@ -1308,8 +1322,7 @@ int lttng_disable_event_ext(struct lttng_handle *handle,
 	lsm.cmd_type = LTTNG_DISABLE_EVENT;
 
 	lttng_ctl_copy_lttng_domain(&lsm.domain, &handle->domain);
-	/* FIXME: copying non-packed struct to packed struct. */
-	memcpy(&lsm.u.disable.event, ev, sizeof(lsm.u.disable.event));
+	lttng_event_no_attr_serialize(&lsm.u.disable.event, ev);
 
 	lttng_ctl_copy_string(lsm.session.name, handle->session_name,
 			sizeof(lsm.session.name));
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 17/20] Add serialized versions of lttng_snapshot_output structs
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (15 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 16/20] Integrate serialized communication " Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 18/20] Add serdes functions for lttng_snapshot_output Yannick Lamarre
                   ` (2 subsequent siblings)
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Serialized version of lttng_snapshot_output is a packed structure to be
used in communication protocols for consistent size across platforms.
The serialized version is stripped of pointers and padding.

Pointers are removed since their size can vary across processes on
platforms supporting multiple pointer sizes (32bits vs 64bits) and this
creates mismatches in structure sizes.
Padding is also removed since it defeats the purpose of a packed struct
and is not used to extend the protocols.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
Reworded commit message to better justify removal of pointer and padding
fields.

 include/lttng/snapshot-internal.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/include/lttng/snapshot-internal.h b/include/lttng/snapshot-internal.h
index b7391c22..9bde25fb 100644
--- a/include/lttng/snapshot-internal.h
+++ b/include/lttng/snapshot-internal.h
@@ -21,6 +21,7 @@
 #include <limits.h>
 #include <stdint.h>
 #include <lttng/constant.h>
+#include <common/macros.h>
 
 /*
  * Object used for the snapshot API. This is opaque to the public library.
@@ -45,6 +46,14 @@ struct lttng_snapshot_output {
 	char data_url[PATH_MAX];
 };
 
+struct lttng_snapshot_output_serialized {
+	uint32_t id;
+	uint64_t max_size;
+	char name[LTTNG_NAME_MAX];
+	char ctrl_url[PATH_MAX];
+	char data_url[PATH_MAX];
+} LTTNG_PACKED;
+
 /*
  * Snapshot output list object opaque to the user.
  */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 18/20] Add serdes functions for lttng_snapshot_output
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (16 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 17/20] Add serialized versions of lttng_snapshot_output structs Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 19/20] Integrate serialized communication " Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 20/20] Introduce serialization unit test Yannick Lamarre
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Since those structs are only transferred across unix sockets, endianness
is kept in host order.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
1) Fixed spelling errors
2) Added missing asserts

 include/lttng/snapshot-internal.h        |  3 +++
 src/common/sessiond-comm/sessiond-comm.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/include/lttng/snapshot-internal.h b/include/lttng/snapshot-internal.h
index 9bde25fb..2280cb85 100644
--- a/include/lttng/snapshot-internal.h
+++ b/include/lttng/snapshot-internal.h
@@ -74,4 +74,7 @@ struct lttng_snapshot_output_list {
 	struct lttng_snapshot_output *array;
 };
 
+int lttng_snapshot_output_serialize(struct lttng_snapshot_output_serialized *dst, const struct lttng_snapshot_output *src);
+int lttng_snapshot_output_deserialize(struct lttng_snapshot_output *dst, const struct lttng_snapshot_output_serialized *src);
+
 #endif /* LTTNG_SNAPSHOT_INTERNAL_ABI_H */
diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c
index 01e564af..1397e8ac 100644
--- a/src/common/sessiond-comm/sessiond-comm.c
+++ b/src/common/sessiond-comm/sessiond-comm.c
@@ -77,6 +77,36 @@ static const char *lttcomm_readable_code[] = {
 static unsigned long network_timeout;
 
 LTTNG_HIDDEN
+int lttng_snapshot_output_serialize(struct lttng_snapshot_output_serialized *dst,
+		const struct lttng_snapshot_output *src)
+{
+	assert(src && dst);
+	dst->id = src->id;
+	dst->max_size = src->max_size;
+
+	memcpy(dst->name, src->name, LTTNG_NAME_MAX);
+	memcpy(dst->ctrl_url, src->ctrl_url, PATH_MAX);
+	memcpy(dst->data_url, src->data_url, PATH_MAX);
+
+	return 0;
+}
+
+LTTNG_HIDDEN
+int lttng_snapshot_output_deserialize(struct lttng_snapshot_output *dst,
+		const struct lttng_snapshot_output_serialized *src)
+{
+	assert(src && dst);
+	dst->id = src->id;
+	dst->max_size = src->max_size;
+
+	memcpy(dst->name, src->name, LTTNG_NAME_MAX);
+	memcpy(dst->ctrl_url, src->ctrl_url, PATH_MAX);
+	memcpy(dst->data_url, src->data_url, PATH_MAX);
+
+	return 0;
+}
+
+LTTNG_HIDDEN
 int lttng_event_common_serialize(struct lttng_event_serialized *dst,
 		const struct lttng_event *src)
 {
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 19/20] Integrate serialized communication for lttng_snapshot_output
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (17 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 18/20] Add serdes functions for lttng_snapshot_output Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 20/20] Introduce serialization unit test Yannick Lamarre
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

lttng-ctl and lttng-sessiond use serialized communication for
messages using lttng_snapshot_output.

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
1) Reworded commit title
2) Fixed spelling error in structure name
3) Fixed coding style issue with index declaration
4) Changed index type to more appropriate size_t
5) Added intermediary variable to avoid dereferencing of pointer more than once

 src/bin/lttng-sessiond/client.c          | 28 +++++++++++++++++++++++-----
 src/common/sessiond-comm/sessiond-comm.h |  4 ++--
 src/lib/lttng-ctl/snapshot.c             | 26 ++++++++++++++++++--------
 3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c
index 3fb69cd9..94293b58 100644
--- a/src/bin/lttng-sessiond/client.c
+++ b/src/bin/lttng-sessiond/client.c
@@ -1738,9 +1738,12 @@ error_add_context:
 	case LTTNG_SNAPSHOT_ADD_OUTPUT:
 	{
 		struct lttcomm_lttng_output_id reply;
+		struct lttng_snapshot_output snapshot_output;
+
+		lttng_snapshot_output_deserialize(&snapshot_output, &cmd_ctx->lsm->u.snapshot_output.output);
 
 		ret = cmd_snapshot_add_output(cmd_ctx->session,
-				&cmd_ctx->lsm->u.snapshot_output.output, &reply.id);
+				&snapshot_output, &reply.id);
 		if (ret != LTTNG_OK) {
 			goto error;
 		}
@@ -1757,14 +1760,20 @@ error_add_context:
 	}
 	case LTTNG_SNAPSHOT_DEL_OUTPUT:
 	{
+		struct lttng_snapshot_output snapshot_output;
+
+		lttng_snapshot_output_deserialize(&snapshot_output, &cmd_ctx->lsm->u.snapshot_output.output);
+
 		ret = cmd_snapshot_del_output(cmd_ctx->session,
-				&cmd_ctx->lsm->u.snapshot_output.output);
+				&snapshot_output);
 		break;
 	}
 	case LTTNG_SNAPSHOT_LIST_OUTPUT:
 	{
 		ssize_t nb_output;
+		size_t i;
 		struct lttng_snapshot_output *outputs = NULL;
+		struct lttng_snapshot_output_serialized *serialized_outputs;
 
 		nb_output = cmd_snapshot_list_outputs(cmd_ctx->session, &outputs);
 		if (nb_output < 0) {
@@ -1773,9 +1782,14 @@ error_add_context:
 		}
 
 		assert((nb_output > 0 && outputs) || nb_output == 0);
-		ret = setup_lttng_msg_no_cmd_header(cmd_ctx, outputs,
-				nb_output * sizeof(struct lttng_snapshot_output));
+		serialized_outputs = malloc(sizeof(struct lttng_snapshot_output_serialized) * nb_output);
+		for (i = 0; i < nb_output; i++) {
+			lttng_snapshot_output_serialize(&serialized_outputs[i], &outputs[i]);
+		}
 		free(outputs);
+		ret = setup_lttng_msg_no_cmd_header(cmd_ctx, serialized_outputs,
+				nb_output * sizeof(struct lttng_snapshot_output_serialized));
+		free(serialized_outputs);
 
 		if (ret < 0) {
 			goto setup_error;
@@ -1786,8 +1800,12 @@ error_add_context:
 	}
 	case LTTNG_SNAPSHOT_RECORD:
 	{
+		struct lttng_snapshot_output snapshot_output;
+
+		lttng_snapshot_output_deserialize(&snapshot_output, &cmd_ctx->lsm->u.snapshot_output.output);
+
 		ret = cmd_snapshot_record(cmd_ctx->session,
-				&cmd_ctx->lsm->u.snapshot_record.output,
+				&snapshot_output,
 				cmd_ctx->lsm->u.snapshot_record.wait);
 		break;
 	}
diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h
index 21535eeb..6f005cc7 100644
--- a/src/common/sessiond-comm/sessiond-comm.h
+++ b/src/common/sessiond-comm/sessiond-comm.h
@@ -428,11 +428,11 @@ struct lttcomm_session_msg {
 			uint32_t size;
 		} LTTNG_PACKED uri;
 		struct {
-			struct lttng_snapshot_output output LTTNG_PACKED;
+			struct lttng_snapshot_output_serialized output;
 		} LTTNG_PACKED snapshot_output;
 		struct {
 			uint32_t wait;
-			struct lttng_snapshot_output output LTTNG_PACKED;
+			struct lttng_snapshot_output_serialized output;
 		} LTTNG_PACKED snapshot_record;
 		struct {
 			uint32_t nb_uri;
diff --git a/src/lib/lttng-ctl/snapshot.c b/src/lib/lttng-ctl/snapshot.c
index b30c4706..21a24d7e 100644
--- a/src/lib/lttng-ctl/snapshot.c
+++ b/src/lib/lttng-ctl/snapshot.c
@@ -47,8 +47,7 @@ int lttng_snapshot_add_output(const char *session_name,
 
 	lttng_ctl_copy_string(lsm.session.name, session_name,
 			sizeof(lsm.session.name));
-	memcpy(&lsm.u.snapshot_output.output, output,
-			sizeof(lsm.u.snapshot_output.output));
+	lttng_snapshot_output_serialize(&lsm.u.snapshot_output.output, output);
 
 	ret = lttng_ctl_ask_sessiond(&lsm, (void **) &reply);
 	if (ret < 0) {
@@ -80,8 +79,7 @@ int lttng_snapshot_del_output(const char *session_name,
 
 	lttng_ctl_copy_string(lsm.session.name, session_name,
 			sizeof(lsm.session.name));
-	memcpy(&lsm.u.snapshot_output.output, output,
-			sizeof(lsm.u.snapshot_output.output));
+	lttng_snapshot_output_serialize(&lsm.u.snapshot_output.output, output);
 
 	return lttng_ctl_ask_sessiond(&lsm, NULL);
 }
@@ -97,8 +95,10 @@ int lttng_snapshot_list_output(const char *session_name,
 		struct lttng_snapshot_output_list **list)
 {
 	int ret;
+	size_t i, count;
 	struct lttcomm_session_msg lsm;
 	struct lttng_snapshot_output_list *new_list = NULL;
+	struct lttng_snapshot_output_serialized *serialized_array;
 
 	if (!session_name || !list) {
 		ret = -LTTNG_ERR_INVALID;
@@ -117,12 +117,23 @@ int lttng_snapshot_list_output(const char *session_name,
 		goto error;
 	}
 
-	ret = lttng_ctl_ask_sessiond(&lsm, (void **) &new_list->array);
+	ret = lttng_ctl_ask_sessiond(&lsm, (void **) &serialized_array);
 	if (ret < 0) {
 		goto free_error;
 	}
 
-	new_list->count = ret / sizeof(struct lttng_snapshot_output);
+	count = ret / sizeof(struct lttng_snapshot_output_serialized);
+	new_list->count = count;
+	new_list->array = zmalloc(sizeof(struct lttng_snapshot_output) * count);
+	if (!new_list) {
+		ret = -LTTNG_ERR_NOMEM;
+		goto free_error;
+	}
+	for (i = 0; i < count; i++) {
+		lttng_snapshot_output_deserialize(&new_list->array[i], &serialized_array[i]);
+	}
+	free(serialized_array);
+
 	*list = new_list;
 	return 0;
 
@@ -207,8 +218,7 @@ int lttng_snapshot_record(const char *session_name,
 	 * record.
 	 */
 	if (output) {
-		memcpy(&lsm.u.snapshot_record.output, output,
-				sizeof(lsm.u.snapshot_record.output));
+		lttng_snapshot_output_serialize(&lsm.u.snapshot_record.output, output);
 	}
 
 	/* The wait param is ignored. */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [RFC PATCH lttng-tools v2 20/20] Introduce serialization unit test
       [not found] <20190501193444.17403-1-ylamarre@efficios.com>
                   ` (18 preceding siblings ...)
  2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 19/20] Integrate serialized communication " Yannick Lamarre
@ 2019-05-01 19:34 ` Yannick Lamarre
  19 siblings, 0 replies; 20+ messages in thread
From: Yannick Lamarre @ 2019-05-01 19:34 UTC (permalink / raw)
  To: lttng-dev; +Cc: jgalar

Signed-off-by: Yannick Lamarre <ylamarre@efficios.com>
---
New unit test for serialization functions.

 .gitignore                                         |   1 +
 tests/unit/Makefile.am                             |   8 +-
 .../unit/test_utils_sessiond_comm_serialization.c  | 290 +++++++++++++++++++++
 3 files changed, 298 insertions(+), 1 deletion(-)
 create mode 100644 tests/unit/test_utils_sessiond_comm_serialization.c

diff --git a/.gitignore b/.gitignore
index 19276012..a73eebc9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -75,6 +75,7 @@ TAGS
 /tests/unit/test_ust_data
 /tests/unit/test_utils_parse_size_suffix
 /tests/unit/test_utils_parse_time_suffix
+/tests/unit/test_utils_sessiond_comm_serialization
 /tests/unit/test_utils_expand_path
 /tests/unit/test_notification
 kernel_all_events_basic
diff --git a/tests/unit/Makefile.am b/tests/unit/Makefile.am
index 26989fa0..f156c640 100644
--- a/tests/unit/Makefile.am
+++ b/tests/unit/Makefile.am
@@ -28,7 +28,8 @@ LIBLTTNG_CTL=$(top_builddir)/src/lib/lttng-ctl/liblttng-ctl.la
 # Define test programs
 noinst_PROGRAMS = test_uri test_session test_kernel_data \
                   test_utils_parse_size_suffix test_utils_parse_time_suffix \
-                  test_utils_expand_path test_string_utils test_notification
+                  test_utils_expand_path test_string_utils test_notification \
+				  test_utils_sessiond_comm_serialization
 
 if HAVE_LIBLTTNG_UST_CTL
 noinst_PROGRAMS += test_ust_data
@@ -156,3 +157,8 @@ test_string_utils_LDADD = $(LIBTAP) $(LIBCOMMON) $(LIBSTRINGUTILS) $(DL_LIBS)
 # Notification api
 test_notification_SOURCES = test_notification.c
 test_notification_LDADD = $(LIBTAP) $(LIBLTTNG_CTL) $(DL_LIBS)
+
+# Serialization unit test
+test_utils_sessiond_comm_serialization_SOURCES = test_utils_sessiond_comm_serialization.c
+test_utils_sessiond_comm_serialization_LDADD = $(LIBTAP) $(LIBSESSIOND_COMM) $(LIBCOMMON) $(LIBLTTNG_CTL) $(DL_LIBS)
+
diff --git a/tests/unit/test_utils_sessiond_comm_serialization.c b/tests/unit/test_utils_sessiond_comm_serialization.c
new file mode 100644
index 00000000..bc9d11cc
--- /dev/null
+++ b/tests/unit/test_utils_sessiond_comm_serialization.c
@@ -0,0 +1,290 @@
+/*
+ * Copyright (c)  2019 Yannick Lamarre <yannick.lamarre@efficios.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * as published by the Free Software Foundation; only version 2
+ * of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include <assert.h>
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/types.h>
+
+#include <tap/tap.h>
+
+#include <common/sessiond-comm/sessiond-comm.h>
+#include <common/common.h>
+
+/* Number of TAP tests in this file */
+#define NUM_TESTS 141
+
+/* For error.h */
+int lttng_opt_quiet = 1;
+int lttng_opt_verbose = 0;
+int lttng_opt_mi;
+
+void test_lttng_channel(void)
+{
+	struct lttng_domain reference_domain;
+	struct lttng_channel channel;
+	struct lttng_channel_extended extended_channel;
+	struct lttng_channel_serialized serialized_channel;
+	struct lttng_channel_extended_serialized serialized_extended_channel;
+
+	reference_domain.type = LTTNG_DOMAIN_KERNEL;
+	reference_domain.buf_type = LTTNG_BUFFER_GLOBAL;
+
+	/* Initializing structures to unmatching values. */
+	memset(&channel, 0, sizeof(struct lttng_channel));
+	memset(&serialized_channel, 0xFF, sizeof(struct lttng_channel_serialized));
+
+	diag("Testing serialization and deserialization of lttng_channel.");
+	ok(lttng_channel_serialize(&serialized_channel, &channel) == 0, "Serialize function returned successfully");
+
+	ok(channel.enabled == serialized_channel.enabled, "enabled field properly copied");
+	ok(sizeof(channel.name) == sizeof(serialized_channel.name), "Size of name field is equal");
+	ok(memcmp(channel.name, serialized_channel.name, sizeof(channel.name)) == 0, "name field properly copied");
+	ok(channel.attr.overwrite == serialized_channel.attr.overwrite, "overwrite field field properly copied");
+	ok(channel.attr.subbuf_size == serialized_channel.attr.subbuf_size, "subbuf_size field properly copied");
+	ok(channel.attr.num_subbuf == serialized_channel.attr.num_subbuf, "num_subbuf field properly copied");
+	ok(channel.attr.switch_timer_interval == serialized_channel.attr.switch_timer_interval, "switch_timer_interval field properly copied");
+	ok(channel.attr.read_timer_interval == serialized_channel.attr.read_timer_interval, "read_timer_interval field properly copied");
+	ok(channel.attr.output == serialized_channel.attr.output, "output field properly copied");
+	ok(channel.attr.tracefile_size == serialized_channel.attr.tracefile_size, "tracefile_size field properly copied");
+	ok(channel.attr.tracefile_count == serialized_channel.attr.tracefile_count, "tracefile_count field properly copied");
+	ok(channel.attr.live_timer_interval == serialized_channel.attr.live_timer_interval, "live_timer_interval field properly copied");
+
+	/* Resetting channel to unmatching value. */
+	memset(&channel, 0xFF, sizeof(struct lttng_channel));
+
+	ok(lttng_channel_deserialize(&channel, &serialized_channel) == 0, "Deserialize function returned successfully");
+
+	ok(channel.enabled == serialized_channel.enabled, "enabled field properly copied");
+	ok(memcmp(channel.name, serialized_channel.name, sizeof(channel.name)) == 0, "name field properly copied");
+	ok(channel.attr.overwrite == serialized_channel.attr.overwrite, "overwrite field field properly copied");
+	ok(channel.attr.subbuf_size == serialized_channel.attr.subbuf_size, "subbuf_size field properly copied");
+	ok(channel.attr.num_subbuf == serialized_channel.attr.num_subbuf, "num_subbuf field properly copied");
+	ok(channel.attr.switch_timer_interval == serialized_channel.attr.switch_timer_interval, "switch_timer_interval field properly copied");
+	ok(channel.attr.read_timer_interval == serialized_channel.attr.read_timer_interval, "read_timer_interval field properly copied");
+	ok(channel.attr.output == serialized_channel.attr.output, "output field properly copied");
+	ok(channel.attr.tracefile_size == serialized_channel.attr.tracefile_size, "tracefile_size field properly copied");
+	ok(channel.attr.tracefile_count == serialized_channel.attr.tracefile_count, "tracefile_count field properly copied");
+	ok(channel.attr.live_timer_interval == serialized_channel.attr.live_timer_interval, "live_timer_interval field properly copied");
+
+	diag("Testing serialized_extended_channel initialization function");
+	ok(init_serialized_extended_channel(&reference_domain, &serialized_extended_channel) == 0, "Initialization function returned successfully");
+
+	ok(serialized_extended_channel.monitor_timer_interval == DEFAULT_KERNEL_CHANNEL_MONITOR_TIMER, "monitor_timer_interval field initialized to the right value");
+	ok(serialized_extended_channel.blocking_timeout == DEFAULT_KERNEL_CHANNEL_BLOCKING_TIMEOUT, "blocking_timeout field initialized to the right value");
+
+	reference_domain.type = LTTNG_DOMAIN_UST;
+	reference_domain.buf_type = LTTNG_BUFFER_PER_PID;
+
+	ok(init_serialized_extended_channel(&reference_domain, &serialized_extended_channel) == 0, "Initialization function returned successfully");
+
+	ok(serialized_extended_channel.monitor_timer_interval == DEFAULT_UST_PID_CHANNEL_MONITOR_TIMER, "monitor_timer_interval field initialized to the right value");
+	ok(serialized_extended_channel.blocking_timeout == DEFAULT_UST_PID_CHANNEL_BLOCKING_TIMEOUT, "blocking_timeout field initialized to the right value");
+
+	reference_domain.type = LTTNG_DOMAIN_UST;
+	reference_domain.buf_type = LTTNG_BUFFER_PER_UID;
+
+	ok(init_serialized_extended_channel(&reference_domain, &serialized_extended_channel) == 0, "Initialization function returned successfully");
+
+	ok(serialized_extended_channel.monitor_timer_interval == DEFAULT_UST_UID_CHANNEL_MONITOR_TIMER, "monitor_timer_interval field initialized to the right value");
+	ok(serialized_extended_channel.blocking_timeout == DEFAULT_UST_UID_CHANNEL_BLOCKING_TIMEOUT, "blocking_timeout field initialized to the right value");
+
+	diag("Testing serialized_extended_channel serialization functions");
+	memset(&extended_channel, 0xFF, sizeof(struct lttng_channel_extended));
+	ok(lttng_channel_extended_deserialize(&extended_channel, &serialized_extended_channel) == 0, "Deserialization function returned successfully");
+
+	ok(extended_channel.monitor_timer_interval == serialized_extended_channel.monitor_timer_interval, "monitor_timer_interval field properly copied");
+	ok(extended_channel.blocking_timeout == serialized_extended_channel.blocking_timeout, "blocking_timeout field properly copied");
+	ok(serialized_extended_channel.discarded_events == extended_channel.discarded_events, "discard_events field properly copied");
+	ok(serialized_extended_channel.lost_packets == extended_channel.lost_packets, "lost_packets field properly copied");
+
+	memset(&serialized_extended_channel, 0xFF, sizeof(struct lttng_channel_extended_serialized));
+	ok(lttng_channel_extended_serialize(&serialized_extended_channel, &extended_channel) == 0, "Serialization function returned successfully");
+
+	ok(extended_channel.monitor_timer_interval == serialized_extended_channel.monitor_timer_interval, "monitor_timer_interval field properly copied");
+	ok(extended_channel.blocking_timeout == serialized_extended_channel.blocking_timeout, "blocking_timeout field properly copied");
+	ok(serialized_extended_channel.discarded_events == extended_channel.discarded_events, "discard_events field properly copied");
+	ok(serialized_extended_channel.lost_packets == extended_channel.lost_packets, "lost_packets field properly copied");
+}
+
+void test_lttng_event_context(void)
+{
+	struct lttng_event_context event_context;
+	struct lttng_event_context_serialized serialized_event_context;
+
+	/* Initialize structures to unmatching values. */
+	memset(&event_context, 0, sizeof(struct lttng_event_context));
+	memset(&serialized_event_context, 0xFF, sizeof(struct lttng_event_context_serialized));
+
+	ok(lttng_event_context_serialize(&serialized_event_context, &event_context) == 0, "Serialization function returned successfully");
+
+	ok(event_context.ctx == serialized_event_context.ctx, "ctx field properly copied");
+	ok(event_context.u.perf_counter.type == serialized_event_context.perf_counter.type, "type field properly copied");
+	ok(event_context.u.perf_counter.config == serialized_event_context.perf_counter.config, "config field properly copied");
+	ok(sizeof(event_context.u.perf_counter.name) == sizeof(serialized_event_context.perf_counter.name), "Size of name field is equal");
+	ok(memcmp(event_context.u.perf_counter.name, serialized_event_context.perf_counter.name, sizeof(event_context.u.perf_counter.name)) == 0, "name field properly copied");
+
+	memset(&event_context, 0xFF, sizeof(struct lttng_event_context));
+	ok(lttng_event_context_deserialize(&event_context, &serialized_event_context) == 0, "Deserialization function returned successfully");
+
+	ok(event_context.ctx == serialized_event_context.ctx, "ctx field properly copied");
+	ok(event_context.u.perf_counter.type == serialized_event_context.perf_counter.type, "type field properly copied");
+	ok(event_context.u.perf_counter.config == serialized_event_context.perf_counter.config, "config field properly copied");
+	ok(memcmp(event_context.u.perf_counter.name, serialized_event_context.perf_counter.name, sizeof(event_context.u.perf_counter.name)) == 0, "name field properly copied");
+}
+
+void test_lttng_event(void)
+{
+	struct lttng_event event;
+	struct lttng_event_serialized serialized_event;
+
+	memset(&event, 0, sizeof(struct lttng_event));
+	memset(&serialized_event, 0xFF, sizeof(struct lttng_event_serialized));
+
+	ok(lttng_event_no_attr_serialize(&serialized_event, &event) == 0, "Serialization function returned successfully");
+
+	ok(event.type == serialized_event.type, "type field properly copied");
+	ok(event.loglevel_type == serialized_event.loglevel_type, "loglevel_type field properly copied");
+	ok(event.loglevel == serialized_event.loglevel, "loglevel field properly copied");
+	ok(event.enabled == serialized_event.enabled, "enabled field properly copied");
+	ok(event.pid == serialized_event.pid, "pid field properly copied");
+	ok(event.filter == serialized_event.filter, "filter field properly copied");
+	ok(event.exclusion == serialized_event.exclusion, "exclusion field properly copied");
+	ok(event.flags == serialized_event.flags, "flags field properly copied");
+	ok(sizeof(event.name) == sizeof(serialized_event.name), "Size of name field is equal");
+	ok(memcmp(event.name, serialized_event.name, sizeof(event.name)) == 0, "name field properly copied");
+
+	memset(&event, 0xFF, sizeof(struct lttng_event));
+	ok(lttng_event_no_attr_deserialize(&event, &serialized_event) == 0, "Deserialization function returned successfully");
+
+	ok(event.type == serialized_event.type, "type field properly copied");
+	ok(event.loglevel_type == serialized_event.loglevel_type, "loglevel_type field properly copied");
+	ok(event.loglevel == serialized_event.loglevel, "loglevel field properly copied");
+	ok(event.enabled == serialized_event.enabled, "enabled field properly copied");
+	ok(event.pid == serialized_event.pid, "pid field properly copied");
+	ok(event.filter == serialized_event.filter, "filter field properly copied");
+	ok(event.exclusion == serialized_event.exclusion, "exclusion field properly copied");
+	ok(event.flags == serialized_event.flags, "flags field properly copied");
+	ok(memcmp(event.name, serialized_event.name, sizeof(event.name)) == 0, "name field properly copied");
+
+	memset(&serialized_event, 0xFF, sizeof(struct lttng_event_serialized));
+	ok(lttng_event_function_attr_serialize(&serialized_event, &event) == 0, "Serialization function returned successfully");
+
+	ok(event.type == serialized_event.type, "type field properly copied");
+	ok(event.loglevel_type == serialized_event.loglevel_type, "loglevel_type field properly copied");
+	ok(event.loglevel == serialized_event.loglevel, "loglevel field properly copied");
+	ok(event.enabled == serialized_event.enabled, "enabled field properly copied");
+	ok(event.pid == serialized_event.pid, "pid field properly copied");
+	ok(event.filter == serialized_event.filter, "filter field properly copied");
+	ok(event.exclusion == serialized_event.exclusion, "exclusion field properly copied");
+	ok(event.flags == serialized_event.flags, "flags field properly copied");
+	ok(memcmp(event.name, serialized_event.name, sizeof(event.name)) == 0, "name field properly copied");
+	ok(sizeof(event.attr.ftrace.symbol_name) == sizeof(serialized_event.attr.ftrace.symbol_name), "Size of symbol_name field is equal");
+	ok(memcmp(event.attr.ftrace.symbol_name, serialized_event.attr.ftrace.symbol_name, sizeof(event.attr.ftrace.symbol_name)) == 0, "symbol_name field properly copied");
+
+	memset(&event, 0xFF, sizeof(struct lttng_event));
+	ok(lttng_event_function_attr_deserialize(&event, &serialized_event) == 0, "Deserialization function returned successfully");
+
+	ok(event.type == serialized_event.type, "type field properly copied");
+	ok(event.loglevel_type == serialized_event.loglevel_type, "loglevel_type field properly copied");
+	ok(event.loglevel == serialized_event.loglevel, "loglevel field properly copied");
+	ok(event.enabled == serialized_event.enabled, "enabled field properly copied");
+	ok(event.pid == serialized_event.pid, "pid field properly copied");
+	ok(event.filter == serialized_event.filter, "filter field properly copied");
+	ok(event.exclusion == serialized_event.exclusion, "exclusion field properly copied");
+	ok(event.flags == serialized_event.flags, "flags field properly copied");
+	ok(memcmp(event.name, serialized_event.name, sizeof(event.name)) == 0, "name field properly copied");
+	ok(memcmp(event.attr.ftrace.symbol_name, serialized_event.attr.ftrace.symbol_name, sizeof(event.attr.ftrace.symbol_name)) == 0, "symbol_name field properly copied");
+
+	memset(&serialized_event, 0xFF, sizeof(struct lttng_event_serialized));
+	ok(lttng_event_probe_attr_serialize(&serialized_event, &event) == 0, "Serialization function returned successfully");
+
+	ok(event.type == serialized_event.type, "type field properly copied");
+	ok(event.loglevel_type == serialized_event.loglevel_type, "loglevel_type field properly copied");
+	ok(event.loglevel == serialized_event.loglevel, "loglevel field properly copied");
+	ok(event.enabled == serialized_event.enabled, "enabled field properly copied");
+	ok(event.pid == serialized_event.pid, "pid field properly copied");
+	ok(event.filter == serialized_event.filter, "filter field properly copied");
+	ok(event.exclusion == serialized_event.exclusion, "exclusion field properly copied");
+	ok(event.flags == serialized_event.flags, "flags field properly copied");
+	ok(memcmp(event.name, serialized_event.name, sizeof(event.name)) == 0, "name field properly copied");
+	ok(event.attr.probe.addr == serialized_event.attr.probe.addr, "addr field properly copied");
+	ok(event.attr.probe.offset == serialized_event.attr.probe.offset, "offset field properly copied");
+	ok(sizeof(event.attr.probe.symbol_name) == sizeof(serialized_event.attr.probe.symbol_name), "Size of symbol_name field is equal");
+	ok(memcmp(event.attr.probe.symbol_name, serialized_event.attr.probe.symbol_name, sizeof(event.attr.probe.symbol_name)) == 0, "symbol_name field properly copied");
+
+	memset(&event, 0xFF, sizeof(struct lttng_event));
+	ok(lttng_event_probe_attr_deserialize(&event, &serialized_event) == 0, "Deserialization function returned successfully");
+
+	ok(event.type == serialized_event.type, "type field properly copied");
+	ok(event.loglevel_type == serialized_event.loglevel_type, "loglevel_type field properly copied");
+	ok(event.loglevel == serialized_event.loglevel, "loglevel field properly copied");
+	ok(event.enabled == serialized_event.enabled, "enabled field properly copied");
+	ok(event.pid == serialized_event.pid, "pid field properly copied");
+	ok(event.filter == serialized_event.filter, "filter field properly copied");
+	ok(event.exclusion == serialized_event.exclusion, "exclusion field properly copied");
+	ok(event.flags == serialized_event.flags, "flags field properly copied");
+	ok(memcmp(event.name, serialized_event.name, sizeof(event.name)) == 0, "name field properly copied");
+	ok(event.attr.probe.addr == serialized_event.attr.probe.addr, "addr field properly copied");
+	ok(event.attr.probe.offset == serialized_event.attr.probe.offset, "offset field properly copied");
+	ok(memcmp(event.attr.probe.symbol_name, serialized_event.attr.probe.symbol_name, sizeof(event.attr.probe.symbol_name)) == 0, "symbol_name field properly copied");
+}
+
+void test_lttng_snapshot_output(void)
+{
+	struct lttng_snapshot_output snapshot_output;
+	struct lttng_snapshot_output_serialized serialized_snapshot_output;
+
+	memset(&snapshot_output, 0, sizeof(struct lttng_snapshot_output));
+	memset(&serialized_snapshot_output, 0xFF, sizeof(struct lttng_snapshot_output_serialized));
+	ok(lttng_snapshot_output_serialize(&serialized_snapshot_output, &snapshot_output) == 0, "Serialization function returned successfully");
+
+	ok(snapshot_output.id == serialized_snapshot_output.id, "id field properly copied");
+	ok(snapshot_output.max_size == serialized_snapshot_output.max_size, "max_size field properly copied");
+	ok(sizeof(snapshot_output.name) == sizeof(serialized_snapshot_output.name), "Size of name field is equal");
+	ok(memcmp(snapshot_output.name, serialized_snapshot_output.name, sizeof(snapshot_output.name)) == 0, "name field properly copied");
+	ok(sizeof(snapshot_output.ctrl_url) == sizeof(serialized_snapshot_output.ctrl_url), "Size if ctrl_url field is equal");
+	ok(memcmp(snapshot_output.ctrl_url, serialized_snapshot_output.ctrl_url, sizeof(snapshot_output.ctrl_url)) == 0, "ctrl_url field properly copied");
+	ok(sizeof(snapshot_output.data_url) == sizeof(serialized_snapshot_output.data_url), "Size of data_url field is equal");
+	ok(memcmp(snapshot_output.data_url, serialized_snapshot_output.data_url, sizeof(snapshot_output.data_url)) == 0, "data_url field properly copied");
+
+	memset(&snapshot_output, 0xFF, sizeof(struct lttng_snapshot_output));
+	ok(lttng_snapshot_output_deserialize(&snapshot_output, &serialized_snapshot_output) == 0, "Deserialization function returned successfully");
+
+	ok(snapshot_output.id == serialized_snapshot_output.id, "id field properly copied");
+	ok(snapshot_output.max_size == serialized_snapshot_output.max_size, "max_size field properly copied");
+	ok(memcmp(snapshot_output.name, serialized_snapshot_output.name, sizeof(snapshot_output.name)) == 0, "name field properly copied");
+	ok(memcmp(snapshot_output.ctrl_url, serialized_snapshot_output.ctrl_url, sizeof(snapshot_output.ctrl_url)) == 0, "ctrl_url field properly copied");
+	ok(memcmp(snapshot_output.data_url, serialized_snapshot_output.data_url, sizeof(snapshot_output.data_url)) == 0, "data_url field properly copied");
+}
+
+int main(void)
+{
+	plan_tests(NUM_TESTS);
+
+	diag("Serialization functions' unit tests");
+	test_lttng_channel();
+	test_lttng_event_context();
+	test_lttng_event();
+	test_lttng_snapshot_output();
+
+	return exit_status();
+}
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-05-01 19:35 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190501193444.17403-1-ylamarre@efficios.com>
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 01/20] Fix: Use platform-independant types in sessiond to consumerd communication Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 02/20] Clean-up: Switch enum fields in lttcomm_consumer_msg Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 03/20] Clean-up: Switch enum fields in lttcomm_consumer_status_msg Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 04/20] Clean-up: Switch enum fields in lttcomm_consumer_status_channel Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 05/20] Clean-up: Remove unused structure Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 06/20] Clean-up: Removed deprecated field Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 07/20] Document inter-proccesses communication protocols requirements Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 08/20] Add serialized versions of lttng_channel structs Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 09/20] Add serdes functions for lttng_channel Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 10/20] Integrate serialized communication " Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 11/20] Add serialized versions of lttng_event_context structs Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 12/20] Add serdes functions for lttng_event_context Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 13/20] Integrate serialized communication " Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 14/20] Add serialized versions of lttng_event structs Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 15/20] Add serdes functions for lttng_event Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 16/20] Integrate serialized communication " Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 17/20] Add serialized versions of lttng_snapshot_output structs Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 18/20] Add serdes functions for lttng_snapshot_output Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 19/20] Integrate serialized communication " Yannick Lamarre
2019-05-01 19:34 ` [RFC PATCH lttng-tools v2 20/20] Introduce serialization unit test Yannick Lamarre

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).