Re: [PATCH] net: filter: Convert the BPF VM to threaded code

[Eric Dumazet <eric.dumazet@gmail.com>]

Le lundi 01 août 2011 à 17:57 -0700, David Miller a écrit :

> Maybe it won't if we use an enum and make sure all enum values are handled
> in the switch? :-)

I tried this idea since its already an enum and all enum values are
handled in the witch, but all gcc versions I used still generate the
useless compare and branch (always predicted by modern CPU, so harmless
anyway ?)

348:   83 c3 08                add    $0x8,%ebx
34b:   66 83 3b 37             cmpw   $0x37,(%ebx)
34f:   77 f7                   ja     348 <sk_run_filter+0x18>
351:   0f b7 03                movzwl (%ebx),%eax
354:   ff 24 85 34 01 00 00    jmp    *0x134(,%eax,4)


It would be nice to try the jump table idea and avoid the array
indirection and get instead :

add	$0xc,%ebx
jmp	*(ebx)

(But this would need to use a larger kernel_sock_filter with not an u16
code, but the target address).


diff --git a/include/linux/filter.h b/include/linux/filter.h
index 741956f..f2a1b37 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -171,7 +171,7 @@ static inline void bpf_jit_free(struct sk_filter *fp)
 #define SK_RUN_FILTER(FILTER, SKB) sk_run_filter(SKB, FILTER->insns)
 #endif
 
-enum {
+enum bpf_inst {
 	BPF_S_RET_K = 1,
 	BPF_S_RET_A,
 	BPF_S_ALU_ADD_K,
diff --git a/net/core/filter.c b/net/core/filter.c
index 36f975f..ea1f467 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -119,17 +119,15 @@ unsigned int sk_run_filter(const struct sk_buff *skb,
 	u32 tmp;
 	int k;
 
+	fentry--;
 	/*
 	 * Process array of filter instructions.
 	 */
-	for (;; fentry++) {
-#if defined(CONFIG_X86_32)
+	for (;;) {
 #define	K (fentry->k)
-#else
-		const u32 K = fentry->k;
-#endif
 
-		switch (fentry->code) {
+		fentry++;
+		switch ((enum bpf_inst)fentry->code) {
 		case BPF_S_ALU_ADD_X:
 			A += X;
 			continue;
@@ -350,11 +348,6 @@ load_b:
 				A = 0;
 			continue;
 		}
-		default:
-			WARN_RATELIMIT(1, "Unknown code:%u jt:%u tf:%u k:%u\n",
-				       fentry->code, fentry->jt,
-				       fentry->jf, fentry->k);
-			return 0;
 		}
 	}