All of lore.kernel.org
 help / color / mirror / Atom feed
* option parsing, run 9
@ 2011-05-23 14:39 Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 01/13] libxtables: retract _NE types and use a flag instead Jan Engelhardt
                   ` (12 more replies)
  0 siblings, 13 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel


The following changes since commit 17f7937f79af4d260c60cb800e56fc0df0a48b37:

  libxt_devgroup: actually set XT_DEVGROUP_OPT_???GROUP flags (2011-05-23 16:28:25 +0200)

are available in the git repository at:
  git://dev.medozas.de/iptables master

JP Abgrall (1):
      libxt_quota: make sure uint64 is not truncated

Jan Engelhardt (11):
      libxtables: retract _NE types and use a flag instead
      libxt_quota: readd missing XTOPT_PUT request
      libxtables: check for negative numbers in xtables_strtou*
      libxt_rateest: streamline case display of units
      doc: add some coded option examples to libxt_hashlimit
      doc: make usage of libxt_rateest more obvious
      doc: clarify that -p all is a special keyword only
      libxt_rateest: avoid optional arguments
      libxt_rateest: fix rateest save operation
      libxt_rateest: use guided option parser
      libxt_ipvs: restore network-byte order

Lutz Jaenicke (1):
      libipt_REDIRECT: "--to-ports" is not mandatory

 extensions/libipt_REDIRECT.c   |    3 +-
 extensions/libxt_TPROXY.c      |    8 +-
 extensions/libxt_hashlimit.man |   26 ++-
 extensions/libxt_ipvs.c        |    6 +-
 extensions/libxt_quota.c       |    3 +-
 extensions/libxt_rateest.c     |  378 +++++++++++++++++-----------------------
 extensions/libxt_rateest.man   |   79 +++++++--
 include/xtables.h.in           |   14 +-
 ip6tables.8.in                 |    5 +-
 iptables.8.in                  |    5 +-
 xtables.c                      |   13 +-
 xtoptions.c                    |   17 +-
 12 files changed, 280 insertions(+), 277 deletions(-)

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 01/13] libxtables: retract _NE types and use a flag instead
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 02/13] libipt_REDIRECT: "--to-ports" is not mandatory Jan Engelhardt
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_TPROXY.c |    8 ++++----
 include/xtables.h.in      |   12 ++++++------
 xtoptions.c               |   13 +++++--------
 3 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/extensions/libxt_TPROXY.c b/extensions/libxt_TPROXY.c
index 61646c9..d13ec85 100644
--- a/extensions/libxt_TPROXY.c
+++ b/extensions/libxt_TPROXY.c
@@ -20,8 +20,8 @@ enum {
 
 #define s struct xt_tproxy_target_info
 static const struct xt_option_entry tproxy_tg0_opts[] = {
-	{.name = "on-port", .id = P_PORT, .type = XTTYPE_PORT_NE,
-	 .flags = XTOPT_MAND | XTOPT_PUT, XTOPT_POINTER(s, lport)},
+	{.name = "on-port", .id = P_PORT, .type = XTTYPE_PORT,
+	 .flags = XTOPT_MAND | XTOPT_NBO | XTOPT_PUT, XTOPT_POINTER(s, lport)},
 	{.name = "on-ip", .id = P_ADDR, .type = XTTYPE_HOST},
 	{.name = "tproxy-mark", .id = P_MARK, .type = XTTYPE_MARKMASK32},
 	XTOPT_TABLEEND,
@@ -29,8 +29,8 @@ static const struct xt_option_entry tproxy_tg0_opts[] = {
 #undef s
 #define s struct xt_tproxy_target_info_v1
 static const struct xt_option_entry tproxy_tg1_opts[] = {
-	{.name = "on-port", .id = P_PORT, .type = XTTYPE_PORT_NE,
-	 .flags = XTOPT_MAND | XTOPT_PUT, XTOPT_POINTER(s, lport)},
+	{.name = "on-port", .id = P_PORT, .type = XTTYPE_PORT,
+	 .flags = XTOPT_MAND | XTOPT_NBO | XTOPT_PUT, XTOPT_POINTER(s, lport)},
 	{.name = "on-ip", .id = P_ADDR, .type = XTTYPE_HOST,
 	 .flags = XTOPT_PUT, XTOPT_POINTER(s, laddr)},
 	{.name = "tproxy-mark", .id = P_MARK, .type = XTTYPE_MARKMASK32},
diff --git a/include/xtables.h.in b/include/xtables.h.in
index 38c0e5e..f88813f 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -64,10 +64,9 @@ struct in_addr;
  * %XTTYPE_HOSTMASK:	one host or address, with an optional prefix length
  * 			(ptr: union nf_inet_addr; only host portion is stored)
  * %XTTYPE_PROTOCOL:	protocol number/name from /etc/protocols (ptr: uint8_t)
- * %XTTYPE_PORT:	16-bit port name or number
- * %XTTYPE_PORT_NE:	16-bit port name or number, stored as network-endian
- * %XTTYPE_PORTRC:	colon-separated port range (names acceptable)
- * %XTTYPE_PORTRC_NE:	same as %XTTYPE_PORTRC, stored in network-endian
+ * %XTTYPE_PORT:	16-bit port name or number (supports %XTOPT_NBO)
+ * %XTTYPE_PORTRC:	colon-separated port range (names acceptable),
+ * 			(supports %XTOPT_NBO)
  * %XTTYPE_PLEN:	prefix length
  * %XTTYPE_PLENMASK:	prefix length (ptr: union nf_inet_addr)
  * %XTTYPE_ETHERMAC:	Ethernet MAC address in hex form
@@ -91,9 +90,7 @@ enum xt_option_type {
 	XTTYPE_HOSTMASK,
 	XTTYPE_PROTOCOL,
 	XTTYPE_PORT,
-	XTTYPE_PORT_NE,
 	XTTYPE_PORTRC,
-	XTTYPE_PORTRC_NE,
 	XTTYPE_PLEN,
 	XTTYPE_PLENMASK,
 	XTTYPE_ETHERMAC,
@@ -104,12 +101,15 @@ enum xt_option_type {
  * %XTOPT_MAND:		option is mandatory
  * %XTOPT_MULTI:	option may be specified multiple times
  * %XTOPT_PUT:		store value into memory at @ptroff
+ * %XTOPT_NBO:		store value in network-byte order
+ * 			(only certain XTTYPEs recognize this)
  */
 enum xt_option_flags {
 	XTOPT_INVERT = 1 << 0,
 	XTOPT_MAND   = 1 << 1,
 	XTOPT_MULTI  = 1 << 2,
 	XTOPT_PUT    = 1 << 3,
+	XTOPT_NBO    = 1 << 4,
 };
 
 /**
diff --git a/xtoptions.c b/xtoptions.c
index eb9e4e6..3c3ce5f 100644
--- a/xtoptions.c
+++ b/xtoptions.c
@@ -509,6 +509,7 @@ static void xtopt_parse_protocol(struct xt_option_call *cb)
  */
 static void xtopt_parse_port(struct xt_option_call *cb)
 {
+	const struct xt_option_entry *entry = cb->entry;
 	int ret;
 
 	ret = xtables_getportbyname(cb->arg);
@@ -516,10 +517,10 @@ static void xtopt_parse_port(struct xt_option_call *cb)
 		xt_params->exit_err(PARAMETER_PROBLEM,
 			"Port \"%s\" does not resolve to anything.\n",
 			cb->arg);
+	if (entry->flags & XTOPT_NBO)
+		ret = htons(ret);
 	cb->val.port = ret;
-	if (cb->entry->type == XTTYPE_PORT_NE)
-		cb->val.port = htons(cb->val.port);
-	if (cb->entry->flags & XTOPT_PUT)
+	if (entry->flags & XTOPT_PUT)
 		*(uint16_t *)XTOPT_MKPTR(cb) = cb->val.port;
 }
 
@@ -561,7 +562,7 @@ static void xtopt_parse_mport(struct xt_option_call *cb)
 			xt_params->exit_err(PARAMETER_PROBLEM,
 				"Port \"%s\" does not resolve to "
 				"anything.\n", arg);
-		if (entry->type == XTTYPE_PORTRC_NE)
+		if (entry->flags & XTOPT_NBO)
 			value = htons(value);
 		if (cb->nvals < ARRAY_SIZE(cb->val.port_range))
 			cb->val.port_range[cb->nvals] = value;
@@ -702,9 +703,7 @@ static void (*const xtopt_subparse[])(struct xt_option_call *) = {
 	[XTTYPE_HOSTMASK]    = xtopt_parse_hostmask,
 	[XTTYPE_PROTOCOL]    = xtopt_parse_protocol,
 	[XTTYPE_PORT]        = xtopt_parse_port,
-	[XTTYPE_PORT_NE]     = xtopt_parse_port,
 	[XTTYPE_PORTRC]      = xtopt_parse_mport,
-	[XTTYPE_PORTRC_NE]   = xtopt_parse_mport,
 	[XTTYPE_PLEN]        = xtopt_parse_plen,
 	[XTTYPE_PLENMASK]    = xtopt_parse_plenmask,
 	[XTTYPE_ETHERMAC]    = xtopt_parse_ethermac,
@@ -730,9 +729,7 @@ static const size_t xtopt_psize[] = {
 	[XTTYPE_HOSTMASK]    = sizeof(union nf_inet_addr),
 	[XTTYPE_PROTOCOL]    = sizeof(uint8_t),
 	[XTTYPE_PORT]        = sizeof(uint16_t),
-	[XTTYPE_PORT_NE]     = sizeof(uint16_t),
 	[XTTYPE_PORTRC]      = sizeof(uint16_t[2]),
-	[XTTYPE_PORTRC_NE]   = sizeof(uint16_t[2]),
 	[XTTYPE_PLENMASK]    = sizeof(union nf_inet_addr),
 	[XTTYPE_ETHERMAC]    = sizeof(uint8_t[6]),
 };
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 02/13] libipt_REDIRECT: "--to-ports" is not mandatory
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 01/13] libxtables: retract _NE types and use a flag instead Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 03/13] libxt_quota: readd missing XTOPT_PUT request Jan Engelhardt
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

From: Lutz Jaenicke <ljaenicke@innominate.com>

The REDIRECT target can be called without the --to-ports option
being specified. From the manual page:
  ...without this, the destination port is never altered.

Signed-off-by: Lutz Jaenicke <ljaenicke@innominate.com>
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libipt_REDIRECT.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/extensions/libipt_REDIRECT.c b/extensions/libipt_REDIRECT.c
index 426a746..e67360a 100644
--- a/extensions/libipt_REDIRECT.c
+++ b/extensions/libipt_REDIRECT.c
@@ -23,8 +23,7 @@ static void REDIRECT_help(void)
 }
 
 static const struct xt_option_entry REDIRECT_opts[] = {
-	{.name = "to-ports", .id = O_TO_PORTS, .type = XTTYPE_STRING,
-	 .flags = XTOPT_MAND},
+	{.name = "to-ports", .id = O_TO_PORTS, .type = XTTYPE_STRING},
 	{.name = "random", .id = O_RANDOM, .type = XTTYPE_NONE},
 	XTOPT_TABLEEND,
 };
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 03/13] libxt_quota: readd missing XTOPT_PUT request
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 01/13] libxtables: retract _NE types and use a flag instead Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 02/13] libipt_REDIRECT: "--to-ports" is not mandatory Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 04/13] libxt_quota: make sure uint64 is not truncated Jan Engelhardt
                   ` (9 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_quota.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/extensions/libxt_quota.c b/extensions/libxt_quota.c
index 988f404..ff498da 100644
--- a/extensions/libxt_quota.c
+++ b/extensions/libxt_quota.c
@@ -13,7 +13,8 @@ enum {
 
 static const struct xt_option_entry quota_opts[] = {
 	{.name = "quota", .id = O_QUOTA, .type = XTTYPE_UINT64,
-	 .flags = XTOPT_MAND | XTOPT_INVERT},
+	 .flags = XTOPT_MAND | XTOPT_INVERT | XTOPT_PUT,
+	 XTOPT_POINTER(struct xt_quota_info, quota)},
 	XTOPT_TABLEEND,
 };
 
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 04/13] libxt_quota: make sure uint64 is not truncated
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (2 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 03/13] libxt_quota: readd missing XTOPT_PUT request Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 05/13] libxtables: check for negative numbers in xtables_strtou* Jan Engelhardt
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

