[PATCH 0/6] Add new comments extension to ipset.

[PATCH 0/6] Add new comments extension to ipset.

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This patch series adds support for a comments extensions on all ipset types via
the new extensions handling code implemented recently. A few small changes have
been made to the destroy code to make it usable in a generic manner as well as
relocating it within the header to enable it to compile (the set struct needed
to be completely defined in order to actually check whether to do cleanup).

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

Oliver Smith (6):
  netfilter: ipset: Support comments for ipset entries in the core.
  netfilter: ipset: Support comments in hash-type ipsets.
  netfilter: ipset: Support comments in bitmap-type ipsets.
  ipset: Rework the "fake" argument parsing for ipset restore.
  ipset: Support comments in the userspace library.
  ipset: Add new userspace set revisions for comment support

 Make_global.am                                     |   2 +-
 include/libipset/data.h                            |   6 +-
 include/libipset/linux_ip_set.h                    |   7 +
 include/libipset/parse.h                           |   2 +
 include/libipset/print.h                           |   3 +
 kernel/include/linux/netfilter/ipset/ip_set.h      |  40 ++++-
 .../include/linux/netfilter/ipset/ip_set_comment.h |  54 ++++++
 kernel/include/uapi/linux/netfilter/ipset/ip_set.h |   4 +
 kernel/net/netfilter/ipset/ip_set_bitmap_gen.h     |  10 +-
 kernel/net/netfilter/ipset/ip_set_bitmap_ip.c      |   3 +-
 kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c   |   3 +-
 kernel/net/netfilter/ipset/ip_set_bitmap_port.c    |   3 +-
 kernel/net/netfilter/ipset/ip_set_core.c           |  14 ++
 kernel/net/netfilter/ipset/ip_set_hash_gen.h       |  10 +-
 kernel/net/netfilter/ipset/ip_set_hash_ip.c        |   3 +-
 kernel/net/netfilter/ipset/ip_set_hash_ipport.c    |   3 +-
 kernel/net/netfilter/ipset/ip_set_hash_ipportip.c  |   3 +-
 kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c |   3 +-
 kernel/net/netfilter/ipset/ip_set_hash_net.c       |   3 +-
 kernel/net/netfilter/ipset/ip_set_hash_netiface.c  |   3 +-
 kernel/net/netfilter/ipset/ip_set_hash_netport.c   |   3 +-
 lib/data.c                                         |  34 ++++
 lib/debug.c                                        |   1 +
 lib/errcode.c                                      |   2 +
 lib/ipset_bitmap_ip.c                              | 114 ++++++++++++
 lib/ipset_bitmap_ipmac.c                           | 118 +++++++++++++
 lib/ipset_bitmap_port.c                            | 107 +++++++++++
 lib/ipset_hash_ip.c                                | 138 +++++++++++++++
 lib/ipset_hash_ipport.c                            | 161 +++++++++++++++++
 lib/ipset_hash_ipportnet.c                         | 195 +++++++++++++++++++++
 lib/ipset_hash_net.c                               | 145 +++++++++++++++
 lib/ipset_hash_netnet.c                            |  14 +-
 lib/ipset_hash_netport.c                           | 158 +++++++++++++++++
 lib/libipset.map                                   |   6 +
 lib/parse.c                                        |  27 +++
 lib/print.c                                        |  31 ++++
 lib/session.c                                      |   8 +-
 lib/types.c                                        |   4 +-
 src/ipset.c                                        |  40 ++++-
 39 files changed, 1451 insertions(+), 34 deletions(-)
 create mode 100644 kernel/include/linux/netfilter/ipset/ip_set_comment.h

-- 
1.8.3.2

[PATCH 1/6] netfilter: ipset: Support comments for ipset entries in the core.

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This adds the core support for having comments on ipset entries.

The comments are stored as standard null-terminated strings in
dynamically allocated memory after being passed to the kernel. As a
result of this, code has been added to the generic destroy function to
iterate all extensions and call that extension's destroy task if the set
has that extension activated, and if such a task is defined.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 kernel/include/linux/netfilter/ipset/ip_set.h      | 40 ++++++++++++----
 .../include/linux/netfilter/ipset/ip_set_comment.h | 54 ++++++++++++++++++++++
 kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  4 ++
 kernel/net/netfilter/ipset/ip_set_core.c           | 14 ++++++
 4 files changed, 104 insertions(+), 8 deletions(-)
 create mode 100644 kernel/include/linux/netfilter/ipset/ip_set_comment.h

