All of lore.kernel.org
 help / color / mirror / Atom feed
* [iptables PATCH v2 00/10] Reduce code size around arptables-nft
@ 2019-10-28 15:48 Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 01/10] ip6tables, xtables-arp: Drop unused struct pprot Phil Sutter
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

A review of xtables-arp.c exposed a significant amount of dead, needless
or duplicated code. This series deals with some low hanging fruits. Most
of the changes affect xtables-arp.c and nft-arp.c only, but where common
issues existed or code was to be shared, other files are touched as
well.

Changes since v1:
- Add missing inverse_for_options array adjustments to patch 7.

Phil Sutter (10):
  ip6tables, xtables-arp: Drop unused struct pprot
  xshared: Share a common add_command() implementation
  xshared: Share a common implementation of parse_rulenumber()
  Merge CMD_* defines
  xtables-arp: Drop generic_opt_check()
  Replace TRUE/FALSE with true/false
  xtables-arp: Integrate OPT_* defines into xshared.h
  xtables-arp: Drop some unused variables
  xtables-arp: Use xtables_parse_interface()
  nft-arp: Use xtables_print_mac_and_mask()

 iptables/ip6tables.c   |  73 +-----------
 iptables/iptables.c    |  64 +----------
 iptables/nft-arp.c     |  31 +----
 iptables/nft-shared.h  |  17 ---
 iptables/xshared.c     |  39 +++++++
 iptables/xshared.h     |  32 ++++++
 iptables/xtables-arp.c | 255 ++++-------------------------------------
 iptables/xtables.c     |  48 +-------
 8 files changed, 107 insertions(+), 452 deletions(-)

-- 
2.23.0


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

* [iptables PATCH v2 01/10] ip6tables, xtables-arp: Drop unused struct pprot
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 02/10] xshared: Share a common add_command() implementation Phil Sutter
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

These seem like leftovers when changing code to use xtables_chain_protos
as struct xtables_pprot is identical to struct pprot removed here.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/ip6tables.c   | 6 ------
 iptables/xtables-arp.c | 7 -------
 2 files changed, 13 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index c160a2dd4e65b..ee463c9586862 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -175,12 +175,6 @@ static const unsigned int inverse_for_options[NUMBER_OF_OPT] =
 #define opts ip6tables_globals.opts
 #define prog_name ip6tables_globals.program_name
 #define prog_vers ip6tables_globals.program_version
-/* A few hardcoded protocols for 'all' and in case the user has no
-   /etc/protocols */
-struct pprot {
-	const char *name;
-	uint8_t num;
-};
 
 static void __attribute__((noreturn))
 exit_tryhelp(int status)
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 1a260e75e3808..8503f47fe2afe 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -209,13 +209,6 @@ static int inverse_for_options[NUMBER_OF_OPT] =
 /* -c */ 0,
 };
 
-/* A few hardcoded protocols for 'all' and in case the user has no
-   /etc/protocols */
-struct pprot {
-	char *name;
-	u_int8_t num;
-};
-
 /* Primitive headers... */
 /* defined in netinet/in.h */
 #if 0
-- 
2.23.0


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

* [iptables PATCH v2 02/10] xshared: Share a common add_command() implementation
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 01/10] ip6tables, xtables-arp: Drop unused struct pprot Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 03/10] xshared: Share a common implementation of parse_rulenumber() Phil Sutter
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

The shared definition of cmdflags is a super set of the previous one in
xtables-arp.c so while not being identical, they're compatible.

Avoid accidental array overstep in cmd2char() by incrementing an index
variable and checking its final value before using it as such.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/ip6tables.c   | 23 -----------------------
 iptables/iptables.c    | 23 -----------------------
 iptables/xshared.c     | 27 +++++++++++++++++++++++++++
 iptables/xshared.h     |  4 ++++
 iptables/xtables-arp.c | 22 ----------------------
 iptables/xtables.c     | 23 -----------------------
 6 files changed, 31 insertions(+), 91 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index ee463c9586862..9a9d71f1cdadc 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -69,8 +69,6 @@
 #define CMD_ZERO_NUM		0x2000U
 #define CMD_CHECK		0x4000U
 #define NUMBER_OF_CMD	16
-static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
-				 'N', 'X', 'P', 'E', 'S', 'Z', 'C' };
 
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
 static const char optflags[]
@@ -336,27 +334,6 @@ opt2char(int option)
 	return *ptr;
 }
 
