| Message ID | 1292478328.2603.56.camel@edumazet-laptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Thu, Dec 16, 2010 at 1:45 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > We can translate pseudo load instructions at filter check time to > dedicated instructions to speed up filtering and avoid one switch(). > libpcap currently uses SKF_AD_PROTOCOL, but custom filters probably use > other ancillary accesses. > > Note : I made the assertion that ancillary data was always accessed with > BPF_LD|BPF_?|BPF_ABS instructions, not with BPF_LD|BPF_?|BPF_IND ones > (offset given by K constant, not by K + X register) > > On x86_64, this saves a few bytes of text : > > # size net/core/filter.o.* > text data bss dec hex filename > 4864 0 0 4864 1300 net/core/filter.o.new > 4944 0 0 4944 1350 net/core/filter.o.old > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> > --- > net/core/filter.c | 72 ++++++++++++++++++++++++++------------------ > 1 file changed, 44 insertions(+), 28 deletions(-) > > diff --git a/net/core/filter.c b/net/core/filter.c > index e8a6ac4..2b27d4e 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -85,6 +85,17 @@ enum { > BPF_S_JMP_JGT_X, > BPF_S_JMP_JSET_K, > BPF_S_JMP_JSET_X, > + /* Ancillary data */ > + BPF_S_ANC_PROTOCOL, > + BPF_S_ANC_PKTTYPE, > + BPF_S_ANC_IFINDEX, > + BPF_S_ANC_NLATTR, > + BPF_S_ANC_NLATTR_NEST, > + BPF_S_ANC_MARK, > + BPF_S_ANC_QUEUE, > + BPF_S_ANC_HATYPE, > + BPF_S_ANC_RXHASH, > + BPF_S_ANC_CPU, > }; > > /* No hurry in this branch */ > @@ -107,11 +118,7 @@ static inline void *load_pointer(const struct sk_buff *skb, int k, > { > if (k >= 0) > return skb_header_pointer(skb, k, size, buffer); > - else { > - if (k >= SKF_AD_OFF) > - return NULL; > - return __load_pointer(skb, k, size); > - } > + return __load_pointer(skb, k, size); > } > > /** > @@ -269,7 +276,7 @@ load_w: > A = get_unaligned_be32(ptr); > continue; > } > - break; > + return 0; > case BPF_S_LD_H_ABS: > k = K; > load_h: > @@ -278,7 +285,7 @@ load_h: > A = get_unaligned_be16(ptr); > continue; > } > - break; > + return 0; > case BPF_S_LD_B_ABS: > k = K; > load_b: > @@ -287,7 +294,7 @@ load_b: > A = *(u8 *)ptr; > continue; > } > - break; > + return 0; > case BPF_S_LD_W_LEN: > A = skb->len; > continue; > @@ -338,45 +345,35 @@ load_b: > case BPF_S_STX: > mem[K] = X; > continue; > - default: > - WARN_ON(1); > - return 0; > - } > - > - /* > - * Handle ancillary data, which are impossible > - * (or very difficult) to get parsing packet contents. > - */ > - switch (k-SKF_AD_OFF) { > - case SKF_AD_PROTOCOL: > + case BPF_S_ANC_PROTOCOL: > A = ntohs(skb->protocol); > continue; > - case SKF_AD_PKTTYPE: > + case BPF_S_ANC_PKTTYPE: > A = skb->pkt_type; > continue; > - case SKF_AD_IFINDEX: > + case BPF_S_ANC_IFINDEX: > if (!skb->dev) > return 0; > A = skb->dev->ifindex; > continue; > - case SKF_AD_MARK: > + case BPF_S_ANC_MARK: > A = skb->mark; > continue; > - case SKF_AD_QUEUE: > + case BPF_S_ANC_QUEUE: > A = skb->queue_mapping; > continue; > - case SKF_AD_HATYPE: > + case BPF_S_ANC_HATYPE: > if (!skb->dev) > return 0; > A = skb->dev->type; > continue; > - case SKF_AD_RXHASH: > + case BPF_S_ANC_RXHASH: > A = skb->rxhash; > continue; > - case SKF_AD_CPU: > + case BPF_S_ANC_CPU: > A = raw_smp_processor_id(); > continue; > - case SKF_AD_NLATTR: { > + case BPF_S_ANC_NLATTR: { > struct nlattr *nla; > > if (skb_is_nonlinear(skb)) > @@ -392,7 +389,7 @@ load_b: > A = 0; > continue; > } > - case SKF_AD_NLATTR_NEST: { > + case BPF_S_ANC_NLATTR_NEST: { > struct nlattr *nla; > > if (skb_is_nonlinear(skb)) > @@ -412,6 +409,7 @@ load_b: > continue; > } > default: > + WARN_ON(1); > return 0; > } > } > @@ -600,6 +598,24 @@ int sk_chk_filter(struct sock_filter *filter, int flen) > pc + ftest->jf + 1 >= flen) > return -EINVAL; > break; > + case BPF_S_LD_W_ABS: > + case BPF_S_LD_H_ABS: > + case BPF_S_LD_B_ABS: > +#define ANCILLARY(CODE) case SKF_AD_OFF + SKF_AD_##CODE: \ > + code = BPF_S_ANC_##CODE; \ > + break > + switch (ftest->k) { > + ANCILLARY(PROTOCOL); > + ANCILLARY(PKTTYPE); > + ANCILLARY(IFINDEX); > + ANCILLARY(NLATTR); > + ANCILLARY(NLATTR_NEST); > + ANCILLARY(MARK); > + ANCILLARY(QUEUE); > + ANCILLARY(HATYPE); > + ANCILLARY(RXHASH); > + ANCILLARY(CPU); A default case should be handled as you remove the k > SKF_AD_OFF from load_pointer(). > + } > } > ftest->code = code; > } >
Le jeudi 16 décembre 2010 à 14:34 +0800, Changli Gao a écrit : > A default case should be handled as you remove the k > SKF_AD_OFF from > load_pointer(). > No, __load_pointer will return NULL in this case. -- 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
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 16 Dec 2010 06:45:28 +0100 > We can translate pseudo load instructions at filter check time to > dedicated instructions to speed up filtering and avoid one switch(). > libpcap currently uses SKF_AD_PROTOCOL, but custom filters probably use > other ancillary accesses. > > Note : I made the assertion that ancillary data was always accessed with > BPF_LD|BPF_?|BPF_ABS instructions, not with BPF_LD|BPF_?|BPF_IND ones > (offset given by K constant, not by K + X register) > > On x86_64, this saves a few bytes of text : > > # size net/core/filter.o.* > text data bss dec hex filename > 4864 0 0 4864 1300 net/core/filter.o.new > 4944 0 0 4944 1350 net/core/filter.o.old > > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Applied, thanks Eric. -- 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 --git a/net/core/filter.c b/net/core/filter.c index e8a6ac4..2b27d4e 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -85,6 +85,17 @@ enum { BPF_S_JMP_JGT_X, BPF_S_JMP_JSET_K, BPF_S_JMP_JSET_X, + /* Ancillary data */ + BPF_S_ANC_PROTOCOL, + BPF_S_ANC_PKTTYPE, + BPF_S_ANC_IFINDEX, + BPF_S_ANC_NLATTR, + BPF_S_ANC_NLATTR_NEST, + BPF_S_ANC_MARK, + BPF_S_ANC_QUEUE, + BPF_S_ANC_HATYPE, + BPF_S_ANC_RXHASH, + BPF_S_ANC_CPU, }; /* No hurry in this branch */ @@ -107,11 +118,7 @@ static inline void *load_pointer(const struct sk_buff *skb, int k, { if (k >= 0) return skb_header_pointer(skb, k, size, buffer); - else { - if (k >= SKF_AD_OFF) - return NULL; - return __load_pointer(skb, k, size); - } + return __load_pointer(skb, k, size); } /** @@ -269,7 +276,7 @@ load_w: A = get_unaligned_be32(ptr); continue; } - break; + return 0; case BPF_S_LD_H_ABS: k = K; load_h: @@ -278,7 +285,7 @@ load_h: A = get_unaligned_be16(ptr); continue; } - break; + return 0; case BPF_S_LD_B_ABS: k = K; load_b: @@ -287,7 +294,7 @@ load_b: A = *(u8 *)ptr; continue; } - break; + return 0; case BPF_S_LD_W_LEN: A = skb->len; continue; @@ -338,45 +345,35 @@ load_b: case BPF_S_STX: mem[K] = X; continue; - default: - WARN_ON(1); - return 0; - } - - /* - * Handle ancillary data, which are impossible - * (or very difficult) to get parsing packet contents. - */ - switch (k-SKF_AD_OFF) { - case SKF_AD_PROTOCOL: + case BPF_S_ANC_PROTOCOL: A = ntohs(skb->protocol); continue; - case SKF_AD_PKTTYPE: + case BPF_S_ANC_PKTTYPE: A = skb->pkt_type; continue; - case SKF_AD_IFINDEX: + case BPF_S_ANC_IFINDEX: if (!skb->dev) return 0; A = skb->dev->ifindex; continue; - case SKF_AD_MARK: + case BPF_S_ANC_MARK: A = skb->mark; continue; - case SKF_AD_QUEUE: + case BPF_S_ANC_QUEUE: A = skb->queue_mapping; continue; - case SKF_AD_HATYPE: + case BPF_S_ANC_HATYPE: if (!skb->dev) return 0; A = skb->dev->type; continue; - case SKF_AD_RXHASH: + case BPF_S_ANC_RXHASH: A = skb->rxhash; continue; - case SKF_AD_CPU: + case BPF_S_ANC_CPU: A = raw_smp_processor_id(); continue; - case SKF_AD_NLATTR: { + case BPF_S_ANC_NLATTR: { struct nlattr *nla; if (skb_is_nonlinear(skb)) @@ -392,7 +389,7 @@ load_b: A = 0; continue; } - case SKF_AD_NLATTR_NEST: { + case BPF_S_ANC_NLATTR_NEST: { struct nlattr *nla; if (skb_is_nonlinear(skb)) @@ -412,6 +409,7 @@ load_b: continue; } default: + WARN_ON(1); return 0; } } @@ -600,6 +598,24 @@ int sk_chk_filter(struct sock_filter *filter, int flen) pc + ftest->jf + 1 >= flen) return -EINVAL; break; + case BPF_S_LD_W_ABS: + case BPF_S_LD_H_ABS: + case BPF_S_LD_B_ABS: +#define ANCILLARY(CODE) case SKF_AD_OFF + SKF_AD_##CODE: \ + code = BPF_S_ANC_##CODE; \ + break + switch (ftest->k) { + ANCILLARY(PROTOCOL); + ANCILLARY(PKTTYPE); + ANCILLARY(IFINDEX); + ANCILLARY(NLATTR); + ANCILLARY(NLATTR_NEST); + ANCILLARY(MARK); + ANCILLARY(QUEUE); + ANCILLARY(HATYPE); + ANCILLARY(RXHASH); + ANCILLARY(CPU); + } } ftest->code = code; }
We can translate pseudo load instructions at filter check time to dedicated instructions to speed up filtering and avoid one switch(). libpcap currently uses SKF_AD_PROTOCOL, but custom filters probably use other ancillary accesses. Note : I made the assertion that ancillary data was always accessed with BPF_LD|BPF_?|BPF_ABS instructions, not with BPF_LD|BPF_?|BPF_IND ones (offset given by K constant, not by K + X register) On x86_64, this saves a few bytes of text : # size net/core/filter.o.* text data bss dec hex filename 4864 0 0 4864 1300 net/core/filter.o.new 4944 0 0 4944 1350 net/core/filter.o.old Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> --- net/core/filter.c | 72 ++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 28 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