From: JP Abgrall <jpa@google.com>

The xtables_strtoul() would cram a long long into a long.
The parse_int would try to cram a UINT64 into a long.
---
 include/xtables.h.in |    2 +-
 xtables.c            |    4 ++--
 xtoptions.c          |    4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/xtables.h.in b/include/xtables.h.in
index f88813f..90eb1b2 100644
--- a/include/xtables.h.in
+++ b/include/xtables.h.in
@@ -408,7 +408,7 @@ extern void xtables_register_matches(struct xtables_match *, unsigned int);
 extern void xtables_register_target(struct xtables_target *me);
 extern void xtables_register_targets(struct xtables_target *, unsigned int);
 
-extern bool xtables_strtoul(const char *, char **, unsigned long *,
+extern bool xtables_strtoul(const char *, char **, unsigned long long *,
 	unsigned long, unsigned long);
 extern bool xtables_strtoui(const char *, char **, unsigned int *,
 	unsigned int, unsigned int);
diff --git a/xtables.c b/xtables.c
index f10cdb7..3c9a13f 100644
--- a/xtables.c
+++ b/xtables.c
@@ -426,7 +426,7 @@ int xtables_load_ko(const char *modprobe, bool quiet)
  * Returns true/false whether number was accepted. On failure, *value has
  * undefined contents.
  */
-bool xtables_strtoul(const char *s, char **end, unsigned long *value,
+bool xtables_strtoul(const char *s, char **end, unsigned long long *value,
                      unsigned long min, unsigned long max)
 {
 	unsigned long v;
@@ -454,7 +454,7 @@ bool xtables_strtoul(const char *s, char **end, unsigned long *value,
 bool xtables_strtoui(const char *s, char **end, unsigned int *value,
                      unsigned int min, unsigned int max)
 {
-	unsigned long v;
+	unsigned long long v;
 	bool ret;
 
 	ret = xtables_strtoul(s, end, &v, min, max);
diff --git a/xtoptions.c b/xtoptions.c
index 3c3ce5f..ec2269b 100644
--- a/xtoptions.c
+++ b/xtoptions.c
@@ -105,7 +105,7 @@ static void xtopt_parse_int(struct xt_option_call *cb)
 {
 	const struct xt_option_entry *entry = cb->entry;
 	unsigned long long lmin = 0, lmax = UINT32_MAX;
-	unsigned int value;
+	unsigned long long value;
 
 	if (entry->type == XTTYPE_UINT8)
 		lmax = UINT8_MAX;
@@ -118,7 +118,7 @@ static void xtopt_parse_int(struct xt_option_call *cb)
 	if (cb->entry->max != 0)
 		lmax = cb->entry->max;
 
-	if (!xtables_strtoui(cb->arg, NULL, &value, lmin, lmax))
+	if (!xtables_strtoul(cb->arg, NULL, &value, lmin, lmax))
 		xt_params->exit_err(PARAMETER_PROBLEM,
 			"%s: bad value for option \"--%s\", "
 			"or out of range (%llu-%llu).\n",
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 05/13] libxtables: check for negative numbers in xtables_strtou*
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (3 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 04/13] libxt_quota: make sure uint64 is not truncated Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 06/13] libxt_rateest: streamline case display of units Jan Engelhardt
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 xtables.c |    9 +++++++--
 1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/xtables.c b/xtables.c
index 3c9a13f..e11a77e 100644
--- a/xtables.c
+++ b/xtables.c
@@ -15,7 +15,7 @@
  *	along with this program; if not, write to the Free Software
  *	Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA.
  */
-
+#include <ctype.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <netdb.h>
@@ -430,11 +430,16 @@ bool xtables_strtoul(const char *s, char **end, unsigned long long *value,
                      unsigned long min, unsigned long max)
 {
 	unsigned long v;
+	const char *p;
 	char *my_end;
 
 	errno = 0;
+	/* Since strtoul allows leading minus, we have to check for ourself. */
+	for (p = s; isspace(*p); ++p)
+		;
+	if (*p == '-')
+		return false;
 	v = strtoul(s, &my_end, 0);
-
 	if (my_end == s)
 		return false;
 	if (end != NULL)
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 06/13] libxt_rateest: streamline case display of units
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (4 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 05/13] libxtables: check for negative numbers in xtables_strtou* Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 07/13] doc: add some coded option examples to libxt_hashlimit Jan Engelhardt
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_rateest.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/extensions/libxt_rateest.c b/extensions/libxt_rateest.c
index e70edc6..509b3e3 100644
--- a/extensions/libxt_rateest.c
+++ b/extensions/libxt_rateest.c
@@ -65,11 +65,11 @@ static const struct rate_suffix {
 	{ "bit",	1. },
 	{ "Kibit",	1024. },
 	{ "kbit",	1000. },
-	{ "mibit",	1024.*1024. },
+	{ "Mibit",	1024.*1024. },
 	{ "mbit",	1000000. },
-	{ "gibit",	1024.*1024.*1024. },
+	{ "Gibit",	1024.*1024.*1024. },
 	{ "gbit",	1000000000. },
-	{ "tibit",	1024.*1024.*1024.*1024. },
+	{ "Tibit",	1024.*1024.*1024.*1024. },
 	{ "tbit",	1000000000000. },
 	{ "Bps",	8. },
 	{ "KiBps",	8.*1024. },
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 07/13] doc: add some coded option examples to libxt_hashlimit
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (5 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 06/13] libxt_rateest: streamline case display of units Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 08/13] doc: make usage of libxt_rateest more obvious Jan Engelhardt
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_hashlimit.man |   26 ++++++++++++++++----------
 1 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/extensions/libxt_hashlimit.man b/extensions/libxt_hashlimit.man
index e91d0c6..f90577e 100644
--- a/extensions/libxt_hashlimit.man
+++ b/extensions/libxt_hashlimit.man
@@ -2,16 +2,7 @@
 \fBlimit\fP match) for a group of connections using a \fBsingle\fP iptables
 rule. Grouping can be done per-hostgroup (source and/or destination address)
 and/or per-port. It gives you the ability to express "\fIN\fP packets per time
-quantum per group":
-.TP
-matching on source host
-"1000 packets per second for every host in 192.168.0.0/16"
-.TP
-matching on source port
-"100 packets per second for every service of 192.168.1.1"
-.TP
-matching on subnet
-"10000 packets per minute for every /28 subnet in 10.0.0.0/8"
+quantum per group" (see below for some examples).
 .PP
 A hash limit option (\fB\-\-hashlimit\-upto\fP, \fB\-\-hashlimit\-above\fP) and
 \fB\-\-hashlimit\-name\fP are required.
@@ -57,3 +48,18 @@ After how many milliseconds do hash entries expire.
 .TP
 \fB\-\-hashlimit\-htable\-gcinterval\fP \fImsec\fP
 How many milliseconds between garbage collection intervals.
+.PP
+Examples:
+.TP
+matching on source host
+"1000 packets per second for every host in 192.168.0.0/16" =>
+\-s 192.168.0.0/16 \-\-hashlimit\-mode srcip \-\-hashlimit\-upto 1000/sec
+.TP
+matching on source port
+"100 packets per second for every service of 192.168.1.1" =>
+\-s 192.168.1.1 \-\-hashlimit\-mode srcport \-\-hashlimit\-upto 100/sec
+.TP
+matching on subnet
+"10000 packets per minute for every /28 subnet (groups of 8 addresses)
+in 10.0.0.0/8" =>
+\-s 10.0.0.8 \-\-hashlimit\-mask 28 \-\-hashlimit\-upto 10000/min
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 08/13] doc: make usage of libxt_rateest more obvious
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (6 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 07/13] doc: add some coded option examples to libxt_hashlimit Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 09/13] doc: clarify that -p all is a special keyword only Jan Engelhardt
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_rateest.man |   71 +++++++++++++++++++++++++++++++++---------
 1 files changed, 56 insertions(+), 15 deletions(-)

