diff mbox series

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

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

Commit Message

Pierre Lebleu Oct. 4, 2017, 10:50 a.m. UTC 
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. UTC | #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. UTC | #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);
Pierre Lebleu Oct. 25, 2017, 3:53 p.m. UTC | #3 
Hi John,

I reworked this patch a bit in order to have the data outside of any
instances. Doing that, I needed also to patch firewall3 in order to
parse this data.
Here come the "dump" of a dummy script:

ubus call service list '{ "name" : "myservice" }'
{
        "myservice": {
                "data": {
                        "firewall": [
                                {
                                        "type": "rule",
                                        "src": "wan",
                                        "proto": "udp",
                                        "dest_port": "xxxx",
                                        "target": "DROP"
                                },
                                {
                                        "type": "rule",
                                        "src": "wan",
                                        "proto": "udp",
                                        "dest_port": "yyyy",
                                        "target": "ACCEPT"
                                },

                        ]
                }
        }
}


ubus call service get_data '{ "name" : "myservice" }'
{
        "myservice": {
                "firewall": [
                        {
                                "type": "rule",
                                "src": "wan",
                                "proto": "udp",
                                "dest_port": "xxxx",
                                "target": "DROP"
                        },
                        {
                                "type": "rule",
                                "src": "wan",
                                "proto": "udp",
                                "dest_port": "yyyy",
                                "target": "ACCEPT"
                        },
                ]
        }
}

2017-10-18 9:25 GMT+02:00 John Crispin <john@phrozen.org>:
> 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);
>
>
diff mbox series

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);