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

[PATCH v2 0/7] 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>

Another re-roll, with requested changes applied. I've also made the allocation
code a bit more robust by having it fall back to vmalloc should kmalloc fail to
do the oh-so-needful. Additionally, the documentation was somewhat lacking
(rather, non-existent) so I've corrected that too.

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

Oliver Smith (7):
  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.
  netfilter: ipset: Support comments in the list-type ipset.
  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                    |  15 ++
 include/libipset/parse.h                           |   2 +
 include/libipset/print.h                           |   3 +
 kernel/include/linux/netfilter/ipset/ip_set.h      |  32 +++-
 .../include/linux/netfilter/ipset/ip_set_comment.h |  65 +++++++
 kernel/include/uapi/linux/netfilter/ipset/ip_set.h |   7 +
 kernel/net/netfilter/ipset/ip_set_bitmap_gen.h     |  18 +-
 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       |  18 +-
 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 +-
 kernel/net/netfilter/ipset/ip_set_list_set.c       |  22 ++-
 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/ipset_list_set.c                               | 108 ++++++++++++
 lib/libipset.map                                   |   7 +
 lib/parse.c                                        |  27 +++
 lib/print.c                                        |  31 ++++
 lib/session.c                                      |   8 +-
 lib/types.c                                        |   4 +-
 src/ipset.8                                        |  59 ++++---
 src/ipset.c                                        |  54 +++++-
 42 files changed, 1649 insertions(+), 70 deletions(-)
 create mode 100644 kernel/include/linux/netfilter/ipset/ip_set_comment.h

-- 
1.8.3.2

[PATCH v2 1/7] 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      | 32 ++++++++---
 .../include/linux/netfilter/ipset/ip_set_comment.h | 65 ++++++++++++++++++++++
 kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  4 ++
 kernel/net/netfilter/ipset/ip_set_core.c           | 14 +++++
 4 files changed, 107 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..aaa166b 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,13 @@ 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)
 
 /* 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 +90,7 @@ struct ip_set_ext {
 	u64 packets;
 	u64 bytes;
 	u32 timeout;
+	char *comment;
 };
 
 struct ip_set_counter {
@@ -93,20 +98,19 @@ 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_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 +228,17 @@ struct ip_set {
 };
 
 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.
+	 */
+	if (SET_WITH_COMMENT(set))
+		ip_set_extensions[IPSET_EXT_ID_COMMENT].destroy(
+			ext_comment(data, set));
+}
+
+static inline void
 ip_set_add_bytes(u64 bytes, struct ip_set_counter *counter)
 {
 	atomic64_add((long long)bytes, &(counter)->bytes);
@@ -426,6 +441,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..71dfe10
--- /dev/null
+++ b/kernel/include/linux/netfilter/ipset/ip_set_comment.h
@@ -0,0 +1,65 @@
+#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_ATOMIC);
+	if (unlikely(!comment->str)) {
+		comment->str = kzalloc(len + 1, GFP_KERNEL);
+		if (unlikely(!comment->str)) {
+			comment->str = vzalloc(len + 1);
+			if (unlikely(!comment->str))
+				return;
+		}
+	}
+	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;
+	if (unlikely(is_vmalloc_addr(comment->str)))
+		vfree(comment->str);
+	else
+		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..f177d99 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_COMMENT = 4,
+	IPSET_FLAG_WITH_COMMENT = (1 << IPSET_FLAG_BIT_WITH_COMMENT),
 	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..29ce622 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_COMMENT,
+		.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 v2 1/7] netfilter: ipset: Support comments for ipset entries in the core.

[Jozsef Kadlecsik]

On Fri, 20 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      | 32 ++++++++---
>  .../include/linux/netfilter/ipset/ip_set_comment.h | 65 ++++++++++++++++++++++
>  kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  4 ++
>  kernel/net/netfilter/ipset/ip_set_core.c           | 14 +++++
>  4 files changed, 107 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..aaa166b 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,13 @@ 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)
>  
>  /* 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 +90,7 @@ struct ip_set_ext {
>  	u64 packets;
>  	u64 bytes;
>  	u32 timeout;
> +	char *comment;
>  };
>  
>  struct ip_set_counter {
> @@ -93,20 +98,19 @@ 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_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 +228,17 @@ struct ip_set {
>  };
>  
>  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.
> +	 */
> +	if (SET_WITH_COMMENT(set))
> +		ip_set_extensions[IPSET_EXT_ID_COMMENT].destroy(
> +			ext_comment(data, set));
> +}
> +
> +static inline void
>  ip_set_add_bytes(u64 bytes, struct ip_set_counter *counter)
>  {
>  	atomic64_add((long long)bytes, &(counter)->bytes);
> @@ -426,6 +441,7 @@ bitmap_bytes(u32 a, u32 b)
>  }
>  
>  #include <linux/netfilter/ipset/ip_set_timeout.h>
> +#include <linux/netfilter/ipset/ip_set_comment.h>

You should have received an error at compiling: 
IP_SET_MAX_COMMENT_SIZE is defined both in 
uapi/linux/netfilter/ipset/ip_set.h and 
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..71dfe10
> --- /dev/null
> +++ b/kernel/include/linux/netfilter/ipset/ip_set_comment.h
> @@ -0,0 +1,65 @@
> +#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

Remove IP_SET_MAX_COMMENT_SIZE from this header file, it must be in
uapi/linux/netfilter/ipset/ip_set.h.

[IPSET_ATTR_COMMENT is also defined in another header file and this one 
depends on it.]

> +#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_ATOMIC);
> +	if (unlikely(!comment->str)) {
> +		comment->str = kzalloc(len + 1, GFP_KERNEL);
> +		if (unlikely(!comment->str)) {
> +			comment->str = vzalloc(len + 1);
> +			if (unlikely(!comment->str))
> +				return;
> +		}
> +	}
> +	strlcpy(comment->str, ext->comment, len + 1);
> +}

This cascading of allocation attempts is simply too much. Also, we must 
use the flag GFP_ATOMIC, because we are holding the set lock (I didn't 
notice it previously, sorry). So please fall back to a single allocation 
with kzalloc and that's all (and keep the checking of it's result, of 
course).

Move the

     if (unlikely(comment->str))
             kfree(comment->str);