-static char
-cmd2char(int option)
-{
-	const char *ptr;
-	for (ptr = cmdflags; option > 1; option >>= 1, ptr++);
-
-	return *ptr;
-}
-
-static void
-add_command(unsigned int *cmd, const int newcmd, const int othercmds,
-	    int invert)
-{
-	if (invert)
-		xtables_error(PARAMETER_PROBLEM, "unexpected '!' flag");
-	if (*cmd & (~othercmds))
-		xtables_error(PARAMETER_PROBLEM, "Cannot use -%c with -%c\n",
-			   cmd2char(newcmd), cmd2char(*cmd & (~othercmds)));
-	*cmd |= newcmd;
-}
-
 /*
  *	All functions starting with "parse" should succeed, otherwise
  *	the program fails.
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 544e87596e7e4..5fec25376c24f 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -65,8 +65,6 @@
 #define CMD_ZERO_NUM		0x2000U
 #define CMD_CHECK		0x4000U
 #define NUMBER_OF_CMD	16
-static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
-				 'N', 'X', 'P', 'E', 'S', 'Z', 'C' };
 
 #define OPT_FRAGMENT    0x00800U
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
@@ -335,27 +333,6 @@ opt2char(int option)
 	return *ptr;
 }
 
-static char
-cmd2char(int option)
-{
-	const char *ptr;
-	for (ptr = cmdflags; option > 1; option >>= 1, ptr++);
-
-	return *ptr;
-}
-
-static void
-add_command(unsigned int *cmd, const int newcmd, const int othercmds, 
-	    int invert)
-{
-	if (invert)
-		xtables_error(PARAMETER_PROBLEM, "unexpected ! flag");
-	if (*cmd & (~othercmds))
-		xtables_error(PARAMETER_PROBLEM, "Cannot use -%c with -%c\n",
-			   cmd2char(newcmd), cmd2char(*cmd & (~othercmds)));
-	*cmd |= newcmd;
-}
-
 /*
  *	All functions starting with "parse" should succeed, otherwise
  *	the program fails.
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 97f1b5d22fdbe..3baa805c64e6d 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -732,3 +732,30 @@ void command_jump(struct iptables_command_state *cs, const char *jumpto)
 		xtables_error(OTHER_PROBLEM, "can't alloc memory!");
 	xt_params->opts = opts;
 }
+
+char cmd2char(int option)
+{
+	/* cmdflags index corresponds with position of bit in CMD_* values */
+	static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
+					 'N', 'X', 'P', 'E', 'S', 'Z', 'C' };
+	int i;
+
+	for (i = 0; option > 1; option >>= 1, i++)
+		;
+	if (i >= ARRAY_SIZE(cmdflags))
+		xtables_error(OTHER_PROBLEM,
+			      "cmd2char(): Invalid command number %u.\n",
+			      1 << i);
+	return cmdflags[i];
+}
+
+void add_command(unsigned int *cmd, const int newcmd,
+		 const int othercmds, int invert)
+{
+	if (invert)
+		xtables_error(PARAMETER_PROBLEM, "unexpected '!' flag");
+	if (*cmd & (~othercmds))
+		xtables_error(PARAMETER_PROBLEM, "Cannot use -%c with -%c\n",
+			   cmd2char(newcmd), cmd2char(*cmd & (~othercmds)));
+	*cmd |= newcmd;
+}
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 64b7e8fc4b690..0b9b357c7bdaa 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -183,4 +183,8 @@ void command_match(struct iptables_command_state *cs);
 const char *xt_parse_target(const char *targetname);
 void command_jump(struct iptables_command_state *cs, const char *jumpto);
 
+char cmd2char(int option);
+void add_command(unsigned int *cmd, const int newcmd,
+		 const int othercmds, int invert);
+
 #endif /* IPTABLES_XSHARED_H */
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 8503f47fe2afe..584b6f0646821 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -81,8 +81,6 @@ typedef char arpt_chainlabel[32];
 #define CMD_CHECK		0x0800U
 #define CMD_RENAME_CHAIN	0x1000U
 #define NUMBER_OF_CMD	13
-static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
-				 'N', 'X', 'P', 'E' };
 
 #define OPTION_OFFSET 256
 
@@ -462,26 +460,6 @@ opt2char(int option)
 	return *ptr;
 }
 
-static char
-cmd2char(int option)
-{
-	const char *ptr;
-	for (ptr = cmdflags; option > 1; option >>= 1, ptr++);
-
-	return *ptr;
-}
-
-static void
-add_command(unsigned int *cmd, const int newcmd, const unsigned int othercmds, int invert)
-{
-	if (invert)
-		xtables_error(PARAMETER_PROBLEM, "unexpected ! flag");
-	if (*cmd & (~othercmds))
-		xtables_error(PARAMETER_PROBLEM, "Can't use -%c with -%c\n",
-			      cmd2char(newcmd), cmd2char(*cmd & (~othercmds)));
-	*cmd |= newcmd;
-}
-
 static int
 check_inverse(const char option[], int *invert, int *optidx, int argc)
 {
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 8a9e0edc3bea2..6dfa3f1171183 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -51,8 +51,6 @@
 #endif
 
 #define NUMBER_OF_CMD	16
-static const char cmdflags[] = { 'I', 'D', 'D', 'R', 'A', 'L', 'F', 'Z',
-				 'N', 'X', 'P', 'E', 'S', 'Z', 'C' };
 
 #define OPT_FRAGMENT	0x00800U
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
@@ -319,27 +317,6 @@ opt2char(int option)
 	return *ptr;
 }
 
-static char
-cmd2char(int option)
-{
-	const char *ptr;
-	for (ptr = cmdflags; option > 1; option >>= 1, ptr++);
-
-	return *ptr;
-}
-
-static void
-add_command(unsigned int *cmd, const int newcmd, const int othercmds,
-	    int invert)
-{
-	if (invert)
-		xtables_error(PARAMETER_PROBLEM, "unexpected ! flag");
-	if (*cmd & (~othercmds))
-		xtables_error(PARAMETER_PROBLEM, "Cannot use -%c with -%c\n",
-			   cmd2char(newcmd), cmd2char(*cmd & (~othercmds)));
-	*cmd |= newcmd;
-}
-
 /*
  *	All functions starting with "parse" should succeed, otherwise
  *	the program fails.
-- 
2.23.0


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

* [iptables PATCH v2 03/10] xshared: Share a common implementation of parse_rulenumber()
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 01/10] ip6tables, xtables-arp: Drop unused struct pprot Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 02/10] xshared: Share a common add_command() implementation Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 04/10] Merge CMD_* defines Phil Sutter
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

The function is really small, but still copied four times.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/ip6tables.c   | 13 -------------
 iptables/iptables.c    | 12 ------------
 iptables/xshared.c     | 12 ++++++++++++
 iptables/xshared.h     |  1 +
 iptables/xtables-arp.c | 13 -------------
 iptables/xtables.c     | 12 ------------
 6 files changed, 13 insertions(+), 50 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index 9a9d71f1cdadc..f4ccfc60de953 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -352,19 +352,6 @@ static int is_exthdr(uint16_t proto)
 		proto == IPPROTO_DSTOPTS);
 }
 
-/* Can't be zero. */
-static int
-parse_rulenumber(const char *rule)
-{
-	unsigned int rulenum;
-
-	if (!xtables_strtoui(rule, NULL, &rulenum, 1, INT_MAX))
-		xtables_error(PARAMETER_PROBLEM,
-			   "Invalid rule number `%s'", rule);
-
-	return rulenum;
-}
-
 static void
 parse_chain(const char *chainname)
 {
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 5fec25376c24f..df371f410a9c2 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -343,18 +343,6 @@ opt2char(int option)
 */
 
 /* Christophe Burki wants `-p 6' to imply `-m tcp'.  */
-/* Can't be zero. */
-static int
-parse_rulenumber(const char *rule)
-{
-	unsigned int rulenum;
-
-	if (!xtables_strtoui(rule, NULL, &rulenum, 1, INT_MAX))
-		xtables_error(PARAMETER_PROBLEM,
-			   "Invalid rule number `%s'", rule);
-
-	return rulenum;
-}
 
 static void
 parse_chain(const char *chainname)
diff --git a/iptables/xshared.c b/iptables/xshared.c
index 3baa805c64e6d..2a0077d9da846 100644
--- a/iptables/xshared.c
+++ b/iptables/xshared.c
@@ -759,3 +759,15 @@ void add_command(unsigned int *cmd, const int newcmd,
 			   cmd2char(newcmd), cmd2char(*cmd & (~othercmds)));
 	*cmd |= newcmd;
 }
