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