diff --git a/kernel/include/linux/netfilter/ipset/ip_set.h b/kernel/include/linux/netfilter/ipset/ip_set.h
index c687abb..ee831f3 100644
--- a/kernel/include/linux/netfilter/ipset/ip_set.h
+++ b/kernel/include/linux/netfilter/ipset/ip_set.h
@@ -54,6 +54,8 @@ enum ip_set_extension {
 	IPSET_EXT_TIMEOUT = (1 << IPSET_EXT_BIT_TIMEOUT),
 	IPSET_EXT_BIT_COUNTER = 1,
 	IPSET_EXT_COUNTER = (1 << IPSET_EXT_BIT_COUNTER),
+	IPSET_EXT_BIT_COMMENT = 2,
+	IPSET_EXT_COMMENT = (1 << IPSET_EXT_BIT_COMMENT),
 	/* Mark set with an extension which needs to call destroy */
 	IPSET_EXT_BIT_DESTROY = 7,
 	IPSET_EXT_DESTROY = (1 << IPSET_EXT_BIT_DESTROY),
@@ -61,11 +63,15 @@ enum ip_set_extension {
 
 #define SET_WITH_TIMEOUT(s)	((s)->extensions & IPSET_EXT_TIMEOUT)
 #define SET_WITH_COUNTER(s)	((s)->extensions & IPSET_EXT_COUNTER)
+#define SET_WITH_COMMENT(s)	((s)->extensions & IPSET_EXT_COMMENT)
+
+/* Comment struct */
 
 /* Extension id, in size order */
 enum ip_set_ext_id {
 	IPSET_EXT_ID_COUNTER = 0,
 	IPSET_EXT_ID_TIMEOUT,
+	IPSET_EXT_ID_COMMENT,
 	IPSET_EXT_ID_MAX,
 };
 
@@ -86,6 +92,7 @@ struct ip_set_ext {
 	u64 packets;
 	u64 bytes;
 	u32 timeout;
+	char *comment;
 };
 
 struct ip_set_counter {
@@ -93,20 +100,21 @@ struct ip_set_counter {
 	atomic64_t packets;
 };
 
-struct ip_set;
+struct ip_set_comment {
+	char *str;
+};
 
-static inline void
-ip_set_ext_destroy(struct ip_set *set, void *data)
-{
-	/* Check that the extension is enabled for the set and
-	 * call it's destroy function for its extension part in data.
-	 */
-}
+struct ip_set;
 
+#define ext_generic(e, s, i)	\
+(void *)(((void *)(e)) + (s)->offset[i])
 #define ext_timeout(e, s)	\
 (unsigned long *)(((void *)(e)) + (s)->offset[IPSET_EXT_ID_TIMEOUT])
 #define ext_counter(e, s)	\
 (struct ip_set_counter *)(((void *)(e)) + (s)->offset[IPSET_EXT_ID_COUNTER])
+#define ext_comment(e, s)	\
+(struct ip_set_comment *)(((void *)(e)) + (s)->offset[IPSET_EXT_ID_COMMENT])
+
 
 typedef int (*ipset_adtfn)(struct ip_set *set, void *value,
 			   const struct ip_set_ext *ext,
@@ -224,6 +232,21 @@ struct ip_set {
 };
 
 static inline void
+ip_set_ext_destroy(struct ip_set *set, void *data)
+{
+	u32 id = 0;
+	/* Check that the extension is enabled for the set and
+	 * call it's destroy function for its extension part in data.
+	 */
+	for (; id < IPSET_EXT_ID_MAX; id++) {
+		if (!(set->extensions & ip_set_extensions[id].type) ||
+		    !ip_set_extensions[id].destroy)
+			continue;
+		ip_set_extensions[id].destroy(ext_generic(data, set, id));
+	}
+}
+
+static inline void
 ip_set_add_bytes(u64 bytes, struct ip_set_counter *counter)
 {
 	atomic64_add((long long)bytes, &(counter)->bytes);
@@ -426,6 +449,7 @@ bitmap_bytes(u32 a, u32 b)
 }
 
 #include <linux/netfilter/ipset/ip_set_timeout.h>
+#include <linux/netfilter/ipset/ip_set_comment.h>
 
 #define IP_SET_INIT_KEXT(skb, opt, set)			\
 	{ .bytes = (skb)->len, .packets = 1,		\
diff --git a/kernel/include/linux/netfilter/ipset/ip_set_comment.h b/kernel/include/linux/netfilter/ipset/ip_set_comment.h
new file mode 100644
index 0000000..981a41e
--- /dev/null
+++ b/kernel/include/linux/netfilter/ipset/ip_set_comment.h
@@ -0,0 +1,54 @@
+#ifndef _IP_SET_COMMENT_H
+#define _IP_SET_COMMENT_H
+
+/* Copyright (C) 2013 Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#define IP_SET_MAX_COMMENT_SIZE 255
+
+#ifdef __KERNEL__
+
+static inline char*
+ip_set_comment_uget(struct nlattr *tb)
+{
+	return nla_data(tb);
+}
+
+static inline void
+ip_set_init_comment(struct ip_set_comment *comment,
+		    const struct ip_set_ext *ext)
+{
+	size_t len = ext->comment ? strlen(ext->comment) : 0;
+	if (!len)
+		return;
+	if (unlikely(len > IP_SET_MAX_COMMENT_SIZE))
+		len = IP_SET_MAX_COMMENT_SIZE;
+	if (unlikely(comment->str))
+		kfree(comment->str);
+	comment->str = kzalloc(len + 1, GFP_KERNEL);
+	strlcpy(comment->str, ext->comment, len + 1);
+}
+
+static inline bool
+ip_set_put_comment(struct sk_buff *skb, struct ip_set_comment *comment)
+{
+	if(!comment->str)
+		return NULL;
+	return nla_put_string(skb, IPSET_ATTR_COMMENT, comment->str);
+}
+
+static inline void
+ip_set_comment_free(struct ip_set_comment *comment)
+{
+	if(unlikely(!comment->str))
+		return;
+	kfree(comment->str);
+	comment->str = NULL;
+}
+
+#endif
+#endif
diff --git a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
index 2b61ac4..98dce6a 100644
--- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
+++ b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
@@ -110,6 +110,7 @@ enum {
 	IPSET_ATTR_IFACE,
 	IPSET_ATTR_BYTES,
 	IPSET_ATTR_PACKETS,
+	IPSET_ATTR_COMMENT,
 	__IPSET_ATTR_ADT_MAX,
 };
 #define IPSET_ATTR_ADT_MAX	(__IPSET_ATTR_ADT_MAX - 1)
@@ -140,6 +141,7 @@ enum ipset_errno {
 	IPSET_ERR_IPADDR_IPV4,
 	IPSET_ERR_IPADDR_IPV6,
 	IPSET_ERR_COUNTER,
+	IPSET_ERR_COMMENT,
 
 	/* Type specific error codes */
 	IPSET_ERR_TYPE_SPECIFIC = 4352,
@@ -176,6 +178,8 @@ enum ipset_cadt_flags {
 	IPSET_FLAG_NOMATCH	= (1 << IPSET_FLAG_BIT_NOMATCH),
 	IPSET_FLAG_BIT_WITH_COUNTERS = 3,
 	IPSET_FLAG_WITH_COUNTERS = (1 << IPSET_FLAG_BIT_WITH_COUNTERS),
+	IPSET_FLAG_BIT_WITH_COMMENTS = 4,
+	IPSET_FLAG_WITH_COMMENTS = (1 << IPSET_FLAG_BIT_WITH_COMMENTS),
 	IPSET_FLAG_CADT_MAX	= 15,
 };
 
diff --git a/kernel/net/netfilter/ipset/ip_set_core.c b/kernel/net/netfilter/ipset/ip_set_core.c
index a2030ee..84b73cf 100644
--- a/kernel/net/netfilter/ipset/ip_set_core.c
+++ b/kernel/net/netfilter/ipset/ip_set_core.c
@@ -321,6 +321,7 @@ ip_set_get_ipaddr6(struct nlattr *nla, union nf_inet_addr *ipaddr)
 }
 EXPORT_SYMBOL_GPL(ip_set_get_ipaddr6);
 
+typedef void (*destroyer)(void *);
 /* ipset data extension types, in size order */
 
 const struct ip_set_ext_type ip_set_extensions[] = {
@@ -335,6 +336,13 @@ const struct ip_set_ext_type ip_set_extensions[] = {
 		.len	= sizeof(unsigned long),
 		.align	= __alignof__(unsigned long),
 	},
+	[IPSET_EXT_ID_COMMENT] = {
+		.type	 = IPSET_EXT_COMMENT | IPSET_EXT_DESTROY,
+		.flag	 = IPSET_FLAG_WITH_COMMENTS,
+		.len	 = sizeof(struct ip_set_comment),
+		.align	 = __alignof__(struct ip_set_comment),
+		.destroy = (destroyer) ip_set_comment_free,
+	},
 };
 EXPORT_SYMBOL_GPL(ip_set_extensions);
 
@@ -386,6 +394,12 @@ ip_set_get_extensions(struct ip_set *set, struct nlattr *tb[],
 			ext->packets = be64_to_cpu(nla_get_be64(
 						   tb[IPSET_ATTR_PACKETS]));
 	}
+	if(tb[IPSET_ATTR_COMMENT]) {
+		if(!(set->extensions & IPSET_EXT_COMMENT))
+			return -IPSET_ERR_COMMENT;
+		ext->comment = ip_set_comment_uget(tb[IPSET_ATTR_COMMENT]);
+	}
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(ip_set_get_extensions);
-- 
1.8.3.2

Re: [PATCH 1/6] netfilter: ipset: Support comments for ipset entries in the core.

[Jozsef Kadlecsik]

On Tue, 17 Sep 2013, Oliver wrote:

> From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> 
> This adds the core support for having comments on ipset entries.
> 
> The comments are stored as standard null-terminated strings in
> dynamically allocated memory after being passed to the kernel. As a
> result of this, code has been added to the generic destroy function to
> iterate all extensions and call that extension's destroy task if the set
> has that extension activated, and if such a task is defined.
> 
> Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> ---
>  kernel/include/linux/netfilter/ipset/ip_set.h      | 40 ++++++++++++----
>  .../include/linux/netfilter/ipset/ip_set_comment.h | 54 ++++++++++++++++++++++
>  kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  4 ++
>  kernel/net/netfilter/ipset/ip_set_core.c           | 14 ++++++
>  4 files changed, 104 insertions(+), 8 deletions(-)
>  create mode 100644 kernel/include/linux/netfilter/ipset/ip_set_comment.h
> 
> diff --git a/kernel/include/linux/netfilter/ipset/ip_set.h b/kernel/include/linux/netfilter/ipset/ip_set.h
> index c687abb..ee831f3 100644
> --- a/kernel/include/linux/netfilter/ipset/ip_set.h
> +++ b/kernel/include/linux/netfilter/ipset/ip_set.h
> @@ -54,6 +54,8 @@ enum ip_set_extension {
>  	IPSET_EXT_TIMEOUT = (1 << IPSET_EXT_BIT_TIMEOUT),
>  	IPSET_EXT_BIT_COUNTER = 1,
>  	IPSET_EXT_COUNTER = (1 << IPSET_EXT_BIT_COUNTER),
> +	IPSET_EXT_BIT_COMMENT = 2,
> +	IPSET_EXT_COMMENT = (1 << IPSET_EXT_BIT_COMMENT),
>  	/* Mark set with an extension which needs to call destroy */
>  	IPSET_EXT_BIT_DESTROY = 7,
>  	IPSET_EXT_DESTROY = (1 << IPSET_EXT_BIT_DESTROY),
> @@ -61,11 +63,15 @@ enum ip_set_extension {
>  
>  #define SET_WITH_TIMEOUT(s)	((s)->extensions & IPSET_EXT_TIMEOUT)
>  #define SET_WITH_COUNTER(s)	((s)->extensions & IPSET_EXT_COUNTER)
> +#define SET_WITH_COMMENT(s)	((s)->extensions & IPSET_EXT_COMMENT)
> +
> +/* Comment struct */

The comment structure then should come here, no?
  
>  /* Extension id, in size order */
>  enum ip_set_ext_id {
>  	IPSET_EXT_ID_COUNTER = 0,
>  	IPSET_EXT_ID_TIMEOUT,
> +	IPSET_EXT_ID_COMMENT,
>  	IPSET_EXT_ID_MAX,
>  };
>  
> @@ -86,6 +92,7 @@ struct ip_set_ext {
>  	u64 packets;
>  	u64 bytes;
>  	u32 timeout;
> +	char *comment;
>  };
>  
>  struct ip_set_counter {
> @@ -93,20 +100,21 @@ struct ip_set_counter {
>  	atomic64_t packets;
>  };
>  
> -struct ip_set;
> +struct ip_set_comment {
> +	char *str;
> +};
>  
> -static inline void
> -ip_set_ext_destroy(struct ip_set *set, void *data)
> -{
> -	/* Check that the extension is enabled for the set and
> -	 * call it's destroy function for its extension part in data.
> -	 */
> -}
> +struct ip_set;
>  
> +#define ext_generic(e, s, i)	\
> +(void *)(((void *)(e)) + (s)->offset[i])
>  #define ext_timeout(e, s)	\
>  (unsigned long *)(((void *)(e)) + (s)->offset[IPSET_EXT_ID_TIMEOUT])
>  #define ext_counter(e, s)	\
>  (struct ip_set_counter *)(((void *)(e)) + (s)->offset[IPSET_EXT_ID_COUNTER])
> +#define ext_comment(e, s)	\
> +(struct ip_set_comment *)(((void *)(e)) + (s)->offset[IPSET_EXT_ID_COMMENT])
> +
>  
>  typedef int (*ipset_adtfn)(struct ip_set *set, void *value,
>  			   const struct ip_set_ext *ext,
> @@ -224,6 +232,21 @@ struct ip_set {
>  };
>  
>  static inline void
> +ip_set_ext_destroy(struct ip_set *set, void *data)
> +{
> +	u32 id = 0;
> +	/* Check that the extension is enabled for the set and
> +	 * call it's destroy function for its extension part in data.
> +	 */
> +	for (; id < IPSET_EXT_ID_MAX; id++) {
> +		if (!(set->extensions & ip_set_extensions[id].type) ||
> +		    !ip_set_extensions[id].destroy)
> +			continue;
> +		ip_set_extensions[id].destroy(ext_generic(data, set, id));
> +	}
> +}

Originally, when preparing ipset for this kind of extensions, I added the 
same generic function body to ip_set_ext_destroy. However, it means 
wasting two loops unnecessarily when we do exactly know which 
extension type needs the call to the destroy function. So a simple

static inline void
ip_set_ext_destroy(struct ip_set *set, void *data)
{
	if (SET_WITH_COMMENT(set))
		ip_set_extensions[IPSET_EXT_ID_COMMENT].destroy(
			ext_comment(data, set));
}

suffices. Extensions cannot be added without modifying the ipset core.

> +static inline void
>  ip_set_add_bytes(u64 bytes, struct ip_set_counter *counter)
>  {
>  	atomic64_add((long long)bytes, &(counter)->bytes);
> @@ -426,6 +449,7 @@ bitmap_bytes(u32 a, u32 b)
>  }
>  
>  #include <linux/netfilter/ipset/ip_set_timeout.h>
> +#include <linux/netfilter/ipset/ip_set_comment.h>
>  
>  #define IP_SET_INIT_KEXT(skb, opt, set)			\
>  	{ .bytes = (skb)->len, .packets = 1,		\
> diff --git a/kernel/include/linux/netfilter/ipset/ip_set_comment.h b/kernel/include/linux/netfilter/ipset/ip_set_comment.h
> new file mode 100644
> index 0000000..981a41e
> --- /dev/null
> +++ b/kernel/include/linux/netfilter/ipset/ip_set_comment.h
> @@ -0,0 +1,54 @@
> +#ifndef _IP_SET_COMMENT_H
> +#define _IP_SET_COMMENT_H
> +
> +/* Copyright (C) 2013 Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#define IP_SET_MAX_COMMENT_SIZE 255
> +
> +#ifdef __KERNEL__
> +
> +static inline char*
> +ip_set_comment_uget(struct nlattr *tb)
> +{
> +	return nla_data(tb);
> +}
> +
> +static inline void
> +ip_set_init_comment(struct ip_set_comment *comment,
> +		    const struct ip_set_ext *ext)
> +{
> +	size_t len = ext->comment ? strlen(ext->comment) : 0;
> +	if (!len)
> +		return;
> +	if (unlikely(len > IP_SET_MAX_COMMENT_SIZE))
> +		len = IP_SET_MAX_COMMENT_SIZE;
> +	if (unlikely(comment->str))
> +		kfree(comment->str);
> +	comment->str = kzalloc(len + 1, GFP_KERNEL);
> +	strlcpy(comment->str, ext->comment, len + 1);
> +}
>
> +static inline bool
> +ip_set_put_comment(struct sk_buff *skb, struct ip_set_comment *comment)
> +{
> +	if(!comment->str)
> +		return NULL;
> +	return nla_put_string(skb, IPSET_ATTR_COMMENT, comment->str);
> +}
> +
> +static inline void
> +ip_set_comment_free(struct ip_set_comment *comment)
> +{
> +	if(unlikely(!comment->str))
> +		return;
> +	kfree(comment->str);
> +	comment->str = NULL;
> +}
> +
> +#endif
> +#endif
> diff --git a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> index 2b61ac4..98dce6a 100644
> --- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> +++ b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> @@ -110,6 +110,7 @@ enum {
>  	IPSET_ATTR_IFACE,
>  	IPSET_ATTR_BYTES,
>  	IPSET_ATTR_PACKETS,
> +	IPSET_ATTR_COMMENT,
>  	__IPSET_ATTR_ADT_MAX,
>  };
>  #define IPSET_ATTR_ADT_MAX	(__IPSET_ATTR_ADT_MAX - 1)
> @@ -140,6 +141,7 @@ enum ipset_errno {
>  	IPSET_ERR_IPADDR_IPV4,
>  	IPSET_ERR_IPADDR_IPV6,
>  	IPSET_ERR_COUNTER,
> +	IPSET_ERR_COMMENT,
>  
>  	/* Type specific error codes */
>  	IPSET_ERR_TYPE_SPECIFIC = 4352,
> @@ -176,6 +178,8 @@ enum ipset_cadt_flags {
>  	IPSET_FLAG_NOMATCH	= (1 << IPSET_FLAG_BIT_NOMATCH),
>  	IPSET_FLAG_BIT_WITH_COUNTERS = 3,
>  	IPSET_FLAG_WITH_COUNTERS = (1 << IPSET_FLAG_BIT_WITH_COUNTERS),
> +	IPSET_FLAG_BIT_WITH_COMMENTS = 4,
> +	IPSET_FLAG_WITH_COMMENTS = (1 << IPSET_FLAG_BIT_WITH_COMMENTS),
>  	IPSET_FLAG_CADT_MAX	= 15,
>  };

Please use singular "COMMENT" everywhere. The "COMMENT" and "COMMENTS" 
mixed usage lead to confusion (see the userspace). (The counters are 
packet and byte counters, therefore plural.)
  
> diff --git a/kernel/net/netfilter/ipset/ip_set_core.c b/kernel/net/netfilter/ipset/ip_set_core.c
> index a2030ee..84b73cf 100644
> --- a/kernel/net/netfilter/ipset/ip_set_core.c
> +++ b/kernel/net/netfilter/ipset/ip_set_core.c
> @@ -321,6 +321,7 @@ ip_set_get_ipaddr6(struct nlattr *nla, union nf_inet_addr *ipaddr)
>  }
>  EXPORT_SYMBOL_GPL(ip_set_get_ipaddr6);
>  
> +typedef void (*destroyer)(void *);
>  /* ipset data extension types, in size order */
>  
>  const struct ip_set_ext_type ip_set_extensions[] = {
> @@ -335,6 +336,13 @@ const struct ip_set_ext_type ip_set_extensions[] = {
>  		.len	= sizeof(unsigned long),
>  		.align	= __alignof__(unsigned long),
>  	},
> +	[IPSET_EXT_ID_COMMENT] = {
> +		.type	 = IPSET_EXT_COMMENT | IPSET_EXT_DESTROY,
> +		.flag	 = IPSET_FLAG_WITH_COMMENTS,
> +		.len	 = sizeof(struct ip_set_comment),
> +		.align	 = __alignof__(struct ip_set_comment),
> +		.destroy = (destroyer) ip_set_comment_free,
> +	},
>  };
>  EXPORT_SYMBOL_GPL(ip_set_extensions);
>  
> @@ -386,6 +394,12 @@ ip_set_get_extensions(struct ip_set *set, struct nlattr *tb[],
>  			ext->packets = be64_to_cpu(nla_get_be64(
>  						   tb[IPSET_ATTR_PACKETS]));
>  	}
> +	if(tb[IPSET_ATTR_COMMENT]) {
> +		if(!(set->extensions & IPSET_EXT_COMMENT))
> +			return -IPSET_ERR_COMMENT;
> +		ext->comment = ip_set_comment_uget(tb[IPSET_ATTR_COMMENT]);
> +	}
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(ip_set_get_extensions);
> -- 
> 1.8.3.2

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

[PATCH 2/6] netfilter: ipset: Support comments in hash-type ipsets.

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This provides kernel support for creating ipsets with comment support.

This does incur a penalty to flushing/destroying an ipset since all
entries are walked in order to free the allocated strings, this penalty
is of course less expensive than the operation of listing an ipset to
userspace, so for general-purpose usage the overall impact is expected
to be little to none.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 kernel/net/netfilter/ipset/ip_set_hash_gen.h       | 10 +++++++++-
 kernel/net/netfilter/ipset/ip_set_hash_ip.c        |  3 ++-
 kernel/net/netfilter/ipset/ip_set_hash_ipport.c    |  3 ++-
 kernel/net/netfilter/ipset/ip_set_hash_ipportip.c  |  3 ++-
 kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c |  3 ++-
 kernel/net/netfilter/ipset/ip_set_hash_net.c       |  3 ++-
 kernel/net/netfilter/ipset/ip_set_hash_netiface.c  |  3 ++-
 kernel/net/netfilter/ipset/ip_set_hash_netport.c   |  3 ++-
 8 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
index 4098edc..193aac9 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
@@ -710,6 +710,8 @@ reuse_slot:
 		ip_set_timeout_set(ext_timeout(data, set), ext->timeout);
 	if (SET_WITH_COUNTER(set))
 		ip_set_init_counter(ext_counter(data, set), ext);
+	if(SET_WITH_COMMENT(set))
+		ip_set_init_comment(ext_comment(data, set), ext);
 
 out:
 	rcu_read_unlock_bh();
@@ -929,7 +931,10 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	     nla_put_net32(skb, IPSET_ATTR_TIMEOUT, htonl(set->timeout))) ||
 	    ((set->extensions & IPSET_EXT_COUNTER) &&
 	     nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
-			   htonl(IPSET_FLAG_WITH_COUNTERS))))
+			   htonl(IPSET_FLAG_WITH_COUNTERS))) ||
+	    ((set->extensions & IPSET_EXT_COMMENT) &&
+	     nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
+			   htonl(IPSET_FLAG_WITH_COMMENTS))))
 		goto nla_put_failure;
 	ipset_nest_end(skb, nested);
 
@@ -986,6 +991,9 @@ mtype_list(const struct ip_set *set,
 			if (SET_WITH_COUNTER(set) &&
 			    ip_set_put_counter(skb, ext_counter(e, set)))
 				goto nla_put_failure;
+			if (SET_WITH_COMMENT(set) &&
+			    ip_set_put_comment(skb, ext_comment(e, set)))
+				goto nla_put_failure;
 			ipset_nest_end(skb, nested);
 		}
 	}
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ip.c b/kernel/net/netfilter/ipset/ip_set_hash_ip.c
index a111ffe..da2433d 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_ip.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_ip.c
@@ -24,7 +24,8 @@
 #include <linux/netfilter/ipset/ip_set_hash.h>
 
 #define IPSET_TYPE_REV_MIN	0
-#define IPSET_TYPE_REV_MAX	1	/* Counters support */
+/*				1	   Counters support */
+#define IPSET_TYPE_REV_MAX	2	/* Comments support */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c
index 5dc735c..c7a9083 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c
@@ -26,7 +26,8 @@
 
 #define IPSET_TYPE_REV_MIN	0
 /*				1    SCTP and UDPLITE support added */
-#define IPSET_TYPE_REV_MAX	2 /* Counters support added */
+/*				2    Counters support added */
+#define IPSET_TYPE_REV_MAX	3 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c
index 8c43dc7..cb17d9a 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c
@@ -26,7 +26,8 @@
 
 #define IPSET_TYPE_REV_MIN	0
 /*				1    SCTP and UDPLITE support added */
-#define IPSET_TYPE_REV_MAX	2 /* Counters support added */
+/*				2    Counters support added */
+#define IPSET_TYPE_REV_MAX	3 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c
index 3489045..071aed7 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c
@@ -28,7 +28,8 @@
 /*				1    SCTP and UDPLITE support added */
 /*				2    Range as input support for IPv4 added */
 /*				3    nomatch flag support added */