+
+/* Can't be zero. */
+int parse_rulenumber(const char *rule)
+{
+	unsigned int rulenum;
+
+	if (!xtables_strtoui(rule, NULL, &rulenum, 1, INT_MAX))
+		xtables_error(PARAMETER_PROBLEM,
+			   "Invalid rule number `%s'", rule);
+
+	return rulenum;
+}
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 0b9b357c7bdaa..85bbfa1250aa3 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -186,5 +186,6 @@ void command_jump(struct iptables_command_state *cs, const char *jumpto);
 char cmd2char(int option);
 void add_command(unsigned int *cmd, const int newcmd,
 		 const int othercmds, int invert);
+int parse_rulenumber(const char *rule);
 
 #endif /* IPTABLES_XSHARED_H */
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 584b6f0646821..79cc83d354fc5 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -518,19 +518,6 @@ parse_interface(const char *arg, char *vianame, unsigned char *mask)
 	}
 }
 
-/* Can't be zero. */
-static int
-parse_rulenumber(const char *rule)
-{
-	unsigned int rulenum;
-
-	if (!xtables_strtoui(rule, NULL, &rulenum, 1, INT_MAX))
-		xtables_error(PARAMETER_PROBLEM,
-			      "Invalid rule number `%s'", rule);
-
-	return rulenum;
-}
-
 static void
 set_option(unsigned int *options, unsigned int option, u_int16_t *invflg,
 	   int invert)
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 6dfa3f1171183..bb76e6a7a1ce8 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -327,18 +327,6 @@ opt2char(int option)
 */
 
 /* Christophe Burki wants `-p 6' to imply `-m tcp'.  */
-/* Can't be zero. */
-static int
-parse_rulenumber(const char *rule)
-{
-	unsigned int rulenum;
-
-	if (!xtables_strtoui(rule, NULL, &rulenum, 1, INT_MAX))
-		xtables_error(PARAMETER_PROBLEM,
-			   "Invalid rule number `%s'", rule);
-
-	return rulenum;
-}
 
 static void
 set_option(unsigned int *options, unsigned int option, uint8_t *invflg,
-- 
2.23.0


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

* [iptables PATCH v2 04/10] Merge CMD_* defines
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (2 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 03/10] xshared: Share a common implementation of parse_rulenumber() Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 05/10] xtables-arp: Drop generic_opt_check() Phil Sutter
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

