[LEDE-DEV,RFC] procd: service: add data within the service itself

Message ID 1507114259-8698-1-git-send-email-pme.lebleu@gmail.com
State Under Review
Delegated to: John Crispin
Headers show
Series
  • [LEDE-DEV,RFC] procd: service: add data within the service itself
Related show

Commit Message

Pierre Lebleu Oct. 4, 2017, 10:50 a.m.
From: Pierre Lebleu <pme.lebleu@gmail.com>

It gives the ability to create firewall data within the
service itself rather than within an instance.

Signed-off-by: Pierre Lebleu <pme.lebleu@gmail.com>
---
 service/service.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 service/service.h |  2 ++
 2 files changed, 45 insertions(+), 2 deletions(-)

Comments

Pierre Lebleu Oct. 4, 2017, 11:01 a.m. | #1
The idea behind this patch is to allow you to create some firewall
rules within the service itself rather than an instance.

Indeed, I have a wrapper script which launches several different
services but no daemon by itself, so I can not create an instance for
it.
Or, if I create an instance without any command, I get a warning and I
am not able to remove this instance because it cannot be killed and
this service instance remains forever...

In order to push the firewall data to procd, I (currently) need an
instance otherwise firewall3 needs also a patch. So, the fake instance
for this service is called "*".

2017-10-04 12:50 GMT+02:00  <pme.lebleu@gmail.com>:
> From: Pierre Lebleu <pme.lebleu@gmail.com>
>
> It gives the ability to create firewall data within the
> service itself rather than within an instance.
>
> Signed-off-by: Pierre Lebleu <pme.lebleu@gmail.com>
> ---
>  service/service.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  service/service.h |  2 ++
>  2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/service/service.c b/service/service.c
> index 9c798aa..595f6f7 100644
> --- a/service/service.c
> +++ b/service/service.c
> @@ -84,6 +84,7 @@ service_alloc(const char *name)
>         s->name = new_name;
>         s->avl.key = s->name;
>         INIT_LIST_HEAD(&s->validators);
> +       blobmsg_list_simple_init(&s->data);
>
>         return s;
>  }
> @@ -95,6 +96,7 @@ enum {
>         SERVICE_SET_TRIGGER,
>         SERVICE_SET_VALIDATE,
>         SERVICE_SET_AUTOSTART,
> +       SERVICE_SET_DATA,
>         __SERVICE_SET_MAX
>  };
>
> @@ -105,8 +107,20 @@ static const struct blobmsg_policy service_set_attrs[__SERVICE_SET_MAX] = {
>         [SERVICE_SET_TRIGGER] = { "triggers", BLOBMSG_TYPE_ARRAY },
>         [SERVICE_SET_VALIDATE] = { "validate", BLOBMSG_TYPE_ARRAY },
>         [SERVICE_SET_AUTOSTART] = { "autostart", BLOBMSG_TYPE_BOOL },
> +       [SERVICE_SET_DATA] = { "data", BLOBMSG_TYPE_TABLE },
>  };
>
> +static void
> +service_fill_any(struct blobmsg_list *l, struct blob_attr *cur)
> +{
> +       if (!cur)
> +               return;
> +
> +       DEBUG(2, "Add data for service\n");
> +
> +       blobmsg_list_fill(l, blobmsg_data(cur), blobmsg_data_len(cur), false);
> +}
> +
>  static int
>  service_update(struct service *s, struct blob_attr **tb, bool add)
>  {
> @@ -148,6 +162,8 @@ service_update(struct service *s, struct blob_attr **tb, bool add)
>                         vlist_flush(&s->instances);
>         }
>
> +       service_fill_any(&s->data, tb[SERVICE_SET_DATA]);
> +
>         s->deleted = false;
>
>         rc(s->name, "running");
> @@ -159,6 +175,7 @@ static void
>  service_delete(struct service *s)
>  {
>         vlist_flush_all(&s->instances);
> +       blobmsg_list_free(&s->data);
>         s->deleted = true;
>         service_stopped(s);
>  }
> @@ -316,6 +333,13 @@ service_dump(struct service *s, bool verbose)
>                 blobmsg_add_blob(&b, s->trigger);
>         if (verbose && !list_empty(&s->validators))
>                 service_validate_dump(&b, s);
> +       if (!avl_is_empty(&s->data.avl)) {
> +               struct blobmsg_list_node *var;
> +               void *e = blobmsg_open_table(&b, "data");
> +               blobmsg_list_for_each(&s->data, var)
> +                       blobmsg_add_blob(&b, var->data);
> +               blobmsg_close_table(&b, e);
> +       }
>         blobmsg_close_table(&b, c);
>  }
>
> @@ -598,13 +622,30 @@ service_get_data(struct ubus_context *ctx, struct ubus_object *obj,
>         blob_buf_init(&b, 0);
>         avl_for_each_element(&services, s, avl) {
>                 void *cs = NULL;
> +               void *ci = NULL;
> +               struct blobmsg_list_node *var;
>
>                 if (name && strcmp(name, s->name))
>                         continue;
>
> +               blobmsg_list_for_each(&s->data, var) {
> +                       if (type && strcmp(blobmsg_name(var->data), type))
> +                               continue;
> +
> +                       if (!cs)
> +                               cs = blobmsg_open_table(&b, s->name);
> +
> +                       if (!ci)
> +                               ci = blobmsg_open_table(&b, "*");
> +
> +                       blobmsg_add_blob(&b, var->data);
> +               }
> +
> +               if (ci)
> +                       blobmsg_close_table(&b, ci), ci = NULL;
> +
>                 vlist_for_each_element(&s->instances, in, node) {
> -                       struct blobmsg_list_node *var;
> -                       void *ci = NULL;
> +                       ci = NULL;
>
>                         if (instance && strcmp(instance, in->name))
>                                 continue;
> diff --git a/service/service.h b/service/service.h
> index a433c9f..15333c4 100644
> --- a/service/service.h
> +++ b/service/service.h
> @@ -18,6 +18,7 @@
>  #include <libubox/avl.h>
>  #include <libubox/vlist.h>
>  #include <libubox/list.h>
> +#include "../utils/utils.h"
>
>  extern struct avl_tree services;
>
> @@ -46,6 +47,7 @@ struct service {
>         struct blob_attr *trigger;
>         struct vlist_tree instances;
>         struct list_head validators;
> +       struct blobmsg_list data;
>  };
>
>  void service_validate_add(struct service *s, struct blob_attr *attr);
> --
> 1.9.1
>
John Crispin Oct. 18, 2017, 7:25 a.m. | #2
Hi Pierre

sorry for the late reply, comments inline


On 04/10/17 12:50, pme.lebleu@gmail.com wrote:
> From: Pierre Lebleu <pme.lebleu@gmail.com>
>
> It gives the ability to create firewall data within the
> service itself rather than within an instance.
>
> Signed-off-by: Pierre Lebleu <pme.lebleu@gmail.com>
> ---
>   service/service.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>   service/service.h |  2 ++
>   2 files changed, 45 insertions(+), 2 deletions(-)
>
> diff --git a/service/service.c b/service/service.c
> index 9c798aa..595f6f7 100644
> --- a/service/service.c
> +++ b/service/service.c
> @@ -84,6 +84,7 @@ service_alloc(const char *name)
>   	s->name = new_name;
>   	s->avl.key = s->name;
>   	INIT_LIST_HEAD(&s->validators);
> +	blobmsg_list_simple_init(&s->data);
>   
>   	return s;
>   }
> @@ -95,6 +96,7 @@ enum {
>   	SERVICE_SET_TRIGGER,
>   	SERVICE_SET_VALIDATE,
>   	SERVICE_SET_AUTOSTART,
> +	SERVICE_SET_DATA,
>   	__SERVICE_SET_MAX
>   };
>   
> @@ -105,8 +107,20 @@ static const struct blobmsg_policy service_set_attrs[__SERVICE_SET_MAX] = {
>   	[SERVICE_SET_TRIGGER] = { "triggers", BLOBMSG_TYPE_ARRAY },
>   	[SERVICE_SET_VALIDATE] = { "validate", BLOBMSG_TYPE_ARRAY },
>   	[SERVICE_SET_AUTOSTART] = { "autostart", BLOBMSG_TYPE_BOOL },
> +	[SERVICE_SET_DATA] = { "data", BLOBMSG_TYPE_TABLE },
>   };
>   
> +static void
> +service_fill_any(struct blobmsg_list *l, struct blob_attr *cur)
> +{
> +	if (!cur)
> +		return;
> +
> +	DEBUG(2, "Add data for service\n");
> +
> +	blobmsg_list_fill(l, blobmsg_data(cur), blobmsg_data_len(cur), false);
> +}
> +
>   static int
>   service_update(struct service *s, struct blob_attr **tb, bool add)
>   {
> @@ -148,6 +162,8 @@ service_update(struct service *s, struct blob_attr **tb, bool add)
>   			vlist_flush(&s->instances);
>   	}
>   
> +	service_fill_any(&s->data, tb[SERVICE_SET_DATA]);
> +
>   	s->deleted = false;
>   
>   	rc(s->name, "running");
> @@ -159,6 +175,7 @@ static void
>   service_delete(struct service *s)
>   {
>   	vlist_flush_all(&s->instances);
> +	blobmsg_list_free(&s->data);
>   	s->deleted = true;
>   	service_stopped(s);
>   }
> @@ -316,6 +333,13 @@ service_dump(struct service *s, bool verbose)
>   		blobmsg_add_blob(&b, s->trigger);
>   	if (verbose && !list_empty(&s->validators))
>   		service_validate_dump(&b, s);
> +	if (!avl_is_empty(&s->data.avl)) {
> +		struct blobmsg_list_node *var;
> +		void *e = blobmsg_open_table(&b, "data");
> +		blobmsg_list_for_each(&s->data, var)
> +			blobmsg_add_blob(&b, var->data);
> +		blobmsg_close_table(&b, e);
> +	}
>   	blobmsg_close_table(&b, c);
>   }
>   
> @@ -598,13 +622,30 @@ service_get_data(struct ubus_context *ctx, struct ubus_object *obj,
>   	blob_buf_init(&b, 0);
>   	avl_for_each_element(&services, s, avl) {
>   		void *cs = NULL;
> +		void *ci = NULL;
> +		struct blobmsg_list_node *var;
>   
>   		if (name && strcmp(name, s->name))
>   			continue;
>   
> +		blobmsg_list_for_each(&s->data, var) {
> +			if (type && strcmp(blobmsg_name(var->data), type))
> +				continue;
> +
> +			if (!cs)
> +				cs = blobmsg_open_table(&b, s->name);
> +
> +			if (!ci)
> +				ci = blobmsg_open_table(&b, "*");
> +
> +			blobmsg_add_blob(&b, var->data);
> +		}
> +
> +		if (ci)
> +			blobmsg_close_table(&b, ci), ci = NULL;
> +

maybe i am reading the code wrong but it looks to me as if this would 
result int he service and instance data being intermingled. not sure if 
this is good or bad. could you possibly post a json dump of what the 
generated blob would looks like ?

     John

>   		vlist_for_each_element(&s->instances, in, node) {
> -			struct blobmsg_list_node *var;
> -			void *ci = NULL;
> +			ci = NULL;
>   
>   			if (instance && strcmp(instance, in->name))
>   				continue;
> diff --git a/service/service.h b/service/service.h
> index a433c9f..15333c4 100644
> --- a/service/service.h
> +++ b/service/service.h
> @@ -18,6 +18,7 @@
>   #include <libubox/avl.h>
>   #include <libubox/vlist.h>
>   #include <libubox/list.h>
> +#include "../utils/utils.h"
>   
>   extern struct avl_tree services;
>   
> @@ -46,6 +47,7 @@ struct service {
>   	struct blob_attr *trigger;
>   	struct vlist_tree instances;
>   	struct list_head validators;
> +	struct blobmsg_list data;
>   };
>   
>   void service_validate_add(struct service *s, struct blob_attr *attr);

Patch

diff --git a/service/service.c b/service/service.c
index 9c798aa..595f6f7 100644
--- a/service/service.c
+++ b/service/service.c
@@ -84,6 +84,7 @@  service_alloc(const char *name)
 	s->name = new_name;
 	s->avl.key = s->name;
 	INIT_LIST_HEAD(&s->validators);
+	blobmsg_list_simple_init(&s->data);
 
 	return s;
 }
@@ -95,6 +96,7 @@  enum {
 	SERVICE_SET_TRIGGER,
 	SERVICE_SET_VALIDATE,
 	SERVICE_SET_AUTOSTART,
+	SERVICE_SET_DATA,
 	__SERVICE_SET_MAX
 };
 
@@ -105,8 +107,20 @@  static const struct blobmsg_policy service_set_attrs[__SERVICE_SET_MAX] = {
 	[SERVICE_SET_TRIGGER] = { "triggers", BLOBMSG_TYPE_ARRAY },
 	[SERVICE_SET_VALIDATE] = { "validate", BLOBMSG_TYPE_ARRAY },
 	[SERVICE_SET_AUTOSTART] = { "autostart", BLOBMSG_TYPE_BOOL },
+	[SERVICE_SET_DATA] = { "data", BLOBMSG_TYPE_TABLE },
 };
 
+static void
+service_fill_any(struct blobmsg_list *l, struct blob_attr *cur)
+{
+	if (!cur)
+		return;
+
+	DEBUG(2, "Add data for service\n");
+
+	blobmsg_list_fill(l, blobmsg_data(cur), blobmsg_data_len(cur), false);
+}
+
 static int
 service_update(struct service *s, struct blob_attr **tb, bool add)
 {
@@ -148,6 +162,8 @@  service_update(struct service *s, struct blob_attr **tb, bool add)
 			vlist_flush(&s->instances);
 	}
 
+	service_fill_any(&s->data, tb[SERVICE_SET_DATA]);
+
 	s->deleted = false;
 
 	rc(s->name, "running");
@@ -159,6 +175,7 @@  static void
 service_delete(struct service *s)
 {
 	vlist_flush_all(&s->instances);
+	blobmsg_list_free(&s->data);
 	s->deleted = true;
 	service_stopped(s);
 }
@@ -316,6 +333,13 @@  service_dump(struct service *s, bool verbose)
 		blobmsg_add_blob(&b, s->trigger);
 	if (verbose && !list_empty(&s->validators))
 		service_validate_dump(&b, s);
+	if (!avl_is_empty(&s->data.avl)) {
+		struct blobmsg_list_node *var;
+		void *e = blobmsg_open_table(&b, "data");
+		blobmsg_list_for_each(&s->data, var)
+			blobmsg_add_blob(&b, var->data);
+		blobmsg_close_table(&b, e);
+	}
 	blobmsg_close_table(&b, c);
 }
 
@@ -598,13 +622,30 @@  service_get_data(struct ubus_context *ctx, struct ubus_object *obj,
 	blob_buf_init(&b, 0);
 	avl_for_each_element(&services, s, avl) {
 		void *cs = NULL;
+		void *ci = NULL;
+		struct blobmsg_list_node *var;
 
 		if (name && strcmp(name, s->name))
 			continue;
 
+		blobmsg_list_for_each(&s->data, var) {
+			if (type && strcmp(blobmsg_name(var->data), type))
+				continue;
+
+			if (!cs)
+				cs = blobmsg_open_table(&b, s->name);
+
+			if (!ci)
+				ci = blobmsg_open_table(&b, "*");
+
+			blobmsg_add_blob(&b, var->data);
+		}
+
+		if (ci)
+			blobmsg_close_table(&b, ci), ci = NULL;
+
 		vlist_for_each_element(&s->instances, in, node) {
-			struct blobmsg_list_node *var;
-			void *ci = NULL;
+			ci = NULL;
 
 			if (instance && strcmp(instance, in->name))
 				continue;
diff --git a/service/service.h b/service/service.h
index a433c9f..15333c4 100644
--- a/service/service.h
+++ b/service/service.h
@@ -18,6 +18,7 @@ 
 #include <libubox/avl.h>
 #include <libubox/vlist.h>
 #include <libubox/list.h>
+#include "../utils/utils.h"
 
 extern struct avl_tree services;
 
@@ -46,6 +47,7 @@  struct service {
 	struct blob_attr *trigger;
 	struct vlist_tree instances;
 	struct list_head validators;
+	struct blobmsg_list data;
 };
 
 void service_validate_add(struct service *s, struct blob_attr *attr);