part before

     if (!len)
 	    return;

and extend it to

     if (unlikely(comment->str)) {
             kfree(comment->str);
	     comment->str = NULL;
     }

That way we support "deleting" just the comment part of an element (user 
can readd the element without comment). But therefore the "unlikely" part 
of the test should probably be removed.

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

This function must be the type of "int" and so the first check return "0".
nla_put_string returns zero or -EMSGSIZE.

> +static inline void
> +ip_set_comment_free(struct ip_set_comment *comment)
> +{
> +	if (unlikely(!comment->str))
> +		return;
> +	if (unlikely(is_vmalloc_addr(comment->str)))
> +		vfree(comment->str);
> +	else
> +		kfree(comment->str);
> +	comment->str = NULL;

Restore the previous ip_set_comment_free version, without the vfree part.

> +
> +#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..f177d99 100644
> --- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> +++ b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h

The definition of IP_SET_MAX_COMMENT_SIZE must come here.

> @@ -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_COMMENT = 4,
> +	IPSET_FLAG_WITH_COMMENT = (1 << IPSET_FLAG_BIT_WITH_COMMENT),
>  	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..29ce622 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_COMMENT,
> +		.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

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

[Jozsef Kadlecsik]

On Fri, 20 Sep 2013, Jozsef Kadlecsik wrote:

> On Fri, 20 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      | 32 ++++++++---
> >  .../include/linux/netfilter/ipset/ip_set_comment.h | 65 ++++++++++++++++++++++
> >  kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  4 ++
> >  kernel/net/netfilter/ipset/ip_set_core.c           | 14 +++++
> >  4 files changed, 107 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..aaa166b 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,13 @@ 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)
> >  
> >  /* 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 +90,7 @@ struct ip_set_ext {
> >  	u64 packets;
> >  	u64 bytes;
> >  	u32 timeout;
> > +	char *comment;
> >  };
> >  
> >  struct ip_set_counter {
> > @@ -93,20 +98,19 @@ 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_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 +228,17 @@ struct ip_set {
> >  };
> >  
> >  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.
> > +	 */
> > +	if (SET_WITH_COMMENT(set))
> > +		ip_set_extensions[IPSET_EXT_ID_COMMENT].destroy(
> > +			ext_comment(data, set));
> > +}
> > +
> > +static inline void
> >  ip_set_add_bytes(u64 bytes, struct ip_set_counter *counter)
> >  {
> >  	atomic64_add((long long)bytes, &(counter)->bytes);
> > @@ -426,6 +441,7 @@ bitmap_bytes(u32 a, u32 b)
> >  }
> >  
> >  #include <linux/netfilter/ipset/ip_set_timeout.h>
> > +#include <linux/netfilter/ipset/ip_set_comment.h>
> 
> You should have received an error at compiling: 
> IP_SET_MAX_COMMENT_SIZE is defined both in 
> uapi/linux/netfilter/ipset/ip_set.h and 
> linux/netfilter/ipset/ip_set_comment.h.

The double definition confused me: in ip_set_comment.h it is 
IP_SET_MAX_COMMENT_SIZE, in ip_set.h it is IPSET_MAX_COMMENT_SIZE. Keep 
the latter only.

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 v2 2/7] 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       | 18 ++++++++++++++----
 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, 28 insertions(+), 11 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
index 59ae854..eb5b71c 100644
--- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h
+++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
@@ -701,6 +701,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();
@@ -891,6 +893,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	const struct htable *t;
 	struct nlattr *nested;
 	size_t memsize;
+	u32 cadt_flags = 0;
 
 	t = rcu_dereference_bh_nfnl(h->table);
 	memsize = mtype_ahash_memsize(h, t, NLEN(set->family), set->dsize);
@@ -910,10 +913,14 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 	if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
 	    nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)) ||
 	    ((set->extensions & IPSET_EXT_TIMEOUT) &&
-	     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))))
+	     nla_put_net32(skb, IPSET_ATTR_TIMEOUT, htonl(set->timeout))))
+		goto nla_put_failure;
+	if (set->extensions & IPSET_EXT_COUNTER)
+		cadt_flags |= IPSET_FLAG_WITH_COUNTERS;
+	if (set->extensions & IPSET_EXT_COMMENT)
+		cadt_flags |= IPSET_FLAG_WITH_COMMENT;
+	if (cadt_flags && nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
+					htonl(cadt_flags)))
 		goto nla_put_failure;
 	ipset_nest_end(skb, nested);
 
@@ -970,6 +977,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 v2 2/7] netfilter: ipset: Support comments in hash-type ipsets.

[Jozsef Kadlecsik]

On Fri, 20 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.

This patch and the one for the bitmap type look all right. However both 
(and the list type) beg for a little simplification:
 
> 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       | 18 ++++++++++++++----
>  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, 28 insertions(+), 11 deletions(-)
> 
> diff --git a/kernel/net/netfilter/ipset/ip_set_hash_gen.h b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
> index 59ae854..eb5b71c 100644
> --- a/kernel/net/netfilter/ipset/ip_set_hash_gen.h
> +++ b/kernel/net/netfilter/ipset/ip_set_hash_gen.h
> @@ -701,6 +701,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();
> @@ -891,6 +893,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
>  	const struct htable *t;
>  	struct nlattr *nested;
>  	size_t memsize;
> +	u32 cadt_flags = 0;
>  
>  	t = rcu_dereference_bh_nfnl(h->table);
>  	memsize = mtype_ahash_memsize(h, t, NLEN(set->family), set->dsize);
> @@ -910,10 +913,14 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
>  	if (nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
>  	    nla_put_net32(skb, IPSET_ATTR_MEMSIZE, htonl(memsize)) ||
>  	    ((set->extensions & IPSET_EXT_TIMEOUT) &&
> -	     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))))
> +	     nla_put_net32(skb, IPSET_ATTR_TIMEOUT, htonl(set->timeout))))
> +		goto nla_put_failure;
> +	if (set->extensions & IPSET_EXT_COUNTER)
> +		cadt_flags |= IPSET_FLAG_WITH_COUNTERS;
> +	if (set->extensions & IPSET_EXT_COMMENT)
> +		cadt_flags |= IPSET_FLAG_WITH_COMMENT;
> +	if (cadt_flags && nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
> +					htonl(cadt_flags)))