They are mostly identical, just xtables-arp ones differ slightly. Though
since they are internal use only and their actual value doesn't matter
(as long as it's a distinct bit), they can be merged anyway.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/ip6tables.c   | 17 -----------------
 iptables/iptables.c    | 17 -----------------
 iptables/nft-shared.h  | 17 -----------------
 iptables/xshared.h     | 20 ++++++++++++++++++++
 iptables/xtables-arp.c | 20 --------------------
 iptables/xtables.c     |  2 --
 6 files changed, 20 insertions(+), 73 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index f4ccfc60de953..e48fdeb1248bd 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -52,23 +52,6 @@
 #define FALSE 0
 #endif
 
-#define CMD_NONE		0x0000U
-#define CMD_INSERT		0x0001U
-#define CMD_DELETE		0x0002U
-#define CMD_DELETE_NUM		0x0004U
-#define CMD_REPLACE		0x0008U
-#define CMD_APPEND		0x0010U
-#define CMD_LIST		0x0020U
-#define CMD_FLUSH		0x0040U
-#define CMD_ZERO		0x0080U
-#define CMD_NEW_CHAIN		0x0100U
-#define CMD_DELETE_CHAIN	0x0200U
-#define CMD_SET_POLICY		0x0400U
-#define CMD_RENAME_CHAIN	0x0800U
-#define CMD_LIST_RULES		0x1000U
-#define CMD_ZERO_NUM		0x2000U
-#define CMD_CHECK		0x4000U
-#define NUMBER_OF_CMD	16
 
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
 static const char optflags[]
diff --git a/iptables/iptables.c b/iptables/iptables.c
index df371f410a9c2..255b61b11ec89 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -48,23 +48,6 @@
 #define FALSE 0
 #endif
 
-#define CMD_NONE		0x0000U
-#define CMD_INSERT		0x0001U
-#define CMD_DELETE		0x0002U
-#define CMD_DELETE_NUM		0x0004U
-#define CMD_REPLACE		0x0008U
-#define CMD_APPEND		0x0010U
-#define CMD_LIST		0x0020U
-#define CMD_FLUSH		0x0040U
-#define CMD_ZERO		0x0080U
-#define CMD_NEW_CHAIN		0x0100U
-#define CMD_DELETE_CHAIN	0x0200U
-#define CMD_SET_POLICY		0x0400U
-#define CMD_RENAME_CHAIN	0x0800U
-#define CMD_LIST_RULES		0x1000U
-#define CMD_ZERO_NUM		0x2000U
-#define CMD_CHECK		0x4000U
-#define NUMBER_OF_CMD	16
 
 #define OPT_FRAGMENT    0x00800U
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
diff --git a/iptables/nft-shared.h b/iptables/nft-shared.h
index 8b073b18fb0d9..e236a981119ac 100644
--- a/iptables/nft-shared.h
+++ b/iptables/nft-shared.h
@@ -199,23 +199,6 @@ struct xtables_args {
 	unsigned long long pcnt_cnt, bcnt_cnt;
 };
 
-#define CMD_NONE		0x0000U
-#define CMD_INSERT		0x0001U
-#define CMD_DELETE		0x0002U
-#define CMD_DELETE_NUM		0x0004U
-#define CMD_REPLACE		0x0008U
-#define CMD_APPEND		0x0010U
-#define CMD_LIST		0x0020U
-#define CMD_FLUSH		0x0040U
-#define CMD_ZERO		0x0080U
-#define CMD_NEW_CHAIN		0x0100U
-#define CMD_DELETE_CHAIN	0x0200U
-#define CMD_SET_POLICY		0x0400U
-#define CMD_RENAME_CHAIN	0x0800U
-#define CMD_LIST_RULES		0x1000U
-#define CMD_ZERO_NUM		0x2000U
-#define CMD_CHECK		0x4000U
-
 struct nft_xt_cmd_parse {
 	unsigned int			command;
 	unsigned int			rulenum;
diff --git a/iptables/xshared.h b/iptables/xshared.h
index 85bbfa1250aa3..b0738b042e95a 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -31,6 +31,26 @@ enum {
 	OPT_COUNTERS    = 1 << 10,
 };
 
+enum {
+	CMD_NONE		= 0,
+	CMD_INSERT		= 1 << 0,
+	CMD_DELETE		= 1 << 1,
+	CMD_DELETE_NUM		= 1 << 2,
+	CMD_REPLACE		= 1 << 3,
+	CMD_APPEND		= 1 << 4,
+	CMD_LIST		= 1 << 5,
+	CMD_FLUSH		= 1 << 6,
+	CMD_ZERO		= 1 << 7,
+	CMD_NEW_CHAIN		= 1 << 8,
+	CMD_DELETE_CHAIN	= 1 << 9,
+	CMD_SET_POLICY		= 1 << 10,
+	CMD_RENAME_CHAIN	= 1 << 11,
+	CMD_LIST_RULES		= 1 << 12,
+	CMD_ZERO_NUM		= 1 << 13,
+	CMD_CHECK		= 1 << 14,
+};
+#define NUMBER_OF_CMD		16
+
 struct xtables_globals;
 struct xtables_rule_match;
 struct xtables_target;
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 79cc83d354fc5..88a7d534da4f1 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -62,26 +62,6 @@ typedef char arpt_chainlabel[32];
 #define FALSE 0
 #endif
 
-/* XXX: command defined by nft-shared.h do not overlap with these two */
-#undef CMD_CHECK
-#undef CMD_RENAME_CHAIN
-
-#define CMD_NONE		0x0000U
-#define CMD_INSERT		0x0001U
-#define CMD_DELETE		0x0002U
-#define CMD_DELETE_NUM		0x0004U
-#define CMD_REPLACE		0x0008U
-#define CMD_APPEND		0x0010U
-#define CMD_LIST		0x0020U
-#define CMD_FLUSH		0x0040U
-#define CMD_ZERO		0x0080U
-#define CMD_NEW_CHAIN		0x0100U
-#define CMD_DELETE_CHAIN	0x0200U
-#define CMD_SET_POLICY		0x0400U
-#define CMD_CHECK		0x0800U
-#define CMD_RENAME_CHAIN	0x1000U
-#define NUMBER_OF_CMD	13
-
 #define OPTION_OFFSET 256
 
 #define OPT_NONE	0x00000U
diff --git a/iptables/xtables.c b/iptables/xtables.c
index bb76e6a7a1ce8..805bd801fb060 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -50,8 +50,6 @@
 #define FALSE 0
 #endif
 
-#define NUMBER_OF_CMD	16
-
 #define OPT_FRAGMENT	0x00800U
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
 static const char optflags[]
-- 
2.23.0


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

* [iptables PATCH v2 05/10] xtables-arp: Drop generic_opt_check()
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (3 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 04/10] Merge CMD_* defines Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 06/10] Replace TRUE/FALSE with true/false Phil Sutter
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

With all fields in commands_v_options[][] being whitespace, the function
is effectively a noop.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-arp.c | 66 ------------------------------------------
 1 file changed, 66 deletions(-)

diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 88a7d534da4f1..ae69baf2a9346 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -139,34 +139,6 @@ struct xtables_globals arptables_globals = {
 	.compat_rev		= nft_compatible_revision,
 };
 
-/* Table of legal combinations of commands and options.  If any of the
- * given commands make an option legal, that option is legal (applies to
- * CMD_LIST and CMD_ZERO only).
- * Key:
- *  +  compulsory
- *  x  illegal
- *     optional
- */
-
-static char commands_v_options[NUMBER_OF_CMD][NUMBER_OF_OPT] =
-/* Well, it's better than "Re: Linux vs FreeBSD" */
-{
-	/*     -n  -s  -d  -p  -j  -v  -x  -i  -o  -f  --line */
-/*INSERT*/    {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*DELETE*/    {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*DELETE_NUM*/{' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*REPLACE*/   {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*APPEND*/    {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*LIST*/      {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*FLUSH*/     {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*ZERO*/      {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*NEW_CHAIN*/ {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*DEL_CHAIN*/ {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*SET_POLICY*/{' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*CHECK*/     {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '},
-/*RENAME*/    {' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' ',' '}
-};
-
 static int inverse_for_options[NUMBER_OF_OPT] =
 {
 /* -n */ 0,
@@ -395,42 +367,6 @@ exit_printhelp(void)
 	exit(0);
 }
 
-static void
-generic_opt_check(int command, int options)
-{
-	int i, j, legal = 0;
-
-	/* Check that commands are valid with options.  Complicated by the
-	 * fact that if an option is legal with *any* command given, it is
-	 * legal overall (ie. -z and -l).
-	 */
-	for (i = 0; i < NUMBER_OF_OPT; i++) {
-		legal = 0; /* -1 => illegal, 1 => legal, 0 => undecided. */
-
-		for (j = 0; j < NUMBER_OF_CMD; j++) {
-			if (!(command & (1<<j)))
-				continue;
-
-			if (!(options & (1<<i))) {
-				if (commands_v_options[j][i] == '+')
-					xtables_error(PARAMETER_PROBLEM,
-						      "You need to supply the `-%c' "
-						      "option for this command\n",
-						      optflags[i]);
-			} else {
-				if (commands_v_options[j][i] != 'x')
-					legal = 1;
-				else if (legal == 0)
-					legal = -1;
-			}
-		}
-		if (legal == -1)
-			xtables_error(PARAMETER_PROBLEM,
-				      "Illegal option `-%c' with this command\n",
-				      optflags[i]);
-	}
-}
-
 static char
 opt2char(int option)
 {
@@ -1059,8 +995,6 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 		xtables_error(PARAMETER_PROBLEM, "Replacement rule does not "
 						 "specify a unique address");
 
-	generic_opt_check(command, options);
-
 	if (chain && strlen(chain) > ARPT_FUNCTION_MAXNAMELEN)
 		xtables_error(PARAMETER_PROBLEM,
 				"chain name `%s' too long (must be under %i chars)",
-- 
2.23.0


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

* [iptables PATCH v2 06/10] Replace TRUE/FALSE with true/false
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (4 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 05/10] xtables-arp: Drop generic_opt_check() Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 07/10] xtables-arp: Integrate OPT_* defines into xshared.h Phil Sutter
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

And drop the conditional defines.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/ip6tables.c   | 14 +++-----------
 iptables/iptables.c    | 12 ++----------
 iptables/xtables-arp.c | 17 +++++------------
 iptables/xtables.c     | 11 ++---------
 4 files changed, 12 insertions(+), 42 deletions(-)

diff --git a/iptables/ip6tables.c b/iptables/ip6tables.c
index e48fdeb1248bd..576c2cf8b0d9f 100644
--- a/iptables/ip6tables.c
+++ b/iptables/ip6tables.c
@@ -45,14 +45,6 @@
 #include "ip6tables-multi.h"
 #include "xshared.h"
 
-#ifndef TRUE
-#define TRUE 1
-#endif
-#ifndef FALSE
-#define FALSE 0
-#endif
-
-
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
 static const char optflags[]
 = { 'n', 's', 'd', 'p', 'j', 'v', 'x', 'i', 'o', '0', 'c'};
@@ -1525,7 +1517,7 @@ int do_command6(int argc, char *argv[], char **table,
 					xtables_error(PARAMETER_PROBLEM,
 						   "multiple consecutive ! not"
 						   " allowed");
-				cs.invert = TRUE;
+				cs.invert = true;
 				optarg[0] = '\0';
 				continue;
 			}
@@ -1537,12 +1529,12 @@ int do_command6(int argc, char *argv[], char **table,
 				/*
 				 * If new options were loaded, we must retry
 				 * getopt immediately and not allow
-				 * cs.invert=FALSE to be executed.
+				 * cs.invert=false to be executed.
 				 */
 				continue;
 			break;
 		}
-		cs.invert = FALSE;
+		cs.invert = false;
 	}
 
 	if (!wait && wait_interval_set)
diff --git a/iptables/iptables.c b/iptables/iptables.c
index 255b61b11ec89..88ef6cf666d4b 100644
--- a/iptables/iptables.c
+++ b/iptables/iptables.c
@@ -41,14 +41,6 @@
 #include <fcntl.h>
 #include "xshared.h"
 
-#ifndef TRUE
-#define TRUE 1
-#endif
-#ifndef FALSE
-#define FALSE 0
-#endif
-
-
 #define OPT_FRAGMENT    0x00800U
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
 static const char optflags[]
@@ -1518,7 +1510,7 @@ int do_command4(int argc, char *argv[], char **table,
 					xtables_error(PARAMETER_PROBLEM,
 						   "multiple consecutive ! not"
 						   " allowed");
-				cs.invert = TRUE;
+				cs.invert = true;
 				optarg[0] = '\0';
 				continue;
 			}
@@ -1531,7 +1523,7 @@ int do_command4(int argc, char *argv[], char **table,
 				continue;
 			break;
 		}
-		cs.invert = FALSE;
+		cs.invert = false;
 	}
 
 	if (!wait && wait_interval_set)
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index ae69baf2a9346..4949ddd3d486c 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -55,13 +55,6 @@
 
 typedef char arpt_chainlabel[32];
 
-#ifndef TRUE
-#define TRUE 1
-#endif
-#ifndef FALSE
-#define FALSE 0
-#endif
-
 #define OPTION_OFFSET 256
 
 #define OPT_NONE	0x00000U
@@ -383,7 +376,7 @@ check_inverse(const char option[], int *invert, int *optidx, int argc)
 		if (*invert)
 			xtables_error(PARAMETER_PROBLEM,
 				      "Multiple `!' flags not allowed");
-		*invert = TRUE;
+		*invert = true;
 		if (optidx) {
 			*optidx = *optidx+1;
 			if (argc && *optidx > argc)
@@ -391,9 +384,9 @@ check_inverse(const char option[], int *invert, int *optidx, int argc)
 					      "no argument following `!'");
 		}
 
-		return TRUE;
+		return true;
 	}
-	return FALSE;
+	return false;
 }
 
 static void
@@ -942,7 +935,7 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 					xtables_error(PARAMETER_PROBLEM,
 						      "multiple consecutive ! not"
 						      " allowed");
-				invert = TRUE;
+				invert = true;
 				optarg[0] = '\0';
 				continue;
 			}
@@ -956,7 +949,7 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 			}
 			break;
 		}
