[LEDE-DEV] [PATCH 5/7] firewall3: add UBUS support for ipset sections

Philip Prindeville philipp_subx at redfish-solutions.com
Fri Apr 28 18:11:22 PDT 2017


Inline…


> On Apr 20, 2017, at 10:05 AM, Pierre Lebleu <pme.lebleu at 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.lebleu at 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 at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/lede-dev




More information about the Lede-dev mailing list