diff mbox

net: reduce the lines of code

Message ID 1290132284-12328-1-git-send-email-xiaosuo@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show

Commit Message

Changli Gao Nov. 19, 2010, 2:04 a.m. UTC 
Use macros to reduce the regularity lines.

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 net/core/filter.c     |   96 +++-----------------------------------------------
 net/core/filter_def.h |   55 ++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 90 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Hagen Paul Pfeifer Nov. 19, 2010, 6:35 a.m. UTC | #1 
On Fri, 19 Nov 2010 10:04:44 +0800, Changli Gao <xiaosuo@gmail.com> wrote:
> Use macros to reduce the regularity lines.

This is complete awkward and does not fix anything - on the contrary it
makes it harder to understand the code and no advantage is achieved.

Hagen
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Changli Gao Nov. 19, 2010, 7:17 a.m. UTC | #2 
On Fri, Nov 19, 2010 at 2:35 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
>
> On Fri, 19 Nov 2010 10:04:44 +0800, Changli Gao <xiaosuo@gmail.com> wrote:
>> Use macros to reduce the regularity lines.
>
> This is complete awkward and does not fix anything - on the contrary it
> makes it harder to understand the code and no advantage is achieved.
>

Although it doesn't fix anything, It can simplify the adding of new
BPF instructions, one place in filter_def.h instead of two in
filter.c. I think if some code can be generated automatically, we'd
better not write it manually, as the chance of error is less when the
code is generated automatically.
Eric Dumazet Nov. 19, 2010, 7:51 a.m. UTC | #3 
Le vendredi 19 novembre 2010 à 15:17 +0800, Changli Gao a écrit : 
> On Fri, Nov 19, 2010 at 2:35 PM, Hagen Paul Pfeifer <hagen@jauu.net> wrote:
> >
> > On Fri, 19 Nov 2010 10:04:44 +0800, Changli Gao <xiaosuo@gmail.com> wrote:
> >> Use macros to reduce the regularity lines.
> >
> > This is complete awkward and does not fix anything - on the contrary it
> > makes it harder to understand the code and no advantage is achieved.
> >
> 
> Although it doesn't fix anything, It can simplify the adding of new
> BPF instructions, one place in filter_def.h instead of two in
> filter.c. I think if some code can be generated automatically, we'd
> better not write it manually, as the chance of error is less when the
> code is generated automatically.
> 

Idea is good, but the way you did it is not.

I have two patches in testing. I'll post them today.

One to remove all the "+ 1" we do in codes[] init

One at check time to replace the divide by a constant instruction
(DIV_K) by a reciprocal one (a multiply at exec time)



--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/net/core/filter.c b/net/core/filter.c
index 15a545d..2a8841d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -39,51 +39,9 @@ 
 #include <linux/filter.h>
 
 enum {
-	BPF_S_RET_K = 0,
-	BPF_S_RET_A,
-	BPF_S_ALU_ADD_K,
-	BPF_S_ALU_ADD_X,
-	BPF_S_ALU_SUB_K,
-	BPF_S_ALU_SUB_X,
-	BPF_S_ALU_MUL_K,
-	BPF_S_ALU_MUL_X,
-	BPF_S_ALU_DIV_X,
-	BPF_S_ALU_AND_K,
-	BPF_S_ALU_AND_X,
-	BPF_S_ALU_OR_K,
-	BPF_S_ALU_OR_X,
-	BPF_S_ALU_LSH_K,
-	BPF_S_ALU_LSH_X,
-	BPF_S_ALU_RSH_K,
-	BPF_S_ALU_RSH_X,
-	BPF_S_ALU_NEG,
-	BPF_S_LD_W_ABS,
-	BPF_S_LD_H_ABS,
-	BPF_S_LD_B_ABS,
-	BPF_S_LD_W_LEN,
-	BPF_S_LD_W_IND,
-	BPF_S_LD_H_IND,
-	BPF_S_LD_B_IND,
-	BPF_S_LD_IMM,
-	BPF_S_LDX_W_LEN,
-	BPF_S_LDX_B_MSH,
-	BPF_S_LDX_IMM,
-	BPF_S_MISC_TAX,
-	BPF_S_MISC_TXA,
-	BPF_S_ALU_DIV_K,
-	BPF_S_LD_MEM,
-	BPF_S_LDX_MEM,
-	BPF_S_ST,
-	BPF_S_STX,
-	BPF_S_JMP_JA,
-	BPF_S_JMP_JEQ_K,
-	BPF_S_JMP_JEQ_X,
-	BPF_S_JMP_JGE_K,
-	BPF_S_JMP_JGE_X,
-	BPF_S_JMP_JGT_K,
-	BPF_S_JMP_JGT_X,
-	BPF_S_JMP_JSET_K,
-	BPF_S_JMP_JSET_X,
+#define BPF_DEFINE(user, kernel) kernel
+#include "filter_def.h"
+#undef BPF_DEFINE
 };
 
 /* No hurry in this branch */