-		invert = FALSE;
+		invert = false;
 	}
 
 	if (cs.target)
diff --git a/iptables/xtables.c b/iptables/xtables.c
index 805bd801fb060..8f9dc628d0029 100644
--- a/iptables/xtables.c
+++ b/iptables/xtables.c
@@ -43,13 +43,6 @@
 #include "nft-shared.h"
 #include "nft.h"
 
-#ifndef TRUE
-#define TRUE 1
-#endif
-#ifndef FALSE
-#define FALSE 0
-#endif
-
 #define OPT_FRAGMENT	0x00800U
 #define NUMBER_OF_OPT	ARRAY_SIZE(optflags)
 static const char optflags[]
@@ -952,7 +945,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
 					xtables_error(PARAMETER_PROBLEM,
 						   "multiple consecutive ! not"
 						   " allowed");
-				cs->invert = TRUE;
+				cs->invert = true;
 				optarg[0] = '\0';
 				continue;
 			}
@@ -965,7 +958,7 @@ void do_parse(struct nft_handle *h, int argc, char *argv[],
 				continue;
 			break;
 		}
-		cs->invert = FALSE;
+		cs->invert = false;
 	}
 
 	if (strcmp(p->table, "nat") == 0 &&
-- 
2.23.0


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

* [iptables PATCH v2 07/10] xtables-arp: Integrate OPT_* defines into xshared.h
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (5 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 06/10] Replace TRUE/FALSE with true/false Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 08/10] xtables-arp: Drop some unused variables Phil Sutter
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

