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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1312866027.2531.42.camel@edumazet-laptop>
Date:	Tue, 09 Aug 2011 07:00:27 +0200
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	hagen@...u.net, rui314@...il.com, netdev@...r.kernel.org
Subject: Re: [PATCH] net: filter: Convert the BPF VM to threaded code

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;
 		}
 	}
 


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