diff --git a/extensions/libxt_rateest.man b/extensions/libxt_rateest.man
index 75303c5..42a82f3 100644
--- a/extensions/libxt_rateest.man
+++ b/extensions/libxt_rateest.man
@@ -1,34 +1,75 @@
 The rate estimator can match on estimated rates as collected by the RATEEST
 target. It supports matching on absolute bps/pps values, comparing two rate
 estimators and matching on the difference between two rate estimators.
+.PP
+For a better understanding of the available options, these are all possible
+combinations:
+.\" * Absolute:
+.IP \(bu 4
+\fBrateest\fP \fIoperator\fP \fBrateest-bps\fP
+.IP \(bu 4
+\fBrateest\fP \fIoperator\fP \fBrateest-pps\fP
+.\" * Absolute + Delta:
+.IP \(bu 4
+(\fBrateest\fP minus \fBrateest-bps1\fP) \fIoperator\fP \fBrateest-bps2\fP
+.IP \(bu 4
+(\fBrateest\fP minus \fBrateest-pps1\fP) \fIoperator\fP \fBrateest-pps2\fP
+.\" * Relative:
+.IP \(bu 4
+\fBrateest1\fP \fIoperator\fP \fBrateest2\fP \fBrateest-bps\fP(without rate!)
+.IP \(bu 4
+\fBrateest1\fP \fIoperator\fP \fBrateest2\fP \fBrateest-pps\fP(without rate!)
+.\" * Relative + Delta:
+.IP \(bu 4
+(\fBrateest1\fP minus \fBrateest-bps1\fP) \fIoperator\fP
+(\fBrateest2\fP minus \fBrateest-bps2\fP)
+.IP \(bu 4
+(\fBrateest1\fP minus \fBrateest-pps1\fP) \fIoperator\fP
+(\fBrateest2\fP minus \fBrateest-pps2\fP)
+.TP
+\fB\-\-rateest\-delta\fP
+For each estimator (either absolute or relative mode), calculate the difference
+between the estimator-determined flow rate and the static value chosen with the
+BPS/PPS options. If the flow rate is higher than the specified BPS/PPS, 0 will
+be used instead of a negative value. In other words, "max(0, rateest#_rate -
+rateest#_bps)" is used.
+.TP
+[\fB!\fP] \fB\-\-rateest\-lt\fP
+Match if rate is less than given rate/estimator.
+.TP
+[\fB!\fP] \fB\-\-rateest\-gt\fP
+Match if rate is greater than given rate/estimator.
+.TP
+[\fB!\fP] \fB\-\-rateest\-eq\fP
+Match if rate is equal to given rate/estimator.
+.PP
+In the so-called "absolute mode", only one rate estimator is used and compared
+against a static value, while in "relative mode", two rate estimators are
+compared against another.
+.TP
+\fB\-\-rateest\fP \fIname\fP
+Name of the one rate estimator for absolute mode.
 .TP
 \fB\-\-rateest1\fP \fIname\fP
-Name of the first rate estimator.
 .TP
 \fB\-\-rateest2\fP \fIname\fP
-Name of the second rate estimator (if difference is to be calculated).
+The names of the two rate estimators for relative mode.
 .TP
-\fB\-\-rateest\-delta\fP
-Compare difference(s) to given rate(s)
+\fB\-\-rateest\-bps\fP [\fIvalue\fP]
+.TP
+\fB\-\-rateest\-pps\fP [\fIvalue\fP]
 .TP
 \fB\-\-rateest\-bps1\fP [\fIvalue\fP]
 .TP
 \fB\-\-rateest\-bps2\fP [\fIvalue\fP]
-Compare bytes per second.
 .TP
 \fB\-\-rateest\-pps1\fP [\fIvalue\fP]
 .TP
 \fB\-\-rateest\-pps2\fP [\fIvalue\fP]
-Compare packets per second.
-.TP
-[\fB!\fP] \fB\-\-rateest\-lt\fP
-Match if rate is less than given rate/estimator.
-.TP
-[\fB!\fP] \fB\-\-rateest\-gt\fP
-Match if rate is greater than given rate/estimator.
-.TP
-[\fB!\fP] \fB\-\-rateest\-eq\fP
-Match if rate is equal to given rate/estimator.
+Compare the estimator(s) by bytes or packets per second, and compare against
+the chosen value. See the above bullet list for which option is to be used in
+which case. A unit suffix may be used - available ones are: bit, [kmgt]bit,
+[KMGT]ibit, Bps, [KMGT]Bps, [KMGT]iBps.
 .PP
 Example: This is what can be used to route outgoing data connections from an
 FTP server over two lines based on the available bandwidth at the time the data
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 09/13] doc: clarify that -p all is a special keyword only
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (7 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 08/13] doc: make usage of libxt_rateest more obvious Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 10/13] libxt_rateest: avoid optional arguments Jan Engelhardt
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 ip6tables.8.in |    5 ++---
 iptables.8.in  |    5 ++---
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/ip6tables.8.in b/ip6tables.8.in
index 61d6667..48ba18e 100644
--- a/ip6tables.8.in
+++ b/ip6tables.8.in
@@ -243,15 +243,14 @@ add, delete, insert, replace and append commands).
 [\fB!\fP] \fB\-p\fP, \fB\-\-protocol\fP \fIprotocol\fP
 The protocol of the rule or of the packet to check.
 The specified protocol can be one of \fBtcp\fP, \fBudp\fP, \fBudplite\fP,
-\fBicmpv6\fP, \fBesp\fP, \fBmh\fP or \fBall\fP,
+\fBicmpv6\fP, \fBesp\fP, \fBmh\fP or the special keyword "\fBall\fP",
 or it can be a numeric value, representing one of these protocols or a
 different one. A protocol name from /etc/protocols is also allowed.
 But IPv6 extension headers except \fBesp\fP are not allowed.
 \fBesp\fP and \fBipv6\-nonext\fP
 can be used with Kernel version 2.6.11 or later.
 A "!" argument before the protocol inverts the
-test.  The number zero is equivalent to \fBall\fP.
-Protocol \fBall\fP
+test.  The number zero is equivalent to \fBall\fP. "\fBall\fP"
 will match with all protocols and is taken as default when this
 option is omitted.
 .TP
diff --git a/iptables.8.in b/iptables.8.in
index 110c599..d09bf7a 100644
--- a/iptables.8.in
+++ b/iptables.8.in
@@ -246,12 +246,11 @@ add, delete, insert, replace and append commands).
 [\fB!\fP] \fB\-p\fP, \fB\-\-protocol\fP \fIprotocol\fP
 The protocol of the rule or of the packet to check.
 The specified protocol can be one of \fBtcp\fP, \fBudp\fP, \fBudplite\fP,
-\fBicmp\fP, \fBesp\fP, \fBah\fP, \fBsctp\fP or \fBall\fP,
+\fBicmp\fP, \fBesp\fP, \fBah\fP, \fBsctp\fP or the special keyword "\fBall\fP",
 or it can be a numeric value, representing one of these protocols or a
 different one.  A protocol name from /etc/protocols is also allowed.
 A "!" argument before the protocol inverts the
-test.  The number zero is equivalent to \fBall\fP.
-Protocol \fBall\fP
+test.  The number zero is equivalent to \fBall\fP. "\fBall\fP"
 will match with all protocols and is taken as default when this
 option is omitted.
 .TP
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 10/13] libxt_rateest: avoid optional arguments
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (8 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 09/13] doc: clarify that -p all is a special keyword only Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-24  6:46   ` Patrick McHardy
  2011-05-23 14:39 ` [PATCH 11/13] libxt_rateest: fix rateest save operation Jan Engelhardt
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Optional arguments make parsing unnecessarily harder - even more so
than two-args. Right now, rateest even crashes because of it.

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_rateest.c   |   75 +++++++++++++++++++-----------------------
 extensions/libxt_rateest.man |   20 +++++++----
 2 files changed, 46 insertions(+), 49 deletions(-)

diff --git a/extensions/libxt_rateest.c b/extensions/libxt_rateest.c
index 509b3e3..62d6033 100644
--- a/extensions/libxt_rateest.c
+++ b/extensions/libxt_rateest.c
@@ -15,16 +15,17 @@ static void rateest_help(void)
 {
 	printf(
 "rateest match options:\n"
-" --rateest1 name		Rate estimator name\n"
-" --rateest2 name		Rate estimator name\n"
 " --rateest-delta		Compare difference(s) to given rate(s)\n"
-" --rateest-bps1 [bps]		Compare bps\n"
-" --rateest-pps1 [pps]		Compare pps\n"
-" --rateest-bps2 [bps]		Compare bps\n"
-" --rateest-pps2 [pps]		Compare pps\n"
 " [!] --rateest-lt		Match if rate is less than given rate/estimator\n"
 " [!] --rateest-gt		Match if rate is greater than given rate/estimator\n"
-" [!] --rateest-eq		Match if rate is equal to given rate/estimator\n");
+" [!] --rateest-eq		Match if rate is equal to given rate/estimator\n"
+" --bytes, --packets            Pick byte/sec or packets/sec comparison\n"
+" --rateest1 name		Rate estimator name\n"
+" --rateest2 name		Rate estimator name\n"
+" --rateest-bps1 bps		Compare bps\n"
+" --rateest-pps1 pps		Compare pps\n"
+" --rateest-bps2 bps		Compare bps\n"
+" --rateest-pps2 pps		Compare pps\n");
 }
 
 enum rateest_options {
@@ -38,22 +39,26 @@ enum rateest_options {
 	OPT_RATEEST_LT,
 	OPT_RATEEST_GT,
 	OPT_RATEEST_EQ,
+	OPT_RATEEST_BYTES,
+	OPT_RATEEST_PACKETS,
 };
 
 static const struct option rateest_opts[] = {
 	{.name = "rateest1",      .has_arg = true,  .val = OPT_RATEEST1},
 	{.name = "rateest",       .has_arg = true,  .val = OPT_RATEEST1}, /* alias for absolute mode */
 	{.name = "rateest2",      .has_arg = true,  .val = OPT_RATEEST2},
-	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
-	{.name = "rateest-pps1",  .has_arg = false, .val = OPT_RATEEST_PPS1},
-	{.name = "rateest-bps2",  .has_arg = false, .val = OPT_RATEEST_BPS2},
-	{.name = "rateest-pps2",  .has_arg = false, .val = OPT_RATEEST_PPS2},
-	{.name = "rateest-bps",   .has_arg = false, .val = OPT_RATEEST_BPS2}, /* alias for absolute mode */
-	{.name = "rateest-pps",   .has_arg = false, .val = OPT_RATEEST_PPS2}, /* alias for absolute mode */
+	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
+	{.name = "rateest-pps1",  .has_arg = true, .val = OPT_RATEEST_PPS1},
+	{.name = "rateest-bps2",  .has_arg = true, .val = OPT_RATEEST_BPS2},
+	{.name = "rateest-pps2",  .has_arg = true, .val = OPT_RATEEST_PPS2},
+	{.name = "rateest-bps",   .has_arg = true, .val = OPT_RATEEST_BPS2}, /* alias for absolute mode */
+	{.name = "rateest-pps",   .has_arg = true, .val = OPT_RATEEST_PPS2}, /* alias for absolute mode */
 	{.name = "rateest-delta", .has_arg = false, .val = OPT_RATEEST_DELTA},
 	{.name = "rateest-lt",    .has_arg = false, .val = OPT_RATEEST_LT},
 	{.name = "rateest-gt",    .has_arg = false, .val = OPT_RATEEST_GT},
 	{.name = "rateest-eq",    .has_arg = false, .val = OPT_RATEEST_EQ},
+	{.name = "bytes",   .has_arg = false, .val = OPT_RATEEST_BYTES},
+	{.name = "packets", .has_arg = false, .val = OPT_RATEEST_PACKETS},
 	XT_GETOPT_TABLEEND,
 };
 
@@ -160,15 +165,10 @@ rateest_parse(int c, char **argv, int invert, unsigned int *flags,
 
 		info->flags |= XT_RATEEST_MATCH_BPS;
 
-		/* The rate is optional and only required in absolute mode */
-		if (!argv[optind] || *argv[optind] == '-' || *argv[optind] == '!')
-			break;
-
-		if (rateest_get_rate(&info->bps1, argv[optind]) < 0)
+		if (rateest_get_rate(&info->bps1, optarg) < 0)
 			xtables_error(PARAMETER_PROBLEM,
 				   "rateest: could not parse rate `%s'",
-				   argv[optind]);
-		optind++;
+				   optarg);
 		break;
 
 	case OPT_RATEEST_PPS1:
@@ -184,16 +184,11 @@ rateest_parse(int c, char **argv, int invert, unsigned int *flags,
 
 		info->flags |= XT_RATEEST_MATCH_PPS;
 
-		/* The rate is optional and only required in absolute mode */
-		if (!argv[optind] || *argv[optind] == '-' || *argv[optind] == '!')
-			break;
-
-		if (!xtables_strtoui(argv[optind], NULL, &val, 0, UINT32_MAX))
+		if (!xtables_strtoui(optarg, NULL, &val, 0, UINT32_MAX))
 			xtables_error(PARAMETER_PROBLEM,
 				   "rateest: could not parse pps `%s'",
-				   argv[optind]);
+				   optarg);
 		info->pps1 = val;
-		optind++;
 		break;
 
 	case OPT_RATEEST_BPS2:
@@ -209,15 +204,10 @@ rateest_parse(int c, char **argv, int invert, unsigned int *flags,
 
 		info->flags |= XT_RATEEST_MATCH_BPS;
 
-		/* The rate is optional and only required in absolute mode */
-		if (!argv[optind] || *argv[optind] == '-' || *argv[optind] == '!')
-			break;
-
-		if (rateest_get_rate(&info->bps2, argv[optind]) < 0)
+		if (rateest_get_rate(&info->bps2, optarg) < 0)
 			xtables_error(PARAMETER_PROBLEM,
 				   "rateest: could not parse rate `%s'",
-				   argv[optind]);
-		optind++;
+				   optarg);
 		break;
 
 	case OPT_RATEEST_PPS2:
@@ -233,16 +223,11 @@ rateest_parse(int c, char **argv, int invert, unsigned int *flags,
 
 		info->flags |= XT_RATEEST_MATCH_PPS;
 
-		/* The rate is optional and only required in absolute mode */
-		if (!argv[optind] || *argv[optind] == '-' || *argv[optind] == '!')
-			break;
-
-		if (!xtables_strtoui(argv[optind], NULL, &val, 0, UINT32_MAX))
+		if (!xtables_strtoui(optarg, NULL, &val, 0, UINT32_MAX))
 			xtables_error(PARAMETER_PROBLEM,
 				   "rateest: could not parse pps `%s'",
-				   argv[optind]);
+				   optarg);
 		info->pps2 = val;
-		optind++;
 		break;
 
 	case OPT_RATEEST_DELTA:
@@ -297,6 +282,14 @@ rateest_parse(int c, char **argv, int invert, unsigned int *flags,
 		if (invert)
 			info->flags |= XT_RATEEST_MATCH_INVERT;
 		break;
+
+	case OPT_RATEEST_BYTES:
+		info->flags |= XT_RATEEST_MATCH_BPS;
+		break;
+
+	case OPT_RATEEST_PACKETS:
+		info->flags |= XT_RATEEST_MATCH_PPS;
+		break;
 	}
 
 	return 1;
diff --git a/extensions/libxt_rateest.man b/extensions/libxt_rateest.man
index 42a82f3..fb81e5c 100644
--- a/extensions/libxt_rateest.man
+++ b/extensions/libxt_rateest.man
@@ -16,9 +16,9 @@ combinations:
 (\fBrateest\fP minus \fBrateest-pps1\fP) \fIoperator\fP \fBrateest-pps2\fP
 .\" * Relative:
 .IP \(bu 4
-\fBrateest1\fP \fIoperator\fP \fBrateest2\fP \fBrateest-bps\fP(without rate!)
+\fBrateest1\fP \fIoperator\fP \fBrateest2\fP \fBbytes\fP
 .IP \(bu 4
-\fBrateest1\fP \fIoperator\fP \fBrateest2\fP \fBrateest-pps\fP(without rate!)
+\fBrateest1\fP \fIoperator\fP \fBrateest2\fP \fBpackets\fP
 .\" * Relative + Delta:
 .IP \(bu 4
 (\fBrateest1\fP minus \fBrateest-bps1\fP) \fIoperator\fP
@@ -55,21 +55,25 @@ Name of the one rate estimator for absolute mode.
 \fB\-\-rateest2\fP \fIname\fP
 The names of the two rate estimators for relative mode.
 .TP
-\fB\-\-rateest\-bps\fP [\fIvalue\fP]
+\fB\-\-rateest\-bps\fP \fIvalue\fP
 .TP
-\fB\-\-rateest\-pps\fP [\fIvalue\fP]
+\fB\-\-rateest\-pps\fP \fIvalue\fP
 .TP
-\fB\-\-rateest\-bps1\fP [\fIvalue\fP]
+\fB\-\-rateest\-bps1\fP \fIvalue\fP
 .TP
-\fB\-\-rateest\-bps2\fP [\fIvalue\fP]
+\fB\-\-rateest\-bps2\fP \fIvalue\fP
 .TP
-\fB\-\-rateest\-pps1\fP [\fIvalue\fP]
+\fB\-\-rateest\-pps1\fP \fIvalue\fP
 .TP
-\fB\-\-rateest\-pps2\fP [\fIvalue\fP]
+\fB\-\-rateest\-pps2\fP \fIvalue\fP
 Compare the estimator(s) by bytes or packets per second, and compare against
 the chosen value. See the above bullet list for which option is to be used in
 which case. A unit suffix may be used - available ones are: bit, [kmgt]bit,
 [KMGT]ibit, Bps, [KMGT]Bps, [KMGT]iBps.
+.TP
+\fB\-\-bytes\fP, \fB\-\-packets\fP
+Compare the estimators by bytes or packets. One of this option is required when
+comparing two estimators without using one of \-\-rateest\-[bp]s[12].
 .PP
 Example: This is what can be used to route outgoing data connections from an
 FTP server over two lines based on the available bandwidth at the time the data
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 11/13] libxt_rateest: fix rateest save operation
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (9 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 10/13] libxt_rateest: avoid optional arguments Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 12/13] libxt_rateest: use guided option parser Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 13/13] libxt_ipvs: restore network-byte order Jan Engelhardt
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

rateest's save operation was pretty much busted.

References: http://marc.info/?l=netfilter&m=130332436408107&w=2
Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_rateest.c |   73 +++++++++++++++++++++++++++++++++-----------
 1 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/extensions/libxt_rateest.c b/extensions/libxt_rateest.c
index 62d6033..2244558 100644
--- a/extensions/libxt_rateest.c
+++ b/extensions/libxt_rateest.c
@@ -399,27 +399,64 @@ rateest_save(const void *ip, const struct xt_entry_match *match)
 {
 	const struct xt_rateest_match_info *info = (const void *)match->data;
 
+	if (info->flags & XT_RATEEST_MATCH_DELTA)
+		printf(" --rateest-delta");
+
 	if (info->flags & XT_RATEEST_MATCH_REL) {
-		printf(" --rateest1 %s", info->name1);
-		if (info->flags & XT_RATEEST_MATCH_BPS)
-			printf(" --rateest-bps");
-		if (info->flags & XT_RATEEST_MATCH_PPS)
-			printf(" --rateest-pps");
-		rateest_print_mode(info, " --rateest-");
-		printf(" --rateest2 %s", info->name2);
-	} else {
-		printf(" --rateest %s", info->name1);
-		if (info->flags & XT_RATEEST_MATCH_BPS) {
-			printf(" --rateest-bps1");
-			rateest_print_rate(info->bps1, 0);
-			printf(" --rateest-bps2");
-			rateest_print_rate(info->bps2, 0);
+		printf(" --rateest1");
+		xtables_save_string(info->name1);
+		if (info->flags & XT_RATEEST_MATCH_DELTA) {
+			if (info->flags & XT_RATEEST_MATCH_BPS) {
+				printf(" --rateest-bps1");
+				rateest_print_rate(info->bps1, 0);
+			} else {
+				printf(" --rateest-pps1 %u", info->pps1);
+			}
 			rateest_print_mode(info, "--rateest-");
-		}
-		if (info->flags & XT_RATEEST_MATCH_PPS) {
-			printf(" --rateest-pps");
+			printf(" --rateest2");
+			xtables_save_string(info->name2);
+			if (info->flags & XT_RATEEST_MATCH_BPS) {
+				printf(" --rateest-bps2");
+				rateest_print_rate(info->bps2, 0);
+			} else {
+				printf(" --rateest-pps2 %u", info->pps2);
+			}
+		} else {
 			rateest_print_mode(info, "--rateest-");
-			printf(" %u", info->pps2);
+			printf(" --rateest2");
+			xtables_save_string(info->name2);
+			printf(info->flags & XT_RATEEST_MATCH_BPS ?
+			       " --bytes" : " --packets");
+		}
+	} else {
+		printf(" --rateest");
+		xtables_save_string(info->name1);
+		if (info->flags & XT_RATEEST_MATCH_DELTA) {
+			if (info->flags & XT_RATEEST_MATCH_BPS) {
+				printf(" --rateest-bps1");
+				rateest_print_rate(info->bps1, 0);
+			} else {
+				printf(" --rateest-pps1 %u",
+				       (unsigned int)info->pps1);
+			}
+		}
+		rateest_print_mode(info, "--rateest-");
+		if (info->flags & XT_RATEEST_MATCH_DELTA) {
+			if (info->flags & XT_RATEEST_MATCH_BPS) {
+				printf(" --rateest-bps2");
+				rateest_print_rate(info->bps2, 0);
+			} else {
+				printf(" --rateest-pps2 %u",
+				       (unsigned int)info->pps2);
+			}
+		} else {
+			if (info->flags & XT_RATEEST_MATCH_BPS) {
+				printf(" --rateest-bps");
+				rateest_print_rate(info->bps2, 0);
+			} else {
+				printf(" --rateest-pps %u",
+				       (unsigned int)info->pps2);
+			}
 		}
 	}
 }
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 12/13] libxt_rateest: use guided option parser
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (10 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 11/13] libxt_rateest: fix rateest save operation Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  2011-05-23 14:39 ` [PATCH 13/13] libxt_ipvs: restore network-byte order Jan Engelhardt
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_rateest.c |  256 +++++++++++++++-----------------------------
 1 files changed, 88 insertions(+), 168 deletions(-)

diff --git a/extensions/libxt_rateest.c b/extensions/libxt_rateest.c
index 2244558..92b8d50 100644
--- a/extensions/libxt_rateest.c
+++ b/extensions/libxt_rateest.c
@@ -8,9 +8,6 @@
 #include <xtables.h>
 #include <linux/netfilter/xt_rateest.h>
 
-/* Ugly hack to pass info to final_check function. We should fix the API */
-static struct xt_rateest_match_info *rateest_info;
-
 static void rateest_help(void)
 {
 	printf(
@@ -29,8 +26,11 @@ static void rateest_help(void)
 }
 
 enum rateest_options {
+	OPT_RATEEST_A,
 	OPT_RATEEST1,
 	OPT_RATEEST2,
+	OPT_RATEEST_ABPS,
+	OPT_RATEEST_APPS,
 	OPT_RATEEST_BPS1,
 	OPT_RATEEST_PPS1,
 	OPT_RATEEST_BPS2,
@@ -41,26 +41,71 @@ enum rateest_options {
 	OPT_RATEEST_EQ,
 	OPT_RATEEST_BYTES,
 	OPT_RATEEST_PACKETS,
+	F_RATEEST_A       = 1 << OPT_RATEEST_A,
+	F_RATEEST1        = 1 << OPT_RATEEST1,
+	F_RATEEST2        = 1 << OPT_RATEEST2,
+	F_RATEEST_ABPS    = 1 << OPT_RATEEST_ABPS,
+	F_RATEEST_APPS    = 1 << OPT_RATEEST_APPS,
+	F_RATEEST_BPS1    = 1 << OPT_RATEEST_BPS1,
+	F_RATEEST_BPS2    = 1 << OPT_RATEEST_BPS2,
+	F_RATEEST_PPS1    = 1 << OPT_RATEEST_PPS1,
+	F_RATEEST_PPS2    = 1 << OPT_RATEEST_PPS2,
+	F_RATEEST_BYTES   = 1 << OPT_RATEEST_BYTES,
+	F_RATEEST_PACKETS = 1 << OPT_RATEEST_PACKETS,
 };
 
-static const struct option rateest_opts[] = {
-	{.name = "rateest1",      .has_arg = true,  .val = OPT_RATEEST1},
-	{.name = "rateest",       .has_arg = true,  .val = OPT_RATEEST1}, /* alias for absolute mode */
-	{.name = "rateest2",      .has_arg = true,  .val = OPT_RATEEST2},
-	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
-	{.name = "rateest-pps1",  .has_arg = true, .val = OPT_RATEEST_PPS1},
-	{.name = "rateest-bps2",  .has_arg = true, .val = OPT_RATEEST_BPS2},
-	{.name = "rateest-pps2",  .has_arg = true, .val = OPT_RATEEST_PPS2},
-	{.name = "rateest-bps",   .has_arg = true, .val = OPT_RATEEST_BPS2}, /* alias for absolute mode */
-	{.name = "rateest-pps",   .has_arg = true, .val = OPT_RATEEST_PPS2}, /* alias for absolute mode */
-	{.name = "rateest-delta", .has_arg = false, .val = OPT_RATEEST_DELTA},
-	{.name = "rateest-lt",    .has_arg = false, .val = OPT_RATEEST_LT},
-	{.name = "rateest-gt",    .has_arg = false, .val = OPT_RATEEST_GT},
-	{.name = "rateest-eq",    .has_arg = false, .val = OPT_RATEEST_EQ},
-	{.name = "bytes",   .has_arg = false, .val = OPT_RATEEST_BYTES},
-	{.name = "packets", .has_arg = false, .val = OPT_RATEEST_PACKETS},
-	XT_GETOPT_TABLEEND,
+#define s struct xt_rateest_match_info
+static const struct xt_option_entry rateest_opts[] = {
+	{.name = "rateest-delta", .id = OPT_RATEEST_DELTA, .type = XTTYPE_NONE},
+	{.name = "rateest-lt", .id = OPT_RATEEST_LT, .type = XTTYPE_NONE,
+	 .flags = XTOPT_INVERT},
+	{.name = "rateest-gt", .id = OPT_RATEEST_GT, .type = XTTYPE_NONE,
+	 .flags = XTOPT_INVERT},
+	{.name = "rateest-eq", .id = OPT_RATEEST_EQ, .type = XTTYPE_NONE,
+	 .flags = XTOPT_INVERT},
+
+	{.name = "rateest", .id = OPT_RATEEST_A, .type = XTTYPE_STRING,
+	 .excl = F_RATEEST1 | F_RATEEST2,
+	 .flags = XTOPT_PUT, XTOPT_POINTER(s, name1)},
+	{.name = "rateest1", .id = OPT_RATEEST1, .type = XTTYPE_STRING,
+	 .excl = F_RATEEST_A, .also = F_RATEEST2,
+	 .flags = XTOPT_PUT, XTOPT_POINTER(s, name1)},
+	{.name = "rateest2", .id = OPT_RATEEST2, .type = XTTYPE_STRING,
+	 .excl = F_RATEEST_A, .also = F_RATEEST1,
+	 .flags = XTOPT_PUT, XTOPT_POINTER(s, name2)},
+
+	{.name = "rateest-bps", .id = OPT_RATEEST_ABPS, .type = XTTYPE_STRING,
+	 .excl = F_RATEEST_BPS1 | F_RATEEST_BPS2 | F_RATEEST_APPS,
+	 F_RATEEST_PPS1 | F_RATEEST_PPS2 | F_RATEEST_BYTES | F_RATEEST_PACKETS,
+	 .also = F_RATEEST_A},
+	{.name = "rateest-pps", .id = OPT_RATEEST_APPS, .type = XTTYPE_UINT32,
+	 .excl = F_RATEEST_BPS1 | F_RATEEST_BPS2 | F_RATEEST_ABPS,
+	 F_RATEEST_PPS1 | F_RATEEST_PPS2 | F_RATEEST_BYTES | F_RATEEST_PACKETS,
+	 .also = F_RATEEST_A, .flags = XTOPT_PUT, XTOPT_POINTER(s, pps2)},
+	{.name = "bytes", .id = OPT_RATEEST_BYTES, .type = XTTYPE_NONE,
+	 .excl = F_RATEEST_PACKETS, .also = F_RATEEST2},
+	{.name = "packets", .id = OPT_RATEEST_PACKETS, .type = XTTYPE_NONE,
+	 .excl = F_RATEEST_BYTES, .also = F_RATEEST2},
+
+	{.name = "rateest-bps1", .id = OPT_RATEEST_BPS1, .type = XTTYPE_STRING,
+	 .excl = F_RATEEST_ABPS | F_RATEEST_APPS | F_RATEEST_PPS1 |
+	  F_RATEEST_PPS2 | F_RATEEST_BYTES | F_RATEEST_PACKETS,
+	 .also = F_RATEEST_BPS2},
+	{.name = "rateest-bps2", .id = OPT_RATEEST_BPS2, .type = XTTYPE_STRING,
+	 .excl = F_RATEEST_ABPS | F_RATEEST_APPS | F_RATEEST_PPS1 |
+	 F_RATEEST_PPS2 | F_RATEEST_BYTES | F_RATEEST_PACKETS,
+	 .also = F_RATEEST_BPS1},
+	{.name = "rateest-pps1", .id = OPT_RATEEST_PPS1, .type = XTTYPE_UINT32,
+	 .excl = F_RATEEST_ABPS | F_RATEEST_APPS | F_RATEEST_BPS1 |
+	 F_RATEEST_BPS2 | F_RATEEST_BYTES | F_RATEEST_PACKETS,
+	 .also = F_RATEEST_PPS2, .flags = XTOPT_PUT, XTOPT_POINTER(s, pps1)},
+	{.name = "rateest-pps2", .id = OPT_RATEEST_PPS2, .type = XTTYPE_UINT32,
+	 .excl = F_RATEEST_ABPS | F_RATEEST_APPS | F_RATEEST_BPS1 |
+	 F_RATEEST_BPS2 | F_RATEEST_BYTES | F_RATEEST_PACKETS,
+	 .also = F_RATEEST_PPS1, .flags = XTOPT_PUT, XTOPT_POINTER(s, pps2)},
+	XTOPT_TABLEEND,
 };
+#undef s
 
 /* Copied from iproute. See http://physics.nist.gov/cuu/Units/binary.html */
 static const struct rate_suffix {
@@ -113,173 +158,51 @@ rateest_get_rate(uint32_t *rate, const char *str)
 	return -1;
 }
 
-static int
-rateest_parse(int c, char **argv, int invert, unsigned int *flags,
-	      const void *entry, struct xt_entry_match **match)
+static void rateest_parse(struct xt_option_call *cb)
 {
-	struct xt_rateest_match_info *info = (void *)(*match)->data;
-	unsigned int val;
-
-	rateest_info = info;
+	struct xt_rateest_match_info *info = cb->data;
 
-	switch (c) {
+	xtables_option_parse(cb);
+	switch (cb->entry->id) {
 	case OPT_RATEEST1:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest1 twice");
-		*flags |= 1 << c;
-
-		strncpy(info->name1, optarg, sizeof(info->name1) - 1);
-		break;
-
 	case OPT_RATEEST2:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest2 twice");
-		*flags |= 1 << c;
-
-		strncpy(info->name2, optarg, sizeof(info->name2) - 1);
 		info->flags |= XT_RATEEST_MATCH_REL;
 		break;
-
+	case OPT_RATEEST_APPS:
+	case OPT_RATEEST_PPS1:
+	case OPT_RATEEST_PPS2:
+		info->flags |= XT_RATEEST_MATCH_PPS;
+		break;
+	case OPT_RATEEST_ABPS:
 	case OPT_RATEEST_BPS1:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest-bps can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest-bps1 twice");
-		*flags |= 1 << c;
-
 		info->flags |= XT_RATEEST_MATCH_BPS;
-
-		if (rateest_get_rate(&info->bps1, optarg) < 0)
+		if (rateest_get_rate(&info->bps1, cb->arg) < 0)
 			xtables_error(PARAMETER_PROBLEM,
 				   "rateest: could not parse rate `%s'",
-				   optarg);
-		break;
-
-	case OPT_RATEEST_PPS1:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest-pps can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest-pps1 twice");
-		*flags |= 1 << c;
-
-		info->flags |= XT_RATEEST_MATCH_PPS;
-
-		if (!xtables_strtoui(optarg, NULL, &val, 0, UINT32_MAX))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: could not parse pps `%s'",
-				   optarg);
-		info->pps1 = val;
-		break;
-
+				   cb->arg);
 	case OPT_RATEEST_BPS2:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest-bps can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest-bps2 twice");
-		*flags |= 1 << c;
-
 		info->flags |= XT_RATEEST_MATCH_BPS;
-
-		if (rateest_get_rate(&info->bps2, optarg) < 0)
+		if (rateest_get_rate(&info->bps2, cb->arg) < 0)
 			xtables_error(PARAMETER_PROBLEM,
 				   "rateest: could not parse rate `%s'",
-				   optarg);
+				   cb->arg);
 		break;