These defines are internal use only, so their actual value doesn't
matter as long as they're unique and inverse_for_options array items
match:

When negating a given option, the corresponding OPT_* value's bit is
used as an index into inverse_for_options to retrieve the corresponding
invflag. If zero, either negating or the option itself is not supported.
(In practice, a lookup for unsupported option won't happen as those are
caught by getopt_long()).

Since xtables-arp's OPT_* values change, adjust the local
inverse_for_options array accordingly.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
Changes since v1:
- Adjust inverse_for_options array size and content to changed OPT_*
  values.
- Add a comment to inverse_for_options array highlighting the
  connection.
- Extend commit message to elaborate on the necessary adjustment.
---
 iptables/xshared.h     |  7 +++++++
 iptables/xtables-arp.c | 43 ++++++++++++++----------------------------
 2 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/iptables/xshared.h b/iptables/xshared.h
index b0738b042e95a..490b19ade5106 100644
--- a/iptables/xshared.h
+++ b/iptables/xshared.h
@@ -29,6 +29,13 @@ enum {
 	OPT_VIANAMEOUT  = 1 << 8,
 	OPT_LINENUMBERS = 1 << 9,
 	OPT_COUNTERS    = 1 << 10,
+	/* below are for arptables only */
+	OPT_S_MAC	= 1 << 11,
+	OPT_D_MAC	= 1 << 12,
+	OPT_H_LENGTH	= 1 << 13,
+	OPT_OPCODE	= 1 << 14,
+	OPT_H_TYPE	= 1 << 15,
+	OPT_P_TYPE	= 1 << 16,
 };
 
 enum {
diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 4949ddd3d486c..8339b2cb6f38c 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -57,23 +57,6 @@ typedef char arpt_chainlabel[32];
 
 #define OPTION_OFFSET 256
 
-#define OPT_NONE	0x00000U
-#define OPT_NUMERIC	0x00001U
-#define OPT_S_IP	0x00002U
-#define OPT_D_IP	0x00004U
-#define OPT_S_MAC	0x00008U
-#define OPT_D_MAC	0x00010U
-#define OPT_H_LENGTH	0x00020U
-#define OPT_P_LENGTH	0x00040U
-#define OPT_OPCODE	0x00080U
-#define OPT_H_TYPE	0x00100U
-#define OPT_P_TYPE	0x00200U
-#define OPT_JUMP	0x00400U
-#define OPT_VERBOSE	0x00800U
-#define OPT_VIANAMEIN	0x01000U
-#define OPT_VIANAMEOUT	0x02000U
-#define OPT_LINENUMBERS 0x04000U
-#define OPT_COUNTERS	0x08000U
 #define NUMBER_OF_OPT	16
 static const char optflags[NUMBER_OF_OPT]
 = { 'n', 's', 'd', 2, 3, 7, 8, 4, 5, 6, 'j', 'v', 'i', 'o', '0', 'c'};
@@ -132,24 +115,26 @@ struct xtables_globals arptables_globals = {
 	.compat_rev		= nft_compatible_revision,
 };
 
-static int inverse_for_options[NUMBER_OF_OPT] =
+/* index relates to bit of each OPT_* value */
+static int inverse_for_options[] =
 {
 /* -n */ 0,
 /* -s */ ARPT_INV_SRCIP,
 /* -d */ ARPT_INV_TGTIP,
-/* 2 */ ARPT_INV_SRCDEVADDR,
-/* 3 */ ARPT_INV_TGTDEVADDR,
-/* -l */ ARPT_INV_ARPHLN,
-/* 8 */ 0,
-/* 4 */ ARPT_INV_ARPOP,
-/* 5 */ ARPT_INV_ARPHRD,
-/* 6 */ ARPT_INV_ARPPRO,
+/* -p */ 0,
 /* -j */ 0,
 /* -v */ 0,
+/* -x */ 0,
 /* -i */ ARPT_INV_VIA_IN,
 /* -o */ ARPT_INV_VIA_OUT,
 /*--line*/ 0,
 /* -c */ 0,
+/* 2 */ ARPT_INV_SRCDEVADDR,
+/* 3 */ ARPT_INV_TGTDEVADDR,
+/* -l */ ARPT_INV_ARPHLN,
+/* 4 */ ARPT_INV_ARPOP,
+/* 5 */ ARPT_INV_ARPHRD,
+/* 6 */ ARPT_INV_ARPPRO,
 };
 
 /* Primitive headers... */
@@ -747,14 +732,14 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 			break;
 		case 's':
 			check_inverse(optarg, &invert, &optind, argc);
-			set_option(&options, OPT_S_IP, &cs.arp.arp.invflags,
+			set_option(&options, OPT_SOURCE, &cs.arp.arp.invflags,
 				   invert);
 			shostnetworkmask = argv[optind-1];
 			break;
 
 		case 'd':
 			check_inverse(optarg, &invert, &optind, argc);
-			set_option(&options, OPT_D_IP, &cs.arp.arp.invflags,
+			set_option(&options, OPT_DESTINATION, &cs.arp.arp.invflags,
 				   invert);
 			dhostnetworkmask = argv[optind-1];
 			break;
@@ -965,9 +950,9 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 			      "nothing appropriate following !");
 
 	if (command & (CMD_REPLACE | CMD_INSERT | CMD_DELETE | CMD_APPEND)) {
-		if (!(options & OPT_D_IP))
+		if (!(options & OPT_DESTINATION))
 			dhostnetworkmask = "0.0.0.0/0";
-		if (!(options & OPT_S_IP))
+		if (!(options & OPT_SOURCE))
 			shostnetworkmask = "0.0.0.0/0";
 	}
 
-- 
2.23.0


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

* [iptables PATCH v2 08/10] xtables-arp: Drop some unused variables
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (6 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 07/10] xtables-arp: Integrate OPT_* defines into xshared.h Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 09/10] xtables-arp: Use xtables_parse_interface() Phil Sutter
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-arp.c | 17 -----------------
 1 file changed, 17 deletions(-)

diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index 8339b2cb6f38c..bbc1ceb6e8e38 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -53,10 +53,6 @@
 #include "nft-arp.h"
 #include <linux/netfilter_arp/arp_tables.h>
 
-typedef char arpt_chainlabel[32];
-
-#define OPTION_OFFSET 256
-
 #define NUMBER_OF_OPT	16
 static const char optflags[NUMBER_OF_OPT]
 = { 'n', 's', 'd', 2, 3, 7, 8, 4, 5, 6, 'j', 'v', 'i', 'o', '0', 'c'};
@@ -102,8 +98,6 @@ static struct option original_opts[] = {
 	{ 0 }
 };
 
-int RUNTIME_NF_ARP_NUMHOOKS = 3;
-
 #define opts xt_params->opts
 
 extern void xtables_exit_error(enum xtables_exittype status, const char *msg, ...) __attribute__((noreturn, format(printf,2,3)));
@@ -137,17 +131,6 @@ static int inverse_for_options[] =
 /* 6 */ ARPT_INV_ARPPRO,
 };
 