These three lines (and the definition of cadt_flags) should be moved into 
a little inline function in ip_set.h and then that simply called like 
this

	if (ip_set_put_flags(skb, set))
		goto nla_put_failure;

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 v2 2/7] netfilter: ipset: Support comments in hash-type ipsets.

[Jozsef Kadlecsik]

On Fri, 20 Sep 2013, Jozsef Kadlecsik wrote:

> On Fri, 20 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.
> 
> This patch and the one for the bitmap type look all right.

I have to correct myself: the IPSET_ATTR_COMMENT attribute is not added to 
the adt_policy of the set type definitions.

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 v2 3/7] 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   | 18 ++++++++++++++----
 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, 20 insertions(+), 7 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..cbc6487 100644
--- a/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
+++ b/kernel/net/netfilter/ipset/ip_set_bitmap_gen.h
@@ -92,6 +92,7 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 {
 	const struct mtype *map = set->data;
 	struct nlattr *nested;
+	u32 cadt_flags = 0;
 
 	nested = ipset_nest_start(skb, IPSET_ATTR_DATA);
 	if (!nested)
@@ -103,10 +104,14 @@ mtype_head(struct ip_set *set, struct sk_buff *skb)
 				map->memsize +
 				set->dsize * map->elements)) ||
 	    (SET_WITH_TIMEOUT(set) &&
-	     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))))
+	     nla_put_net32(skb, IPSET_ATTR_TIMEOUT, htonl(set->timeout))))
+		goto nla_put_failure;
+	if (SET_WITH_COUNTER(set))
+		cadt_flags |= IPSET_FLAG_WITH_COUNTERS;
+	if (SET_WITH_COMMENT(set))
+		cadt_flags |= IPSET_FLAG_WITH_COMMENT;
+	if (cadt_flags && nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
+	    htonl(cadt_flags)))
 		goto nla_put_failure;
 	ipset_nest_end(skb, nested);
 
@@ -162,6 +167,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 +240,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 v2 4/7] netfilter: ipset: Support comments in the list-type ipset.

[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 list ipsets with the comment
annotation extension.

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_list_set.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/kernel/net/netfilter/ipset/ip_set_list_set.c b/kernel/net/netfilter/ipset/ip_set_list_set.c
index 30bf1dd..f1ca3d0 100644
--- a/kernel/net/netfilter/ipset/ip_set_list_set.c
+++ b/kernel/net/netfilter/ipset/ip_set_list_set.c
@@ -16,7 +16,8 @@
 #include <linux/netfilter/ipset/ip_set_list.h>
 
 #define IPSET_TYPE_REV_MIN	0
-#define IPSET_TYPE_REV_MAX	1 /* Counters support added */
+/*				1    Counters support added */
+#define IPSET_TYPE_REV_MAX	2 /* Comments support added */
 
 MODULE_LICENSE("GPL");
 MODULE_AUTHOR("Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>");
@@ -191,6 +192,8 @@ list_set_add(struct ip_set *set, u32 i, struct set_adt_elem *d,
 		ip_set_timeout_set(ext_timeout(e, set), ext->timeout);
 	if (SET_WITH_COUNTER(set))
 		ip_set_init_counter(ext_counter(e, set), ext);
+	if (SET_WITH_COMMENT(set) && ext->comment)
+		ip_set_init_comment(ext_comment(e, set), ext);
 	return 0;
 }
 
@@ -299,6 +302,8 @@ list_set_uadd(struct ip_set *set, void *value, const struct ip_set_ext *ext,
 			ip_set_timeout_set(ext_timeout(e, set), ext->timeout);
 		if (SET_WITH_COUNTER(set))
 			ip_set_init_counter(ext_counter(e, set), ext);
+		if (SET_WITH_COMMENT(set))
+			ip_set_init_comment(ext_comment(e, set), ext);
 		/* Set is already added to the list */
 		ip_set_put_byindex(d->id);
 		return 0;
@@ -456,6 +461,7 @@ list_set_head(struct ip_set *set, struct sk_buff *skb)
 {
 	const struct list_set *map = set->data;
 	struct nlattr *nested;
+	u32 cadt_flags = 0;
 
 	nested = ipset_nest_start(skb, IPSET_ATTR_DATA);
 	if (!nested)
@@ -463,13 +469,17 @@ list_set_head(struct ip_set *set, struct sk_buff *skb)
 	if (nla_put_net32(skb, IPSET_ATTR_SIZE, htonl(map->size)) ||
 	    (SET_WITH_TIMEOUT(set) &&
 	     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))) ||
 	    nla_put_net32(skb, IPSET_ATTR_REFERENCES, htonl(set->ref - 1)) ||
 	    nla_put_net32(skb, IPSET_ATTR_MEMSIZE,
 			  htonl(sizeof(*map) + map->size * set->dsize)))
 		goto nla_put_failure;
+	if (SET_WITH_COUNTER(set))
+		cadt_flags |= IPSET_FLAG_WITH_COUNTERS;
+	if (SET_WITH_COMMENT(set))
+		cadt_flags |= IPSET_FLAG_WITH_COMMENT;
+	if (cadt_flags && nla_put_net32(skb, IPSET_ATTR_CADT_FLAGS,
+					htonl(cadt_flags)))
+		goto nla_put_failure;
 	ipset_nest_end(skb, nested);
 
 	return 0;
@@ -516,6 +526,9 @@ list_set_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);
 	}
 finish:
@@ -660,6 +673,7 @@ static struct ip_set_type list_set_type __read_mostly = {
 		[IPSET_ATTR_CADT_FLAGS]	= { .type = NLA_U32 },
 		[IPSET_ATTR_BYTES]	= { .type = NLA_U64 },
 		[IPSET_ATTR_PACKETS]	= { .type = NLA_U64 },
+		[IPSET_ATTR_COMMENT]	= { .type = NLA_STRING },
 	},
 	.me		= THIS_MODULE,
 };
-- 
1.8.3.2

[PATCH v2 5/7] 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 | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 45 insertions(+), 9 deletions(-)