-#define IPSET_TYPE_REV_MAX	4 /* Counters support added */
+/*				4    Counters support added */
+#define IPSET_TYPE_REV_MAX	5 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_net.c b/kernel/net/netfilter/ipset/ip_set_hash_net.c
index d559855..7ff21b9 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_net.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_net.c
@@ -25,7 +25,8 @@
 #define IPSET_TYPE_REV_MIN	0
 /*				1    Range as input support for IPv4 added */
 /*				2    nomatch flag support added */
-#define IPSET_TYPE_REV_MAX	3 /* Counters support added */
+/*				3    Counters support added */
+#define IPSET_TYPE_REV_MAX	4 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c
index 26703e9..fb49cb5 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c
@@ -26,7 +26,8 @@
 #define IPSET_TYPE_REV_MIN	0
 /*				1    nomatch flag support added */
 /*				2    /0 support added */
-#define IPSET_TYPE_REV_MAX	3 /* Counters support added */
+/*				3    Counters support added */
+#define IPSET_TYPE_REV_MAX	4 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netport.c b/kernel/net/netfilter/ipset/ip_set_hash_netport.c
index 45b6e91..e3e6fd8 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_netport.c
+++ b/kernel/net/netfilter/ipset/ip_set_hash_netport.c
@@ -27,7 +27,8 @@
 /*				1    SCTP and UDPLITE support added */
 /*				2    Range as input support for IPv4 added */
 /*				3    nomatch flag support added */
-#define IPSET_TYPE_REV_MAX	4 /* Counters support added */
+/*				4    Counters support added */
+#define IPSET_TYPE_REV_MAX	5 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
-- 
1.8.3.2

Re: [PATCH 2/6] netfilter: ipset: Support comments in hash-type ipsets.

[Jozsef Kadlecsik]

On Tue, 17 Sep 2013, Oliver wrote:

> From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> 
> This provides kernel support for creating ipsets with comment support.
> 
> This does incur a penalty to flushing/destroying an ipset since all
> entries are walked in order to free the allocated strings, this penalty
> is of course less expensive than the operation of listing an ipset to
> userspace, so for general-purpose usage the overall impact is expected
> to be little to none.
> 
> Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> ---
>  kernel/net/netfilter/ipset/ip_set_hash_gen.h       | 10 +++++++++-
>  kernel/net/netfilter/ipset/ip_set_hash_ip.c        |  3 ++-
>  kernel/net/netfilter/ipset/ip_set_hash_ipport.c    |  3 ++-
>  kernel/net/netfilter/ipset/ip_set_hash_ipportip.c  |  3 ++-
>  kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c |  3 ++-
>  kernel/net/netfilter/ipset/ip_set_hash_net.c       |  3 ++-
>  kernel/net/netfilter/ipset/ip_set_hash_netiface.c  |  3 ++-
>  kernel/net/netfilter/ipset/ip_set_hash_netport.c   |  3 ++-
>  8 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
> index 4098edc..193aac9 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -710,6 +710,8 @@ reuse_slot:
>  		ip_set_timeout_set(ext_timeout(data, set), ext->timeout);
>  	if (SET_WITH_COUNTER(set))
>  		ip_set_init_counter(ext_counter(data, set), ext);
> +	if(SET_WITH_COMMENT(set))
> +		ip_set_init_comment(ext_comment(data, set), ext);
>  
>  out:
>  	rcu_read_unlock_bh();
> @@ -929,7 +931,10 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
>  	     nla_put_net32(skb, IPSET_ATTR_TIMEOUT, htonl(set->timeout))) ||
>  	    ((set->extensions & IPSET_EXT_COUNTER) &&
>  	     nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
> -			   htonl(IPSET_FLAG_WITH_COUNTERS))))
> +			   htonl(IPSET_FLAG_WITH_COUNTERS))) ||
> +	    ((set->extensions & IPSET_EXT_COMMENT) &&
> +	     nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
> +			   htonl(IPSET_FLAG_WITH_COMMENTS))))
>  		goto nla_put_failure;
>  	ipset_nest_end(skb, nested);

The protocol doesn't support returning the IPSET_ATTR_CADT_FLAGS attribute 
multiple times. Initialize the flag and send if not zero, like in the 
*_data_list functions of the hash:*net* types.

The same applies to the bitmap types.

As I see, the extension is missing for the list:set type, please add it 
there too.
  
> @@ -986,6 +991,9 @@ mtype_list(const struct ip_set *set,
>  			if (SET_WITH_COUNTER(set) &&
>  			    ip_set_put_counter(skb, ext_counter(e, set)))
>  				goto nla_put_failure;
> +			if (SET_WITH_COMMENT(set) &&
> +			    ip_set_put_comment(skb, ext_comment(e, set)))
> +				goto nla_put_failure;
>  			ipset_nest_end(skb, nested);
>  		}
>  	}
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ip.c b/kernel/net/netfilter/ipset/ip_set_hash_ip.c
> index a111ffe..da2433d 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_ip.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_ip.c
> @@ -24,7 +24,8 @@
>  #include <linux/netfilter/ipset/ip_set_hash.h>
>  
>  #define IPSET_TYPE_REV_MIN	0
> -#define IPSET_TYPE_REV_MAX	1	/* Counters support */
> +/*				1	   Counters support */
> +#define IPSET_TYPE_REV_MAX	2	/* Comments support */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c
> index 5dc735c..c7a9083 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_ipport.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipport.c
> @@ -26,7 +26,8 @@
>  
>  #define IPSET_TYPE_REV_MIN	0
>  /*				1    SCTP and UDPLITE support added */
> -#define IPSET_TYPE_REV_MAX	2 /* Counters support added */
> +/*				2    Counters support added */
> +#define IPSET_TYPE_REV_MAX	3 /* Comments support added */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c
> index 8c43dc7..cb17d9a 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportip.c
> @@ -26,7 +26,8 @@
>  
>  #define IPSET_TYPE_REV_MIN	0
>  /*				1    SCTP and UDPLITE support added */
> -#define IPSET_TYPE_REV_MAX	2 /* Counters support added */
> +/*				2    Counters support added */
> +#define IPSET_TYPE_REV_MAX	3 /* Comments support added */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c
> index 3489045..071aed7 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_ipportnet.c
> @@ -28,7 +28,8 @@
>  /*				1    SCTP and UDPLITE support added */
>  /*				2    Range as input support for IPv4 added */
>  /*				3    nomatch flag support added */
> -#define IPSET_TYPE_REV_MAX	4 /* Counters support added */
> +/*				4    Counters support added */
> +#define IPSET_TYPE_REV_MAX	5 /* Comments support added */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_net.c b/kernel/net/netfilter/ipset/ip_set_hash_net.c
> index d559855..7ff21b9 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_net.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_net.c
> @@ -25,7 +25,8 @@
>  #define IPSET_TYPE_REV_MIN	0
>  /*				1    Range as input support for IPv4 added */
>  /*				2    nomatch flag support added */
> -#define IPSET_TYPE_REV_MAX	3 /* Counters support added */
> +/*				3    Counters support added */
> +#define IPSET_TYPE_REV_MAX	4 /* Comments support added */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c
> index 26703e9..fb49cb5 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_netiface.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_netiface.c
> @@ -26,7 +26,8 @@
>  #define IPSET_TYPE_REV_MIN	0
>  /*				1    nomatch flag support added */
>  /*				2    /0 support added */
> -#define IPSET_TYPE_REV_MAX	3 /* Counters support added */
> +/*				3    Counters support added */
> +#define IPSET_TYPE_REV_MAX	4 /* Comments support added */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_netport.c b/kernel/net/netfilter/ipset/ip_set_hash_netport.c
> index 45b6e91..e3e6fd8 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_netport.c
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_netport.c
> @@ -27,7 +27,8 @@
>  /*				1    SCTP and UDPLITE support added */
>  /*				2    Range as input support for IPv4 added */
>  /*				3    nomatch flag support added */
> -#define IPSET_TYPE_REV_MAX	4 /* Counters support added */
> +/*				4    Counters support added */
> +#define IPSET_TYPE_REV_MAX	5 /* Comments support added */
>  
>  MODULE_LICENSE("GPL");
>  MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
> -- 
> 1.8.3.2

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

[PATCH 3/6] netfilter: ipset: Support comments in bitmap-type ipsets.

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This provides kernel support for creating bitmap ipsets with comment
support.

As is the case for hashes, this incurs a penalty when flushing or
destroying the entire ipset as the entries must first be walked in order
to free the comment strings. This penalty is of course far less than the
cost of listing an ipset to userspace. Any set created without support
for comments will be flushed/destroyed as before.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 kernel/net/netfilter/ipset/ip_set_bitmap_gen.h   | 10 +++++++++-
 kernel/net/netfilter/ipset/ip_set_bitmap_ip.c    |  3 ++-
 kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c |  3 ++-
 kernel/net/netfilter/ipset/ip_set_bitmap_port.c  |  3 ++-
 4 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
index 4515fe8..99d7683 100644
--- a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -106,7 +106,10 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	     nla_put_net32(skb, IPSET_ATTR_TIMEOUT, htonl(set->timeout))) ||
 	    (SET_WITH_COUNTER(set) &&
 	     nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
-			   htonl(IPSET_FLAG_WITH_COUNTERS))))
+			   htonl(IPSET_FLAG_WITH_COUNTERS))) ||
+	    (SET_WITH_COMMENT(set) &&
+	     nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
+			   htonl(IPSET_FLAG_WITH_COMMENTS))))
 		goto nla_put_failure;
 	ipset_nest_end(skb, nested);
 
@@ -162,6 +165,8 @@ mtype_add(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 
 	if (SET_WITH_COUNTER(set))
 		ip_set_init_counter(ext_counter(x, set), ext);
+	if (SET_WITH_COMMENT(set))
+		ip_set_init_comment(ext_comment(x, set), ext);
 	return 0;
 }
 
@@ -233,6 +238,9 @@ mtype_list(const struct ip_set *set,
 		if (SET_WITH_COUNTER(set) &&
 		    ip_set_put_counter(skb, ext_counter(x, set)))
 			goto nla_put_failure;
+		if (SET_WITH_COMMENT(set) &&
+		    ip_set_put_comment(skb, ext_comment(x, set)))
+			goto nla_put_failure;
 		ipset_nest_end(skb, nested);
 	}
 	ipset_nest_end(skb, adt);
diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c
index 94d9854..4d49b1c 100644
--- a/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c
+++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ip.c
@@ -26,7 +26,8 @@
 #include <linux/netfilter/ipset/ip_set_bitmap.h>
 
 #define IPSET_TYPE_REV_MIN	0
-#define IPSET_TYPE_REV_MAX	1	/* Counter support added */
+/*				1	   Counter support added */
+#define IPSET_TYPE_REV_MAX	2	/* Comment support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
index 654a97b..1d81f02 100644
--- a/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
+++ b/kernel/net/netfilter/ipset/ip_set_bitmap_ipmac.c
@@ -26,7 +26,8 @@
 #include <linux/netfilter/ipset/ip_set_bitmap.h>
 
 #define IPSET_TYPE_REV_MIN	0
-#define IPSET_TYPE_REV_MAX	1	/* Counter support added */
+/*				1	   Counter support added */
+#define IPSET_TYPE_REV_MAX	2	/* Comment support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
diff --git a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c
index 1ef2f31..3cff821 100644
--- a/kernel/net/netfilter/ipset/ip_set_bitmap_port.c
+++ b/kernel/net/netfilter/ipset/ip_set_bitmap_port.c
@@ -21,7 +21,8 @@
 #include <linux/netfilter/ipset/ip_set_getport.h>
 
 #define IPSET_TYPE_REV_MIN	0
-#define IPSET_TYPE_REV_MAX	1	/* Counter support added */
+/*				1	   Counter support added */
+#define IPSET_TYPE_REV_MAX	2	/* Comment support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
-- 
1.8.3.2

[PATCH 4/6] ipset: Rework the "fake" argument parsing for ipset restore.

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This reworks the argument parsing functionality of ipset to handle
quote-delimited lines in such a way that they are considered to be a
single argument.

This commit is necessary for ipset to successfully restore sets that
have comments.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 src/ipset.c | 40 +++++++++++++++++++++++++++++++++-------
 1 file changed, 33 insertions(+), 7 deletions(-)