-
-	case OPT_RATEEST_PPS2:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest-pps can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest-pps2 twice");
-		*flags |= 1 << c;
-
-		info->flags |= XT_RATEEST_MATCH_PPS;
-
-		if (!xtables_strtoui(optarg, NULL, &val, 0, UINT32_MAX))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: could not parse pps `%s'",
-				   optarg);
-		info->pps2 = val;
-		break;
-
 	case OPT_RATEEST_DELTA:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-		if (invert)
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: rateest-delta can't be inverted");
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify --rateest-delta twice");
-		*flags |= 1 << c;
-
 		info->flags |= XT_RATEEST_MATCH_DELTA;
 		break;
-
 	case OPT_RATEEST_EQ:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify lt/gt/eq twice");
-		*flags |= 1 << c;
-
 		info->mode = XT_RATEEST_MATCH_EQ;
-		if (invert)
+		if (cb->invert)
 			info->flags |= XT_RATEEST_MATCH_INVERT;
 		break;
-
 	case OPT_RATEEST_LT:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify lt/gt/eq twice");
-		*flags |= 1 << c;
-
 		info->mode = XT_RATEEST_MATCH_LT;
-		if (invert)
+		if (cb->invert)
 			info->flags |= XT_RATEEST_MATCH_INVERT;
 		break;
-
 	case OPT_RATEEST_GT:
-		xtables_check_inverse(optarg, &invert, &optind, 0, argv);
-
-		if (*flags & (1 << c))
-			xtables_error(PARAMETER_PROBLEM,
-				   "rateest: can't specify lt/gt/eq twice");
-		*flags |= 1 << c;
-
 		info->mode = XT_RATEEST_MATCH_GT;
-		if (invert)
+		if (cb->invert)
 			info->flags |= XT_RATEEST_MATCH_INVERT;
 		break;
 
@@ -291,16 +214,13 @@ rateest_parse(int c, char **argv, int invert, unsigned int *flags,
 		info->flags |= XT_RATEEST_MATCH_PPS;
 		break;
 	}
-
-	return 1;
 }
 
-static void
-rateest_final_check(unsigned int flags)
+static void rateest_final_check(struct xt_fcheck_call *cb)
 {
-	struct xt_rateest_match_info *info = rateest_info;
+	struct xt_rateest_match_info *info = cb->data;
 
-	if (info == NULL)
+	if (cb->xflags == 0)
 		xtables_error(PARAMETER_PROBLEM, "rateest match: "
 		           "you need to specify some flags");
 	if (!(info->flags & XT_RATEEST_MATCH_REL))
@@ -468,11 +388,11 @@ static struct xtables_match rateest_mt_reg = {
 	.size		= XT_ALIGN(sizeof(struct xt_rateest_match_info)),
 	.userspacesize	= XT_ALIGN(offsetof(struct xt_rateest_match_info, est1)),
 	.help		= rateest_help,
-	.parse		= rateest_parse,
-	.final_check	= rateest_final_check,
+	.x6_parse	= rateest_parse,
+	.x6_fcheck	= rateest_final_check,
 	.print		= rateest_print,
 	.save		= rateest_save,
-	.extra_opts	= rateest_opts,
+	.x6_options	= rateest_opts,
 };
 
 void _init(void)
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 13/13] libxt_ipvs: restore network-byte order
  2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
                   ` (11 preceding siblings ...)
  2011-05-23 14:39 ` [PATCH 12/13] libxt_rateest: use guided option parser Jan Engelhardt
@ 2011-05-23 14:39 ` Jan Engelhardt
  12 siblings, 0 replies; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-23 14:39 UTC (permalink / raw)
  To: kaber; +Cc: netfilter-devel

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 extensions/libxt_ipvs.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/extensions/libxt_ipvs.c b/extensions/libxt_ipvs.c
index 88d235f..4672766 100644
--- a/extensions/libxt_ipvs.c
+++ b/extensions/libxt_ipvs.c
@@ -32,12 +32,14 @@ static const struct xt_option_entry ipvs_mt_opts[] = {
 	{.name = "vaddr", .id = O_VADDR, .type = XTTYPE_HOSTMASK,
 	 .flags = XTOPT_INVERT},
 	{.name = "vport", .id = O_VPORT, .type = XTTYPE_PORT,
-	 .flags = XTOPT_INVERT | XTOPT_PUT, XTOPT_POINTER(s, vport)},
+	 .flags = XTOPT_NBO | XTOPT_INVERT | XTOPT_PUT,
+	 XTOPT_POINTER(s, vport)},
 	{.name = "vdir", .id = O_VDIR, .type = XTTYPE_STRING},
 	{.name = "vmethod", .id = O_VMETHOD, .type = XTTYPE_STRING,
 	 .flags = XTOPT_INVERT},
 	{.name = "vportctl", .id = O_VPORTCTL, .type = XTTYPE_PORT,
-	 .flags = XTOPT_INVERT | XTOPT_PUT, XTOPT_POINTER(s, vportctl)},
+	 .flags = XTOPT_NBO | XTOPT_INVERT | XTOPT_PUT,
+	 XTOPT_POINTER(s, vportctl)},
 	XTOPT_TABLEEND,
 };
 #undef s