diff --git a/src/ipset.c b/src/ipset.c
index 4f308da..fe73f96 100644
--- a/src/ipset.c
+++ b/src/ipset.c
@@ -159,25 +159,59 @@ ipset_print_file(const char *fmt, ...)
 static void
 build_argv(char *buffer)
 {
-	char *ptr;
+	char *tmp, *arg;
 	int i;
+	bool quoted = false;
 
 	/* 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;
-	while ((ptr = strtok(NULL, " \t\r\n")) != NULL) {
-		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *)))
-			newargv[newargc++] = ptr;
-		else {
+	arg = calloc(strlen(buffer) + 1, sizeof(*buffer));
+	for (tmp = buffer, i = 0; *tmp; tmp++) {
+		if ((newargc + 1) == (int)(sizeof(newargv)/sizeof(char *))) {
 			exit_error(PARAMETER_PROBLEM,
 				   "Line is too long to parse.");
 			return;
 		}
+		switch (*tmp) {
+		case '\\':
+			arg[i++] = *++tmp;
+			if (*(tmp+1))
+				continue;
+			break;
+		case '"':
+			quoted = !quoted;
+			if (*(tmp+1))
+				continue;
+			break;
+		case ' ':
+		case '\r':
+		case '\n':
+			if (!quoted)
+				break;
+			arg[i++] = *tmp;
+			continue;
+		default:
+			arg[i++] = *tmp;
+			if (*(tmp+1))
+				continue;
+			break;
+		}
+		if (!*(tmp+1) && quoted) {
+			exit_error(PARAMETER_PROBLEM, "missing close quote");
+			return;
+		}
+		newargv[newargc] = calloc(strlen(arg) + 1, sizeof(*arg));
+		ipset_strlcpy(newargv[newargc++], arg, strlen(arg) + 1);
+		memset(arg, 0, strlen(arg) + 1);
+		i = 0;
 	}
+	free(arg);
 }
 
 /* Main parser function, workhorse */
@@ -195,7 +229,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 +267,7 @@ restore(char *argv0)
 	if (ret < 0)
 		handle_error();
 
+	free(newargv[0]);
 	return ret;
 }
 
-- 
1.8.3.2

Re: [PATCH v2 5/7] ipset: Rework the "fake" argument parsing for ipset restore.

[Jozsef Kadlecsik]

On Fri, 20 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 | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 45 insertions(+), 9 deletions(-)
> 
> diff --git a/src/ipset.c b/src/ipset.c
> index 4f308da..fe73f96 100644
> --- a/src/ipset.c
> +++ b/src/ipset.c
> @@ -159,25 +159,59 @@ ipset_print_file(const char *fmt, ...)
>  static void
>  build_argv(char *buffer)
>  {
> -	char *ptr;
> +	char *tmp, *arg;
>  	int i;
> +	bool quoted = false;
>  
>  	/* 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;
> -	while ((ptr = strtok(NULL, " \t\r\n")) != NULL) {
> -		if ((newargc + 1) < (int)(sizeof(newargv)/sizeof(char *)))
> -			newargv[newargc++] = ptr;
> -		else {
> +	arg = calloc(strlen(buffer) + 1, sizeof(*buffer));
> +	for (tmp = buffer, i = 0; *tmp; tmp++) {
> +		if ((newargc + 1) == (int)(sizeof(newargv)/sizeof(char *))) {
>  			exit_error(PARAMETER_PROBLEM,
>  				   "Line is too long to parse.");
>  			return;
>  		}
> +		switch (*tmp) {
> +		case '\\':
> +			arg[i++] = *++tmp;
> +			if (*(tmp+1))
> +				continue;
> +			break;

This opens up a can of worms: if escape character is accepted, then that 
should be put back at printing/saving. Otherwise comments like
"This is \"a funny\" comment" will be parsed right, but printed/saved 
broken. Maybe it's much simpler to emit an error and reject escaping.

> +		case '"':
> +			quoted = !quoted;
> +			if (*(tmp+1))
> +				continue;
> +			break;
> +		case ' ':
> +		case '\r':
> +		case '\n':
> +			if (!quoted)
> +				break;
> +			arg[i++] = *tmp;
> +			continue;

The handling of "\t" is missing.

> +		default:
> +			arg[i++] = *tmp;
> +			if (*(tmp+1))
> +				continue;
> +			break;
> +		}
> +		if (!*(tmp+1) && quoted) {
> +			exit_error(PARAMETER_PROBLEM, "missing close quote");
> +			return;
> +		}
> +		newargv[newargc] = calloc(strlen(arg) + 1, sizeof(*arg));
> +		ipset_strlcpy(newargv[newargc++], arg, strlen(arg) + 1);
> +		memset(arg, 0, strlen(arg) + 1);
> +		i = 0;
>  	}
> +	free(arg);
>  }
>  
>  /* Main parser function, workhorse */
> @@ -195,7 +229,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 +267,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

[PATCH v2 6/7] 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                    | 15 ++++++++++
 include/libipset/parse.h                           |  2 ++
 include/libipset/print.h                           |  3 ++
 kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  3 ++
 lib/data.c                                         | 34 ++++++++++++++++++++++
 lib/debug.c                                        |  1 +
 lib/errcode.c                                      |  2 ++
 lib/libipset.map                                   |  7 +++++
 lib/parse.c                                        | 27 +++++++++++++++++
 lib/print.c                                        | 31 ++++++++++++++++++++
 lib/session.c                                      |  8 ++++-
 lib/types.c                                        |  4 +--
 14 files changed, 140 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..8806f15 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_CREATE_COMMENT,
+	IPSET_OPT_ADT_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_ADT_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..847bbff 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_MAX_COMMENT_SIZE	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_COMMENT = 4,
+	IPSET_FLAG_WITH_COMMENT = (1 << IPSET_FLAG_BIT_WITH_COMMENT),
 	IPSET_FLAG_CADT_MAX	= 15,
 };
 
