Inline…
> On Apr 20, 2017, at 10:05 AM, Pierre Lebleu <pme.leb...@gmail.com> wrote:
>
> It gives the ability to create ipset rules via procd
> services and netifd interface firewall data.
>
> Signed-off-by: Pierre Lebleu <pme.leb...@gmail.com>
> ---
> ipsets.c | 83 +++++++++++++++++++++++++++++++++++++++++++-------------------
> ipsets.h | 11 +++++----
> main.c | 2 +-
> 3 files changed, 65 insertions(+), 31 deletions(-)
>
> diff --git a/ipsets.c b/ipsets.c
> index 3b1ba00..a3e8ee7 100644
> --- a/ipsets.c
> +++ b/ipsets.c
> @@ -85,7 +85,7 @@ static struct ipset_type ipset_types[] = {
>
>
> static bool
> -check_types(struct uci_element *e, struct fw3_ipset *ipset)
> +check_types(struct fw3_ipset *ipset)
> {
> int i = 0;
> uint32_t typelist = 0;
> @@ -95,7 +95,8 @@ check_types(struct uci_element *e, struct fw3_ipset *ipset)
> {
> if (i >= 3)
> {
> - warn_elem(e, "must not have more than 3 datatypes
> assigned");
> + warn("%s must not have more than 3 datatypes assigned",
> + ipset->name);
> return false;
> }
>
> @@ -116,8 +117,9 @@ check_types(struct uci_element *e, struct fw3_ipset
> *ipset)
> {
> ipset->method = ipset_types[i].method;
>
> - warn_elem(e, "defines no storage method,
> assuming '%s'",
> -
> fw3_ipset_method_names[ipset->method]);
> + warn("%s defines no storage method, assuming
> '%s'",
> + ipset->name,
> + fw3_ipset_method_names[ipset->method]);
>
> break;
> }
> @@ -136,56 +138,56 @@ check_types(struct uci_element *e, struct fw3_ipset
> *ipset)
> if ((ipset_types[i].required & OPT_IPRANGE) &&
> !ipset->iprange.set)
> {
> - warn_elem(e, "requires an ip range");
> + warn("%s requires an ip range",
> ipset->name);
> return false;
> }
>
> if ((ipset_types[i].required & OPT_PORTRANGE) &&
> !ipset->portrange.set)
> {
> - warn_elem(e, "requires a port range");
> + warn("%s requires a port range",
> ipset->name);
> return false;
> }
>
> if (!(ipset_types[i].required & OPT_IPRANGE) &&
> ipset->iprange.set)
> {
> - warn_elem(e, "iprange ignored");
> + warn("%s iprange ignored", ipset->name);
> ipset->iprange.set = false;
> }
>
> if (!(ipset_types[i].required & OPT_PORTRANGE)
> &&
> ipset->portrange.set)
> {
> - warn_elem(e, "portrange ignored");
> + warn("%s portrange ignored",
> ipset->name);
> ipset->portrange.set = false;
> }
>
> if (!(ipset_types[i].optional & OPT_NETMASK) &&
> ipset->netmask > 0)
> {
> - warn_elem(e, "netmask ignored");
> + warn("%s netmask ignored", ipset->name);
> ipset->netmask = 0;
> }
>
> if (!(ipset_types[i].optional & OPT_HASHSIZE) &&
> ipset->hashsize > 0)
> {
> - warn_elem(e, "hashsize ignored");
> + warn("%s hashsize ignored",
> ipset->name);
> ipset->hashsize = 0;
> }
>
> if (!(ipset_types[i].optional & OPT_MAXELEM) &&
> ipset->maxelem > 0)
> {
> - warn_elem(e, "maxelem ignored");
> + warn("%s maxelem ignored", ipset->name);
> ipset->maxelem = 0;
> }
>
> if (!(ipset_types[i].optional & OPT_FAMILY) &&
> ipset->family != FW3_FAMILY_V4)
> {
> - warn_elem(e, "family ignored");
> + warn("%s family ignored", ipset->name);
> ipset->family = FW3_FAMILY_V4;
> }
> }
> @@ -194,12 +196,12 @@ check_types(struct uci_element *e, struct fw3_ipset
> *ipset)
> }
> }
>
> - warn_elem(e, "has an invalid combination of storage method and
> matches");
> + warn("%s has an invalid combination of storage method and matches",
> ipset->name);
> return false;
> }
>
> -struct fw3_ipset *
> -fw3_alloc_ipset(void)
> +static struct fw3_ipset *
> +fw3_alloc_ipset(struct fw3_state *state)
> {
> struct fw3_ipset *ipset;
>
> @@ -212,21 +214,50 @@ fw3_alloc_ipset(void)
> ipset->enabled = true;
> ipset->family = FW3_FAMILY_V4;
>
> + list_add_tail(&ipset->list, &state->ipsets);
> +
> return ipset;
> }
>
> void
> -fw3_load_ipsets(struct fw3_state *state, struct uci_package *p)
> +fw3_load_ipsets(struct fw3_state *state, struct uci_package *p,
> + struct blob_attr *a)
> {
> struct uci_section *s;
> struct uci_element *e;
> - struct fw3_ipset *ipset;
> + struct fw3_ipset *ipset, *n;
> + struct blob_attr *entry, *opt;
> + unsigned rem, orem;
>
> INIT_LIST_HEAD(&state->ipsets);
>
> if (state->disable_ipsets)
> return;
>
> + blob_for_each_attr(entry, a, rem)
> + {
> + const char *type = NULL;
> + const char *name = "ubus ipset";
> + blobmsg_for_each_attr(opt, entry, orem)
> + if (!strcmp(blobmsg_name(opt), "type"))
> + type = blobmsg_get_string(opt);
> + else if (!strcmp(blobmsg_name(opt), "name"))
> + name = blobmsg_get_string(opt);
> +
> + if (!type || strcmp(type, "ipset"))
> + continue;
> +
> + if (!(ipset = fw3_alloc_ipset(state)))
Minor nit… Assignments inside of conditionals are a bear to step through in a
debugger like GDB.
-Philip
> + continue;
> +
> + if (!fw3_parse_blob_options(ipset, fw3_ipset_opts, entry, name))
> + {
> + warn("%s skipped due to invalid options\n", name);
> + fw3_free_ipset(ipset);
> + continue;
> + }
> + }
> +
> uci_foreach_element(&p->sections, e)
> {
> s = uci_to_section(e);
> @@ -234,7 +265,7 @@ fw3_load_ipsets(struct fw3_state *state, struct
> uci_package *p)
> if (strcmp(s->type, "ipset"))
> continue;
>
> - ipset = fw3_alloc_ipset();
> + ipset = fw3_alloc_ipset(state);
>
> if (!ipset)
> continue;
> @@ -245,7 +276,10 @@ fw3_load_ipsets(struct fw3_state *state, struct
> uci_package *p)
> fw3_free_ipset(ipset);
> continue;
> }
> + }
>
> + list_for_each_entry_safe(ipset, n, &state->ipsets, list)
> + {
> if (ipset->external)
> {
> if (!*ipset->external)
> @@ -256,27 +290,26 @@ fw3_load_ipsets(struct fw3_state *state, struct
> uci_package *p)
>
> if (!ipset->name || !*ipset->name)
> {
> - warn_elem(e, "must have a name assigned");
> + warn("ipset must have a name assigned");
> }
> //else if (fw3_lookup_ipset(state, ipset->name) != NULL)
> //{
> - // warn_elem(e, "has duplicated set name '%s'",
> ipset->name);
> + // warn("%s has duplicated set name", ipset->name);
> //}
> else if (ipset->family == FW3_FAMILY_ANY)
> {
> - warn_elem(e, "must not have family 'any'");
> + warn("%s must not have family 'any'", ipset->name);
> }
> else if (ipset->iprange.set && ipset->family !=
> ipset->iprange.family)
> {
> - warn_elem(e, "has iprange of wrong address family");
> + warn("%s has iprange of wrong address family",
> ipset->name);
> }
> else if (list_empty(&ipset->datatypes))
> {
> - warn_elem(e, "has no datatypes assigned");
> + warn("%s has no datatypes assigned", ipset->name);
> }
> - else if (check_types(e, ipset))
> + else if (check_types(ipset))
> {
> - list_add_tail(&ipset->list, &state->ipsets);
> continue;
> }
>
> diff --git a/ipsets.h b/ipsets.h
> index b5fee6c..2ba862d 100644
> --- a/ipsets.h
> +++ b/ipsets.h
> @@ -27,8 +27,7 @@
>
> extern const struct fw3_option fw3_ipset_opts[];
>
> -struct fw3_ipset * fw3_alloc_ipset(void);
> -void fw3_load_ipsets(struct fw3_state *state, struct uci_package *p);
> +void fw3_load_ipsets(struct fw3_state *state, struct uci_package *p, struct
> blob_attr *a);
> void fw3_create_ipsets(struct fw3_state *state);
> void fw3_destroy_ipsets(struct fw3_state *state);
>
> @@ -36,9 +35,11 @@ struct fw3_ipset * fw3_lookup_ipset(struct fw3_state
> *state, const char *name);
>
> bool fw3_check_ipset(struct fw3_ipset *set);
>
> -#define fw3_free_ipset(ipset) \
> - fw3_free_object(ipset, fw3_ipset_opts)
> -
> +static inline void fw3_free_ipset(struct fw3_ipset *ipset)
> +{
> + list_del(&ipset->list);
> + fw3_free_object(ipset, fw3_ipset_opts);
> +}
>
> #ifndef SO_IP_SET
>
> diff --git a/main.c b/main.c
> index 4cf46fd..6e275ef 100644
> --- a/main.c
> +++ b/main.c
> @@ -101,7 +101,7 @@ build_state(bool runtime)
> fw3_ubus_rules(&b);
>
> fw3_load_defaults(state, p);
> - fw3_load_ipsets(state, p);
> + fw3_load_ipsets(state, p, b.head);
> fw3_load_zones(state, p);
> fw3_load_rules(state, p, b.head);
> fw3_load_redirects(state, p, b.head);
> --
> 1.7.9.5
>
>
> _______________________________________________
> Lede-dev mailing list
> Lede-dev@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