-- 
1.7.3.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/13] libxt_rateest: avoid optional arguments
  2011-05-23 14:39 ` [PATCH 10/13] libxt_rateest: avoid optional arguments Jan Engelhardt
@ 2011-05-24  6:46   ` Patrick McHardy
  2011-05-24  8:03     ` Jan Engelhardt
  0 siblings, 1 reply; 19+ messages in thread
From: Patrick McHardy @ 2011-05-24  6:46 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

On 23.05.2011 16:39, Jan Engelhardt wrote:
> Optional arguments make parsing unnecessarily harder - even more so
> than two-args. Right now, rateest even crashes because of it.
> 
>  static const struct option rateest_opts[] = {
>  	{.name = "rateest1",      .has_arg = true,  .val = OPT_RATEEST1},
>  	{.name = "rateest",       .has_arg = true,  .val = OPT_RATEEST1}, /* alias for absolute mode */
>  	{.name = "rateest2",      .has_arg = true,  .val = OPT_RATEEST2},
> -	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
> -	{.name = "rateest-pps1",  .has_arg = false, .val = OPT_RATEEST_PPS1},
> -	{.name = "rateest-bps2",  .has_arg = false, .val = OPT_RATEEST_BPS2},
> -	{.name = "rateest-pps2",  .has_arg = false, .val = OPT_RATEEST_PPS2},
> -	{.name = "rateest-bps",   .has_arg = false, .val = OPT_RATEEST_BPS2}, /* alias for absolute mode */
> -	{.name = "rateest-pps",   .has_arg = false, .val = OPT_RATEEST_PPS2}, /* alias for absolute mode */
> +	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
> +	{.name = "rateest-pps1",  .has_arg = true, .val = OPT_RATEEST_PPS1},
> +	{.name = "rateest-bps2",  .has_arg = true, .val = OPT_RATEEST_BPS2},
> +	{.name = "rateest-pps2",  .has_arg = true, .val = OPT_RATEEST_PPS2},
> +	{.name = "rateest-bps",   .has_arg = true, .val = OPT_RATEEST_BPS2}, /* alias for absolute mode */
> +	{.name = "rateest-pps",   .has_arg = true, .val = OPT_RATEEST_PPS2}, /* alias for absolute mode */
>  	{.name = "rateest-delta", .has_arg = false, .val = OPT_RATEEST_DELTA},
>  	{.name = "rateest-lt",    .has_arg = false, .val = OPT_RATEEST_LT},
>  	{.name = "rateest-gt",    .has_arg = false, .val = OPT_RATEEST_GT},
>  	{.name = "rateest-eq",    .has_arg = false, .val = OPT_RATEEST_EQ},
> +	{.name = "bytes",   .has_arg = false, .val = OPT_RATEEST_BYTES},
> +	{.name = "packets", .has_arg = false, .val = OPT_RATEEST_PACKETS},
>  	XT_GETOPT_TABLEEND,
>  };

This appears to be breaking backwards compatibility.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/13] libxt_rateest: avoid optional arguments
  2011-05-24  6:46   ` Patrick McHardy
