[RFC] genl: Add a message builder API to help creating complex nl messages
by Tomasz Bursztyka
---
Hi Marcel,
Quickly, as an RFC, does this look better?
For building message I think either one uses l_genl_msg or l_genl_msg_builder.
Thus I changed l_genl_msg_builder_new(): it creates the l_genl_msg by itself.
Only when one calls l_genl_msg_builder_finalize(), he will get a reference on
the message. Such reference l_genl_family_send() will unref, or that he will
in case of error.
Does that look saner? At least it avoids the user to call l_genl_msg_new()
and again l_genl_msg_builder() afterwards. I think it's better to separate
properly both.
I kept the queue for the nested of nested etc... I still want your opinion:
either a queue or a fixed depth of nested of nested ? or we don't care?
(I think we do care).
ell/genl.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
ell/genl.h | 19 ++++++++++
2 files changed, 137 insertions(+)
diff --git a/ell/genl.c b/ell/genl.c
index 1b1afab..460d078 100644
--- a/ell/genl.c
+++ b/ell/genl.c
@@ -115,6 +115,12 @@ struct l_genl_family {
unsigned int nlctrl_cmd;
};
+struct l_genl_msg_builder {
+ struct l_genl_msg *message;
+ struct l_queue *nests;
+ struct nlattr *last_nested;
+};
+
static void destroy_request(void *data)
{
struct genl_request *request = data;
@@ -1280,3 +1286,115 @@ LIB_EXPORT bool l_genl_family_unregister(struct l_genl_family *family,
return true;
}
+
+LIB_EXPORT struct l_genl_msg_builder *l_genl_msg_builder_new(uint8_t cmd,
+ uint32_t size)
+{
+ struct l_genl_msg_builder *ret;
+
+ ret = l_new(struct l_genl_msg_builder, 1);
+ ret->message = l_genl_msg_new_sized(cmd, size);
+
+ return ret;
+}
+
+static inline void builder_message_realign(struct l_genl_msg_builder *builder)
+{
+ if (!builder->last_nested)
+ return;
+
+ if (!builder->nests)
+ builder->message->len = NLA_ALIGN(builder->message->len);
+
+ builder->last_nested = NULL;
+}
+
+LIB_EXPORT bool l_genl_msg_builder_append_attr(
+ struct l_genl_msg_builder *builder,
+ uint16_t type, uint16_t len,
+ const void *data)
+{
+ if (unlikely(!builder))
+ return false;
+
+ builder_message_realign(builder);
+
+ return l_genl_msg_append_attr(builder->message, type, len, data);
+}
+
+LIB_EXPORT bool l_genl_msg_builder_enter_nested(
+ struct l_genl_msg_builder *builder,
+ uint16_t type)
+{
+ struct nlattr *nla;
+
+ if (unlikely(!builder))
+ return false;
+
+ if (!builder->nests) {
+ builder->nests = l_queue_new();
+ if (!builder->nests)
+ return false;
+ }
+
+ builder_message_realign(builder);
+
+ nla = builder->message->data + builder->message->len;
+ nla->nla_len = NLA_HDRLEN;
+ nla->nla_type = type;
+
+ if (!l_genl_msg_append_attr(builder->message, type, 0, NULL))
+ return false;
+
+ return l_queue_push_head(builder->nests, nla);
+}
+
+LIB_EXPORT bool l_genl_msg_builder_leave_nested(
+ struct l_genl_msg_builder *builder)
+{
+ struct nlattr *nla;
+ uint32_t len;
+
+ if (unlikely(!builder) || unlikely(!builder->nests))
+ return false;
+
+ nla = l_queue_pop_head(builder->nests);
+ if (!nla)
+ return false;
+
+ if (l_queue_isempty(builder->nests)) {
+ l_queue_destroy(builder->nests, NULL);
+ builder->nests = NULL;
+ }
+
+ len = builder->message->len - ((void *)nla - builder->message->data);
+ if (len <= NLA_HDRLEN)
+ return false;
+
+ nla->nla_len = len;
+
+ builder->last_nested = nla;
+
+ return true;
+}
+
+LIB_EXPORT void l_genl_msg_builder_destroy(struct l_genl_msg_builder *builder)
+{
+ if (unlikely(!builder))
+ return;
+
+ l_queue_destroy(builder->nests, NULL);
+ l_genl_msg_unref(builder->message);
+ l_free(builder);
+}
+
+LIB_EXPORT struct l_genl_msg *l_genl_msg_builder_finalize(
+ struct l_genl_msg_builder *builder)
+{
+ if (unlikely(!builder) || unlikely(builder->nests))
+ return NULL;
+
+ builder_message_realign(builder);
+
+ return l_genl_msg_ref(builder->message);
+}
diff --git a/ell/genl.h b/ell/genl.h
index c628b8c..773682a 100644
--- a/ell/genl.h
+++ b/ell/genl.h
@@ -112,6 +112,25 @@ unsigned int l_genl_family_register(struct l_genl_family *family,
void *user_data, l_genl_destroy_func_t destroy);
bool l_genl_family_unregister(struct l_genl_family *family, unsigned int id);
+struct l_genl_msg_builder;
+
+struct l_genl_msg_builder *l_genl_msg_builder_new(uint8_t cmd,
+ uint32_t size);
+
+bool l_genl_msg_builder_append_attr(struct l_genl_msg_builder *builder,
+ uint16_t type, uint16_t len,
+ const void *data);
+
+bool l_genl_msg_builder_enter_nested(struct l_genl_msg_builder *builder,
+ uint16_t type);
+
+bool l_genl_msg_builder_leave_nested(struct l_genl_msg_builder *builder);
+
+void l_genl_msg_builder_destroy(struct l_genl_msg_builder *builder);
+
+struct l_genl_msg *l_genl_msg_builder_finalize(
+ struct l_genl_msg_builder *builder);
+
#ifdef __cplusplus
}
#endif
--
2.0.5
5 years, 10 months
[PATCH 1/2] genl: Implement message nesting
by Denis Kenzior
---
ell/genl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
ell/genl.h | 2 ++
2 files changed, 50 insertions(+)
diff --git a/ell/genl.c b/ell/genl.c
index 1b1afab..290325f 100644
--- a/ell/genl.c
+++ b/ell/genl.c
@@ -37,6 +37,8 @@
#include "genl-private.h"
#include "private.h"
+#define MAX_NESTING_LEVEL 4
+
struct l_genl {
int ref_count;
int fd;
@@ -65,6 +67,8 @@ struct l_genl_msg {
void *data;
uint32_t size;
uint32_t len;
+ struct nlattr *nests[MAX_NESTING_LEVEL];
+ uint8_t nesting_level;
};
struct genl_request {
@@ -241,6 +245,7 @@ static struct l_genl_msg *msg_alloc(uint8_t cmd, uint8_t version, uint32_t size)
msg->size = msg->len + NLMSG_ALIGN(size);
msg->data = l_new(unsigned char, msg->size);
+ msg->nesting_level = 0;
return l_genl_msg_ref(msg);
}
@@ -737,6 +742,49 @@ LIB_EXPORT bool l_genl_msg_append_attr(struct l_genl_msg *msg, uint16_t type,
return true;
}
+LIB_EXPORT bool l_genl_msg_enter_nested(struct l_genl_msg *msg, uint16_t type)
+{
+ struct nlattr *nla;
+
+ if (unlikely(!msg))
+ return false;
+
+ if (unlikely(msg->nesting_level == MAX_NESTING_LEVEL))
+ return false;
+
+ if (msg->len + NLA_HDRLEN > msg->size)
+ return false;
+
+ nla = msg->data + msg->len;
+ nla->nla_type = type;
+ nla->nla_len = msg->len; /* Save position */
+
+ msg->nests[msg->nesting_level] = nla;
+ msg->nesting_level += 1;
+
+ msg->len += NLA_HDRLEN;
+
+ return true;
+}
+
+LIB_EXPORT bool l_genl_msg_leave_nested(struct l_genl_msg *msg)
+{
+ struct nlattr *nla;
+
+ if (unlikely(!msg))
+ return false;
+
+ if (unlikely(msg->nesting_level == 0))
+ return false;
+
+ nla = msg->nests[msg->nesting_level - 1];
+ nla->nla_len = msg->len - nla->nla_len;
+
+ msg->nesting_level -= 1;
+
+ return true;
+}
+
#define NLA_OK(nla,len) ((len) >= (int) sizeof(struct nlattr) && \
(nla)->nla_len >= sizeof(struct nlattr) && \
(nla)->nla_len <= (len))
diff --git a/ell/genl.h b/ell/genl.h
index c628b8c..8f5fd52 100644
--- a/ell/genl.h
+++ b/ell/genl.h
@@ -71,6 +71,8 @@ int l_genl_msg_get_error(struct l_genl_msg *msg);
bool l_genl_msg_append_attr(struct l_genl_msg *msg, uint16_t type,
uint16_t len, const void *data);
+bool l_genl_msg_enter_nested(struct l_genl_msg *msg, uint16_t type);
+bool l_genl_msg_leave_nested(struct l_genl_msg *msg);
bool l_genl_attr_init(struct l_genl_attr *attr, struct l_genl_msg *msg);
bool l_genl_attr_next(struct l_genl_attr *attr, uint16_t *type,
--
2.0.4
5 years, 10 months
[PATCH v2 0/4] Improve genl with a message builder
by Tomasz Bursztyka
Hi,
It took a bit long as I was messing up with the wrong dump of a message
(a private vs a broadcasted one...) and of course it took a while before
I realized it.
Anyway, seems to work. I only had an issue with the nested message length
calculation when leaving it.
Tomasz Bursztyka (4):
genl: Normilize functions exports and parameters check as it should
genl: Add a message builder API to help creating complex nl messages
genl: Add a function to grab the message's raw data for unit purpose
unit: Add a message builder unit test for genl
Makefile.am | 1 +
ell/genl-private.h | 25 +++++
ell/genl.c | 284 ++++++++++++++++++++++++++++++++++++++++-------------
ell/genl.h | 18 ++++
unit/test-genl.c | 136 +++++++++++++++++++++----
5 files changed, 379 insertions(+), 85 deletions(-)
create mode 100644 ell/genl-private.h
--
2.0.5
5 years, 10 months
[PATCH 0/2] Improve genl with a message builder
by Tomasz Bursztyka
Hi,
I could not test yet the whole patch set so it's sent here for a quick review.
First patch is there since I realized genl lacked of those LIB_EXPORT and unlikely().
Minor stuff so.
Second patch is the interesting one.
I am currently building a unit test for it. Getting some reliable nested based messages
to compare with is the annoyingly time consuming part here.
Tomasz Bursztyka (2):
genl: Normilize functions exports and parameters check as it should
genl: Add a message builder API to help creating complex nl messages
ell/genl.c | 275 ++++++++++++++++++++++++++++++++++++++++++++++---------------
ell/genl.h | 18 ++++
2 files changed, 226 insertions(+), 67 deletions(-)
--
2.0.5
5 years, 10 months
[PATCH] genl: Add utility function no append nested attribute into a message
by Tomasz Bursztyka
It will fill-in a l_genl_attr structure with the necessary information
where the nested attribute starts. Thus at the end, it will update the
nested attribute with the final length.
---
ell/genl.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
ell/genl.h | 3 +++
2 files changed, 51 insertions(+)
diff --git a/ell/genl.c b/ell/genl.c
index 2ef6a34..b74f3c0 100644
--- a/ell/genl.c
+++ b/ell/genl.c
@@ -707,6 +707,54 @@ bool l_genl_msg_append_attr(struct l_genl_msg *msg, uint16_t type,
return true;
}
+bool l_genl_msg_append_init_recurse(struct l_genl_msg *msg, uint16_t type,
+ struct l_genl_attr *nested)
+{
+ struct nlattr *nla;
+
+ if (!msg || !nested)
+ return false;
+
+ nla = msg->data + msg->len;
+ nla->nla_len = NLA_HDRLEN;
+ nla->nla_type = type;
+
+ if (!l_genl_msg_append_attr(msg, type, 0, NULL))
+ return false;
+
+ nested->msg = msg;
+ nested->data = nla;
+ nested->len = 0;
+ nested->next_data = NULL;
+ nested->next_len = 0;
+
+ return true;
+}
+
+bool l_genl_msg_append_end_recurse(struct l_genl_attr *nested)
+{
+ struct l_genl_msg *msg;
+ struct nlattr *nla;
+ uint32_t len;
+
+ if (!nested || !nested->msg)
+ return false;
+
+ msg = nested->msg;
+ len = nested->data - msg->data;
+ nla = msg->data + len;
+
+ len = msg->len - len;
+ if (len <= NLA_HDRLEN)
+ return false;
+
+ nla->nla_len = len;
+
+ msg->len = NLA_ALIGN(msg->len);
+
+ return true;
+}
+
#define NLA_OK(nla,len) ((len) >= (int) sizeof(struct nlattr) && \
(nla)->nla_len >= sizeof(struct nlattr) && \
(nla)->nla_len <= (len))
diff --git a/ell/genl.h b/ell/genl.h
index c628b8c..13fe149 100644
--- a/ell/genl.h
+++ b/ell/genl.h
@@ -71,6 +71,9 @@ int l_genl_msg_get_error(struct l_genl_msg *msg);
bool l_genl_msg_append_attr(struct l_genl_msg *msg, uint16_t type,
uint16_t len, const void *data);
+bool l_genl_msg_append_init_recurse(struct l_genl_msg *msg, uint16_t type,
+ struct l_genl_attr *nested);
+bool l_genl_msg_append_end_recurse(struct l_genl_attr *nested);
bool l_genl_attr_init(struct l_genl_attr *attr, struct l_genl_msg *msg);
bool l_genl_attr_next(struct l_genl_attr *attr, uint16_t *type,
--
2.0.5
5 years, 11 months