@@ -436,51 +394,9 @@  int sk_chk_filter(struct sock_filter *filter, int flen)
 	 * Invalid instructions are initialized to 0.
 	 */
 	static const u8 codes[] = {
-		[BPF_ALU|BPF_ADD|BPF_K]  = BPF_S_ALU_ADD_K + 1,
-		[BPF_ALU|BPF_ADD|BPF_X]  = BPF_S_ALU_ADD_X + 1,
-		[BPF_ALU|BPF_SUB|BPF_K]  = BPF_S_ALU_SUB_K + 1,
-		[BPF_ALU|BPF_SUB|BPF_X]  = BPF_S_ALU_SUB_X + 1,
-		[BPF_ALU|BPF_MUL|BPF_K]  = BPF_S_ALU_MUL_K + 1,
-		[BPF_ALU|BPF_MUL|BPF_X]  = BPF_S_ALU_MUL_X + 1,
-		[BPF_ALU|BPF_DIV|BPF_X]  = BPF_S_ALU_DIV_X + 1,
-		[BPF_ALU|BPF_AND|BPF_K]  = BPF_S_ALU_AND_K + 1,
-		[BPF_ALU|BPF_AND|BPF_X]  = BPF_S_ALU_AND_X + 1,
-		[BPF_ALU|BPF_OR|BPF_K]   = BPF_S_ALU_OR_K + 1,
-		[BPF_ALU|BPF_OR|BPF_X]   = BPF_S_ALU_OR_X + 1,
-		[BPF_ALU|BPF_LSH|BPF_K]  = BPF_S_ALU_LSH_K + 1,
-		[BPF_ALU|BPF_LSH|BPF_X]  = BPF_S_ALU_LSH_X + 1,
-		[BPF_ALU|BPF_RSH|BPF_K]  = BPF_S_ALU_RSH_K + 1,
-		[BPF_ALU|BPF_RSH|BPF_X]  = BPF_S_ALU_RSH_X + 1,
-		[BPF_ALU|BPF_NEG]        = BPF_S_ALU_NEG + 1,
-		[BPF_LD|BPF_W|BPF_ABS]   = BPF_S_LD_W_ABS + 1,
-		[BPF_LD|BPF_H|BPF_ABS]   = BPF_S_LD_H_ABS + 1,
-		[BPF_LD|BPF_B|BPF_ABS]   = BPF_S_LD_B_ABS + 1,
-		[BPF_LD|BPF_W|BPF_LEN]   = BPF_S_LD_W_LEN + 1,
-		[BPF_LD|BPF_W|BPF_IND]   = BPF_S_LD_W_IND + 1,
-		[BPF_LD|BPF_H|BPF_IND]   = BPF_S_LD_H_IND + 1,
-		[BPF_LD|BPF_B|BPF_IND]   = BPF_S_LD_B_IND + 1,
-		[BPF_LD|BPF_IMM]         = BPF_S_LD_IMM + 1,
-		[BPF_LDX|BPF_W|BPF_LEN]  = BPF_S_LDX_W_LEN + 1,
-		[BPF_LDX|BPF_B|BPF_MSH]  = BPF_S_LDX_B_MSH + 1,
-		[BPF_LDX|BPF_IMM]        = BPF_S_LDX_IMM + 1,
-		[BPF_MISC|BPF_TAX]       = BPF_S_MISC_TAX + 1,
-		[BPF_MISC|BPF_TXA]       = BPF_S_MISC_TXA + 1,
-		[BPF_RET|BPF_K]          = BPF_S_RET_K + 1,
-		[BPF_RET|BPF_A]          = BPF_S_RET_A + 1,
-		[BPF_ALU|BPF_DIV|BPF_K]  = BPF_S_ALU_DIV_K + 1,
-		[BPF_LD|BPF_MEM]         = BPF_S_LD_MEM + 1,
-		[BPF_LDX|BPF_MEM]        = BPF_S_LDX_MEM + 1,
-		[BPF_ST]                 = BPF_S_ST + 1,
-		[BPF_STX]                = BPF_S_STX + 1,
-		[BPF_JMP|BPF_JA]         = BPF_S_JMP_JA + 1,
-		[BPF_JMP|BPF_JEQ|BPF_K]  = BPF_S_JMP_JEQ_K + 1,
-		[BPF_JMP|BPF_JEQ|BPF_X]  = BPF_S_JMP_JEQ_X + 1,
-		[BPF_JMP|BPF_JGE|BPF_K]  = BPF_S_JMP_JGE_K + 1,
-		[BPF_JMP|BPF_JGE|BPF_X]  = BPF_S_JMP_JGE_X + 1,
-		[BPF_JMP|BPF_JGT|BPF_K]  = BPF_S_JMP_JGT_K + 1,
-		[BPF_JMP|BPF_JGT|BPF_X]  = BPF_S_JMP_JGT_X + 1,
-		[BPF_JMP|BPF_JSET|BPF_K] = BPF_S_JMP_JSET_K + 1,
-		[BPF_JMP|BPF_JSET|BPF_X] = BPF_S_JMP_JSET_X + 1,
+#define BPF_DEFINE(user, kernel) [user] = kernel + 1
+#include "filter_def.h"
+#undef BPF_DEFINE
 	};
 	int pc;
 