diff --git a/src/ipset.c b/src/ipset.c
index 4f308da..d9d609c 100644
--- a/src/ipset.c
+++ b/src/ipset.c
@@ -159,20 +159,44 @@ ipset_print_file(const char *fmt, ...)
 static void
 build_argv(char *buffer)
 {
-	char *ptr;
+	char *ptr, *tmp;
 	int i;
 
 	/* Reset */
-	for (i = 1; i < newargc; i++)
+	for (i = 1; i < newargc; i++) {
+		if(newargv[i])
+			free(newargv[i]);
 		newargv[i] = NULL;
+	}
 	newargc = 1;
 
 	ptr = strtok(buffer, " \t\r\n");
-	newargv[newargc++] = ptr;
+	newargv[newargc] = calloc(strlen(ptr) + 1, sizeof(*ptr));
+	ipset_strlcpy(newargv[newargc++], ptr, strlen(ptr) + 1);
 	while ((ptr = strtok(NULL, " \t\r\n")) != NULL) {
-		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *)))
-			newargv[newargc++] = ptr;
-		else {
+		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *))) {
+			tmp = newargv[newargc] = calloc(strlen(ptr) + 1,
+							sizeof(*ptr));
+			if(*ptr == '"') {
+				ipset_strlcpy(tmp, ptr + 1, (strlen(ptr) + 1) *
+					      sizeof(*ptr));
+				ipset_strlcat(tmp, " ", (strlen(ptr) + 1) *
+					      sizeof(*ptr));
+				while((ptr = strtok(NULL, "\"")) != NULL) {
+					if(*ptr == '\n' || *ptr == '\r')
+						continue;
+					tmp = realloc(tmp, (strlen(tmp) +
+						      strlen(ptr) + 1) *
+						      sizeof(*ptr));
+					ipset_strlcat(tmp, ptr, (strlen(tmp) +
+						      strlen(ptr) + 1) *
+						      sizeof(*ptr));
+				}
+			} else
+				ipset_strlcpy(tmp, ptr, (strlen(ptr) + 1) *
+					      sizeof(*ptr));
+			newargv[newargc++] = tmp;
+		} else {
 			exit_error(PARAMETER_PROBLEM,
 				   "Line is too long to parse.");
 			return;
@@ -195,7 +219,8 @@ restore(char *argv0)
 
 	/* Initialize newargv/newargc */
 	newargc = 0;
-	newargv[newargc++] = argv0;
+	newargv[newargc] = calloc(strlen(argv0) + 1, sizeof(*argv0));
+	ipset_strlcpy(newargv[newargc++], argv0, strlen(argv0) + 1);
 	if (filename) {
 		fd = fopen(filename, "r");
 		if (!fd) {
@@ -232,6 +257,7 @@ restore(char *argv0)
 	if (ret < 0)
 		handle_error();
 
+	free(newargv[0]);
 	return ret;
 }
 
-- 
1.8.3.2

Re: [PATCH 4/6] ipset: Rework the "fake" argument parsing for ipset restore.

[Jozsef Kadlecsik]

On Tue, 17 Sep 2013, Oliver wrote:

> From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> 
> This reworks the argument parsing functionality of ipset to handle
> quote-delimited lines in such a way that they are considered to be a
> single argument.
> 
> This commit is necessary for ipset to successfully restore sets that
> have comments.
> 
> Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> ---
>  src/ipset.c | 40 +++++++++++++++++++++++++++++++++-------
>  1 file changed, 33 insertions(+), 7 deletions(-)
> 
> diff --git a/src/ipset.c b/src/ipset.c
> index 4f308da..d9d609c 100644
> --- a/src/ipset.c
> +++ b/src/ipset.c
> @@ -159,20 +159,44 @@ ipset_print_file(const char *fmt, ...)
>  static void
>  build_argv(char *buffer)
>  {
> -	char *ptr;
> +	char *ptr, *tmp;
>  	int i;
>  
>  	/* Reset */
> -	for (i = 1; i < newargc; i++)
> +	for (i = 1; i < newargc; i++) {
> +		if(newargv[i])
> +			free(newargv[i]);
>  		newargv[i] = NULL;
> +	}
>  	newargc = 1;
>  
>  	ptr = strtok(buffer, " \t\r\n");
> -	newargv[newargc++] = ptr;
> +	newargv[newargc] = calloc(strlen(ptr) + 1, sizeof(*ptr));
> +	ipset_strlcpy(newargv[newargc++], ptr, strlen(ptr) + 1);
>  	while ((ptr = strtok(NULL, " \t\r\n")) != NULL) {
> -		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *)))
> -			newargv[newargc++] = ptr;
> -		else {
> +		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *))) {
> +			tmp = newargv[newargc] = calloc(strlen(ptr) + 1,
> +							sizeof(*ptr));
> +			if(*ptr == '"') {
> +				ipset_strlcpy(tmp, ptr + 1, (strlen(ptr) + 1) *
> +					      sizeof(*ptr));
> +				ipset_strlcat(tmp, " ", (strlen(ptr) + 1) *
> +					      sizeof(*ptr));
> +				while((ptr = strtok(NULL, "\"")) != NULL) {
> +					if(*ptr == '\n' || *ptr == '\r')
> +						continue;
> +					tmp = realloc(tmp, (strlen(tmp) +
> +						      strlen(ptr) + 1) *
> +						      sizeof(*ptr));
> +					ipset_strlcat(tmp, ptr, (strlen(tmp) +
> +						      strlen(ptr) + 1) *
> +						      sizeof(*ptr));
> +				}

Why is there the inside while loop? We look for a single closing quote. 
Also, I'd better like to see an error reported if the closing quote is 
missing instead of silently accepting it.

> +			} else
> +				ipset_strlcpy(tmp, ptr, (strlen(ptr) + 1) *
> +					      sizeof(*ptr));
> +			newargv[newargc++] = tmp;
> +		} else {
>  			exit_error(PARAMETER_PROBLEM,
>  				   "Line is too long to parse.");
>  			return;
> @@ -195,7 +219,8 @@ restore(char *argv0)
>  
>  	/* Initialize newargv/newargc */
>  	newargc = 0;
> -	newargv[newargc++] = argv0;
> +	newargv[newargc] = calloc(strlen(argv0) + 1, sizeof(*argv0));
> +	ipset_strlcpy(newargv[newargc++], argv0, strlen(argv0) + 1);
>  	if (filename) {
>  		fd = fopen(filename, "r");
>  		if (!fd) {
> @@ -232,6 +257,7 @@ restore(char *argv0)
>  	if (ret < 0)
>  		handle_error();
>  
> +	free(newargv[0]);
>  	return ret;
>  }
>  
> -- 
> 1.8.3.2

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 4/6] ipset: Rework the "fake" argument parsing for ipset restore.

[Oliver]

On Wednesday 18 September 2013 20:22:41 Jozsef Kadlecsik wrote:
> On Tue, 17 Sep 2013, Oliver wrote:
<snip>
> > -	newargv[newargc++] = ptr;
> > +	newargv[newargc] = calloc(strlen(ptr) + 1, sizeof(*ptr));
> > +	ipset_strlcpy(newargv[newargc++], ptr, strlen(ptr) + 1);
> > 
> >  	while ((ptr = strtok(NULL, " \t\r\n")) != NULL) {
> > 
> > -		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *)))
> > -			newargv[newargc++] = ptr;
> > -		else {
> > +		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *))) {
> > +			tmp = newargv[newargc] = calloc(strlen(ptr) + 1,
> > +							sizeof(*ptr));
> > +			if(*ptr == '"') {
> > +				ipset_strlcpy(tmp, ptr + 1, (strlen(ptr) + 1) *
> > +					      sizeof(*ptr));
> > +				ipset_strlcat(tmp, " ", (strlen(ptr) + 1) *
> > +					      sizeof(*ptr));
> > +				while((ptr = strtok(NULL, "\"")) != NULL) {
> > +					if(*ptr == '\n' || *ptr == '\r')
> > +						continue;
> > +					tmp = realloc(tmp, (strlen(tmp) +
> > +						      strlen(ptr) + 1) *
> > +						      sizeof(*ptr));
> > +					ipset_strlcat(tmp, ptr, (strlen(tmp) +
> > +						      strlen(ptr) + 1) *
> > +						      sizeof(*ptr));
> > +				}
> 
> Why is there the inside while loop? We look for a single closing quote.
> Also, I'd better like to see an error reported if the closing quote is
> missing instead of silently accepting it.
So, I've come to the conclusion that is is all horribly ugly and strtok is 
more of a hinderance than a help here, so... I've redone it in a very similar 
vein to that of the iptables_restore code.

Kind Regards,
Oliver

[PATCH 5/6] ipset: Support comments in the userspace library.

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This adds support to the userspace portion of ipset for handling ipsets
with the comment extension enabled. The library revision has been raised
accordingly.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 Make_global.am                  |  2 +-
 include/libipset/data.h         |  6 +++++-
 include/libipset/linux_ip_set.h |  7 +++++++
 include/libipset/parse.h        |  2 ++
 include/libipset/print.h        |  3 +++
 lib/data.c                      | 34 ++++++++++++++++++++++++++++++++++
 lib/debug.c                     |  1 +
 lib/errcode.c                   |  2 ++
 lib/libipset.map                |  6 ++++++
 lib/parse.c                     | 27 +++++++++++++++++++++++++++
 lib/print.c                     | 31 +++++++++++++++++++++++++++++++
 lib/session.c                   |  8 +++++++-
 lib/types.c                     |  4 ++--
 13 files changed, 128 insertions(+), 5 deletions(-)

diff --git a/Make_global.am b/Make_global.am
index 29b5678..9c228cc 100644
--- a/Make_global.am
+++ b/Make_global.am
@@ -69,7 +69,7 @@
 # interface. 
 
 #            curr:rev:age
-LIBVERSION = 4:0:1
+LIBVERSION = 4:1:2
 
 AM_CPPFLAGS = $(kinclude_CFLAGS) $(all_includes) -I$(top_srcdir)/include \
 	-I/usr/local/include
diff --git a/include/libipset/data.h b/include/libipset/data.h
index 2b6b8cd..ae0d099 100644
--- a/include/libipset/data.h
+++ b/include/libipset/data.h
@@ -57,6 +57,8 @@ enum ipset_opt {
 	IPSET_OPT_COUNTERS,
 	IPSET_OPT_PACKETS,
 	IPSET_OPT_BYTES,
+	IPSET_OPT_COMMENTS,
+	IPSET_OPT_COMMENT,
 	/* Internal options */
 	IPSET_OPT_FLAGS = 48,	/* IPSET_FLAG_EXIST| */
 	IPSET_OPT_CADT_FLAGS,	/* IPSET_FLAG_BEFORE| */
@@ -106,11 +108,13 @@ enum ipset_opt {
 	| IPSET_FLAG(IPSET_OPT_CADT_FLAGS)\
 	| IPSET_FLAG(IPSET_OPT_BEFORE) \
 	| IPSET_FLAG(IPSET_OPT_PHYSDEV) \
-	| IPSET_FLAG(IPSET_OPT_NOMATCH))
+	| IPSET_FLAG(IPSET_OPT_NOMATCH) \
+	| IPSET_FLAG(IPSET_OPT_COMMENT))
 
 struct ipset_data;
 
 extern void ipset_strlcpy(char *dst, const char *src, size_t len);
+extern void ipset_strlcat(char *dst, const char *src, size_t len);
 extern bool ipset_data_flags_test(const struct ipset_data *data,
 				  uint64_t flags);
 extern void ipset_data_flags_set(struct ipset_data *data, uint64_t flags);
diff --git a/include/libipset/linux_ip_set.h b/include/libipset/linux_ip_set.h
index 8024cdf..2278a4e 100644
--- a/include/libipset/linux_ip_set.h
+++ b/include/libipset/linux_ip_set.h
@@ -19,6 +19,9 @@
 /* The max length of strings including NUL: set and type identifiers */
 #define IPSET_MAXNAMELEN	32
 
+/* The maximum permissible length we will accept over netlink (inc. comments) */
+#define IPSET_MAXSTRLEN		255
+
 /* Message types and commands */
 enum ipset_cmd {
 	IPSET_CMD_NONE,
@@ -110,6 +113,7 @@ enum {
 	IPSET_ATTR_IFACE,
 	IPSET_ATTR_BYTES,
 	IPSET_ATTR_PACKETS,
+	IPSET_ATTR_COMMENT,
 	__IPSET_ATTR_ADT_MAX,
 };
 #define IPSET_ATTR_ADT_MAX	(__IPSET_ATTR_ADT_MAX - 1)
@@ -140,6 +144,7 @@ enum ipset_errno {
 	IPSET_ERR_IPADDR_IPV4,
 	IPSET_ERR_IPADDR_IPV6,
 	IPSET_ERR_COUNTER,
+	IPSET_ERR_COMMENT,
 
 	/* Type specific error codes */
 	IPSET_ERR_TYPE_SPECIFIC = 4352,
@@ -176,6 +181,8 @@ enum ipset_cadt_flags {
 	IPSET_FLAG_NOMATCH	= (1 << IPSET_FLAG_BIT_NOMATCH),
 	IPSET_FLAG_BIT_WITH_COUNTERS = 3,
 	IPSET_FLAG_WITH_COUNTERS = (1 << IPSET_FLAG_BIT_WITH_COUNTERS),
+	IPSET_FLAG_BIT_WITH_COMMENTS = 4,
+	IPSET_FLAG_WITH_COMMENTS = (1 << IPSET_FLAG_BIT_WITH_COMMENTS),
 	IPSET_FLAG_CADT_MAX	= 15,
 };
 
diff --git a/include/libipset/parse.h b/include/libipset/parse.h
index 014c62f..5c46a88 100644
--- a/include/libipset/parse.h
+++ b/include/libipset/parse.h
@@ -90,6 +90,8 @@ extern int ipset_parse_typename(struct ipset_session *session,
 				enum ipset_opt opt, const char *str);
 extern int ipset_parse_iface(struct ipset_session *session,
 			     enum ipset_opt opt, const char *str);
+extern int ipset_parse_comment(struct ipset_session *session,
+			     enum ipset_opt opt, const char *str);
 extern int ipset_parse_output(struct ipset_session *session,
 			      int opt, const char *str);
 extern int ipset_parse_ignored(struct ipset_session *session,
diff --git a/include/libipset/print.h b/include/libipset/print.h
index 1d537bd..f2a6095 100644
--- a/include/libipset/print.h
+++ b/include/libipset/print.h
@@ -40,6 +40,9 @@ extern int ipset_print_port(char *buf, unsigned int len,
 extern int ipset_print_iface(char *buf, unsigned int len,
 			     const struct ipset_data *data,
 			     enum ipset_opt opt, uint8_t env);
+extern int ipset_print_comment(char *buf, unsigned int len,
+			     const struct ipset_data *data,
+			     enum ipset_opt opt, uint8_t env);
 extern int ipset_print_proto(char *buf, unsigned int len,
 			     const struct ipset_data *data,
 			     enum ipset_opt opt, uint8_t env);
diff --git a/lib/data.c b/lib/data.c
index 04a5997..9aaa550 100644
--- a/lib/data.c
+++ b/lib/data.c
@@ -75,6 +75,7 @@ struct ipset_data {
 			char iface[IFNAMSIZ];
 			uint64_t packets;
 			uint64_t bytes;
+			char comment[IPSET_MAXSTRLEN];
 		} adt;
 	};
 };
@@ -108,6 +109,25 @@ ipset_strlcpy(char *dst, const char *src, size_t len)
 }
 
 /**
+ * ipset_strlcat - concatenate the string from src to the end of dst
+ * @dst: the target string buffer
+ * @src: the source string buffer
+ * @len: the length of bytes to concat, including the terminating null byte.
+ *
+ * Cooncatenate the string in src to destination, but at most len bytes are
+ * copied. The target is unconditionally terminated by the null byte.
+ */
+void
+ipset_strlcat(char *dst, const char *src, size_t len)
+{
+	assert(dst);
+	assert(src);
+
+	strncat(dst, src, len);
+	dst[len - 1] = '\0';
+}
+
+/**
  * ipset_data_flags_test - test option bits in the data blob
  * @data: data blob
  * @flags: the option flags to test
@@ -278,6 +298,9 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt opt, const void *value)
 	case IPSET_OPT_COUNTERS:
 		cadt_flag_type_attr(data, opt, IPSET_FLAG_WITH_COUNTERS);
 		break;
+	case IPSET_OPT_COMMENTS:
+		cadt_flag_type_attr(data, opt, IPSET_FLAG_WITH_COMMENTS);
+		break;
 	/* Create-specific options, filled out by the kernel */
 	case IPSET_OPT_ELEMENTS:
 		data->create.elements = *(const uint32_t *) value;
@@ -336,6 +359,9 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt opt, const void *value)
 	case IPSET_OPT_BYTES:
 		data->adt.bytes = *(const uint64_t *) value;
 		break;
+	case IPSET_OPT_COMMENT:
+		ipset_strlcpy(data->adt.comment, value, IPSET_MAXSTRLEN);
+		break;
 	/* Swap/rename */
 	case IPSET_OPT_SETNAME2:
 		ipset_strlcpy(data->setname2, value, IPSET_MAXNAMELEN);
@@ -370,6 +396,9 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt opt, const void *value)
 		if (data->cadt_flags & IPSET_FLAG_WITH_COUNTERS)
 			ipset_data_flags_set(data,
 					     IPSET_FLAG(IPSET_OPT_COUNTERS));
+		if (data->cadt_flags & IPSET_FLAG_WITH_COMMENTS)
+			ipset_data_flags_set(data,
+					     IPSET_FLAG(IPSET_OPT_COMMENTS));
 		break;
 	default:
 		return -1;
@@ -472,6 +501,8 @@ ipset_data_get(const struct ipset_data *data, enum ipset_opt opt)
 		return &data->adt.packets;
 	case IPSET_OPT_BYTES:
 		return &data->adt.bytes;
+	case IPSET_OPT_COMMENT:
+		return &data->adt.comment;
 	/* Swap/rename */
 	case IPSET_OPT_SETNAME2:
 		return data->setname2;
@@ -484,6 +515,7 @@ ipset_data_get(const struct ipset_data *data, enum ipset_opt opt)
 	case IPSET_OPT_PHYSDEV:
 	case IPSET_OPT_NOMATCH:
 	case IPSET_OPT_COUNTERS:
+	case IPSET_OPT_COMMENTS:
 		return &data->cadt_flags;
 	default:
 		return NULL;
@@ -543,6 +575,8 @@ ipset_data_sizeof(enum ipset_opt opt, uint8_t family)
 	case IPSET_OPT_NOMATCH:
 	case IPSET_OPT_COUNTERS:
 		return sizeof(uint32_t);
+	case IPSET_OPT_COMMENT:
+		return IPSET_MAXSTRLEN;
 	default:
 		return 0;
 	};