@ 2011-05-24  8:03     ` Jan Engelhardt
  2011-05-24  8:14       ` Patrick McHardy
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-24  8:03 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel

On Tuesday 2011-05-24 08:46, Patrick McHardy wrote:

>On 23.05.2011 16:39, Jan Engelhardt wrote:
>> Optional arguments make parsing unnecessarily harder - even more so
>> than two-args. Right now, rateest even crashes because of it.
>> 
>>  static const struct option rateest_opts[] = {
[...]
>> -	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
>> +	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
[...]
>
>This appears to be breaking backwards compatibility.

Admittedly yes, though the fact that this has remained unseen for so
long suggests that the potential user base is very small or not yet
existing.

In my time with users in IRC, I notice that they in particular prefer
hard stops in parsing over silent upgrades of rules[1], so as to
actually become aware of the change upfront. As such, I believe the
impact is well justified.

[1] i.e. `diff -u rules <(xtables-main save6)` is supposed to yield
no diff except for counters

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/13] libxt_rateest: avoid optional arguments
  2011-05-24  8:03     ` Jan Engelhardt
@ 2011-05-24  8:14       ` Patrick McHardy
  2011-05-24  8:51         ` Jan Engelhardt
  0 siblings, 1 reply; 19+ messages in thread
From: Patrick McHardy @ 2011-05-24  8:14 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

On 24.05.2011 10:03, Jan Engelhardt wrote:
> On Tuesday 2011-05-24 08:46, Patrick McHardy wrote:
> 
>> On 23.05.2011 16:39, Jan Engelhardt wrote:
>>> Optional arguments make parsing unnecessarily harder - even more so
>>> than two-args. Right now, rateest even crashes because of it.
>>>
>>>  static const struct option rateest_opts[] = {
> [...]
>>> -	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
>>> +	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
> [...]
>>
>> This appears to be breaking backwards compatibility.
> 
> Admittedly yes, though the fact that this has remained unseen for so
> long suggests that the potential user base is very small or not yet
> existing.

I'm pretty sure this used to work at some point. Let me check history.

> In my time with users in IRC, I notice that they in particular prefer
> hard stops in parsing over silent upgrades of rules[1], so as to
> actually become aware of the change upfront. As such, I believe the
> impact is well justified.

Well, if it really was broken from the beginning I'm fine of course,
but I don't think that's case.

> [1] i.e. `diff -u rules <(xtables-main save6)` is supposed to yield
> no diff except for counters
> 


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/13] libxt_rateest: avoid optional arguments
  2011-05-24  8:14       ` Patrick McHardy