@@ -250,6 +257,14 @@ struct ip_set_req_get_set {
 #define IP_SET_OP_GET_BYINDEX	0x00000007	/* Get set name by index */
 /* Uses ip_set_req_get_set */
 
+#define IP_SET_OP_GET_FNAME	0x00000008	/* Get set index and family */
+struct ip_set_req_get_set_family {
+	unsigned int op;
+	unsigned int version;
+	unsigned int family;
+	union ip_set_name_index set;
+};
+
 #define IP_SET_OP_VERSION	0x00000100	/* Ask kernel version */
 struct ip_set_req_version {
 	unsigned int op;
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/kernel/include/uapi/linux/netfilter/ipset/ip_set.h b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
index f177d99..847bbff 100644
--- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
+++ b/kernel/include/uapi/linux/netfilter/ipset/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_MAX_COMMENT_SIZE	255
+
 /* Message types and commands */
 enum ipset_cmd {
 	IPSET_CMD_NONE,
diff --git a/lib/data.c b/lib/data.c
index 04a5997..17c2b68 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_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_CREATE_COMMENT:
+		cadt_flag_type_attr(data, opt, IPSET_FLAG_WITH_COMMENT);
+		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_ADT_COMMENT:
+		ipset_strlcpy(data->adt.comment, value, IPSET_MAX_COMMENT_SIZE);
+		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_COMMENT)
+			ipset_data_flags_set(data,
+					  IPSET_FLAG(IPSET_OPT_CREATE_COMMENT));
 		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_ADT_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_CREATE_COMMENT:
 		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_ADT_COMMENT:
+		return IPSET_MAX_COMMENT_SIZE;
 	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..160d9ad 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 string is too long!" },
 
 	/* ADD specific error codes */
 	{ IPSET_ERR_EXIST, IPSET_CMD_ADD,
diff --git a/lib/libipset.map b/lib/libipset.map
index 271fe59..ab0b96f 100644
--- a/lib/libipset.map
+++ b/lib/libipset.map
@@ -127,3 +127,10 @@ LIBIPSET_4.0 {
 global:
   ipset_parse_uint64;
 } LIBIPSET_3.0;
+
+LIBIPSET_4.1 {
+global:
+  ipset_parse_comment;
+  ipset_print_comment;
+  ipset_strlcat;
+} LIBIPSET_4.0;
diff --git a/lib/parse.c b/lib/parse.c
index 112b273..487d82d 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_ADT_COMMENT);
+	assert(str);
+
+	data = ipset_session_data(session);
+	if (strlen(str) > IPSET_MAX_COMMENT_SIZE)
+		return syntax_err("comment is longer than the maximum allowed "
+				  "%d characters", IPSET_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..abdfd34 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_ADT_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..546431d 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_ADT_COMMENT,
+		.len  = IPSET_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) > policy[type].len) {
 		return MNL_CB_ERROR;
+	}
 	tb[type] = attr;
 	return MNL_CB_OK;
 }
diff --git a/lib/types.c b/lib/types.c
index adaba83..e2a41ad 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 < 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 v2 6/7] ipset: Support comments in the userspace library.

[Jozsef Kadlecsik]

On Fri, 20 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                    | 15 ++++++++++
>  include/libipset/parse.h                           |  2 ++
>  include/libipset/print.h                           |  3 ++
>  kernel/include/uapi/linux/netfilter/ipset/ip_set.h |  3 ++
>  lib/data.c                                         | 34 ++++++++++++++++++++++
>  lib/debug.c                                        |  1 +
>  lib/errcode.c                                      |  2 ++
>  lib/libipset.map                                   |  7 +++++
>  lib/parse.c                                        | 27 +++++++++++++++++
>  lib/print.c                                        | 31 ++++++++++++++++++++
>  lib/session.c                                      |  8 ++++-
>  lib/types.c                                        |  4 +--
>  14 files changed, 140 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..8806f15 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_CREATE_COMMENT,
> +	IPSET_OPT_ADT_COMMENT,
>  	/* Internal options */
>  	IPSET_OPT_FLAGS = 48,	/* IPSET_FLAG_EXIST| */
>  	IPSET_OPT_CADT_FLAGS,	/* IPSET_FLAG_BEFORE| */

The IPSET_FLAG(IPSET_OPT_CREATE_COMMENT) must be added to the definition 
of IPSET_CREATE_FLAGS.


> @@ -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_ADT_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..847bbff 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_MAX_COMMENT_SIZE	255
> +

In ip_set_comment.h (from where to be moved into uapi/.../ip_set.h) it was 
named as IP_SET_MAX_COMMENT_SIZE. Use the same name everywhere, i.e. 
IPSET_MAX_COMMENT_SIZE.

This file is a plain copy of uapi/linux/netfilter/ipset/ip_set.h.

