* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: Changli Gao @ 2010-11-19 12:32 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Hagen Paul Pfeifer, netdev
In-Reply-To: <1290165472.3034.109.camel@edumazet-laptop>
On Fri, Nov 19, 2010 at 7:17 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le vendredi 19 novembre 2010 à 10:54 +0100, Eric Dumazet a écrit :
>
>> I believe we should revert the u32 f_k = fentry->k; part
>>
>> fentry->k as is fast as f_k if stored on stack, and avoids one
>> instruction if fentry->k is not needed.
>>
>>
>
> A revert is not good on arches with decent number of registers (x86_64
> for example).
>
> Maybe have some CONFIG_ARCH_HAS_{FEW|MANY}_REGISTERS is needed, (or
> already exist ?)
>
> Here is the patch to save 400 bytes on x86_32, and really speedup the
> damn thing on all arches.
>
> Thanks
>
> [PATCH net-next-2.6] filter: optimize sk_run_filter
>
> remove pc variable to avoid arithmetic to compute fentry at each filter
> instruction. Jumps directly manipulate fentry pointer.
>
> As the last instruction of filter[] is guaranteed to be a RETURN, and
> all jumps are before the last instruction, we dont need to check filter
> bounds (number of instructions in filter array) at each iteration, so we
> remove it from sk_run_filter() params.
>
> On x86_32 remove f_k var introduced in commit 57fe93b374a6b871
> (filter: make sure filters dont read uninitialized memory)
>
> Note : We could use a CONFIG_ARCH_HAS_{FEW|MANY}_REGISTERS in order to
> avoid too many ifdefs in this code.
>
> This helps compiler to use cpu registers to hold fentry and A
> accumulator.
>
> On x86_32, this saves 401 bytes, and more important, sk_run_filter()
> runs much faster because less register pressure (One less conditional
> branch per BPF instruction)
>
> # size net/core/filter.o net/core/filter_pre.o
> text data bss dec hex filename
> 2948 0 0 2948 b84 net/core/filter.o
> 3349 0 0 3349 d15 net/core/filter_pre.o
>
> on x86_64 :
> # size net/core/filter.o net/core/filter_pre.o
> text data bss dec hex filename
> 5173 0 0 5173 1435 net/core/filter.o
> 5224 0 0 5224 1468 net/core/filter_pre.o
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Changli Gao <xiaosuo@gmail.com>
> Cc: Hagen Paul Pfeifer <hagen@jauu.net>
> ---
> include/linux/filter.h | 2
> net/core/filter.c | 93 +++++++++++++++++++-------------------
> net/core/timestamping.c | 2
> net/packet/af_packet.c | 2
> 4 files changed, 51 insertions(+), 48 deletions(-)
>
you missed the users of sk_run_filter in directory dev/.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC PATCH] netfilter: remove the duplicate tables
From: Changli Gao @ 2010-11-19 12:48 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jan Engelhardt, Patrick McHardy, David S. Miller, netfilter-devel,
netdev, Stephen Hemminger
In-Reply-To: <1290166180.3034.119.camel@edumazet-laptop>
On Fri, Nov 19, 2010 at 7:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
> IMHO, the real problem is not the table duplication. We know that adding
> a level of indirection is going to hurt a lot because of cache misses.
>
Currently, multi-core CPU is common, the cores in a CPU share the
lowest level cache. Duplicate tables use more RAM, and may cause more
pressure of the lowest level cache.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC PATCH] netfilter: remove the duplicate tables
From: Jan Engelhardt @ 2010-11-19 12:57 UTC (permalink / raw)
To: Changli Gao
Cc: Eric Dumazet, Patrick McHardy, David S. Miller, netfilter-devel,
netdev, Stephen Hemminger
In-Reply-To: <AANLkTikF83kxggqyxoxw0m6L5km3zogPsiV7wVO1jbXy@mail.gmail.com>
On Friday 2010-11-19 13:48, Changli Gao wrote:
>On Fri, Nov 19, 2010 at 7:29 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> IMHO, the real problem is not the table duplication. We know that adding
>> a level of indirection is going to hurt a lot because of cache misses.
>>
>
>Currently, multi-core CPU is common, the cores in a CPU share the
>lowest level cache. Duplicate tables use more RAM, and may cause more
>pressure of the lowest level cache.
There probably should be at most one copy per NUMA node. Maybe less,
depending on what the benchmarks will say.
I started on some patch to reduce the ruleset from #cpu to #numa_nodes,
but then stopped when I ran into the obvious fact that it would require
locking the counters because xtables can actually run on more than one
core within a given numa node. When decoupling counters from the
ruleset, reducing the ruleset copies would become easier.
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: Changli Gao @ 2010-11-19 12:57 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Hagen Paul Pfeifer, netdev
In-Reply-To: <AANLkTi=ed79a5HjGULPdwuqmUhj=W8BGzRh7tm2L11ef@mail.gmail.com>
On Fri, Nov 19, 2010 at 8:32 PM, Changli Gao <xiaosuo@gmail.com> wrote:
>
> you missed the users of sk_run_filter in directory dev/.
>
Sorry, the directory is drivers/.
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC PATCH] netfilter: remove the duplicate tables
From: Changli Gao @ 2010-11-19 13:03 UTC (permalink / raw)
To: Jan Engelhardt
Cc: Eric Dumazet, Patrick McHardy, David S. Miller, netfilter-devel,
netdev, Stephen Hemminger
In-Reply-To: <alpine.LNX.2.01.1011191354160.20769@obet.zrqbmnf.qr>
On Fri, Nov 19, 2010 at 8:57 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
> There probably should be at most one copy per NUMA node. Maybe less,
> depending on what the benchmarks will say.
>
> I started on some patch to reduce the ruleset from #cpu to #numa_nodes,
It is a great idea! :)
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [RFC PATCH] netfilter: remove the duplicate tables
From: Jan Engelhardt @ 2010-11-19 13:11 UTC (permalink / raw)
To: Changli Gao
Cc: Eric Dumazet, Patrick McHardy, David S. Miller, netfilter-devel,
netdev, Stephen Hemminger
In-Reply-To: <AANLkTinR4b_V1J7TDL6jXCmJNL8cHaWSbteoLWF60w_M@mail.gmail.com>
On Friday 2010-11-19 14:03, Changli Gao wrote:
>On Fri, Nov 19, 2010 at 8:57 PM, Jan Engelhardt <jengelh@medozas.de> wrote:
>> There probably should be at most one copy per NUMA node. Maybe less,
>> depending on what the benchmarks will say.
>>
>> I started on some patch to reduce the ruleset from #cpu to #numa_nodes,
>
>It is a great idea! :)
Feel free to do it. (I don't have the patch anymore. It was just a few
lines anyway.)
^ permalink raw reply
* [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
From: Eric Dumazet @ 2010-11-19 13:16 UTC (permalink / raw)
To: Changli Gao; +Cc: David Miller, Hagen Paul Pfeifer, netdev
In-Reply-To: <AANLkTi==Ovw8xFA1K6rVabg9MW0w6wd=2qvo=XojUXpM@mail.gmail.com>
Le vendredi 19 novembre 2010 à 20:57 +0800, Changli Gao a écrit :
> On Fri, Nov 19, 2010 at 8:32 PM, Changli Gao <xiaosuo@gmail.com> wrote:
> >
> > you missed the users of sk_run_filter in directory dev/.
> >
>
> Sorry, the directory is drivers/.
>
>
You're right, thanks !
[PATCH net-next-2.6 v2] filter: optimize sk_run_filter
Remove pc variable to avoid arithmetic to compute fentry at each filter
instruction. Jumps directly manipulate fentry pointer.
As the last instruction of filter[] is guaranteed to be a RETURN, and
all jumps are before the last instruction, we dont need to check filter
bounds (number of instructions in filter array) at each iteration, so we
remove it from sk_run_filter() params.
On x86_32 remove f_k var introduced in commit 57fe93b374a6b871
(filter: make sure filters dont read uninitialized memory)
Note : We could use a CONFIG_ARCH_HAS_{FEW|MANY}_REGISTERS in order to
avoid too many ifdefs in this code.
This helps compiler to use cpu registers to hold fentry and A
accumulator.
On x86_32, this saves 401 bytes, and more important, sk_run_filter()
runs much faster because less register pressure (One less conditional
branch per BPF instruction)
# size net/core/filter.o net/core/filter_pre.o
text data bss dec hex filename
2948 0 0 2948 b84 net/core/filter.o
3349 0 0 3349 d15 net/core/filter_pre.o
on x86_64 :
# size net/core/filter.o net/core/filter_pre.o
text data bss dec hex filename
5173 0 0 5173 1435 net/core/filter.o
5224 0 0 5224 1468 net/core/filter_pre.o
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Changli Gao <xiaosuo@gmail.com>
Cc: Hagen Paul Pfeifer <hagen@jauu.net>
---
V2: add missing changes for drivers/isdn/i4l/isdn_ppp.c &
drivers/net/ppp_generic.c
drivers/isdn/i4l/isdn_ppp.c | 14 ++---
drivers/net/ppp_generic.c | 12 +---
include/linux/filter.h | 2
net/core/filter.c | 93 +++++++++++++++++-----------------
net/core/timestamping.c | 2
net/packet/af_packet.c | 2
6 files changed, 61 insertions(+), 64 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_ppp.c b/drivers/isdn/i4l/isdn_ppp.c
index 97c5cc2..9e8162c 100644
--- a/drivers/isdn/i4l/isdn_ppp.c
+++ b/drivers/isdn/i4l/isdn_ppp.c
@@ -1147,15 +1147,14 @@ isdn_ppp_push_higher(isdn_net_dev * net_dev, isdn_net_local * lp, struct sk_buff
}
if (is->pass_filter
- && sk_run_filter(skb, is->pass_filter, is->pass_len) == 0) {
+ && sk_run_filter(skb, is->pass_filter) == 0) {
if (is->debug & 0x2)
printk(KERN_DEBUG "IPPP: inbound frame filtered.\n");
kfree_skb(skb);
return;
}
if (!(is->active_filter
- && sk_run_filter(skb, is->active_filter,
- is->active_len) == 0)) {
+ && sk_run_filter(skb, is->active_filter) == 0)) {
if (is->debug & 0x2)
printk(KERN_DEBUG "IPPP: link-active filter: reseting huptimer.\n");
lp->huptimer = 0;
@@ -1294,15 +1293,14 @@ isdn_ppp_xmit(struct sk_buff *skb, struct net_device *netdev)
}
if (ipt->pass_filter
- && sk_run_filter(skb, ipt->pass_filter, ipt->pass_len) == 0) {
+ && sk_run_filter(skb, ipt->pass_filter) == 0) {
if (ipt->debug & 0x4)
printk(KERN_DEBUG "IPPP: outbound frame filtered.\n");
kfree_skb(skb);
goto unlock;
}
if (!(ipt->active_filter
- && sk_run_filter(skb, ipt->active_filter,
- ipt->active_len) == 0)) {
+ && sk_run_filter(skb, ipt->active_filter) == 0)) {
if (ipt->debug & 0x4)
printk(KERN_DEBUG "IPPP: link-active filter: reseting huptimer.\n");
lp->huptimer = 0;
@@ -1492,9 +1490,9 @@ int isdn_ppp_autodial_filter(struct sk_buff *skb, isdn_net_local *lp)
}
drop |= is->pass_filter
- && sk_run_filter(skb, is->pass_filter, is->pass_len) == 0;
+ && sk_run_filter(skb, is->pass_filter) == 0;
drop |= is->active_filter
- && sk_run_filter(skb, is->active_filter, is->active_len) == 0;
+ && sk_run_filter(skb, is->active_filter) == 0;
skb_push(skb, IPPP_MAX_HEADER - 4);
return drop;
diff --git a/drivers/net/ppp_generic.c b/drivers/net/ppp_generic.c
index 09cf56d..0c91598a 100644
--- a/drivers/net/ppp_generic.c
+++ b/drivers/net/ppp_generic.c
@@ -1136,8 +1136,7 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
a four-byte PPP header on each packet */
*skb_push(skb, 2) = 1;
if (ppp->pass_filter &&
- sk_run_filter(skb, ppp->pass_filter,
- ppp->pass_len) == 0) {
+ sk_run_filter(skb, ppp->pass_filter) == 0) {
if (ppp->debug & 1)
printk(KERN_DEBUG "PPP: outbound frame not passed\n");
kfree_skb(skb);
@@ -1145,8 +1144,7 @@ ppp_send_frame(struct ppp *ppp, struct sk_buff *skb)
}
/* if this packet passes the active filter, record the time */
if (!(ppp->active_filter &&
- sk_run_filter(skb, ppp->active_filter,
- ppp->active_len) == 0))
+ sk_run_filter(skb, ppp->active_filter) == 0))
ppp->last_xmit = jiffies;
skb_pull(skb, 2);
#else
@@ -1758,8 +1756,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
*skb_push(skb, 2) = 0;
if (ppp->pass_filter &&
- sk_run_filter(skb, ppp->pass_filter,
- ppp->pass_len) == 0) {
+ sk_run_filter(skb, ppp->pass_filter) == 0) {
if (ppp->debug & 1)
printk(KERN_DEBUG "PPP: inbound frame "
"not passed\n");
@@ -1767,8 +1764,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
return;
}
if (!(ppp->active_filter &&
- sk_run_filter(skb, ppp->active_filter,
- ppp->active_len) == 0))
+ sk_run_filter(skb, ppp->active_filter) == 0))
ppp->last_recv = jiffies;
__skb_pull(skb, 2);
} else
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 151f5d7..447a775 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -147,7 +147,7 @@ struct sock;
extern int sk_filter(struct sock *sk, struct sk_buff *skb);
extern unsigned int sk_run_filter(struct sk_buff *skb,
- struct sock_filter *filter, int flen);
+ const struct sock_filter *filter);
extern int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk);
extern int sk_detach_filter(struct sock *sk);
extern int sk_chk_filter(struct sock_filter *filter, int flen);
diff --git a/net/core/filter.c b/net/core/filter.c
index 15a545d..9e77b3c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -137,7 +137,7 @@ int sk_filter(struct sock *sk, struct sk_buff *skb)
rcu_read_lock_bh();
filter = rcu_dereference_bh(sk->sk_filter);
if (filter) {
- unsigned int pkt_len = sk_run_filter(skb, filter->insns, filter->len);
+ unsigned int pkt_len = sk_run_filter(skb, filter->insns);
err = pkt_len ? pskb_trim(skb, pkt_len) : -EPERM;
}
@@ -151,14 +151,15 @@ EXPORT_SYMBOL(sk_filter);
* sk_run_filter - run a filter on a socket
* @skb: buffer to run the filter on
* @filter: filter to apply
- * @flen: length of filter
*
* Decode and apply filter instructions to the skb->data.
- * Return length to keep, 0 for none. skb is the data we are
- * filtering, filter is the array of filter instructions, and
- * len is the number of filter blocks in the array.
+ * Return length to keep, 0 for none. @skb is the data we are
+ * filtering, @filter is the array of filter instructions.
+ * Because all jumps are guaranteed to be before last instruction,
+ * and last instruction guaranteed to be a RET, we dont need to check
+ * flen. (We used to pass to this function the length of filter)
*/
-unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int flen)
+unsigned int sk_run_filter(struct sk_buff *skb, const struct sock_filter *fentry)
{
void *ptr;
u32 A = 0; /* Accumulator */
@@ -167,34 +168,36 @@ unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int
unsigned long memvalid = 0;
u32 tmp;
int k;
- int pc;
BUILD_BUG_ON(BPF_MEMWORDS > BITS_PER_LONG);
/*
* Process array of filter instructions.
*/
- for (pc = 0; pc < flen; pc++) {
- const struct sock_filter *fentry = &filter[pc];
- u32 f_k = fentry->k;
+ for (;; fentry++) {
+#if defined(CONFIG_X86_32)
+#define K (fentry->k)
+#else
+ const u32 K = fentry->k;
+#endif
switch (fentry->code) {
case BPF_S_ALU_ADD_X:
A += X;
continue;
case BPF_S_ALU_ADD_K:
- A += f_k;
+ A += K;
continue;
case BPF_S_ALU_SUB_X:
A -= X;
continue;
case BPF_S_ALU_SUB_K:
- A -= f_k;
+ A -= K;
continue;
case BPF_S_ALU_MUL_X:
A *= X;
continue;
case BPF_S_ALU_MUL_K:
- A *= f_k;
+ A *= K;
continue;
case BPF_S_ALU_DIV_X:
if (X == 0)
@@ -202,64 +205,64 @@ unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int
A /= X;
continue;
case BPF_S_ALU_DIV_K:
- A /= f_k;
+ A /= K;
continue;
case BPF_S_ALU_AND_X:
A &= X;
continue;
case BPF_S_ALU_AND_K:
- A &= f_k;
+ A &= K;
continue;
case BPF_S_ALU_OR_X:
A |= X;
continue;
case BPF_S_ALU_OR_K:
- A |= f_k;
+ A |= K;
continue;
case BPF_S_ALU_LSH_X:
A <<= X;
continue;
case BPF_S_ALU_LSH_K:
- A <<= f_k;
+ A <<= K;
continue;
case BPF_S_ALU_RSH_X:
A >>= X;
continue;
case BPF_S_ALU_RSH_K:
- A >>= f_k;
+ A >>= K;
continue;
case BPF_S_ALU_NEG:
A = -A;
continue;
case BPF_S_JMP_JA:
- pc += f_k;
+ fentry += K;
continue;
case BPF_S_JMP_JGT_K:
- pc += (A > f_k) ? fentry->jt : fentry->jf;
+ fentry += (A > K) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JGE_K:
- pc += (A >= f_k) ? fentry->jt : fentry->jf;
+ fentry += (A >= K) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JEQ_K:
- pc += (A == f_k) ? fentry->jt : fentry->jf;
+ fentry += (A == K) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JSET_K:
- pc += (A & f_k) ? fentry->jt : fentry->jf;
+ fentry += (A & K) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JGT_X:
- pc += (A > X) ? fentry->jt : fentry->jf;
+ fentry += (A > X) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JGE_X:
- pc += (A >= X) ? fentry->jt : fentry->jf;
+ fentry += (A >= X) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JEQ_X:
- pc += (A == X) ? fentry->jt : fentry->jf;
+ fentry += (A == X) ? fentry->jt : fentry->jf;
continue;
case BPF_S_JMP_JSET_X:
- pc += (A & X) ? fentry->jt : fentry->jf;
+ fentry += (A & X) ? fentry->jt : fentry->jf;
continue;
case BPF_S_LD_W_ABS:
- k = f_k;
+ k = K;
load_w:
ptr = load_pointer(skb, k, 4, &tmp);
if (ptr != NULL) {
@@ -268,7 +271,7 @@ load_w:
}
break;
case BPF_S_LD_H_ABS:
- k = f_k;
+ k = K;
load_h:
ptr = load_pointer(skb, k, 2, &tmp);
if (ptr != NULL) {
@@ -277,7 +280,7 @@ load_h:
}
break;
case BPF_S_LD_B_ABS:
- k = f_k;
+ k = K;
load_b:
ptr = load_pointer(skb, k, 1, &tmp);
if (ptr != NULL) {
@@ -292,34 +295,34 @@ load_b:
X = skb->len;
continue;
case BPF_S_LD_W_IND:
- k = X + f_k;
+ k = X + K;
goto load_w;
case BPF_S_LD_H_IND:
- k = X + f_k;
+ k = X + K;
goto load_h;
case BPF_S_LD_B_IND:
- k = X + f_k;
+ k = X + K;
goto load_b;
case BPF_S_LDX_B_MSH:
- ptr = load_pointer(skb, f_k, 1, &tmp);
+ ptr = load_pointer(skb, K, 1, &tmp);
if (ptr != NULL) {
X = (*(u8 *)ptr & 0xf) << 2;
continue;
}
return 0;
case BPF_S_LD_IMM:
- A = f_k;
+ A = K;
continue;
case BPF_S_LDX_IMM:
- X = f_k;
+ X = K;
continue;
case BPF_S_LD_MEM:
- A = (memvalid & (1UL << f_k)) ?
- mem[f_k] : 0;
+ A = (memvalid & (1UL << K)) ?
+ mem[K] : 0;
continue;
case BPF_S_LDX_MEM:
- X = (memvalid & (1UL << f_k)) ?
- mem[f_k] : 0;
+ X = (memvalid & (1UL << K)) ?
+ mem[K] : 0;
continue;
case BPF_S_MISC_TAX:
X = A;
@@ -328,16 +331,16 @@ load_b:
A = X;
continue;
case BPF_S_RET_K:
- return f_k;
+ return K;
case BPF_S_RET_A:
return A;
case BPF_S_ST:
- memvalid |= 1UL << f_k;
- mem[f_k] = A;
+ memvalid |= 1UL << K;
+ mem[K] = A;
continue;
case BPF_S_STX:
- memvalid |= 1UL << f_k;
- mem[f_k] = X;
+ memvalid |= 1UL << K;
+ mem[K] = X;
continue;
default:
WARN_ON(1);
diff --git a/net/core/timestamping.c b/net/core/timestamping.c
index 0ae6c22..dac7ed6 100644
--- a/net/core/timestamping.c
+++ b/net/core/timestamping.c
@@ -31,7 +31,7 @@ static unsigned int classify(struct sk_buff *skb)
if (likely(skb->dev &&
skb->dev->phydev &&
skb->dev->phydev->drv))
- return sk_run_filter(skb, ptp_filter, ARRAY_SIZE(ptp_filter));
+ return sk_run_filter(skb, ptp_filter);
else
return PTP_CLASS_NONE;
}
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 2096456..b6372dd 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -519,7 +519,7 @@ static inline unsigned int run_filter(struct sk_buff *skb, struct sock *sk,
rcu_read_lock_bh();
filter = rcu_dereference_bh(sk->sk_filter);
if (filter != NULL)
- res = sk_run_filter(skb, filter->insns, filter->len);
+ res = sk_run_filter(skb, filter->insns);
rcu_read_unlock_bh();
return res;
^ permalink raw reply related
* Re: [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
From: Changli Gao @ 2010-11-19 14:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, Hagen Paul Pfeifer, netdev
In-Reply-To: <1290172607.3034.124.camel@edumazet-laptop>
On Fri, Nov 19, 2010 at 9:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le vendredi 19 novembre 2010 à 20:57 +0800, Changli Gao a écrit :
>> On Fri, Nov 19, 2010 at 8:32 PM, Changli Gao <xiaosuo@gmail.com> wrote:
>> >
>> > you missed the users of sk_run_filter in directory dev/.
>> >
>>
>> Sorry, the directory is drivers/.
>>
>>
>
> You're right, thanks !
>
> [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
>
> Remove pc variable to avoid arithmetic to compute fentry at each filter
> instruction. Jumps directly manipulate fentry pointer.
>
> As the last instruction of filter[] is guaranteed to be a RETURN, and
> all jumps are before the last instruction, we dont need to check filter
> bounds (number of instructions in filter array) at each iteration, so we
> remove it from sk_run_filter() params.
>
> On x86_32 remove f_k var introduced in commit 57fe93b374a6b871
> (filter: make sure filters dont read uninitialized memory)
>
> Note : We could use a CONFIG_ARCH_HAS_{FEW|MANY}_REGISTERS in order to
> avoid too many ifdefs in this code.
>
> This helps compiler to use cpu registers to hold fentry and A
> accumulator.
>
> On x86_32, this saves 401 bytes, and more important, sk_run_filter()
> runs much faster because less register pressure (One less conditional
> branch per BPF instruction)
>
> # size net/core/filter.o net/core/filter_pre.o
> text data bss dec hex filename
> 2948 0 0 2948 b84 net/core/filter.o
> 3349 0 0 3349 d15 net/core/filter_pre.o
>
> on x86_64 :
> # size net/core/filter.o net/core/filter_pre.o
> text data bss dec hex filename
> 5173 0 0 5173 1435 net/core/filter.o
> 5224 0 0 5224 1468 net/core/filter_pre.o
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Changli Gao <xiaosuo@gmail.com>
> Cc: Hagen Paul Pfeifer <hagen@jauu.net>
Acked-by: Changli Gao <xiaosuo@gmail.com>
--
Regards,
Changli Gao(xiaosuo@gmail.com)
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
From: Tetsuo Handa @ 2010-11-19 14:17 UTC (permalink / raw)
To: eric.dumazet, xiaosuo; +Cc: davem, hagen, netdev
In-Reply-To: <1290172607.3034.124.camel@edumazet-laptop>
Just my thought again...
Eric Dumazet wrote:
> @@ -167,34 +168,36 @@ unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int
> unsigned long memvalid = 0;
> u32 tmp;
> int k;
Is this 'k' useful?
> - int pc;
>
> BUILD_BUG_ON(BPF_MEMWORDS > BITS_PER_LONG);
> /*
> * Process array of filter instructions.
> */
> - for (pc = 0; pc < flen; pc++) {
> - const struct sock_filter *fentry = &filter[pc];
> - u32 f_k = fentry->k;
> + for (;; fentry++) {
> +#if defined(CONFIG_X86_32)
> +#define K (fentry->k)
> +#else
> + const u32 K = fentry->k;
What happens if we use
u32 f_k = fentry->k;
and
> case BPF_S_LD_W_ABS:
> - k = f_k;
> + k = K;
remove this assignment and
> load_w:
> ptr = load_pointer(skb, k, 4, &tmp);
change to
ptr = load_pointer(skb, (int) f_k, 4, &tmp);
and
> case BPF_S_LD_W_IND:
> - k = X + f_k;
> + k = X + K;
change to
f_k += X;
?
> goto load_w;
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
From: Eric Dumazet @ 2010-11-19 14:35 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: xiaosuo, davem, hagen, netdev
In-Reply-To: <201011192317.CFB48465.OtLFOJOMHQSFFV@I-love.SAKURA.ne.jp>
Le vendredi 19 novembre 2010 à 23:17 +0900, Tetsuo Handa a écrit :
> Just my thought again...
>
> Eric Dumazet wrote:
> > @@ -167,34 +168,36 @@ unsigned int sk_run_filter(struct sk_buff *skb, struct sock_filter *filter, int
> > unsigned long memvalid = 0;
> > u32 tmp;
> > int k;
>
> Is this 'k' useful?
>
Yes this is useful, please read later we have :
k = X + K;
goto load_w;
K is readonly, k is a rw temp variable (on stack)
> > - int pc;
> >
> > BUILD_BUG_ON(BPF_MEMWORDS > BITS_PER_LONG);
> > /*
> > * Process array of filter instructions.
> > */
> > - for (pc = 0; pc < flen; pc++) {
> > - const struct sock_filter *fentry = &filter[pc];
> > - u32 f_k = fentry->k;
> > + for (;; fentry++) {
> > +#if defined(CONFIG_X86_32)
> > +#define K (fentry->k)
> > +#else
> > + const u32 K = fentry->k;
>
> What happens if we use
>
> u32 f_k = fentry->k;
>
> and
>
> > case BPF_S_LD_W_ABS:
> > - k = f_k;
> > + k = K;
>
> remove this assignment and
>
> > load_w:
> > ptr = load_pointer(skb, k, 4, &tmp);
>
> change to
>
> ptr = load_pointer(skb, (int) f_k, 4, &tmp);
>
> and
>
> > case BPF_S_LD_W_IND:
> > - k = X + f_k;
> > + k = X + K;
>
> change to
>
> f_k += X;
>
> ?
> > goto load_w;
This wont work, K is really a constant, part of the filter program,
while k is the (existing) temp variable.
^ permalink raw reply
* Re:Proposal
From: Jerrold R. David Esq. @ 2010-11-17 22:43 UTC (permalink / raw)
To: netdev
Attention:
I am Jerrold David Esq., an Attorney at Law, I would like to present a proposal to you that we will both benefit from only if you will permit me. Although, this medium (Internet) has been greatly abused, but i chose to reach you through it because it still remains the fastest medium of communication so far, I shall release the details of my proposal in your positive response to this mail.
Regards,
Jerrold R. David Esq.
Jerrold & Co. sub. JH.Ltd. UK
Bracken House,Charles Street
Manchester,M1 7BD.UK
Gleneida Avenue,Carmel,NY, USA
Tel. UK: 0044 70 31813794
Fax: 0044 84 47743889
USA: 001 914 205 8012
^ permalink raw reply
* Re: [PATCH 3/3] SELinux: return -ECONNREFUSED from ip_postroute to signal fatal error
From: Paul Moore @ 2010-11-19 15:11 UTC (permalink / raw)
To: Eric Paris
Cc: netdev, linux-kernel, selinux, netfilter-devel, equinox,
eric.dumazet, davem, hzhong, jmorris, kaber, kuznet, pekkas, sds,
yoshfuji
In-Reply-To: <20101116215257.6727.12163.stgit@paris.rdu.redhat.com>
On Tue, 2010-11-16 at 16:52 -0500, Eric Paris wrote:
> The SELinux netfilter hooks just return NF_DROP if they drop a packet. We
> want to signal that a drop in this hook is a permanant fatal error and is not
> transient. If we do this the error will be passed back up the stack in some
> places and applications will get a faster interaction that something went
> wrong.
Sorry for the delay, but I wasn't able to respond until today ...
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 8ba5001..b1104f9 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4594,11 +4594,11 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> secmark_perm = PACKET__SEND;
> break;
> default:
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
> }
> if (secmark_perm == PACKET__FORWARD_OUT) {
> if (selinux_skb_peerlbl_sid(skb, family, &peer_sid))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
The error condition here isn't due to a policy decision, but some
runtime error that happened when trying to determine the peer label of
an individual packet. I think leaving this as just NF_DROP is the right
thing to do as an error here can be temporary.
> } else
> peer_sid = SECINITSID_KERNEL;
> } else {
> @@ -4611,28 +4611,28 @@ static unsigned int selinux_ip_postroute(struct sk_buff *skb, int ifindex,
> ad.u.net.netif = ifindex;
> ad.u.net.family = family;
> if (selinux_parse_skb(skb, &ad, &addrp, 0, NULL))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Same thing, this is a transient error and not due to a policy decision.
We should keep this as NF_DROP.
> if (secmark_active)
> if (avc_has_perm(peer_sid, skb->secmark,
> SECCLASS_PACKET, secmark_perm, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Yep, this is a good place as the error is the result of a policy
decision, i.e. an avc_has_perm() call.
> if (peerlbl_active) {
> u32 if_sid;
> u32 node_sid;
>
> if (sel_netif_sid(ifindex, &if_sid))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Another transient error case, should be NF_DROP.
> if (avc_has_perm(peer_sid, if_sid,
> SECCLASS_NETIF, NETIF__EGRESS, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Good.
> if (sel_netnode_sid(addrp, family, &node_sid))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Bad.
> if (avc_has_perm(peer_sid, node_sid,
> SECCLASS_NODE, NODE__SENDTO, &ad))
> - return NF_DROP;
> + return NF_DROP_ERR(-ECONNREFUSED);
Good. I think you get the idea now.
Also, while I think we can ignore the forwarding and output hooks, we do
need to change the NF_DROPs in selinux_ip_postroute_compat() ... I'd
suggest the following (the last change catches more than just policy
decisions but considering the "compat" nature I think a little wiggle
room is okay here):
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 65fa8bf..de1b79e 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4520,11 +4520,11 @@ static unsigned int selinux_ip_postroute_compat(struct sk_buff *skb,
if (selinux_secmark_enabled())
if (avc_has_perm(sksec->sid, skb->secmark,
SECCLASS_PACKET, PACKET__SEND, &ad))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
if (selinux_policycap_netpeer)
if (selinux_xfrm_postroute_last(sksec->sid, skb, &ad, proto))
- return NF_DROP;
+ return NF_DROP_ERR(-ECONNREFUSED);
return NF_ACCEPT;
}
--
paul moore
linux @ hp
^ permalink raw reply related
* Updating
From: xmission.com @ 2010-11-19 16:13 UTC (permalink / raw)
Dear xmission.com User
HTK4S a virus was detected in xmission.com Webmail server, and xmission.com all webmail accounts must
be updated immediately to prevent damage to the webmail xmission.com.
You are asked to provide the identity account listed below to allow us to check and maintain your
account with the new version HTK4S anti-virus/anti-Spam 2010. failure to provide valid information,
your account will be temporarily suspended from our services.
Name :..................
User Name ...............
Password :............
Date of Birth :...........
Webmail Copyright © xmission.com 2010 All rights reserved
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: David Miller @ 2010-11-19 16:21 UTC (permalink / raw)
To: eric.dumazet; +Cc: hagen, netdev, xiaosuo
In-Reply-To: <1290165472.3034.109.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 Nov 2010 12:17:52 +0100
> Le vendredi 19 novembre 2010 à 10:54 +0100, Eric Dumazet a écrit :
>
>> I believe we should revert the u32 f_k = fentry->k; part
>>
>> fentry->k as is fast as f_k if stored on stack, and avoids one
>> instruction if fentry->k is not needed.
>>
>>
>
> A revert is not good on arches with decent number of registers (x86_64
> for example).
-EFIX_THE_DAMN_COMPILER
We never make calls out of this function or touch volatile memory or
create a full memory barrier between the assignment of f_k and it's
uses.
Therefore if common sub-expression elimination is working the compiler
will be able to decide properly whether to access things via memory or
use a register for the value.
Remember this is why we have that ACCESS_ONCE() thing.
We can't have it both ways, either ACCESS_ONCE() should be removed or
we should never make changes like your's and instead should submit
compiler bug reports :-)
^ permalink raw reply
* Re: 2.6.37-rc2-git4: Reported regressions 2.6.35 -> 2.6.36
From: Alex Deucher @ 2010-11-19 16:39 UTC (permalink / raw)
To: Mark Lord
Cc: Rafael J. Wysocki, Linux SCSI List, Linux ACPI,
Network Development, Linux Wireless List,
Linux Kernel Mailing List, DRI, Florian Mickler, Andrew Morton,
Kernel Testers List, Linus Torvalds, Linux PM List,
Maciej Rutecki
In-Reply-To: <4CE5C919.7090504@teksavvy.com>
On Thu, Nov 18, 2010 at 7:47 PM, Mark Lord <kernel@teksavvy.com> wrote:
> On 10-11-18 06:50 PM, Rafael J. Wysocki wrote:
>>
>> This message contains a list of some post-2.6.35 regressions introduced
>> before
>> 2.6.36, for which there are no fixes in the mainline known to the tracking
>> team.
>> If any of them have been fixed already, please let us know.
>>
>> If you know of any other unresolved post-2.6.35 regressions, please let us
>> know
>> either and we'll add them to the list. Also, please let us know if any
>> of the entries below are invalid.
>
>> Bug-Entry : http://bugzilla.kernel.org/show_bug.cgi?id=21652
>> Subject : several problems with intel graphics since 2.6.36
>> Submitter : Norbert Preining<preining@logic.at>
>> Date : 2010-10-27 14:32 (23 days old)
>> Message-ID :<20101027143252.GA8676@gamma.logic.tuwien.ac.at>
>> References : http://marc.info/?l=linux-kernel&m=128818998630241&w=2
>
> That one is interesting to me.. I suspect it may be the same cause
> as for https://bugzilla.kernel.org/show_bug.cgi?id=21952
>
> I have one of those Samsung (N210) Netbooks here: works fine with 2.6.34 and
> earlier,
> but fails to come out of suspend on 2.6.35/2.6.36 (haven't tried 2.6.37).
>
> So perhaps add 21952 to the list, or link it to the 21652
> (kind of amusing how similar the bug numbers are..).
>
>
> My non-Intel graphics notebook (has ATI X1400 graphics) also has a resume
> regression with 2.6.36. But it does work fine with 2.6.35 (and earlier,
> back many years). As a result, I'm stuck with 2.6.35 for the time being,
> and lack the time for a concerted debug effort on 2.6.36+ right now.
>
Can you bisect? Does this patch help?
diff --git a/drivers/gpu/drm/radeon/atom.c b/drivers/gpu/drm/radeon/atom.c
index 8e421f6..05efb5b 100644
--- a/drivers/gpu/drm/radeon/atom.c
+++ b/drivers/gpu/drm/radeon/atom.c
@@ -112,6 +112,7 @@ static uint32_t atom_iio_execute(struct
atom_context *ctx, int base,
base += 3;
break;
case ATOM_IIO_WRITE:
+ (void)ctx->card->ioreg_read(ctx->card, CU16(base + 1));
ctx->card->ioreg_write(ctx->card, CU16(base + 1), temp);
base += 3;
break;
Alex
> Cheers
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>
^ permalink raw reply related
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: Eric Dumazet @ 2010-11-19 16:55 UTC (permalink / raw)
To: David Miller; +Cc: hagen, netdev, xiaosuo
In-Reply-To: <20101119.082125.193710226.davem@davemloft.net>
Le vendredi 19 novembre 2010 à 08:21 -0800, David Miller a écrit :
> -EFIX_THE_DAMN_COMPILER
>
> We never make calls out of this function or touch volatile memory or
> create a full memory barrier between the assignment of f_k and it's
> uses.
>
> Therefore if common sub-expression elimination is working the compiler
> will be able to decide properly whether to access things via memory or
> use a register for the value.
>
> Remember this is why we have that ACCESS_ONCE() thing.
>
> We can't have it both ways, either ACCESS_ONCE() should be removed or
> we should never make changes like your's and instead should submit
> compiler bug reports :-)
Compiler is OK IMHO in this case. It does exactly what is required.
Compiler cannot load fentry->k before the switch() if some expression
dont use it, as it could trigger a fault.
After the "f_k = fentry->k;" commit, it was requested to do so.
Unfortunatly on x86_32 it also chose that f_k was more valuable in a cpu
register and accumulator A lost its register to get a stack slot
instead.
Not many BPF instructions use K, and if used, its used _once_ per BPF
instruction. There is no real gain to put it on a register, but code
size if (and only if) it is held in a cpu register, because each
assembler instruction using a register instead of stack is a bit
shorter.
In the end, I believe the "f_k = fentry->k;" was a good looking idea,
and good for some arches, but we forgot x86_32 (and probably some others)
have few available registers to play with.
Have a good week end !
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: David Miller @ 2010-11-19 17:05 UTC (permalink / raw)
To: eric.dumazet; +Cc: hagen, netdev, xiaosuo
In-Reply-To: <1290185759.3034.179.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 Nov 2010 17:55:59 +0100
> Unfortunatly on x86_32 it also chose that f_k was more valuable in a cpu
> register and accumulator A lost its register to get a stack slot
> instead.
Ok that tradeoff is terrible, but it depends upon knowledge we haven't
given to the compiler (yet).
Let me think about this a bit...
^ permalink raw reply
* Re: [net-2.6 PATCH] be2net: Fix to avoid firmware update when interface is not open.
From: David Miller @ 2010-11-19 17:06 UTC (permalink / raw)
To: Sarveshwar.Bandi; +Cc: netdev
In-Reply-To: <20101119094445.GA10165@emulex.com>
From: Sarveshwar Bandi <Sarveshwar.Bandi@emulex.com>
Date: Fri, 19 Nov 2010 15:14:45 +0530
> Since interrupts are enabled only when open is called on the interface,
> Attempting a firmware update operation when interface is down could lead to
> partial success or failure of operation. This fix fails the request if
> netif_running is false.
>
> Signed-off-by: Sarveshwar Bandi <Sarveshwar.Bandi@emulex.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: Stephen Hemminger @ 2010-11-19 17:15 UTC (permalink / raw)
To: David Miller; +Cc: eric.dumazet, hagen, netdev, xiaosuo
In-Reply-To: <20101119.090516.104057453.davem@davemloft.net>
On Fri, 19 Nov 2010 09:05:16 -0800 (PST)
David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 19 Nov 2010 17:55:59 +0100
>
> > Unfortunatly on x86_32 it also chose that f_k was more valuable in a cpu
> > register and accumulator A lost its register to get a stack slot
> > instead.
>
> Ok that tradeoff is terrible, but it depends upon knowledge we haven't
> given to the compiler (yet).
>
> Let me think about this a bit...
Are 'register' modifiers a no-op on current GCC?
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: Eric Dumazet @ 2010-11-19 17:16 UTC (permalink / raw)
To: David Miller; +Cc: hagen, netdev, xiaosuo
In-Reply-To: <20101119.090516.104057453.davem@davemloft.net>
Le vendredi 19 novembre 2010 à 09:05 -0800, David Miller a écrit :
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Fri, 19 Nov 2010 17:55:59 +0100
>
> > Unfortunatly on x86_32 it also chose that f_k was more valuable in a cpu
> > register and accumulator A lost its register to get a stack slot
> > instead.
>
> Ok that tradeoff is terrible, but it depends upon knowledge we haven't
> given to the compiler (yet).
>
> Let me think about this a bit...
By the way, I tried the 'register' keyword, and I knew it was a stupid
idea before even trying, since compiler ignored me : "What the hell do
you think you can tell me how to optimize this code ?"
(I am quite sure my laptop even smiled and sent to his neighbours a
broadcast saying "Hey buddies, this dumb guy tried the register C
keyword, isnt it funny ?")
Oh well...
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: David Miller @ 2010-11-19 17:21 UTC (permalink / raw)
To: shemminger; +Cc: eric.dumazet, hagen, netdev, xiaosuo
In-Reply-To: <20101119091537.5d1fed2e@nehalam>
From: Stephen Hemminger <shemminger@vyatta.com>
Date: Fri, 19 Nov 2010 09:15:37 -0800
> On Fri, 19 Nov 2010 09:05:16 -0800 (PST)
> David Miller <davem@davemloft.net> wrote:
>
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Fri, 19 Nov 2010 17:55:59 +0100
>>
>> > Unfortunatly on x86_32 it also chose that f_k was more valuable in a cpu
>> > register and accumulator A lost its register to get a stack slot
>> > instead.
>>
>> Ok that tradeoff is terrible, but it depends upon knowledge we haven't
>> given to the compiler (yet).
>>
>> Let me think about this a bit...
>
> Are 'register' modifiers a no-op on current GCC?
Yes, they are.
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: optimize sk_run_filter
From: David Miller @ 2010-11-19 17:25 UTC (permalink / raw)
To: eric.dumazet; +Cc: hagen, netdev, xiaosuo
In-Reply-To: <1290186984.3034.187.camel@edumazet-laptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 19 Nov 2010 18:16:24 +0100
> Le vendredi 19 novembre 2010 à 09:05 -0800, David Miller a écrit :
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Fri, 19 Nov 2010 17:55:59 +0100
>>
>> > Unfortunatly on x86_32 it also chose that f_k was more valuable in a cpu
>> > register and accumulator A lost its register to get a stack slot
>> > instead.
>>
>> Ok that tradeoff is terrible, but it depends upon knowledge we haven't
>> given to the compiler (yet).
>>
>> Let me think about this a bit...
>
> By the way, I tried the 'register' keyword, and I knew it was a stupid
> idea before even trying, since compiler ignored me : "What the hell do
> you think you can tell me how to optimize this code ?"
>
> (I am quite sure my laptop even smiled and sent to his neighbours a
> broadcast saying "Hey buddies, this dumb guy tried the register C
> keyword, isnt it funny ?")
:-)
I wonder though, since "fentry" is const in your recent patch,
you might not even need that X86_32 ifdef thing. GCC should
actually now be able to see that fentry->k cannot change, even
when we make external function calls via load_pointer() and such.
^ permalink raw reply
* Re: [PATCH] net: fix kernel-doc for sk_filter_rcu_release
From: David Miller @ 2010-11-19 17:27 UTC (permalink / raw)
To: randy.dunlap; +Cc: netdev
In-Reply-To: <20101118150237.1f0ba4b5.randy.dunlap@oracle.com>
From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Thu, 18 Nov 2010 15:02:37 -0800
> From: Randy Dunlap <randy.dunlap@oracle.com>
>
> Fix kernel-doc warning for sk_filter_rcu_release():
>
> Warning(net/core/filter.c:586): missing initial short description on line:
> * sk_filter_rcu_release: Release a socket filter by rcu_head
>
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Applied, thanks Randy.
^ permalink raw reply
* Re: [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
From: David Miller @ 2010-11-19 17:52 UTC (permalink / raw)
To: xiaosuo; +Cc: eric.dumazet, hagen, netdev
In-Reply-To: <AANLkTimiMRL+DM+XzxLoK_zdHv_KGhbxyBRCazStpZ6c@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Fri, 19 Nov 2010 22:13:07 +0800
> On Fri, Nov 19, 2010 at 9:16 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> [PATCH net-next-2.6 v2] filter: optimize sk_run_filter
>>
>> Remove pc variable to avoid arithmetic to compute fentry at each filter
>> instruction. Jumps directly manipulate fentry pointer.
>>
>> As the last instruction of filter[] is guaranteed to be a RETURN, and
>> all jumps are before the last instruction, we dont need to check filter
>> bounds (number of instructions in filter array) at each iteration, so we
>> remove it from sk_run_filter() params.
>>
>> On x86_32 remove f_k var introduced in commit 57fe93b374a6b871
>> (filter: make sure filters dont read uninitialized memory)
>>
>> Note : We could use a CONFIG_ARCH_HAS_{FEW|MANY}_REGISTERS in order to
>> avoid too many ifdefs in this code.
>>
>> This helps compiler to use cpu registers to hold fentry and A
>> accumulator.
>>
>> On x86_32, this saves 401 bytes, and more important, sk_run_filter()
>> runs much faster because less register pressure (One less conditional
>> branch per BPF instruction)
>>
>> # size net/core/filter.o net/core/filter_pre.o
>> text data bss dec hex filename
>> 2948 0 0 2948 b84 net/core/filter.o
>> 3349 0 0 3349 d15 net/core/filter_pre.o
>>
>> on x86_64 :
>> # size net/core/filter.o net/core/filter_pre.o
>> text data bss dec hex filename
>> 5173 0 0 5173 1435 net/core/filter.o
>> 5224 0 0 5224 1468 net/core/filter_pre.o
>>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> Cc: Changli Gao <xiaosuo@gmail.com>
>> Cc: Hagen Paul Pfeifer <hagen@jauu.net>
>
> Acked-by: Changli Gao <xiaosuo@gmail.com>
Ok, I'm applying this to net-next-2.6 for now. It keeps the
"f_k" situation optimal for all cases, on every platform I've
taken a look at the asm output (sparc64, x86-32, x86-64).
I can't currently think of a way to get rid of that ifdef,
so for now it's a small price to pay to get this optimal.
Thanks Eric!
^ permalink raw reply
* Re: [PATCH net-next-2.6] filter: cleanup codes[] init
From: David Miller @ 2010-11-19 18:07 UTC (permalink / raw)
To: xiaosuo; +Cc: eric.dumazet, hagen, netdev
In-Reply-To: <AANLkTi=GLX-biB0ApDFMD2GOYyqb2qghMSHScmBTJv-O@mail.gmail.com>
From: Changli Gao <xiaosuo@gmail.com>
Date: Fri, 19 Nov 2010 20:21:57 +0800
> On Fri, Nov 19, 2010 at 5:54 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>> Most probably you have "CONFIG_CC_OPTIMIZE_FOR_SIZE=y" which
>> unfortunately is known to generate poor looking code.
>
> Yes. So
>
> Acked-by: Changli Gao <xiaosuo@gmail.com>
Applied.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox