[PATCH 0/5] ipset: some small userland improvements

[PATCH 0/5] ipset: some small userland improvements

[Holger Eitzenberger]

Hi Jozsef,

what follows are some smaller patches I collected while starting to
implement sorted ipset list ("ipset -s list") output.

They are not directly related to the sorted output, but are IMO still
usefull to have.

I'd like to get them out before sending the initial RFC patches for
the sorted output.

Please check.

Cheers,

 /Holger

---

Holger Eitzenberger (5):
      Remove a duplicate debug print
      ipset: avoid duplicate command flags
      ipset: coalesce two ifs
      ipset: remove extran newline on debug output
      ipset: data callback does not need to be part of cb_ctl[]


 lib/mnl.c     |    6 ++----
 lib/session.c |   12 ++++--------
 2 files changed, 6 insertions(+), 12 deletions(-)

-- 
A dirty mind is a terrible thing to waste.

[PATCH 1/5] Remove a duplicate debug print

[Holger Eitzenberger]

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
---
 lib/session.c |    3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/session.c b/lib/session.c
index d2957a5..7a719b2 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -1038,8 +1038,7 @@ callback_list(struct ipset_session *session, struct nlattr *nla[],
 		ipset_data_flags_unset(data, IPSET_CREATE_FLAGS);
 		D("nla typename %s",
 		  (char *) mnl_attr_get_payload(nla[IPSET_ATTR_TYPENAME]));
-		D("nla typename %s",
-		  (char *) mnl_attr_get_payload(nla[IPSET_ATTR_TYPENAME]));
+
 		ATTR2DATA(session, nla, IPSET_ATTR_FAMILY, cmd_attrs);
 		ATTR2DATA(session, nla, IPSET_ATTR_TYPENAME, cmd_attrs);
 		ATTR2DATA(session, nla, IPSET_ATTR_REVISION, cmd_attrs);

[PATCH 2/5] ipset: avoid duplicate command flags

[Holger Eitzenberger]

NLM_F_DUMP is #defined as (NLM_F_ROOT | NLM_F_ACK), so specifying
all of them is redundant.

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
---
 lib/mnl.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/mnl.c b/lib/mnl.c
index 786c9e5..a0fa66e 100644
--- a/lib/mnl.c
+++ b/lib/mnl.c
@@ -38,10 +38,8 @@ static const uint16_t cmdflags[] = {
 	[IPSET_CMD_FLUSH-1]	= NLM_F_REQUEST|NLM_F_ACK,
 	[IPSET_CMD_RENAME-1]	= NLM_F_REQUEST|NLM_F_ACK,
 	[IPSET_CMD_SWAP-1]	= NLM_F_REQUEST|NLM_F_ACK,
-	[IPSET_CMD_LIST-1]	= NLM_F_REQUEST|NLM_F_ACK|
-					NLM_F_ROOT|NLM_F_MATCH|NLM_F_DUMP,
-	[IPSET_CMD_SAVE-1]	= NLM_F_REQUEST|NLM_F_ACK|
-					NLM_F_ROOT|NLM_F_MATCH|NLM_F_DUMP,
+	[IPSET_CMD_LIST-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_DUMP,
+	[IPSET_CMD_SAVE-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_DUMP,
 	[IPSET_CMD_ADD-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_EXCL,
 	[IPSET_CMD_DEL-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_EXCL,
 	[IPSET_CMD_TEST-1]	= NLM_F_REQUEST|NLM_F_ACK,

[PATCH 3/5] ipset: coalesce two ifs

[Holger Eitzenberger]

Because they are doing the same for both IPSET_CMD_LIST and
IPSET_CMD_SAVE.

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
---
 lib/session.c |    6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/session.c b/lib/session.c
index 7a719b2..d4fe836 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -1936,13 +1936,11 @@ ipset_cmd(struct ipset_session *session, enum ipset_cmd cmd, uint32_t lineno)
 	session->lineno = lineno;
 
 	/* Set default output mode */
-	if (cmd == IPSET_CMD_LIST) {
+	if (cmd == IPSET_CMD_LIST || cmd == IPSET_CMD_SAVE) {
 		if (session->mode == IPSET_LIST_NONE)
 			session->mode = IPSET_LIST_PLAIN;
-	} else if (cmd == IPSET_CMD_SAVE) {
-		if (session->mode == IPSET_LIST_NONE)
-			session->mode = IPSET_LIST_SAVE;
 	}
+
 	/* Start the root element in XML mode */
 	if ((cmd == IPSET_CMD_LIST || cmd == IPSET_CMD_SAVE) &&
 	    session->mode == IPSET_LIST_XML)

[PATCH 4/5] ipset: remove extran newline on debug output

[Holger Eitzenberger]

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
---
 lib/session.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/session.c b/lib/session.c
index d4fe836..8d79a96 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -804,7 +804,7 @@ list_adt(struct ipset_session *session, struct nlattr *nla[])
 		safe_snprintf(session, "</elem>");
 
 	for (arg = type->args[IPSET_ADD]; arg != NULL && arg->opt; arg++) {
-		D("print arg opt %u %s\n", arg->opt,
+		D("print arg opt %u %s", arg->opt,
 		   ipset_data_test(data, arg->opt) ? "(yes)" : "(missing)");
 		if (!(arg->print && ipset_data_test(data, arg->opt)))
 			continue;

[PATCH 5/5] ipset: data callback does not need to be part of cb_ctl[]

[Holger Eitzenberger]

Because the data callback is passed explicitely to mnl_cb_run2()
there is no need to add it to cb_ctl[].  For nlmsg_types
>= NLMSG_MIN_TYPE the data callback is used explicitely.

Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
---
 lib/session.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/session.c b/lib/session.c
index 8d79a96..807f0d3 100644
--- a/lib/session.c
+++ b/lib/session.c
@@ -1868,7 +1868,6 @@ static mnl_cb_t cb_ctl[] = {
 	[NLMSG_ERROR] = callback_error,
 	[NLMSG_DONE]  = callback_done,
 	[NLMSG_OVERRUN] = callback_noop,
-	[NLMSG_MIN_TYPE] = callback_data,
 };
 
 static inline struct ipset_handle *

Re: [PATCH 5/5] ipset: data callback does not need to be part of cb_ctl[]

[Holger Eitzenberger]

Hi Jozsef,

> Because the data callback is passed explicitely to mnl_cb_run2()
> there is no need to add it to cb_ctl[].  For nlmsg_types
> >= NLMSG_MIN_TYPE the data callback is used explicitely.

Just noticed that there is a side effect to this one.  So please
just ignore this patch for now.

Re: [PATCH 1/5] Remove a duplicate debug print

[Jozsef Kadlecsik]

On Mon, 8 Sep 2014, Holger Eitzenberger wrote:

> Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
> ---
>  lib/session.c |    3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/lib/session.c b/lib/session.c
> index d2957a5..7a719b2 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -1038,8 +1038,7 @@ callback_list(struct ipset_session *session, struct nlattr *nla[],
>  		ipset_data_flags_unset(data, IPSET_CREATE_FLAGS);
>  		D("nla typename %s",
>  		  (char *) mnl_attr_get_payload(nla[IPSET_ATTR_TYPENAME]));
> -		D("nla typename %s",
> -		  (char *) mnl_attr_get_payload(nla[IPSET_ATTR_TYPENAME]));
> +
>  		ATTR2DATA(session, nla, IPSET_ATTR_FAMILY, cmd_attrs);
>  		ATTR2DATA(session, nla, IPSET_ATTR_TYPENAME, cmd_attrs);
>  		ATTR2DATA(session, nla, IPSET_ATTR_REVISION, cmd_attrs);
> 

Patch is applied, thanks!

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 2/5] ipset: avoid duplicate command flags

[Jozsef Kadlecsik]

On Mon, 8 Sep 2014, Holger Eitzenberger wrote:

> NLM_F_DUMP is #defined as (NLM_F_ROOT | NLM_F_ACK), so specifying
> all of them is redundant.
> 
> Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
> ---
>  lib/mnl.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/mnl.c b/lib/mnl.c
> index 786c9e5..a0fa66e 100644
> --- a/lib/mnl.c
> +++ b/lib/mnl.c
> @@ -38,10 +38,8 @@ static const uint16_t cmdflags[] = {
>  	[IPSET_CMD_FLUSH-1]	= NLM_F_REQUEST|NLM_F_ACK,
>  	[IPSET_CMD_RENAME-1]	= NLM_F_REQUEST|NLM_F_ACK,
>  	[IPSET_CMD_SWAP-1]	= NLM_F_REQUEST|NLM_F_ACK,
> -	[IPSET_CMD_LIST-1]	= NLM_F_REQUEST|NLM_F_ACK|
> -					NLM_F_ROOT|NLM_F_MATCH|NLM_F_DUMP,
> -	[IPSET_CMD_SAVE-1]	= NLM_F_REQUEST|NLM_F_ACK|
> -					NLM_F_ROOT|NLM_F_MATCH|NLM_F_DUMP,
> +	[IPSET_CMD_LIST-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_DUMP,
> +	[IPSET_CMD_SAVE-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_DUMP,
>  	[IPSET_CMD_ADD-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_EXCL,
>  	[IPSET_CMD_DEL-1]	= NLM_F_REQUEST|NLM_F_ACK|NLM_F_EXCL,
>  	[IPSET_CMD_TEST-1]	= NLM_F_REQUEST|NLM_F_ACK,
> 

Patch is applied.

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 3/5] ipset: coalesce two ifs

[Jozsef Kadlecsik]

On Mon, 8 Sep 2014, Holger Eitzenberger wrote:

> Because they are doing the same for both IPSET_CMD_LIST and
> IPSET_CMD_SAVE.
> 
> Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
> ---
>  lib/session.c |    6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/session.c b/lib/session.c
> index 7a719b2..d4fe836 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -1936,13 +1936,11 @@ ipset_cmd(struct ipset_session *session, enum ipset_cmd cmd, uint32_t lineno)
>  	session->lineno = lineno;
>  
>  	/* Set default output mode */
> -	if (cmd == IPSET_CMD_LIST) {
> +	if (cmd == IPSET_CMD_LIST || cmd == IPSET_CMD_SAVE) {
>  		if (session->mode == IPSET_LIST_NONE)
>  			session->mode = IPSET_LIST_PLAIN;
> -	} else if (cmd == IPSET_CMD_SAVE) {
> -		if (session->mode == IPSET_LIST_NONE)
> -			session->mode = IPSET_LIST_SAVE;
>  	}
> +
>  	/* Start the root element in XML mode */
>  	if ((cmd == IPSET_CMD_LIST || cmd == IPSET_CMD_SAVE) &&
>  	    session->mode == IPSET_LIST_XML)
> 

The two ifs don't do the same thing: by coalescing them the safe mode 
would be broken (unless XML mode is specified). So this patch is skipped.

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

Re: [PATCH 4/5] ipset: remove extran newline on debug output

[Jozsef Kadlecsik]

On Mon, 8 Sep 2014, Holger Eitzenberger wrote:

> Signed-off-by: Holger Eitzenberger <holger@eitzenberger.org>
> ---
>  lib/session.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/session.c b/lib/session.c
> index d4fe836..8d79a96 100644
> --- a/lib/session.c
> +++ b/lib/session.c
> @@ -804,7 +804,7 @@ list_adt(struct ipset_session *session, struct nlattr *nla[])
>  		safe_snprintf(session, "</elem>");
>  
>  	for (arg = type->args[IPSET_ADD]; arg != NULL && arg->opt; arg++) {
> -		D("print arg opt %u %s\n", arg->opt,
> +		D("print arg opt %u %s", arg->opt,
>  		   ipset_data_test(data, arg->opt) ? "(yes)" : "(missing)");
>  		if (!(arg->print && ipset_data_test(data, arg->opt)))
>  			continue;
> 

This patch is also applied, thanks Holger.

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