>  /* 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_COMMENT = 4,
> +	IPSET_FLAG_WITH_COMMENT = (1 << IPSET_FLAG_BIT_WITH_COMMENT),
>  	IPSET_FLAG_CADT_MAX	= 15,
>  };
>  
> @@ -250,6 +257,14 @@ struct ip_set_req_get_set {
>  #define IP_SET_OP_GET_BYINDEX	0x00000007	/* Get set name by index */
>  /* Uses ip_set_req_get_set */
>  
> +#define IP_SET_OP_GET_FNAME	0x00000008	/* Get set index and family */
> +struct ip_set_req_get_set_family {
> +	unsigned int op;
> +	unsigned int version;
> +	unsigned int family;
> +	union ip_set_name_index set;
> +};
> +
>  #define IP_SET_OP_VERSION	0x00000100	/* Ask kernel version */
>  struct ip_set_req_version {
>  	unsigned int op;
> 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/kernel/include/uapi/linux/netfilter/ipset/ip_set.h b/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> index f177d99..847bbff 100644
> --- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> +++ b/kernel/include/uapi/linux/netfilter/ipset/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_MAX_COMMENT_SIZE	255
> +
>  /* Message types and commands */
>  enum ipset_cmd {
>  	IPSET_CMD_NONE,

The patch above for kernel/include/... should not be here at all.

> diff --git a/lib/data.c b/lib/data.c
> index 04a5997..17c2b68 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_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_CREATE_COMMENT:
> +		cadt_flag_type_attr(data, opt, IPSET_FLAG_WITH_COMMENT);
> +		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_ADT_COMMENT:
> +		ipset_strlcpy(data->adt.comment, value, IPSET_MAX_COMMENT_SIZE);
> +		break;

In the kernel part you interpreted IPSET_MAX_COMMENT_SIZE as the max 
length without the null character. So be consistent with it in userspace: 
above it should be IPSET_MAX_COMMENT_SIZE + 1.

>  	/* 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_COMMENT)
> +			ipset_data_flags_set(data,
> +					  IPSET_FLAG(IPSET_OPT_CREATE_COMMENT));
>  		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_ADT_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_CREATE_COMMENT:
>  		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_ADT_COMMENT:
> +		return IPSET_MAX_COMMENT_SIZE;

This is also IPSET_MAX_COMMENT_SIZE + 1;

>  	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..160d9ad 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 string is too long!" },
>  
>  	/* ADD specific error codes */
>  	{ IPSET_ERR_EXIST, IPSET_CMD_ADD,
> diff --git a/lib/libipset.map b/lib/libipset.map
> index 271fe59..ab0b96f 100644
> --- a/lib/libipset.map
> +++ b/lib/libipset.map
> @@ -127,3 +127,10 @@ LIBIPSET_4.0 {
>  global:
>    ipset_parse_uint64;
>  } LIBIPSET_3.0;
> +
> +LIBIPSET_4.1 {
> +global:
> +  ipset_parse_comment;
> +  ipset_print_comment;
> +  ipset_strlcat;
> +} LIBIPSET_4.0;
> diff --git a/lib/parse.c b/lib/parse.c
> index 112b273..487d82d 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_ADT_COMMENT);
> +	assert(str);
> +
> +	data = ipset_session_data(session);
> +	if (strlen(str) > IPSET_MAX_COMMENT_SIZE)
> +		return syntax_err("comment is longer than the maximum allowed "
> +				  "%d characters", IPSET_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..abdfd34 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_ADT_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..546431d 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_ADT_COMMENT,
> +		.len  = IPSET_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) > policy[type].len) {
>  		return MNL_CB_ERROR;
> +	}
>  	tb[type] = attr;
>  	return MNL_CB_OK;
>  }
> diff --git a/lib/types.c b/lib/types.c
> index adaba83..e2a41ad 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 < 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

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 v2 7/7] 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 ++++++++++++++++++++++++++++++++++++
 lib/ipset_list_set.c       | 108 +++++++++++++++++++++++++
 src/ipset.8                |  59 ++++++++------
 11 files changed, 1291 insertions(+), 26 deletions(-)

diff --git a/lib/ipset_bitmap_ip.c b/lib/ipset_bitmap_ip.c
index a4726db..af63c99 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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..084f2fc 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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..26b2023 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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..45185ec 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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..c9dc4c1 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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..4baabe5 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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..99ffc1f 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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 ea65c8f..0e617af 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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_CREATE_COMMENT),
 		[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_ADT_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..3a41456 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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .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_ADT_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] [comment]\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_CREATE_COMMENT),
+		[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_ADT_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 = "comment 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);
 }
diff --git a/lib/ipset_list_set.c b/lib/ipset_list_set.c
index 6cec67c..9da3204 100644
--- a/lib/ipset_list_set.c
+++ b/lib/ipset_list_set.c
@@ -189,9 +189,117 @@ static struct ipset_type ipset_list_set1 = {
 	.description = "counters support",
 };
 
+/* Parse commandline arguments */
+static const struct ipset_arg list_set_create_args2[] = {
+	{ .name = { "size", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_SIZE,
+	  .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 = { "comment", NULL },
+	  .has_arg = IPSET_NO_ARG,		.opt = IPSET_OPT_CREATE_COMMENT,
+	  .parse = ipset_parse_flag,		.print = ipset_print_flag,
+	},
+	{ },
+};
+
+static const struct ipset_arg list_set_adt_args2[] = {
+	{ .name = { "timeout", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_TIMEOUT,
+	  .parse = ipset_parse_timeout,		.print = ipset_print_number,
+	},
+	{ .name = { "before", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_NAMEREF,
+	  .parse = ipset_parse_before,
+	},
+	{ .name = { "after", NULL },
+	  .has_arg = IPSET_MANDATORY_ARG,	.opt = IPSET_OPT_NAMEREF,
+	  .parse = ipset_parse_after,
+	},
+	{ .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_ADT_COMMENT,
+	  .parse = ipset_parse_comment,		.print = ipset_print_comment,
+	},
+	{ },
+};
+
+static const char list_set_usage2[] =
+"create SETNAME list:set\n"
+"               [size VALUE] [timeout VALUE] [counters] [comment]\n"
+"add    SETNAME NAME [before|after NAME] [timeout VALUE]\n"
+"               [packets VALUE] [bytes VALUE] [comment STRING]\n"
+"del    SETNAME NAME [before|after NAME]\n"
+"test   SETNAME NAME [before|after NAME]\n\n"
+"where NAME are existing set names.\n";
+
+static struct ipset_type ipset_list_set2 = {
+	.name = "list:set",
+	.alias = { "setlist", NULL },
+	.revision = 2,
+	.family = NFPROTO_UNSPEC,
+	.dimension = IPSET_DIM_ONE,
+	.elem = {
+		[IPSET_DIM_ONE - 1] = {
+			.parse = ipset_parse_setname,
+			.print = ipset_print_name,
+			.opt = IPSET_OPT_NAME
+		},
+	},
+	.compat_parse_elem = ipset_parse_name_compat,
+	.args = {
+		[IPSET_CREATE] = list_set_create_args2,
+		[IPSET_ADD] = list_set_adt_args2,
+		[IPSET_DEL] = list_set_adt_args2,
+		[IPSET_TEST] = list_set_adt_args2,
+	},
+	.mandatory = {
+		[IPSET_CREATE] = 0,
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_NAME),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_NAME),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_NAME),
+	},
+	.full = {
+		[IPSET_CREATE] = IPSET_FLAG(IPSET_OPT_SIZE)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_COUNTERS)
+			| IPSET_FLAG(IPSET_OPT_CREATE_COMMENT),
+		[IPSET_ADD] = IPSET_FLAG(IPSET_OPT_NAME)
+			| IPSET_FLAG(IPSET_OPT_BEFORE)
+			| IPSET_FLAG(IPSET_OPT_NAMEREF)
+			| IPSET_FLAG(IPSET_OPT_TIMEOUT)
+			| IPSET_FLAG(IPSET_OPT_PACKETS)
+			| IPSET_FLAG(IPSET_OPT_BYTES)
+			| IPSET_FLAG(IPSET_OPT_ADT_COMMENT),
+		[IPSET_DEL] = IPSET_FLAG(IPSET_OPT_NAME)
+			| IPSET_FLAG(IPSET_OPT_BEFORE)
+			| IPSET_FLAG(IPSET_OPT_NAMEREF),
+		[IPSET_TEST] = IPSET_FLAG(IPSET_OPT_NAME)
+			| IPSET_FLAG(IPSET_OPT_BEFORE)
+			| IPSET_FLAG(IPSET_OPT_NAMEREF),
+	},
+
+	.usage = list_set_usage2,
+	.description = "comment support",
+};
 void _init(void);
 void _init(void)
 {
 	ipset_type_add(&ipset_list_set0);
 	ipset_type_add(&ipset_list_set1);
+	ipset_type_add(&ipset_list_set2);
 }