-/* Primitive headers... */
-/* defined in netinet/in.h */
-#if 0
-#ifndef IPPROTO_ESP
-#define IPPROTO_ESP 50
-#endif
-#ifndef IPPROTO_AH
-#define IPPROTO_AH 51
-#endif
-#endif
-
 /***********************************************/
 /* ARPTABLES SPECIFIC NEW FUNCTIONS ADDED HERE */
 /***********************************************/
-- 
2.23.0


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

* [iptables PATCH v2 09/10] xtables-arp: Use xtables_parse_interface()
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (7 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 08/10] xtables-arp: Drop some unused variables Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-28 15:48 ` [iptables PATCH v2 10/10] nft-arp: Use xtables_print_mac_and_mask() Phil Sutter
  2019-10-30  8:24 ` [iptables PATCH v2 00/10] Reduce code size around arptables-nft Pablo Neira Ayuso
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

The local implementation differs just slightly but libxtables version
seems more correct (no needless memsetting of mask, more relevant
illegal character checking) so use that one.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/xtables-arp.c | 50 +++++-------------------------------------
 1 file changed, 6 insertions(+), 44 deletions(-)

diff --git a/iptables/xtables-arp.c b/iptables/xtables-arp.c
index bbc1ceb6e8e38..9cfad76263d32 100644
--- a/iptables/xtables-arp.c
+++ b/iptables/xtables-arp.c
@@ -357,44 +357,6 @@ check_inverse(const char option[], int *invert, int *optidx, int argc)
 	return false;
 }
 
-static void
-parse_interface(const char *arg, char *vianame, unsigned char *mask)
-{
-	int vialen = strlen(arg);
-	unsigned int i;
-
-	memset(mask, 0, IFNAMSIZ);
-	memset(vianame, 0, IFNAMSIZ);
-
-	if (vialen + 1 > IFNAMSIZ)
-		xtables_error(PARAMETER_PROBLEM,
-			      "interface name `%s' must be shorter than IFNAMSIZ"
-			      " (%i)", arg, IFNAMSIZ-1);
-
-	strcpy(vianame, arg);
-	if (vialen == 0)
-		memset(mask, 0, IFNAMSIZ);
-	else if (vianame[vialen - 1] == '+') {
-		memset(mask, 0xFF, vialen - 1);
-		memset(mask + vialen - 1, 0, IFNAMSIZ - vialen + 1);
-		/* Don't remove `+' here! -HW */
-	} else {
-		/* Include nul-terminator in match */
-		memset(mask, 0xFF, vialen + 1);
-		memset(mask + vialen + 1, 0, IFNAMSIZ - vialen - 1);
-		for (i = 0; vianame[i]; i++) {
-			if (!isalnum(vianame[i])
-			    && vianame[i] != '_'
-			    && vianame[i] != '.') {
-				printf("Warning: weird character in interface"
-				       " `%s' (No aliases, :, ! or *).\n",
-				       vianame);
-				break;
-			}
-		}
-	}
-}
-
 static void
 set_option(unsigned int *options, unsigned int option, u_int16_t *invflg,
 	   int invert)