@ 2011-05-24  8:51         ` Jan Engelhardt
  2011-05-24 12:49           ` Patrick McHardy
  0 siblings, 1 reply; 19+ messages in thread
From: Jan Engelhardt @ 2011-05-24  8:51 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: netfilter-devel


On Tuesday 2011-05-24 10:14, Patrick McHardy wrote:
>>> On 23.05.2011 16:39, Jan Engelhardt wrote:
>>>> Optional arguments make parsing unnecessarily harder - even more so
>>>> than two-args. Right now, rateest even crashes because of it.
>>>>
>>>>  static const struct option rateest_opts[] = {
>> [...]
>>>> -	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
>>>> +	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
>> [...]
>>>
>>> This appears to be breaking backwards compatibility.
>> 
>> Admittedly yes, though the fact that this has remained unseen for so
>> long suggests that the potential user base is very small or not yet
>> existing.
>
>I'm pretty sure this used to work at some point. Let me check history.
>
>> In my time with users in IRC, I notice that they in particular prefer
>> hard stops in parsing over silent upgrades of rules[1], so as to
>> actually become aware of the change upfront. As such, I believe the
>> impact is well justified.
>
>Well, if it really was broken from the beginning I'm fine of course,
>but I don't think that's case.

It works half of the time, and fails half of the time because it can
run - since the beginning - into UB when using argv[optind].

Yes, the fix can be done in many ways, silent update is possible, but
that is undesirable, as are optional arguments in the first place.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 10/13] libxt_rateest: avoid optional arguments
  2011-05-24  8:51         ` Jan Engelhardt
@ 2011-05-24 12:49           ` Patrick McHardy
  0 siblings, 0 replies; 19+ messages in thread