diff --git a/net/core/filter_def.h b/net/core/filter_def.h
new file mode 100644
index 0000000..51bd662
--- /dev/null
+++ b/net/core/filter_def.h
@@ -0,0 +1,55 @@ 
+#define BPF_A1(arg1) BPF_DEFINE(BPF_##arg1, BPF_S_##arg1)
+#define BPF_A2(arg1, arg2) \
+	BPF_DEFINE(BPF_##arg1|BPF_##arg2, BPF_S_##arg1##_##arg2)
+#define BPF_A3(arg1, arg2, arg3) \
+	BPF_DEFINE(BPF_##arg1|BPF_##arg2|BPF_##arg3, \
+		   BPF_S_##arg1##_##arg2##_##arg3)
+
+BPF_A2(RET, K),
+BPF_A2(RET, A),
+BPF_A3(ALU, ADD, K),
+BPF_A3(ALU, ADD, X),
+BPF_A3(ALU, SUB, K),
+BPF_A3(ALU, SUB, X),
+BPF_A3(ALU, MUL, K),
+BPF_A3(ALU, MUL, X),
+BPF_A3(ALU, DIV, X),
+BPF_A3(ALU, AND, K),
+BPF_A3(ALU, AND, X),
+BPF_A3(ALU, OR, K),
+BPF_A3(ALU, OR, X),
+BPF_A3(ALU, LSH, K),
+BPF_A3(ALU, LSH, X),
+BPF_A3(ALU, RSH, K),
+BPF_A3(ALU, RSH, X),
+BPF_A2(ALU, NEG),
+BPF_A3(LD, W, ABS),
+BPF_A3(LD, H, ABS),
+BPF_A3(LD, B, ABS),
+BPF_A3(LD, W, LEN),
+BPF_A3(LD, W, IND),
+BPF_A3(LD, H, IND),
+BPF_A3(LD, B, IND),
+BPF_A2(LD, IMM),
+BPF_A3(LDX, W, LEN),
+BPF_A3(LDX, B, MSH),
+BPF_A2(LDX, IMM),
+BPF_A2(MISC, TAX),
+BPF_A2(MISC, TXA),
+BPF_A3(ALU, DIV, K),
+BPF_A2(LD, MEM),
+BPF_A2(LDX, MEM),
+BPF_A1(ST),
+BPF_A1(STX),
+BPF_A2(JMP, JA),
+BPF_A3(JMP, JEQ, K),
+BPF_A3(JMP, JEQ, X),
+BPF_A3(JMP, JGE, K),
+BPF_A3(JMP, JGE, X),
+BPF_A3(JMP, JGT, K),
+BPF_A3(JMP, JGT, X),
+BPF_A3(JMP, JSET, K),
+BPF_A3(JMP, JSET, X),
+#undef BPF_A1
+#undef BPF_A2
+#undef BPF_A3