diff --git a/lib/debug.c b/lib/debug.c
index 3aa5a99..a204940 100644
--- a/lib/debug.c
+++ b/lib/debug.c
@@ -64,6 +64,7 @@ static const struct ipset_attrname adtattr2name[] = {
 	[IPSET_ATTR_CIDR2]	= { .name = "CIDR2" },
 	[IPSET_ATTR_IP2_TO]	= { .name = "IP2_TO" },
 	[IPSET_ATTR_IFACE]	= { .name = "IFACE" },
+	[IPSET_ATTR_COMMENT]	= { .name = "COMMENT" },
 };
 
 static void
diff --git a/lib/errcode.c b/lib/errcode.c
index c939949..c824cc3 100644
--- a/lib/errcode.c
+++ b/lib/errcode.c
@@ -72,6 +72,8 @@ static const struct ipset_errcode_table core_errcode_table[] = {
 	  "An IPv6 address is expected, but not received" },
 	{ IPSET_ERR_COUNTER, 0,
 	  "Packet/byte counters cannot be used: set was created without counter support" },
+	{ IPSET_ERR_COMMENT, 0,
+	  "Comment too long!" },
 
 	/* ADD specific error codes */
 	{ IPSET_ERR_EXIST, IPSET_CMD_ADD,
diff --git a/lib/libipset.map b/lib/libipset.map
index 271fe59..06216e7 100644
--- a/lib/libipset.map
+++ b/lib/libipset.map
@@ -127,3 +127,9 @@ LIBIPSET_4.0 {
 global:
   ipset_parse_uint64;
 } LIBIPSET_3.0;
+
+LIBIPSET_4.1 {
+global:
+  ipset_parse_comment;
+  ipset_strlcat;
+} LIBIPSET_4.0;
diff --git a/lib/parse.c b/lib/parse.c
index 112b273..bde56b3 100644
--- a/lib/parse.c
+++ b/lib/parse.c
@@ -1739,6 +1739,33 @@ ipset_parse_iface(struct ipset_session *session,
 }
 
 /**
+ * ipset_parse_comment - parse string as a comment
+ * @session: session structure
+ * @opt: option kind of the data
+ * @str: string to parse
+ *
+ * Parse string for use as a comment on an ipset entry.
+ * Gets stored in the data blob as usual.
+ *
+ * Returns 0 on success or a negative error code.
+ */
+int ipset_parse_comment(struct ipset_session *session,
+		       enum ipset_opt opt, const char *str)
+{
+	struct ipset_data *data;
+
+	assert(session);
+	assert(opt == IPSET_OPT_COMMENT);
+	assert(str);
+
+	data = ipset_session_data(session);
+	if (strlen(str) > IPSET_MAXSTRLEN)
+		return syntax_err("comment is longer than the maximum permitted");
+
+	return ipset_data_set(data, opt, str);
+}
+
+/**
  * ipset_parse_output - parse output format name
  * @session: session structure
  * @opt: option kind of the data
diff --git a/lib/print.c b/lib/print.c
index 86a7674..9ccc3c8 100644
--- a/lib/print.c
+++ b/lib/print.c
@@ -530,6 +530,37 @@ ipset_print_iface(char *buf, unsigned int len,
 }
 
 /**
+ * ipset_print_comment - print arbitrary parameter string
+ * @buf: printing buffer
+ * @len: length of available buffer space
+ * @data: data blob
+ * @opt: the option kind
+ * @env: environment flags
+ *
+ * Print arbitrary string to output buffer.
+ *
+ * Return length of printed string or error size.
+ */
+int ipset_print_comment(char *buf, unsigned int len,
+		       const struct ipset_data *data, enum ipset_opt opt,
+		       uint8_t env UNUSED)
+{
+	const char *comment;
+	int size, offset = 0;
+
+	assert(buf);
+	assert(len > 0);
+	assert(data);
+	assert(opt == IPSET_OPT_COMMENT);
+
+	comment = ipset_data_get(data, opt);
+	assert(comment);
+	size = snprintf(buf + offset, len, "\"%s\"", comment);
+	SNPRINTF_FAILURE(size, len, offset);
+	return offset;
+}
+
+/**
  * ipset_print_proto - print protocol name
  * @buf: printing buffer
  * @len: length of available buffer space
diff --git a/lib/session.c b/lib/session.c
index f1df515..94aa9a7 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -488,6 +488,11 @@ static const struct ipset_attr_policy adt_attrs[] = {
 		.type = MNL_TYPE_U64,
 		.opt = IPSET_OPT_BYTES,
 	},
+	[IPSET_ATTR_COMMENT] = {
+		.type = MNL_TYPE_NUL_STRING,
+		.opt = IPSET_OPT_COMMENT,
+		.len  = IPSET_MAXSTRLEN,
+	},
 };
 
 static const struct ipset_attr_policy ipaddr_attrs[] = {
@@ -522,8 +527,9 @@ generic_data_attr_cb(const struct nlattr *attr, void *data,
 		return MNL_CB_ERROR;
 	}
 	if (policy[type].type == MNL_TYPE_NUL_STRING &&
-	    mnl_attr_get_payload_len(attr) > IPSET_MAXNAMELEN)
+	    mnl_attr_get_payload_len(attr) > IPSET_MAXSTRLEN) {
 		return MNL_CB_ERROR;
+	}
 	tb[type] = attr;
 	return MNL_CB_OK;
 }
diff --git a/lib/types.c b/lib/types.c
index adaba83..b95114f 100644
--- a/lib/types.c
+++ b/lib/types.c
@@ -607,7 +607,7 @@ ipset_load_types(void)
 		len = snprintf(path, sizeof(path), "%.*s",
 			       (unsigned int)(next - dir), dir);
 
-		if (len >= sizeof(path) || len < 0)
+		if (len >= (int)sizeof(path) || len < (int)0)
 			continue;
 
 		n = scandir(path, &list, NULL, alphasort);
@@ -620,7 +620,7 @@ ipset_load_types(void)
 
 			len = snprintf(file, sizeof(file), "%s/%s",
 				       path, list[n]->d_name);
-			if (len >= sizeof(file) || len < 0)
+			if (len >= (int)sizeof(file) || len < (int)0)
 				goto nextf;
 
 			if (dlopen(file, RTLD_NOW) == NULL)
-- 
1.8.3.2

Re: [PATCH 5/6] ipset: Support comments in the userspace library.

[Jozsef Kadlecsik]

On Tue, 17 Sep 2013, Oliver wrote:

> From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> 
> This adds support to the userspace portion of ipset for handling ipsets
> with the comment extension enabled. The library revision has been raised
> accordingly.
> 
> Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> ---
>  Make_global.am                  |  2 +-
>  include/libipset/data.h         |  6 +++++-
>  include/libipset/linux_ip_set.h |  7 +++++++
>  include/libipset/parse.h        |  2 ++
>  include/libipset/print.h        |  3 +++
>  lib/data.c                      | 34 ++++++++++++++++++++++++++++++++++
>  lib/debug.c                     |  1 +
>  lib/errcode.c                   |  2 ++
>  lib/libipset.map                |  6 ++++++
>  lib/parse.c                     | 27 +++++++++++++++++++++++++++
>  lib/print.c                     | 31 +++++++++++++++++++++++++++++++
>  lib/session.c                   |  8 +++++++-
>  lib/types.c                     |  4 ++--
>  13 files changed, 128 insertions(+), 5 deletions(-)
> 
> diff --git a/Make_global.am b/Make_global.am
> index 29b5678..9c228cc 100644
> --- a/Make_global.am
> +++ b/Make_global.am
> @@ -69,7 +69,7 @@
>  # interface. 
>  
>  #            curr:rev:age
> -LIBVERSION = 4:0:1
> +LIBVERSION = 4:1:2
>  
>  AM_CPPFLAGS = $(kinclude_CFLAGS) $(all_includes) -I$(top_srcdir)/include \
>  	-I/usr/local/include
> diff --git a/include/libipset/data.h b/include/libipset/data.h
> index 2b6b8cd..ae0d099 100644
> --- a/include/libipset/data.h
> +++ b/include/libipset/data.h
> @@ -57,6 +57,8 @@ enum ipset_opt {
>  	IPSET_OPT_COUNTERS,
>  	IPSET_OPT_PACKETS,
>  	IPSET_OPT_BYTES,
> +	IPSET_OPT_COMMENTS,
> +	IPSET_OPT_COMMENT,

Both cases are here, and then used mixed below. Stick to "COMMENT" 
everywhere.

>  	/* Internal options */
>  	IPSET_OPT_FLAGS = 48,	/* IPSET_FLAG_EXIST| */
>  	IPSET_OPT_CADT_FLAGS,	/* IPSET_FLAG_BEFORE| */
> @@ -106,11 +108,13 @@ enum ipset_opt {
>  	| IPSET_FLAG(IPSET_OPT_CADT_FLAGS)\
>  	| IPSET_FLAG(IPSET_OPT_BEFORE) \
>  	| IPSET_FLAG(IPSET_OPT_PHYSDEV) \
> -	| IPSET_FLAG(IPSET_OPT_NOMATCH))
> +	| IPSET_FLAG(IPSET_OPT_NOMATCH) \
> +	| IPSET_FLAG(IPSET_OPT_COMMENT))
>  
>  struct ipset_data;
>  
>  extern void ipset_strlcpy(char *dst, const char *src, size_t len);
> +extern void ipset_strlcat(char *dst, const char *src, size_t len);
>  extern bool ipset_data_flags_test(const struct ipset_data *data,
>  				  uint64_t flags);
>  extern void ipset_data_flags_set(struct ipset_data *data, uint64_t flags);
> diff --git a/include/libipset/linux_ip_set.h b/include/libipset/linux_ip_set.h
> index 8024cdf..2278a4e 100644
> --- a/include/libipset/linux_ip_set.h
> +++ b/include/libipset/linux_ip_set.h
> @@ -19,6 +19,9 @@
>  /* The max length of strings including NUL: set and type identifiers */
>  #define IPSET_MAXNAMELEN	32
>  
> +/* The maximum permissible length we will accept over netlink (inc. comments) */
> +#define IPSET_MAXSTRLEN		255

The include/libipset/linux_* header files are updated by "make 
update_includes", which'll overwrite this.

So move IP_SET_MAX_COMMENT_SIZE from 
include/linux/netfilter/ipset/ip_set_comment.h into 
include/uapi/linux/netfilter/ipset/ip_set.h - the separated size checking 
of setnames, typenames must be kept anyway.