From: Patrick McHardy @ 2011-05-24 12:49 UTC (permalink / raw)
  To: Jan Engelhardt; +Cc: netfilter-devel

On 24.05.2011 10:51, Jan Engelhardt wrote:
> 
> On Tuesday 2011-05-24 10:14, Patrick McHardy wrote:
>>>> On 23.05.2011 16:39, Jan Engelhardt wrote:
>>>>> Optional arguments make parsing unnecessarily harder - even more so
>>>>> than two-args. Right now, rateest even crashes because of it.
>>>>>
>>>>>  static const struct option rateest_opts[] = {
>>> [...]
>>>>> -	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
>>>>> +	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
>>> [...]
>>>>
>>>> This appears to be breaking backwards compatibility.
>>>
>>> Admittedly yes, though the fact that this has remained unseen for so
>>> long suggests that the potential user base is very small or not yet
>>> existing.
>>
>> I'm pretty sure this used to work at some point. Let me check history.
>>
>>> In my time with users in IRC, I notice that they in particular prefer
>>> hard stops in parsing over silent upgrades of rules[1], so as to
>>> actually become aware of the change upfront. As such, I believe the
>>> impact is well justified.
>>
>> Well, if it really was broken from the beginning I'm fine of course,
>> but I don't think that's case.
> 
> It works half of the time, and fails half of the time because it can
> run - since the beginning - into UB when using argv[optind].
> 
> Yes, the fix can be done in many ways, silent update is possible, but
> that is undesirable, as are optional arguments in the first place.
> 

Well, not breaking compatibility for the people it used to work for
certainly is desirable. If its not too much trouble, I very much
prefer a soft update and if necessary replacement of the old options
after some warning period.

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2011-05-24 12:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-23 14:39 option parsing, run 9 Jan Engelhardt
2011-05-23 14:39 ` [PATCH 01/13] libxtables: retract _NE types and use a flag instead Jan Engelhardt
2011-05-23 14:39 ` [PATCH 02/13] libipt_REDIRECT: "--to-ports" is not mandatory Jan Engelhardt
2011-05-23 14:39 ` [PATCH 03/13] libxt_quota: readd missing XTOPT_PUT request Jan Engelhardt
2011-05-23 14:39 ` [PATCH 04/13] libxt_quota: make sure uint64 is not truncated Jan Engelhardt
2011-05-23 14:39 ` [PATCH 05/13] libxtables: check for negative numbers in xtables_strtou* Jan Engelhardt
2011-05-23 14:39 ` [PATCH 06/13] libxt_rateest: streamline case display of units Jan Engelhardt
2011-05-23 14:39 ` [PATCH 07/13] doc: add some coded option examples to libxt_hashlimit Jan Engelhardt
2011-05-23 14:39 ` [PATCH 08/13] doc: make usage of libxt_rateest more obvious Jan Engelhardt
2011-05-23 14:39 ` [PATCH 09/13] doc: clarify that -p all is a special keyword only Jan Engelhardt
2011-05-23 14:39 ` [PATCH 10/13] libxt_rateest: avoid optional arguments Jan Engelhardt
2011-05-24  6:46   ` Patrick McHardy
2011-05-24  8:03     ` Jan Engelhardt
2011-05-24  8:14       ` Patrick McHardy
2011-05-24  8:51         ` Jan Engelhardt
2011-05-24 12:49           ` Patrick McHardy
2011-05-23 14:39 ` [PATCH 11/13] libxt_rateest: fix rateest save operation Jan Engelhardt
2011-05-23 14:39 ` [PATCH 12/13] libxt_rateest: use guided option parser Jan Engelhardt
2011-05-23 14:39 ` [PATCH 13/13] libxt_ipvs: restore network-byte order Jan Engelhardt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.