diff --git a/src/ipset.8 b/src/ipset.8
index b53e94d..ccb4bde 100644
--- a/src/ipset.8
+++ b/src/ipset.8
@@ -304,17 +304,28 @@ ipset create foo hash:ip counters
 .IP 
 ipset add foo 192.168.1.1 packets 42 bytes 1024
 .PP 
+.SS comment
+All set types support the optional \fBcomment\fR extension.
+Enabling this extension on an ipset enables you to annotate an ipset entry with
+an arbitrary string. This string is completely ignored by both the kernel and ipset
+itself and is purely for providing a convenient means to document the reason for an
+entry's existence.
+.IP
+ipset create foo hash:ip comment
+.IP
+ipset add foo 192.168.1.1 comment "foobar comment here"
+.PP
 .SH "SET TYPES"
 .SS bitmap:ip
 The \fBbitmap:ip\fR set type uses a memory range to store either IPv4 host
 (default) or IPv4 network addresses. A \fBbitmap:ip\fR type of set can store up
 to 65536 entries.
 .PP 
-\fICREATE\-OPTIONS\fR := \fBrange\fP \fIfromip\fP\-\fItoip\fR|\fIip\fR/\fIcidr\fR [ \fBnetmask\fP \fIcidr\fP ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := \fBrange\fP \fIfromip\fP\-\fItoip\fR|\fIip\fR/\fIcidr\fR [ \fBnetmask\fP \fIcidr\fP ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := { \fIip\fR | \fIfromip\fR\-\fItoip\fR | \fIip\fR/\fIcidr\fR }
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := { \fIip\fR | \fIfromip\fR\-\fItoip\fR | \fIip\fR/\fIcidr\fR }
 .PP 
@@ -349,11 +360,11 @@ ipset test foo 192.168.1.1
 .SS bitmap:ip,mac
 The \fBbitmap:ip,mac\fR set type uses a memory range to store IPv4 and a MAC address pairs. A \fBbitmap:ip,mac\fR type of set can store up to 65536 entries.
 .PP 
-\fICREATE\-OPTIONS\fR := \fBrange\fP \fIfromip\fP\-\fItoip\fR|\fIip\fR/\fIcidr\fR [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := \fBrange\fP \fIfromip\fP\-\fItoip\fR|\fIip\fR/\fIcidr\fR [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fIip\fR[,\fImacaddr\fR]
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fIip\fR[,\fImacaddr\fR]
 .PP 
@@ -389,11 +400,11 @@ ipset test foo 192.168.1.1
 The \fBbitmap:port\fR set type uses a memory range to store port numbers
 and such a set can store up to 65536 ports.
 .PP 
-\fICREATE\-OPTIONS\fR := \fBrange\fP \fIfromport\fP\-\fItoport [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := \fBrange\fP \fIfromport\fP\-\fItoport [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := { \fI[proto:]port\fR | \fI[proto:]fromport\fR\-\fItoport\fR }
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := { \fI[proto:]port\fR | \fI[proto:]fromport\fR\-\fItoport\fR }
 .PP 
@@ -424,11 +435,11 @@ The \fBhash:ip\fR set type uses a hash to store IP host addresses (default) or
 network addresses. Zero valued IP address cannot be stored in a \fBhash:ip\fR
 type of set.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBnetmask\fP \fIcidr\fP ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBnetmask\fP \fIcidr\fP ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fIipaddr\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fIipaddr\fR
 .PP 
@@ -471,11 +482,11 @@ ipset test foo 192.168.1.2
 The \fBhash:net\fR set type uses a hash to store different sized IP network addresses.
 Network address with zero prefix size cannot be stored in this type of sets.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fInetaddr\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fInetaddr\fR
 .PP 
@@ -541,11 +552,11 @@ over the second, so a nomatch entry could be potentially be ineffective if a mor
 first parameter existed with a suitable second parameter.
 Network address with zero prefix size cannot be stored in this type of set.
 .PP
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP
 \fIADD\-ENTRY\fR := \fInetaddr\fR,\fInetaddr\fR
 .PP
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP
 \fIDEL\-ENTRY\fR := \fInetaddr\fR,\fInetaddr\fR
 .PP
@@ -613,11 +624,11 @@ The \fBhash:ip,port\fR set type uses a hash to store IP address and port number
 The port number is interpreted together with a protocol (default TCP) and zero
 protocol number cannot be used.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fIipaddr\fR,[\fIproto\fR:]\fIport\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fIipaddr\fR,[\fIproto\fR:]\fIport\fR
 .PP 
@@ -689,11 +700,11 @@ address and port pairs. The port number is interpreted together with a protocol
 (default TCP) and zero protocol number cannot be used. Network
 address with zero prefix size is not accepted either.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fInetaddr\fR,[\fIproto\fR:]\fIport\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ]  [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ]  [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fInetaddr\fR,[\fIproto\fR:]\fIport\fR
 .PP 
@@ -753,11 +764,11 @@ The \fBhash:ip,port,ip\fR set type uses a hash to store IP address, port number
 and a second IP address triples. The port number is interpreted together with a
 protocol (default TCP) and zero protocol number cannot be used.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fIipaddr\fR,[\fIproto\fR:]\fIport\fR,\fIip\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fIipaddr\fR,[\fIproto\fR:]\fIport\fR,\fIip\fR
 .PP 
@@ -799,11 +810,11 @@ and IP network address triples. The port number is interpreted together with a
 protocol (default TCP) and zero protocol number cannot be used. Network
 address with zero prefix size cannot be stored either.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fIipaddr\fR,[\fIproto\fR:]\fIport\fR,\fInetaddr\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ]  [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ]  [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fIipaddr\fR,[\fIproto\fR:]\fIport\fR,\fInetaddr\fR
 .PP 
@@ -859,11 +870,11 @@ ipset test foo 192.168.1,80.10.0.0/24
 The \fBhash:net,iface\fR set type uses a hash to store different sized IP network
 address and interface name pairs.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBfamily\fR { \fBinet\fR | \fBinet6\fR } ] | [ \fBhashsize\fR \fIvalue\fR ] [ \fBmaxelem\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fInetaddr\fR,[\fBphysdev\fR:]\fIiface\fR
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ]  [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ]  [ \fBnomatch\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fInetaddr\fR,[\fBphysdev\fR:]\fIiface\fR
 .PP 
@@ -930,11 +941,11 @@ ipset test foo 192.168.0/24,eth0
 The \fBlist:set\fR type uses a simple list in which you can store
 set names.
 .PP 
-\fICREATE\-OPTIONS\fR := [ \fBsize\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ]
+\fICREATE\-OPTIONS\fR := [ \fBsize\fR \fIvalue\fR ] [ \fBtimeout\fR \fIvalue\fR ] [ \fBcounters\fP ] [ \fBcomment\fP ]
 .PP 
 \fIADD\-ENTRY\fR := \fIsetname\fR [ { \fBbefore\fR | \fBafter\fR } \fIsetname\fR ]
 .PP 
-\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ]
+\fIADD\-OPTIONS\fR := [ \fBtimeout\fR \fIvalue\fR ] [ \fBpackets\fR \fIvalue\fR ] [ \fBbytes\fR \fIvalue\fR ] [ \fBcomment\fR \fIstring\fR ]
 .PP 
 \fIDEL\-ENTRY\fR := \fIsetname\fR [ { \fBbefore\fR | \fBafter\fR } \fIsetname\fR ]
 .PP 
-- 
1.8.3.2

Re: [PATCH v2 0/7] Add new comments extension to ipset.

[Oliver]

On Saturday 21 September 2013 09:46:09 Jozsef Kadlecsik wrote:
> Hi,
> 
> On Fri, 20 Sep 2013, Oliver wrote:
> > From: Oliver Smith <oliver@8.c.9.b.0.7.4.0.1.0.0.2.ip6.arpa>
> > 
> > Another re-roll, with requested changes applied. I've also made the
> > allocation code a bit more robust by having it fall back to vmalloc
> > should kmalloc fail to do the oh-so-needful. Additionally, the
> > documentation was somewhat lacking (rather, non-existent) so I've
> > corrected that too.
> 
> In order to make sure nothing is missed, I list all which needs to be
> fixed:

Alrighty, checking these things off as I go. Please be sure to let me know if I 
need to undo the lib include header stuff because that was a result of "make 
update_modules" - I didn't manually edit that file, so presumably, this is the 
the desirable way to go.

> 
> - A single IPSET_MAX_COMMENT_SIZE in uapi/linux/netfilter/ipset/ip_set.h
>   and no IP_SET_MAX_COMMENT_SIZE definition anywhere and the changes
>   according to this at kernel side.

Done.

> - uapi/linux/netfilter/ipset/ip_set.h copied to userspace part as
>   include/libipset/linux_ip_set.h

I just ran "make update_includes"

> - ip_set_init_comment: handle resetting the comment field to none and
>   use a single allocation with kzalloc(len + 1, GFP_KERNEL).

OK. I'm assuming you meant GFP_ATOMIC since we're in a softirq.

> - ip_set_comment_free updated according to ip_set_init_comment.

Done.

> - ip_set_put_comment is "static inline int" and thus returns 0 in
>   the first "if" branch.

OK, assuming you mean it *should* be an int since nla_put_string() returns an 
int, so I've done that and have the return as 0 rather than NULL.

> - The *_head functions use an inline function which adds the
>   IPSET_ATTR_CADT_FLAGS attribute to skb.

Done, also moved timeout into it since that can easily be handled there too.

> - The bitmap and hash type definitions (struct ip_set_type) are extended
>   with the IPSET_ATTR_COMMENT attribute in the adt_policy field.

Done.

> - The new build_argv either accepts the "\" character, but then
>   ipset_print_comment is updated so that it puts back those, or
>   simply rejects strings where there's a escape character.
>   The function must handle "\t" and multiple consecutive whitespace
>   characters outside of quoted string too.

Bah, I knew I'd forgotten something, I just reject quotes now. the escape char 
no longer has any meaning. Also now handles contiguous whitespace.

> - IPSET_FLAG(IPSET_OPT_CREATE_COMMENT) must be added to IPSET_CREATE_FLAGS

Done.

> - In ipset_data_set and ipset_data_sizeof the length of
>   IPSET_OPT_ADT_COMMENT is (IPSET_MAX_COMMENT_SIZE + 1)

Done.

_______________________________________________________________________________

I have one issue in your other e-mails where there's a bit of a contradiction:

> > --- a/kernel/include/uapi/linux/netfilter/ipset/ip_set.h
> > +++ b/kernel/include/uapi/linux/netfilter/ipset/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_MAX_COMMENT_SIZE	255
> > +
> > 
> >  /* Message types and commands */
> >  enum ipset_cmd {
> >  
> >  	IPSET_CMD_NONE,
> 
> The patch above for kernel/include/... should not be here at all.

This conflicts with your statement of:
> 
> - A single IPSET_MAX_COMMENT_SIZE in uapi/linux/netfilter/ipset/ip_set.h
>   and no IP_SET_MAX_COMMENT_SIZE definition anywhere and the changes
>   according to this at kernel side.

So I'm assuming I should ignore the former and retain the latter.

I'll land v3 into the mailing list around 20:00-21:00 CEST or sooner if you 
confirm all is good with the above.

And just in case you find this whole patch e-mailing thing a bit archaic, I 
have it in a git repo at https://github.com/Olipro/ipset on branch "ipset-
comments". If you prefer I formally submit it as a [GIT PULL] instead of a 
patch series, let me know and I'll do that instead of what I stated above.

Kind Regards,
Oliver.