>  /* Message types and commands */
>  enum ipset_cmd {
>  	IPSET_CMD_NONE,
> @@ -110,6 +113,7 @@ enum {
>  	IPSET_ATTR_IFACE,
>  	IPSET_ATTR_BYTES,
>  	IPSET_ATTR_PACKETS,
> +	IPSET_ATTR_COMMENT,
>  	__IPSET_ATTR_ADT_MAX,
>  };
>  #define IPSET_ATTR_ADT_MAX	(__IPSET_ATTR_ADT_MAX - 1)
> @@ -140,6 +144,7 @@ enum ipset_errno {
>  	IPSET_ERR_IPADDR_IPV4,
>  	IPSET_ERR_IPADDR_IPV6,
>  	IPSET_ERR_COUNTER,
> +	IPSET_ERR_COMMENT,
>  
>  	/* Type specific error codes */
>  	IPSET_ERR_TYPE_SPECIFIC = 4352,
> @@ -176,6 +181,8 @@ enum ipset_cadt_flags {
>  	IPSET_FLAG_NOMATCH	= (1 << IPSET_FLAG_BIT_NOMATCH),
>  	IPSET_FLAG_BIT_WITH_COUNTERS = 3,
>  	IPSET_FLAG_WITH_COUNTERS = (1 << IPSET_FLAG_BIT_WITH_COUNTERS),
> +	IPSET_FLAG_BIT_WITH_COMMENTS = 4,
> +	IPSET_FLAG_WITH_COMMENTS = (1 << IPSET_FLAG_BIT_WITH_COMMENTS),
>  	IPSET_FLAG_CADT_MAX	= 15,
>  };
>  
> diff --git a/include/libipset/parse.h b/include/libipset/parse.h
> index 014c62f..5c46a88 100644
> --- a/include/libipset/parse.h
> +++ b/include/libipset/parse.h
> @@ -90,6 +90,8 @@ extern int ipset_parse_typename(struct ipset_session *session,
>  				enum ipset_opt opt, const char *str);
>  extern int ipset_parse_iface(struct ipset_session *session,
>  			     enum ipset_opt opt, const char *str);
> +extern int ipset_parse_comment(struct ipset_session *session,
> +			     enum ipset_opt opt, const char *str);
>  extern int ipset_parse_output(struct ipset_session *session,
>  			      int opt, const char *str);
>  extern int ipset_parse_ignored(struct ipset_session *session,
> diff --git a/include/libipset/print.h b/include/libipset/print.h
> index 1d537bd..f2a6095 100644
> --- a/include/libipset/print.h
> +++ b/include/libipset/print.h
> @@ -40,6 +40,9 @@ extern int ipset_print_port(char *buf, unsigned int len,
>  extern int ipset_print_iface(char *buf, unsigned int len,
>  			     const struct ipset_data *data,
>  			     enum ipset_opt opt, uint8_t env);
> +extern int ipset_print_comment(char *buf, unsigned int len,
> +			     const struct ipset_data *data,
> +			     enum ipset_opt opt, uint8_t env);
>  extern int ipset_print_proto(char *buf, unsigned int len,
>  			     const struct ipset_data *data,
>  			     enum ipset_opt opt, uint8_t env);
> diff --git a/lib/data.c b/lib/data.c
> index 04a5997..9aaa550 100644
> --- a/lib/data.c
> +++ b/lib/data.c
> @@ -75,6 +75,7 @@ struct ipset_data {
>  			char iface[IFNAMSIZ];
>  			uint64_t packets;
>  			uint64_t bytes;
> +			char comment[IPSET_MAXSTRLEN];

That should be
			char comment[IP_SET_MAX_COMMENT_SIZE+1];

>  		} adt;
>  	};
>  };
> @@ -108,6 +109,25 @@ ipset_strlcpy(char *dst, const char *src, size_t len)
>  }
>  
>  /**
> + * ipset_strlcat - concatenate the string from src to the end of dst
> + * @dst: the target string buffer
> + * @src: the source string buffer
> + * @len: the length of bytes to concat, including the terminating null byte.
> + *
> + * Cooncatenate the string in src to destination, but at most len bytes are
> + * copied. The target is unconditionally terminated by the null byte.
> + */
> +void
> +ipset_strlcat(char *dst, const char *src, size_t len)
> +{
> +	assert(dst);
> +	assert(src);
> +
> +	strncat(dst, src, len);
> +	dst[len - 1] = '\0';
> +}
> +
> +/**
>   * ipset_data_flags_test - test option bits in the data blob
>   * @data: data blob
>   * @flags: the option flags to test
> @@ -278,6 +298,9 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt opt, const void *value)
>  	case IPSET_OPT_COUNTERS:
>  		cadt_flag_type_attr(data, opt, IPSET_FLAG_WITH_COUNTERS);
>  		break;
> +	case IPSET_OPT_COMMENTS:
> +		cadt_flag_type_attr(data, opt, IPSET_FLAG_WITH_COMMENTS);
> +		break;
>  	/* Create-specific options, filled out by the kernel */
>  	case IPSET_OPT_ELEMENTS:
>  		data->create.elements = *(const uint32_t *) value;
> @@ -336,6 +359,9 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt opt, const void *value)
>  	case IPSET_OPT_BYTES:
>  		data->adt.bytes = *(const uint64_t *) value;
>  		break;
> +	case IPSET_OPT_COMMENT:
> +		ipset_strlcpy(data->adt.comment, value, IPSET_MAXSTRLEN);
> +		break;
>  	/* Swap/rename */
>  	case IPSET_OPT_SETNAME2:
>  		ipset_strlcpy(data->setname2, value, IPSET_MAXNAMELEN);
> @@ -370,6 +396,9 @@ ipset_data_set(struct ipset_data *data, enum ipset_opt opt, const void *value)
>  		if (data->cadt_flags & IPSET_FLAG_WITH_COUNTERS)
>  			ipset_data_flags_set(data,
>  					     IPSET_FLAG(IPSET_OPT_COUNTERS));
> +		if (data->cadt_flags & IPSET_FLAG_WITH_COMMENTS)
> +			ipset_data_flags_set(data,
> +					     IPSET_FLAG(IPSET_OPT_COMMENTS));
>  		break;
>  	default:
>  		return -1;
> @@ -472,6 +501,8 @@ ipset_data_get(const struct ipset_data *data, enum ipset_opt opt)
>  		return &data->adt.packets;
>  	case IPSET_OPT_BYTES:
>  		return &data->adt.bytes;
> +	case IPSET_OPT_COMMENT:
> +		return &data->adt.comment;
>  	/* Swap/rename */
>  	case IPSET_OPT_SETNAME2:
>  		return data->setname2;
> @@ -484,6 +515,7 @@ ipset_data_get(const struct ipset_data *data, enum ipset_opt opt)
>  	case IPSET_OPT_PHYSDEV:
>  	case IPSET_OPT_NOMATCH:
>  	case IPSET_OPT_COUNTERS:
> +	case IPSET_OPT_COMMENTS:
>  		return &data->cadt_flags;
>  	default:
>  		return NULL;
> @@ -543,6 +575,8 @@ ipset_data_sizeof(enum ipset_opt opt, uint8_t family)
>  	case IPSET_OPT_NOMATCH:
>  	case IPSET_OPT_COUNTERS:
>  		return sizeof(uint32_t);
> +	case IPSET_OPT_COMMENT:
> +		return IPSET_MAXSTRLEN;
>  	default:
>  		return 0;
>  	};
> diff --git a/lib/debug.c b/lib/debug.c
> index 3aa5a99..a204940 100644
> --- a/lib/debug.c
> +++ b/lib/debug.c
> @@ -64,6 +64,7 @@ static const struct ipset_attrname adtattr2name[] = {
>  	[IPSET_ATTR_CIDR2]	= { .name = "CIDR2" },
>  	[IPSET_ATTR_IP2_TO]	= { .name = "IP2_TO" },
>  	[IPSET_ATTR_IFACE]	= { .name = "IFACE" },
> +	[IPSET_ATTR_COMMENT]	= { .name = "COMMENT" },
>  };
>  
>  static void
> diff --git a/lib/errcode.c b/lib/errcode.c
> index c939949..c824cc3 100644
> --- a/lib/errcode.c
> +++ b/lib/errcode.c
> @@ -72,6 +72,8 @@ static const struct ipset_errcode_table core_errcode_table[] = {
>  	  "An IPv6 address is expected, but not received" },
>  	{ IPSET_ERR_COUNTER, 0,
>  	  "Packet/byte counters cannot be used: set was created without counter support" },
> +	{ IPSET_ERR_COMMENT, 0,
> +	  "Comment too long!" },

"The comment value is too long!" or something like that.
 