@@ -816,18 +778,18 @@ int do_commandarp(struct nft_handle *h, int argc, char *argv[], char **table,
 			check_inverse(optarg, &invert, &optind, argc);
 			set_option(&options, OPT_VIANAMEIN, &cs.arp.arp.invflags,
 				   invert);
-			parse_interface(argv[optind-1],
-					cs.arp.arp.iniface,
-					cs.arp.arp.iniface_mask);
+			xtables_parse_interface(argv[optind-1],
+						cs.arp.arp.iniface,
+						cs.arp.arp.iniface_mask);
 			break;
 
 		case 'o':
 			check_inverse(optarg, &invert, &optind, argc);
 			set_option(&options, OPT_VIANAMEOUT, &cs.arp.arp.invflags,
 				   invert);
-			parse_interface(argv[optind-1],
-					cs.arp.arp.outiface,
-					cs.arp.arp.outiface_mask);
+			xtables_parse_interface(argv[optind-1],
+						cs.arp.arp.outiface,
+						cs.arp.arp.outiface_mask);
 			break;
 
 		case 'v':
-- 
2.23.0


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

* [iptables PATCH v2 10/10] nft-arp: Use xtables_print_mac_and_mask()
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (8 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 09/10] xtables-arp: Use xtables_parse_interface() Phil Sutter
@ 2019-10-28 15:48 ` Phil Sutter
  2019-10-30  8:24 ` [iptables PATCH v2 00/10] Reduce code size around arptables-nft Pablo Neira Ayuso
  10 siblings, 0 replies; 12+ messages in thread
From: Phil Sutter @ 2019-10-28 15:48 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel

This libxtables function does exactly what the local implementation did.
The only noteworthy difference is that it assumes MAC/mask lengths, but
the local implementation was passed ETH_ALEN in each invocation, so no
practical difference.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 iptables/nft-arp.c | 31 ++++---------------------------
 1 file changed, 4 insertions(+), 27 deletions(-)

diff --git a/iptables/nft-arp.c b/iptables/nft-arp.c
index 9805bbe0de87b..7068f82c5495a 100644
--- a/iptables/nft-arp.c
+++ b/iptables/nft-arp.c
@@ -114,29 +114,6 @@ mask_to_dotted(const struct in_addr *mask)
 	return buf;
 }
 
-static void print_mac(const unsigned char *mac, int l)
-{
-	int j;
-
-	for (j = 0; j < l; j++)
-		printf("%02x%s", mac[j],
-			(j==l-1) ? "" : ":");
-}
-
-static void print_mac_and_mask(const unsigned char *mac, const unsigned char *mask, int l)
-{
-	int i;
-
-	print_mac(mac, l);
-	for (i = 0; i < l ; i++)
-		if (mask[i] != 255)
-			break;
-	if (i == l)
-		return;
-	printf("/");
-	print_mac(mask, l);
-}
-
 static bool need_devaddr(struct arpt_devaddr_info *info)
 {
 	int i;
@@ -506,8 +483,8 @@ static void nft_arp_print_rule_details(const struct iptables_command_state *cs,
 	printf("%s%s", sep, fw->arp.invflags & ARPT_INV_SRCDEVADDR
 		? "! " : "");
 	printf("--src-mac ");
-	print_mac_and_mask((unsigned char *)fw->arp.src_devaddr.addr,
-		(unsigned char *)fw->arp.src_devaddr.mask, ETH_ALEN);
+	xtables_print_mac_and_mask((unsigned char *)fw->arp.src_devaddr.addr,
+				   (unsigned char *)fw->arp.src_devaddr.mask);
 	sep = " ";
 after_devsrc:
 
@@ -532,8 +509,8 @@ after_devsrc:
 	printf("%s%s", sep, fw->arp.invflags & ARPT_INV_TGTDEVADDR
 		? "! " : "");
 	printf("--dst-mac ");
-	print_mac_and_mask((unsigned char *)fw->arp.tgt_devaddr.addr,
-		(unsigned char *)fw->arp.tgt_devaddr.mask, ETH_ALEN);
+	xtables_print_mac_and_mask((unsigned char *)fw->arp.tgt_devaddr.addr,
+				   (unsigned char *)fw->arp.tgt_devaddr.mask);
 	sep = " ";
 
 after_devdst:
-- 
2.23.0


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

* Re: [iptables PATCH v2 00/10] Reduce code size around arptables-nft
  2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
                   ` (9 preceding siblings ...)
  2019-10-28 15:48 ` [iptables PATCH v2 10/10] nft-arp: Use xtables_print_mac_and_mask() Phil Sutter
@ 2019-10-30  8:24 ` Pablo Neira Ayuso
  10 siblings, 0 replies; 12+ messages in thread
From: Pablo Neira Ayuso @ 2019-10-30  8:24 UTC (permalink / raw)
  To: Phil Sutter; +Cc: netfilter-devel

On Mon, Oct 28, 2019 at 04:48:08PM +0100, Phil Sutter wrote:
> A review of xtables-arp.c exposed a significant amount of dead, needless
> or duplicated code. This series deals with some low hanging fruits. Most
> of the changes affect xtables-arp.c and nft-arp.c only, but where common
> issues existed or code was to be shared, other files are touched as
> well.

Acked-by: Pablo Neira Ayuso <pablo@netfilter.org>

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

end of thread, other threads:[~2019-10-30  8:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28 15:48 [iptables PATCH v2 00/10] Reduce code size around arptables-nft Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 01/10] ip6tables, xtables-arp: Drop unused struct pprot Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 02/10] xshared: Share a common add_command() implementation Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 03/10] xshared: Share a common implementation of parse_rulenumber() Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 04/10] Merge CMD_* defines Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 05/10] xtables-arp: Drop generic_opt_check() Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 06/10] Replace TRUE/FALSE with true/false Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 07/10] xtables-arp: Integrate OPT_* defines into xshared.h Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 08/10] xtables-arp: Drop some unused variables Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 09/10] xtables-arp: Use xtables_parse_interface() Phil Sutter
2019-10-28 15:48 ` [iptables PATCH v2 10/10] nft-arp: Use xtables_print_mac_and_mask() Phil Sutter
2019-10-30  8:24 ` [iptables PATCH v2 00/10] Reduce code size around arptables-nft Pablo Neira Ayuso

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.