On 21.01.2011 15:01, Jozsef Kadlecsik wrote:
> diff --git a/net/netfilter/ipset/ip_set_bitmap_ip.c b/net/netfilter/ipset/ip_set_bitmap_ip.c
> new file mode 100644
> index 0000000..4fbb360
> --- /dev/null
> +++ b/net/netfilter/ipset/ip_set_bitmap_ip.c
> +static const struct nla_policy bitmap_ip_adt_policy[IPSET_ATTR_ADT_MAX+1] = {
> + [IPSET_ATTR_IP] = { .type = NLA_NESTED },
> + [IPSET_ATTR_IP_TO] = { .type = NLA_NESTED },
> + [IPSET_ATTR_CIDR] = { .type = NLA_U8 },
> + [IPSET_ATTR_TIMEOUT] = { .type = NLA_U32 },
> + [IPSET_ATTR_LINENO] = { .type = NLA_U32 },
> +};
> +
> +static int
> +bitmap_ip_uadt(struct ip_set *set, struct nlattr *head, int len,
> + enum ipset_adt adt, u32 *lineno, u32 flags)
> +{
> + struct bitmap_ip *map = set->data;
> + struct nlattr *tb[IPSET_ATTR_ADT_MAX+1];
> + u32 ip, ip_to, id;
> + int ret = 0;
> +
> + if (nla_parse(tb, IPSET_ATTR_ADT_MAX, head, len,
> + bitmap_ip_adt_policy))
You can simply pass the container attribute instead of the
contents and length from ip_set_core.c and use nla_parse_nested().
This could even be done centrally in ip_set_core.c and you
just hand a set of parsed and validated attributes to this
function. Basically what you would do is:
- add nla_policy member to the ip_set_type_variant structure
- add type/variant specific max_attribute member to the
ip_set_type_variant structure
initialize both appropriately for each set type variant.
In ip_set_core.c, do:
struct nlattr *nla[set->variant->maxattr + 1];
err = nla_parse_nested(nla, set->variant->maxattr,
attr[IPSET_ATTR_DATA],
set->variant->policy);
if (err < 0)
return err;
set->variant->uadt(..., nla, ...)
That way you avoid duplicating the parsing in every set type.
> + return -IPSET_ERR_PROTOCOL;
> +
> + if (unlikely(!tb[IPSET_ATTR_IP]))
> + return -IPSET_ERR_PROTOCOL;
> +
> + if (tb[IPSET_ATTR_LINENO])
> + *lineno = nla_get_u32(tb[IPSET_ATTR_LINENO]);
> +
> + ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP], &ip);
> + if (ret)
> + return ret;
> +
> + if (ip < map->first_ip || ip > map->last_ip)
> + return -IPSET_ERR_BITMAP_RANGE;
> +
> + /* Set was defined without timeout support:
> + * don't ignore the attribute silently */
> + if (tb[IPSET_ATTR_TIMEOUT])
> + return -IPSET_ERR_TIMEOUT;
> +
> + if (adt == IPSET_TEST)
> + return bitmap_ip_test(map, ip_to_id(map, ip));
> +
> + if (tb[IPSET_ATTR_IP_TO]) {
> + ret = ip_set_get_hostipaddr4(tb[IPSET_ATTR_IP_TO], &ip_to);
> + if (ret)
> + return ret;
> + if (ip > ip_to) {
> + swap(ip, ip_to);
> + if (ip < map->first_ip)
> + return -IPSET_ERR_BITMAP_RANGE;
> + }
> + } else if (tb[IPSET_ATTR_CIDR]) {
> + u8 cidr = nla_get_u8(tb[IPSET_ATTR_CIDR]);
> +
> + if (cidr > 32)
> + return -IPSET_ERR_INVALID_CIDR;
> + ip &= ip_set_hostmask(cidr);
> + ip_to = ip | ~ip_set_hostmask(cidr);
> + } else
> + ip_to = ip;
> +
> + if (ip_to > map->last_ip)
> + return -IPSET_ERR_BITMAP_RANGE;
> +
> + for (; !before(ip_to, ip); ip += map->hosts) {
> + id = ip_to_id(map, ip);
> + ret = adt == IPSET_ADD ? bitmap_ip_add(map, id)
> + : bitmap_ip_del(map, id);
> +
> + if (ret && !ip_set_eexist(ret, flags))
> + return ret;
> + else
> + ret = 0;
> + }
> + return ret;
> +}
> +
> +static void
> +bitmap_ip_destroy(struct ip_set *set)
> +{
> + struct bitmap_ip *map = set->data;
> +
> + ip_set_free(map->members);
> + kfree(map);
> +
> + set->data = NULL;
> +}
> +
> +static void
> +bitmap_ip_flush(struct ip_set *set)
> +{
> + struct bitmap_ip *map = set->data;
> +
> + memset(map->members, 0, map->memsize);
> +}
> +
> +static int
> +bitmap_ip_head(struct ip_set *set, struct sk_buff *skb)
> +{
> + const struct bitmap_ip *map = set->data;
> + struct nlattr *nested;
> +
> + nested = ipset_nest_start(skb, IPSET_ATTR_DATA);
> + if (!nested)
> + goto nla_put_failure;
> + NLA_PUT_IPADDR4(skb, IPSET_ATTR_IP, htonl(map->first_ip));
> + NLA_PUT_IPADDR4(skb, IPSET_ATTR_IP_TO, htonl(map->last_ip));
> + if (map->netmask != 32)
> + NLA_PUT_U8(skb, IPSET_ATTR_NETMASK, map->netmask);
> + NLA_PUT_NET32(skb, IPSET_ATTR_REFERENCES,
> + htonl(atomic_read(&set->ref) - 1));
> + NLA_PUT_NET32(skb, IPSET_ATTR_MEMSIZE,
> + htonl(sizeof(*map) + map->memsize));
> + ipset_nest_end(skb, nested);
> +
> + return 0;
> +nla_put_failure:
> + return -EFAULT;
Same as in ip_set_core, this should be EMSGSIZE (probably applies
to all set types).
> +}
> +
> +static int
> +bitmap_ip_list(const struct ip_set *set,
> + struct sk_buff *skb, struct netlink_callback *cb)
> +{
> + const struct bitmap_ip *map = set->data;
> + struct nlattr *atd, *nested;
> + u32 id, first = cb->args[2];
> +
> + atd = ipset_nest_start(skb, IPSET_ATTR_ADT);
> + if (!atd)
> + return -EFAULT;
Same here.
> + for (; cb->args[2] < map->elements; cb->args[2]++) {
> + id = cb->args[2];
> + if (!bitmap_ip_test(map, id))
> + continue;
> + nested = ipset_nest_start(skb, IPSET_ATTR_DATA);
> + if (!nested) {
> + if (id == first) {
> + nla_nest_cancel(skb, atd);
> + return -EFAULT;
And here.
> + } else
> + goto nla_put_failure;
> + }
> + NLA_PUT_IPADDR4(skb, IPSET_ATTR_IP,
> + htonl(map->first_ip + id * map->hosts));
> + ipset_nest_end(skb, nested);
> + }
> + ipset_nest_end(skb, atd);
> + /* Set listing finished */
> + cb->args[2] = 0;
> + return 0;
> +
> +nla_put_failure:
> + nla_nest_cancel(skb, nested);
> + ipset_nest_end(skb, atd);
> + return 0;
Doesn't this need to return an errno value to indicate that the
dump is incomplete?
> +/* Timeout variant */
> +
> +struct bitmap_ip_timeout {
> + unsigned long *members; /* the set members */
> + u32 first_ip; /* host byte order, included in range */
> + u32 last_ip; /* host byte order, included in range */
> + u32 elements; /* number of max elements in the set */
> + u32 hosts; /* number of hosts in a subnet */
> + size_t memsize; /* members size */
> + u8 netmask; /* subnet netmask */
> +
> + u32 timeout; /* timeout parameter */
> + struct timer_list gc; /* garbage collection */
There's a lot of duplicated code just because the structures are
different. It seems this could be avoided if the common members
were in a common structure and just the timeout and timer_list
members were specific to the timeout variant.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