>  	/* ADD specific error codes */
>  	{ IPSET_ERR_EXIST, IPSET_CMD_ADD,
> diff --git a/lib/libipset.map b/lib/libipset.map
> index 271fe59..06216e7 100644
> --- a/lib/libipset.map
> +++ b/lib/libipset.map
> @@ -127,3 +127,9 @@ LIBIPSET_4.0 {
>  global:
>    ipset_parse_uint64;
>  } LIBIPSET_3.0;
> +
> +LIBIPSET_4.1 {
> +global:
> +  ipset_parse_comment;

Plus ipset_print_comment, too.

> +  ipset_strlcat;
> +} LIBIPSET_4.0;
> diff --git a/lib/parse.c b/lib/parse.c
> index 112b273..bde56b3 100644
> --- a/lib/parse.c
> +++ b/lib/parse.c
> @@ -1739,6 +1739,33 @@ ipset_parse_iface(struct ipset_session *session,
>  }
>  
>  /**
> + * ipset_parse_comment - parse string as a comment
> + * @session: session structure
> + * @opt: option kind of the data
> + * @str: string to parse
> + *
> + * Parse string for use as a comment on an ipset entry.
> + * Gets stored in the data blob as usual.
> + *
> + * Returns 0 on success or a negative error code.
> + */
> +int ipset_parse_comment(struct ipset_session *session,
> +		       enum ipset_opt opt, const char *str)
> +{
> +	struct ipset_data *data;
> +
> +	assert(session);
> +	assert(opt == IPSET_OPT_COMMENT);
> +	assert(str);
> +
> +	data = ipset_session_data(session);
> +	if (strlen(str) > IPSET_MAXSTRLEN)
> +		return syntax_err("comment is longer than the maximum permitted");

Print the maximum length too, like

		return syntax_err("The comment value is longer than the "
				  "maximum allowed %d characters.\n",
				  IP_SET_MAX_COMMENT_SIZE);

> +
> +	return ipset_data_set(data, opt, str);
> +}
> +
> +/**
>   * ipset_parse_output - parse output format name
>   * @session: session structure
>   * @opt: option kind of the data
> diff --git a/lib/print.c b/lib/print.c
> index 86a7674..9ccc3c8 100644
> --- a/lib/print.c
> +++ b/lib/print.c
> @@ -530,6 +530,37 @@ ipset_print_iface(char *buf, unsigned int len,
>  }
>  
>  /**
> + * ipset_print_comment - print arbitrary parameter string
> + * @buf: printing buffer
> + * @len: length of available buffer space
> + * @data: data blob
> + * @opt: the option kind
> + * @env: environment flags
> + *
> + * Print arbitrary string to output buffer.
> + *
> + * Return length of printed string or error size.
> + */
> +int ipset_print_comment(char *buf, unsigned int len,
> +		       const struct ipset_data *data, enum ipset_opt opt,
> +		       uint8_t env UNUSED)
> +{
> +	const char *comment;
> +	int size, offset = 0;
> +
> +	assert(buf);
> +	assert(len > 0);
> +	assert(data);
> +	assert(opt == IPSET_OPT_COMMENT);
> +
> +	comment = ipset_data_get(data, opt);
> +	assert(comment);
> +	size = snprintf(buf + offset, len, "\"%s\"", comment);
> +	SNPRINTF_FAILURE(size, len, offset);
> +	return offset;
> +}
> +
> +/**
>   * ipset_print_proto - print protocol name
>   * @buf: printing buffer
>   * @len: length of available buffer space
> diff --git a/lib/session.c b/lib/session.c
> index f1df515..94aa9a7 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -488,6 +488,11 @@ static const struct ipset_attr_policy adt_attrs[] = {
>  		.type = MNL_TYPE_U64,
>  		.opt = IPSET_OPT_BYTES,
>  	},
> +	[IPSET_ATTR_COMMENT] = {
> +		.type = MNL_TYPE_NUL_STRING,
> +		.opt = IPSET_OPT_COMMENT,
> +		.len  = IPSET_MAXSTRLEN,

Take into account the terminating null character:

		.len = IP_SET_MAX_COMMENT_SIZE + 1,

> +	},
>  };
>  
>  static const struct ipset_attr_policy ipaddr_attrs[] = {
> @@ -522,8 +527,9 @@ generic_data_attr_cb(const struct nlattr *attr, void *data,
>  		return MNL_CB_ERROR;
>  	}
>  	if (policy[type].type == MNL_TYPE_NUL_STRING &&
> -	    mnl_attr_get_payload_len(attr) > IPSET_MAXNAMELEN)
> +	    mnl_attr_get_payload_len(attr) > IPSET_MAXSTRLEN) {

Compare to policy[type].len: the different string types have got different 
size limits (actually, interface names were not handled properly).

>  		return MNL_CB_ERROR;
> +	}
>  	tb[type] = attr;
>  	return MNL_CB_OK;
>  }
> diff --git a/lib/types.c b/lib/types.c
> index adaba83..b95114f 100644
> --- a/lib/types.c
> +++ b/lib/types.c
> @@ -607,7 +607,7 @@ ipset_load_types(void)
>  		len = snprintf(path, sizeof(path), "%.*s",
>  			       (unsigned int)(next - dir), dir);
>  
> -		if (len >= sizeof(path) || len < 0)
> +		if (len >= (int)sizeof(path) || len < (int)0)
>  			continue;
>  
>  		n = scandir(path, &list, NULL, alphasort);
> @@ -620,7 +620,7 @@ ipset_load_types(void)
>  
>  			len = snprintf(file, sizeof(file), "%s/%s",
>  				       path, list[n]->d_name);
> -			if (len >= sizeof(file) || len < 0)
> +			if (len >= (int)sizeof(file) || len < (int)0)
>  				goto nextf;
>  
>  			if (dlopen(file, RTLD_NOW) == NULL)

I don't see why these two modifications are required.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 5/6] ipset: Support comments in the userspace library.

[Jozsef Kadlecsik]

On Wed, 18 Sep 2013, Jozsef Kadlecsik wrote:

> On Tue, 17 Sep 2013, Oliver wrote:
> 
> > From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> > 
> > This adds support to the userspace portion of ipset for handling ipsets
> > with the comment extension enabled. The library revision has been raised
> > accordingly.
> > 
> > Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> > ---
> >  Make_global.am                  |  2 +-
> >  include/libipset/data.h         |  6 +++++-
> >  include/libipset/linux_ip_set.h |  7 +++++++
> >  include/libipset/parse.h        |  2 ++
> >  include/libipset/print.h        |  3 +++
> >  lib/data.c                      | 34 ++++++++++++++++++++++++++++++++++
> >  lib/debug.c                     |  1 +
> >  lib/errcode.c                   |  2 ++
> >  lib/libipset.map                |  6 ++++++
> >  lib/parse.c                     | 27 +++++++++++++++++++++++++++
> >  lib/print.c                     | 31 +++++++++++++++++++++++++++++++
> >  lib/session.c                   |  8 +++++++-
> >  lib/types.c                     |  4 ++--
> >  13 files changed, 128 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Make_global.am b/Make_global.am
> > index 29b5678..9c228cc 100644
> > --- a/Make_global.am
> > +++ b/Make_global.am
> > @@ -69,7 +69,7 @@
> >  # interface. 
> >  
> >  #            curr:rev:age
> > -LIBVERSION = 4:0:1
> > +LIBVERSION = 4:1:2
> >  
> >  AM_CPPFLAGS = $(kinclude_CFLAGS) $(all_includes) -I$(top_srcdir)/include \
> >  	-I/usr/local/include
> > diff --git a/include/libipset/data.h b/include/libipset/data.h
> > index 2b6b8cd..ae0d099 100644
> > --- a/include/libipset/data.h
> > +++ b/include/libipset/data.h
> > @@ -57,6 +57,8 @@ enum ipset_opt {
> >  	IPSET_OPT_COUNTERS,
> >  	IPSET_OPT_PACKETS,
> >  	IPSET_OPT_BYTES,
> > +	IPSET_OPT_COMMENTS,
> > +	IPSET_OPT_COMMENT,
> 
> Both cases are here, and then used mixed below. Stick to "COMMENT" 
> everywhere.

Ohh, I got it: the first is for the create option and the second is for 
the attribute.

This is highly error prone. Please rename the first one to 
IPSET_OPT_CREATE_COMMENT, or the second one to IPSET_OPT_ADT_COMMENT, 
whichever you prefer.
 
Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 5/6] ipset: Support comments in the userspace library.

[Oliver]

On Wednesday 18 September 2013 20:43:30 Jozsef Kadlecsik wrote:
> On Tue, 17 Sep 2013, Oliver wrote:
<snip>
> > diff --git a/lib/types.c b/lib/types.c
> > index adaba83..b95114f 100644
> > --- a/lib/types.c
> > +++ b/lib/types.c
> > @@ -607,7 +607,7 @@ ipset_load_types(void)
> > 
> >  		len = snprintf(path, sizeof(path), "%.*s",
> >  		
> >  			       (unsigned int)(next - dir), dir);
> > 
> > -		if (len >= sizeof(path) || len < 0)
> > +		if (len >= (int)sizeof(path) || len < (int)0)
> > 
> >  			continue;
> >  		
> >  		n = scandir(path, &list, NULL, alphasort);
> > 
> > @@ -620,7 +620,7 @@ ipset_load_types(void)
> > 
> >  			len = snprintf(file, sizeof(file), "%s/%s",
> >  			
> >  				       path, list[n]->d_name);
> > 
> > -			if (len >= sizeof(file) || len < 0)
> > +			if (len >= (int)sizeof(file) || len < (int)0)
> > 
> >  				goto nextf;
> >  			
> >  			if (dlopen(file, RTLD_NOW) == NULL)
> 
> I don't see why these two modifications are required.

Just to prevent a build failure in debug mode because warnings are treated as 
errors and comparing an int to a size_t obviously makes it unhappy.

All other other changes you specified have been made. :)

Thanks,
Oliver.

Re: [PATCH 5/6] ipset: Support comments in the userspace library.

[Jozsef Kadlecsik]

On Thu, 19 Sep 2013, Oliver wrote:

> On Wednesday 18 September 2013 20:43:30 Jozsef Kadlecsik wrote:
> > On Tue, 17 Sep 2013, Oliver wrote:
> <snip>
> > > diff --git a/lib/types.c b/lib/types.c
> > > index adaba83..b95114f 100644
> > > --- a/lib/types.c
> > > +++ b/lib/types.c
> > > @@ -607,7 +607,7 @@ ipset_load_types(void)
> > > 
> > >  		len = snprintf(path, sizeof(path), "%.*s",
> > >  		
> > >  			       (unsigned int)(next - dir), dir);
> > > 
> > > -		if (len >= sizeof(path) || len < 0)
> > > +		if (len >= (int)sizeof(path) || len < (int)0)
> > > 
> > >  			continue;
> > >  		
> > >  		n = scandir(path, &list, NULL, alphasort);
> > > 
> > > @@ -620,7 +620,7 @@ ipset_load_types(void)
> > > 
> > >  			len = snprintf(file, sizeof(file), "%s/%s",
> > >  			
> > >  				       path, list[n]->d_name);
> > > 
> > > -			if (len >= sizeof(file) || len < 0)
> > > +			if (len >= (int)sizeof(file) || len < (int)0)
> > > 
> > >  				goto nextf;
> > >  			
> > >  			if (dlopen(file, RTLD_NOW) == NULL)
> > 
> > I don't see why these two modifications are required.
> 
> Just to prevent a build failure in debug mode because warnings are 
> treated as errors and comparing an int to a size_t obviously makes it 
> unhappy.

That's good that you compiled the code in debug mode too. But what is your 
gcc version? That "len < (int)0" looks really strange.

Best regards,
Jozsef
-
E-mail  : kadlec@blackhole.kfki.hu, kadlecsik.jozsef@wigner.mta.hu
PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences
          H-1525 Budapest 114, POB. 49, Hungary

Re: [PATCH 5/6] ipset: Support comments in the userspace library.

[Oliver]

On Thursday 19 September 2013 20:37:12 Jozsef Kadlecsik wrote:
> On Thu, 19 Sep 2013, Oliver wrote:
> > On Wednesday 18 September 2013 20:43:30 Jozsef Kadlecsik wrote:
> > > On Tue, 17 Sep 2013, Oliver wrote:
> > <snip>
> > 
> > > > diff --git a/lib/types.c b/lib/types.c
> > > > index adaba83..b95114f 100644
> > > > --- a/lib/types.c
> > > > +++ b/lib/types.c
> > > > @@ -607,7 +607,7 @@ ipset_load_types(void)
> > > > 
> > > >  		len = snprintf(path, sizeof(path), "%.*s",
> > > >  		
> > > >  			       (unsigned int)(next - dir), dir);
> > > > 
> > > > -		if (len >= sizeof(path) || len < 0)
> > > > +		if (len >= (int)sizeof(path) || len < (int)0)
> > > > 
> > > >  			continue;
> > > >  		
> > > >  		n = scandir(path, &list, NULL, alphasort);
> > > > 
> > > > @@ -620,7 +620,7 @@ ipset_load_types(void)
> > > > 
> > > >  			len = snprintf(file, sizeof(file), "%s/%s",
> > > >  			
> > > >  				       path, list[n]->d_name);
> > > > 
> > > > -			if (len >= sizeof(file) || len < 0)
> > > > +			if (len >= (int)sizeof(file) || len < (int)0)
> > > > 
> > > >  				goto nextf;
> > > >  			
> > > >  			if (dlopen(file, RTLD_NOW) == NULL)
> > > 
> > > I don't see why these two modifications are required.
> > 
> > Just to prevent a build failure in debug mode because warnings are
> > treated as errors and comparing an int to a size_t obviously makes it
> > unhappy.
> 
> That's good that you compiled the code in debug mode too. But what is your
> gcc version? That "len < (int)0" looks really strange.

GCC 4.7.3, but that cast on the zero constant isn't needed and frankly I'm not 
entirely sure what possessed me to put it there in the first place - I'm going 
to put it down to a lack of coffee.

Kind Regards,
Oliver.

[PATCH 6/6] ipset: Add new userspace set revisions for comment support

[Oliver]

From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>

This introduces new revisions of all hash and bitmap ipsets to
complement the comment functionality introduced into the kernel modules.

Currently all sets have a compile-time limit of 255 characters including
\0. This can otherwise be arbitrarily modified.

Signed-off-by: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
---
 lib/ipset_bitmap_ip.c      | 114 ++++++++++++++++++++++++++
 lib/ipset_bitmap_ipmac.c   | 118 +++++++++++++++++++++++++++
 lib/ipset_bitmap_port.c    | 107 +++++++++++++++++++++++++
 lib/ipset_hash_ip.c        | 138 ++++++++++++++++++++++++++++++++
 lib/ipset_hash_ipport.c    | 161 +++++++++++++++++++++++++++++++++++++
 lib/ipset_hash_ipportnet.c | 195 +++++++++++++++++++++++++++++++++++++++++++++
 lib/ipset_hash_net.c       | 145 +++++++++++++++++++++++++++++++++
 lib/ipset_hash_netnet.c    |  14 +++-
 lib/ipset_hash_netport.c   | 158 ++++++++++++++++++++++++++++++++++++
 9 files changed, 1148 insertions(+), 2 deletions(-)

diff --git a/lib/ipset_bitmap_ip.c b/lib/ipset_bitmap_ip.c
index a4726db..3b58661 100644
--- a/lib/ipset_bitmap_ip.c
+++ b/lib/ipset_bitmap_ip.c
@@ -201,9 +201,123 @@ static struct ipset_type ipset_bitmap_ip1 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg bitmap_ip_create_args2[] = {
+	{ .name = { "range", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_netrange,	.print = ipset_print_ip,
+	},
+	{ .name = { "netmask", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_NETMASK,
+	  .parse = ipset_parse_netmask,		.print = ipset_print_number,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Backward compatibility */
+	{ .name = { "from", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_single_ip,
+	},
+	{ .name = { "to", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP_TO,
+	  .parse = ipset_parse_single_ip,
+	},
+	{ .name = { "network", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_net,
+	},
+	{ },
+};
+
+static const struct ipset_arg bitmap_ip_add_args2[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const char bitmap_ip_usage2[] =
+"create SETNAME bitmap:ip range IP/CIDR|FROM-TO\n"
+"               [netmask CIDR] [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME IP|IP/CIDR|FROM-TO [timeout VALUE]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP|IP/CIDR|FROM-TO\n"
+"test   SETNAME IP\n\n"
+"where IP, FROM and TO are IPv4 addresses (or hostnames),\n"
+"      CIDR is a valid IPv4 CIDR prefix.\n";
+
+static struct ipset_type ipset_bitmap_ip2 = {
+	.name = "bitmap:ip",
+	.alias = { "ipmap", NULL },
+	.revision = 2,
+	.family = NFPROTO_IPV4,
+	.dimension = IPSET_DIM_ONE,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_ip,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = bitmap_ip_create_args2,
+		[IPSET_ADD] = bitmap_ip_add_args2,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_NETMASK)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP),
+	},
+
+	.usage = bitmap_ip_usage2,
+	.description = "comments support",
+};
 void _init(void);
 void _init(void)
 {
 	ipset_type_add(&ipset_bitmap_ip0);
 	ipset_type_add(&ipset_bitmap_ip1);
+	ipset_type_add(&ipset_bitmap_ip2);
 }
diff --git a/lib/ipset_bitmap_ipmac.c b/lib/ipset_bitmap_ipmac.c
index 67217a9..34f9b24 100644
--- a/lib/ipset_bitmap_ipmac.c
+++ b/lib/ipset_bitmap_ipmac.c
@@ -207,9 +207,127 @@ static struct ipset_type ipset_bitmap_ipmac1 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg bitmap_ipmac_create_args2[] = {
+	{ .name = { "range", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_netrange,	.print = ipset_print_ip,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Backward compatibility */
+	{ .name = { "from", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_single_ip,
+	},
+	{ .name = { "to", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP_TO,
+	  .parse = ipset_parse_single_ip,
+	},
+	{ .name = { "network", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_net,
+	},
+	{ },
+};
+
+static const struct ipset_arg bitmap_ipmac_add_args2[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const char bitmap_ipmac_usage2[] =
+"create SETNAME bitmap:ip,mac range IP/CIDR|FROM-TO\n"
+"               [matchunset] [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME IP[,MAC] [timeout VALUE]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP[,MAC]\n"
+"test   SETNAME IP[,MAC]\n\n"
+"where IP, FROM and TO are IPv4 addresses (or hostnames),\n"
+"      CIDR is a valid IPv4 CIDR prefix,\n"
+"      MAC is a valid MAC address.\n";
+
+static struct ipset_type ipset_bitmap_ipmac2 = {
+	.name = "bitmap:ip,mac",
+	.alias = { "macipmap", NULL },
+	.revision = 2,
+	.family = NFPROTO_IPV4,
+	.dimension = IPSET_DIM_TWO,
+	.last_elem_optional = true,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_single_ip,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+		[IPSET_DIM_TWO - 1] = {
+			.parse = ipset_parse_ether,
+			.print = ipset_print_ether,
+			.opt = IPSET_OPT_ETHER
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = bitmap_ipmac_create_args2,
+		[IPSET_ADD] = bitmap_ipmac_add_args2,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_ETHER)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_ETHER),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_ETHER),
+	},
+
+	.usage = bitmap_ipmac_usage2,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
 	ipset_type_add(&ipset_bitmap_ipmac0);
 	ipset_type_add(&ipset_bitmap_ipmac1);
+	ipset_type_add(&ipset_bitmap_ipmac2);
 }
diff --git a/lib/ipset_bitmap_port.c b/lib/ipset_bitmap_port.c
index a706d80..f42a3d2 100644
--- a/lib/ipset_bitmap_port.c
+++ b/lib/ipset_bitmap_port.c
@@ -185,9 +185,116 @@ static struct ipset_type ipset_bitmap_port1 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg bitmap_port_create_args2[] = {
+	{ .name = { "range", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PORT,
+	  .parse = ipset_parse_tcp_udp_port,	.print = ipset_print_port,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Backward compatibility */
+	{ .name = { "from", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PORT,
+	  .parse = ipset_parse_single_tcp_port,
+	},
+	{ .name = { "to", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PORT_TO,
+	  .parse = ipset_parse_single_tcp_port,
+	},
+	{ },
+};
+
+static const struct ipset_arg bitmap_port_add_args2[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const char bitmap_port_usage2[] =
+"create SETNAME bitmap:port range [PROTO:]FROM-TO\n"
+"               [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME [PROTO:]PORT|FROM-TO [timeout VALUE]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME [PROTO:]PORT|FROM-TO\n"
+"test   SETNAME [PROTO:]PORT\n\n"
+"where PORT, FROM and TO are port numbers or port names from /etc/services.\n"
+"PROTO is only needed if a service name is used and it does not exist as a TCP service;\n"
+"it isn't used otherwise with the bitmap.\n";
+
+static struct ipset_type ipset_bitmap_port2 = {
+	.name = "bitmap:port",
+	.alias = { "portmap", NULL },
+	.revision = 2,
+	.family = NFPROTO_UNSPEC,
+	.dimension = IPSET_DIM_ONE,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_tcp_udp_port,
+			.print = ipset_print_port,
+			.opt = IPSET_OPT_PORT
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = bitmap_port_create_args2,
+		[IPSET_ADD] = bitmap_port_add_args2,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_PORT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_PORT),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_PORT),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_PORT),
+	},
+
+	.usage = bitmap_port_usage2,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
 	ipset_type_add(&ipset_bitmap_port0);
 	ipset_type_add(&ipset_bitmap_port1);
+	ipset_type_add(&ipset_bitmap_port2);
 }
diff --git a/lib/ipset_hash_ip.c b/lib/ipset_hash_ip.c
index 19688db..d910d1f 100644
--- a/lib/ipset_hash_ip.c
+++ b/lib/ipset_hash_ip.c
@@ -246,9 +246,147 @@ static struct ipset_type ipset_hash_ip1 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg hash_ip_create_args2[] = {
+	{ .name = { "family", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,		.print = ipset_print_family,
+	},
+	/* Alias: family inet */
+	{ .name = { "-4", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	/* Alias: family inet6 */
+	{ .name = { "-6", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	{ .name = { "hashsize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_HASHSIZE,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "maxelem", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_MAXELEM,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "netmask", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_NETMASK,
+	  .parse = ipset_parse_netmask,		.print = ipset_print_number,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Ignored options: backward compatibilty */
+	{ .name = { "probes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PROBES,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "resize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_RESIZE,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "gc", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_GC,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_ip_add_args2[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const char hash_ip_usage2[] =
+"create SETNAME hash:ip\n"
+"		[family inet|inet6]\n"
+"               [hashsize VALUE] [maxelem VALUE]\n"
+"               [netmask CIDR] [timeout VALUE]\n"
+"               [counters] [comments]\n"
+"add    SETNAME IP [timeout VALUE]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP\n"
+"test   SETNAME IP\n\n"
+"where depending on the INET family\n"
+"      IP is a valid IPv4 or IPv6 address (or hostname),\n"
+"      CIDR is a valid IPv4 or IPv6 CIDR prefix.\n"
+"      Adding/deleting multiple elements in IP/CIDR or FROM-TO form\n"
+"      is supported for IPv4.\n";
+
+static struct ipset_type ipset_hash_ip2 = {
+	.name = "hash:ip",
+	.alias = { "iphash", NULL },
+	.revision = 2,
+	.family = NFPROTO_IPSET_IPV46,
+	.dimension = IPSET_DIM_ONE,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_ip4_single6,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = hash_ip_create_args2,
+		[IPSET_ADD] = hash_ip_add_args2,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = 0,
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_HASHSIZE)
+			| IPSET_FLAG(IPSET_OPT_MAXELEM)
+			| IPSET_FLAG(IPSET_OPT_NETMASK)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP),
+	},
+
+	.usage = hash_ip_usage2,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
 	ipset_type_add(&ipset_hash_ip0);
 	ipset_type_add(&ipset_hash_ip1);
+	ipset_type_add(&ipset_hash_ip2);
 }
diff --git a/lib/ipset_hash_ipport.c b/lib/ipset_hash_ipport.c
index b1c9f72..61848d5 100644
--- a/lib/ipset_hash_ipport.c
+++ b/lib/ipset_hash_ipport.c
@@ -294,9 +294,170 @@ static struct ipset_type ipset_hash_ipport2 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg hash_ipport_create_args3[] = {
+	{ .name = { "family", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,		.print = ipset_print_family,
+	},
+	/* Alias: family inet */
+	{ .name = { "-4", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	/* Alias: family inet6 */
+	{ .name = { "-6", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	{ .name = { "hashsize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_HASHSIZE,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "maxelem", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_MAXELEM,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Backward compatibility */
+	{ .name = { "probes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PROBES,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "resize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_RESIZE,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "from", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_ignored,
+	},
+	{ .name = { "to", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP_TO,
+	  .parse = ipset_parse_ignored,
+	},
+	{ .name = { "network", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_ignored,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_ipport_add_args3[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const char hash_ipport_usage3[] =
+"create SETNAME hash:ip,port\n"
+"		[family inet|inet6]\n"
+"               [hashsize VALUE] [maxelem VALUE]\n"
+"               [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME IP,PROTO:PORT [timeout VALUE]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP,PROTO:PORT\n"
+"test   SETNAME IP,PROTO:PORT\n\n"
+"where depending on the INET family\n"
+"      IP is a valid IPv4 or IPv6 address (or hostname).\n"
+"      Adding/deleting multiple elements in IP/CIDR or FROM-TO form\n"
+"      is supported for IPv4.\n"
+"      Adding/deleting multiple elements with TCP/SCTP/UDP/UDPLITE\n"
+"      port range is supported both for IPv4 and IPv6.\n";
+
+static struct ipset_type ipset_hash_ipport3 = {
+	.name = "hash:ip,port",
+	.alias = { "ipporthash", NULL },
+	.revision = 3,
+	.family = NFPROTO_IPSET_IPV46,
+	.dimension = IPSET_DIM_TWO,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_ip4_single6,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+		[IPSET_DIM_TWO - 1] = {
+			.parse = ipset_parse_proto_port,
+			.print = ipset_print_proto_port,
+			.opt = IPSET_OPT_PORT
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = hash_ipport_create_args3,
+		[IPSET_ADD] = hash_ipport_add_args3,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = 0,
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_PORT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_PORT),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_PORT),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_HASHSIZE)
+			| IPSET_FLAG(IPSET_OPT_MAXELEM)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_PROTO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PROTO),
+	},
+
+	.usage = hash_ipport_usage3,
+	.usagefn = ipset_port_usage,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
 	ipset_type_add(&ipset_hash_ipport1);
 	ipset_type_add(&ipset_hash_ipport2);
+	ipset_type_add(&ipset_hash_ipport3);
 }
diff --git a/lib/ipset_hash_ipportnet.c b/lib/ipset_hash_ipportnet.c
index 2c2e014..0ea1f96 100644
--- a/lib/ipset_hash_ipportnet.c
+++ b/lib/ipset_hash_ipportnet.c
@@ -544,6 +544,200 @@ static struct ipset_type ipset_hash_ipportnet4 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg hash_ipportnet_create_args5[] = {
+	{ .name = { "family", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,		.print = ipset_print_family,
+	},
+	/* Alias: family inet */
+	{ .name = { "-4", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	/* Alias: family inet6 */
+	{ .name = { "-6", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	{ .name = { "hashsize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_HASHSIZE,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "maxelem", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_MAXELEM,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Backward compatibility */
+	{ .name = { "probes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PROBES,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "resize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_RESIZE,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "from", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_ignored,
+	},
+	{ .name = { "to", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP_TO,
+	  .parse = ipset_parse_ignored,
+	},
+	{ .name = { "network", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_IP,
+	  .parse = ipset_parse_ignored,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_ipportnet_add_args5[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "nomatch", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_NOMATCH,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_ipportnet_test_args5[] = {
+	{ .name = { "nomatch", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_NOMATCH,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ },
+};
+
+static const char hash_ipportnet_usage5[] =
+"create SETNAME hash:ip,port,net\n"
+"		[family inet|inet6]\n"
+"               [hashsize VALUE] [maxelem VALUE]\n"
+"               [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME IP,PROTO:PORT,IP[/CIDR] [timeout VALUE] [nomatch]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP,PROTO:PORT,IP[/CIDR]\n"
+"test   SETNAME IP,PROTO:PORT,IP[/CIDR]\n\n"
+"where depending on the INET family\n"
+"      IP are valid IPv4 or IPv6 addresses (or hostnames),\n"
+"      CIDR is a valid IPv4 or IPv6 CIDR prefix.\n"
+"      Adding/deleting multiple elements in IP/CIDR or FROM-TO form\n"
+"      in both IP components are supported for IPv4.\n"
+"      Adding/deleting multiple elements with TCP/SCTP/UDP/UDPLITE\n"
+"      port range is supported both for IPv4 and IPv6.\n";
+
+static struct ipset_type ipset_hash_ipportnet5 = {
+	.name = "hash:ip,port,net",
+	.alias = { "ipportnethash", NULL },
+	.revision = 5,
+	.family = NFPROTO_IPSET_IPV46,
+	.dimension = IPSET_DIM_THREE,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_ip4_single6,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+		[IPSET_DIM_TWO - 1] = {
+			.parse = ipset_parse_proto_port,
+			.print = ipset_print_proto_port,
+			.opt = IPSET_OPT_PORT
+		},
+		[IPSET_DIM_THREE - 1] = {
+			.parse = ipset_parse_ip4_net6,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP2
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = hash_ipportnet_create_args5,
+		[IPSET_ADD] = hash_ipportnet_add_args5,
+		[IPSET_TEST] = hash_ipportnet_test_args5,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = 0,
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_IP2),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_IP2),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_IP2),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_HASHSIZE)
+			| IPSET_FLAG(IPSET_OPT_MAXELEM)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_IP2)
+			| IPSET_FLAG(IPSET_OPT_CIDR2)
+			| IPSET_FLAG(IPSET_OPT_IP2_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_NOMATCH)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_IP2)
+			| IPSET_FLAG(IPSET_OPT_CIDR2)
+			| IPSET_FLAG(IPSET_OPT_IP2_TO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_IP2)
+			| IPSET_FLAG(IPSET_OPT_CIDR2)
+			| IPSET_FLAG(IPSET_OPT_NOMATCH),
+	},
+
+	.usage = hash_ipportnet_usage5,
+	.usagefn = ipset_port_usage,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
@@ -551,4 +745,5 @@ void _init(void)
 	ipset_type_add(&ipset_hash_ipportnet2);
 	ipset_type_add(&ipset_hash_ipportnet3);
 	ipset_type_add(&ipset_hash_ipportnet4);
+	ipset_type_add(&ipset_hash_ipportnet5);
 }
diff --git a/lib/ipset_hash_net.c b/lib/ipset_hash_net.c
index a80d732..0b13304 100644
--- a/lib/ipset_hash_net.c
+++ b/lib/ipset_hash_net.c
@@ -366,6 +366,150 @@ static struct ipset_type ipset_hash_net3 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg hash_net_create_args4[] = {
+	{ .name = { "family", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,		.print = ipset_print_family,
+	},
+	/* Alias: family inet */
+	{ .name = { "-4", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	/* Alias: family inet6 */
+	{ .name = { "-6", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	{ .name = { "hashsize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_HASHSIZE,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "maxelem", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_MAXELEM,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	/* Ignored options: backward compatibilty */
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "probes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PROBES,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ .name = { "resize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_RESIZE,
+	  .parse = ipset_parse_ignored,		.print = ipset_print_number,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_net_add_args4[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "nomatch", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_NOMATCH,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_net_test_args4[] = {
+	{ .name = { "nomatch", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_NOMATCH,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ },
+};
+
+static const char hash_net_usage4[] =
+"create SETNAME hash:net\n"
+"		[family inet|inet6]\n"
+"               [hashsize VALUE] [maxelem VALUE]\n"
+"               [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME IP[/CIDR]|FROM-TO [timeout VALUE] [nomatch]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP[/CIDR]|FROM-TO\n"
+"test   SETNAME IP[/CIDR]\n\n"
+"where depending on the INET family\n"
+"      IP is an IPv4 or IPv6 address (or hostname),\n"
+"      CIDR is a valid IPv4 or IPv6 CIDR prefix.\n"
+"      IP range is not supported with IPv6.\n";
+
+static struct ipset_type ipset_hash_net4 = {
+	.name = "hash:net",
+	.alias = { "nethash", NULL },
+	.revision = 4,
+	.family = NFPROTO_IPSET_IPV46,
+	.dimension = IPSET_DIM_ONE,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_ip4_net6,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = hash_net_create_args4,
+		[IPSET_ADD] = hash_net_add_args4,
+		[IPSET_TEST] = hash_net_test_args4,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = 0,
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_HASHSIZE)
+			| IPSET_FLAG(IPSET_OPT_MAXELEM)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_NOMATCH)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_IP_TO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_NOMATCH),
+	},
+
+	.usage = hash_net_usage4,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
@@ -373,4 +517,5 @@ void _init(void)
 	ipset_type_add(&ipset_hash_net1);
 	ipset_type_add(&ipset_hash_net2);
 	ipset_type_add(&ipset_hash_net3);
+	ipset_type_add(&ipset_hash_net4);
 }
diff --git a/lib/ipset_hash_netnet.c b/lib/ipset_hash_netnet.c
index 5cc5a40..fff7909 100644
--- a/lib/ipset_hash_netnet.c
+++ b/lib/ipset_hash_netnet.c
@@ -42,6 +42,10 @@ static const struct ipset_arg hash_netnet_create_args0[] = {
 	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
 	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
 	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
 	{ },
 };
 
@@ -62,6 +66,10 @@ static const struct ipset_arg hash_netnet_add_args0[] = {
 	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
 	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
 	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
 	{ },
 };
 
@@ -123,7 +131,8 @@ static struct ipset_type ipset_hash_netnet0 = {
 		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_HASHSIZE)
 			| IPSET_FLAG(IPSET_OPT_MAXELEM)
 			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
-			| IPSET_FLAG(IPSET_OPT_COUNTERS),
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
 		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
 			| IPSET_FLAG(IPSET_OPT_CIDR)
 			| IPSET_FLAG(IPSET_OPT_IP_TO)
@@ -133,7 +142,8 @@ static struct ipset_type ipset_hash_netnet0 = {
 			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
 			| IPSET_FLAG(IPSET_OPT_NOMATCH)
 			| IPSET_FLAG(IPSET_OPT_PACKETS)
-			| IPSET_FLAG(IPSET_OPT_BYTES),
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
 		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
 			| IPSET_FLAG(IPSET_OPT_CIDR)
 			| IPSET_FLAG(IPSET_OPT_IP_TO)
diff --git a/lib/ipset_hash_netport.c b/lib/ipset_hash_netport.c
index 2b26cf2..a9fe4eb 100644
--- a/lib/ipset_hash_netport.c
+++ b/lib/ipset_hash_netport.c
@@ -437,6 +437,163 @@ static struct ipset_type ipset_hash_netport4 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg hash_netport_create_args5[] = {
+	{ .name = { "family", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,		.print = ipset_print_family,
+	},
+	/* Alias: family inet */
+	{ .name = { "-4", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	/* Alias: family inet6 */
+	{ .name = { "-6", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_FAMILY,
+	  .parse = ipset_parse_family,
+	},
+	{ .name = { "hashsize", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_HASHSIZE,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "maxelem", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_MAXELEM,
+	  .parse = ipset_parse_uint32,		.print = ipset_print_number,
+	},
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "counters", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COUNTERS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "comments", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_COMMENTS,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_netport_add_args5[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "nomatch", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_NOMATCH,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ .name = { "packets", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_PACKETS,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "bytes", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_BYTES,
+	  .parse = ipset_parse_uint64,		.print = ipset_print_number,
+	},
+	{ .name = { "comment", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const struct ipset_arg hash_netport_test_args5[] = {
+	{ .name = { "nomatch", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_NOMATCH,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ },
+};
+
+static const char hash_netport_usage5[] =
+"create SETNAME hash:net,port\n"
+"		[family inet|inet6]\n"
+"               [hashsize VALUE] [maxelem VALUE]\n"
+"               [timeout VALUE] [counters] [comments]\n"
+"add    SETNAME IP[/CIDR]|FROM-TO,PROTO:PORT [timeout VALUE] [nomatch]\n"
+"               [packets VALUE] [bytes VALUE] [comment \"string\"]\n"
+"del    SETNAME IP[/CIDR]|FROM-TO,PROTO:PORT\n"
+"test   SETNAME IP[/CIDR],PROTO:PORT\n\n"
+"where depending on the INET family\n"
+"      IP is a valid IPv4 or IPv6 address (or hostname),\n"
+"      CIDR is a valid IPv4 or IPv6 CIDR prefix.\n"
+"      Adding/deleting multiple elements with IPv4 is supported.\n"
+"      Adding/deleting multiple elements with TCP/SCTP/UDP/UDPLITE\n"
+"      port range is supported both for IPv4 and IPv6.\n";
+
+static struct ipset_type ipset_hash_netport5 = {
+	.name = "hash:net,port",
+	.alias = { "netporthash", NULL },
+	.revision = 5,
+	.family = NFPROTO_IPSET_IPV46,
+	.dimension = IPSET_DIM_TWO,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_ip4_net6,
+			.print = ipset_print_ip,
+			.opt = IPSET_OPT_IP
+		},
+		[IPSET_DIM_TWO - 1] = {
+			.parse = ipset_parse_proto_port,
+			.print = ipset_print_proto_port,
+			.opt = IPSET_OPT_PORT
+		},
+	},
+	.args = {
+		[IPSET_CREATE] = hash_netport_create_args5,
+		[IPSET_ADD] = hash_netport_add_args5,
+		[IPSET_TEST] = hash_netport_test_args5,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = 0,
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_PORT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_PORT),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_PORT),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_HASHSIZE)
+			| IPSET_FLAG(IPSET_OPT_MAXELEM)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_COMMENTS),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_NOMATCH)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_IP_TO)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PORT_TO)
+			| IPSET_FLAG(IPSET_OPT_PROTO),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_IP)
+			| IPSET_FLAG(IPSET_OPT_CIDR)
+			| IPSET_FLAG(IPSET_OPT_PORT)
+			| IPSET_FLAG(IPSET_OPT_PROTO)
+			| IPSET_FLAG(IPSET_OPT_NOMATCH),
+	},
+
+	.usage = hash_netport_usage5,
+	.usagefn = ipset_port_usage,
+	.description = "comments support",
+};
+
 void _init(void);
 void _init(void)
 {
@@ -444,4 +601,5 @@ void _init(void)
 	ipset_type_add(&ipset_hash_netport2);
 	ipset_type_add(&ipset_hash_netport3);
 	ipset_type_add(&ipset_hash_netport4);
+	ipset_type_add(&ipset_hash_netport5);
 }
-- 
1.8.3.2