Re: [PATCH] fragment: add fast path

Eric Dumazet
2010-06-14T05:35:50Z

Le lundi 14 juin 2010 à 07:16 +0800, Changli Gao a écrit :
> add fast path
>
> As the fragments are usually in order, it is likely the new fragments are
> at the end of the inet_frag_queue. In the fast path, we check if the skb at the
> end of the inet_frag_queue is the prev we expect.
>
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ----
> include/net/inet_frag.h | 1 +
> net/ipv4/ip_fragment.c | 17 +++++++++++++++++
> net/ipv6/reassembly.c | 16 ++++++++++++++++
> 3 files changed, 34 insertions(+)
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index 39f2dc9..16ff29a 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -20,6 +20,7 @@ struct inet_frag_queue {
> atomic_t refcnt;
> struct timer_list timer; /* when will this queue expire? */
> struct sk_buff *fragments; /* list of received fragments */
> + struct sk_buff *fragments_tail;
> ktime_t stamp;
> int len; /* total length of orig datagram */
> int meat;
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index 75347ea..d8c36d4 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -317,6 +317,7 @@ static int ip_frag_reinit(struct ipq *qp)
> qp->q.len = 0;
> qp->q.meat = 0;
> qp->q.fragments = NULL;
> + qp->q.fragments_tail = NULL;
> qp->iif = 0;
>
> return 0;
> @@ -389,6 +390,16 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb)
> * in the chain of fragments so far. We must know where to put
> * this fragment, right?
> */
> + prev = qp->q.fragments_tail;
> + if (prev) {
> + if (FRAG_CB(prev)->offset < offset) {
> + next = NULL;
> + goto found;
> + }
> + } else {
> + next = NULL;

How can this chunk be a win ? queue is empty anyway.
You add tests and slow the 'other path'

> + goto found;
> + }

Quite frankly, one easy way to speedup things would be to move 'offset'
from ipfrag_skb_cb close to skb->next field so that only one cache line
per frag is used during lookup.

I am not sure why we need "struct inet_skb_parm h;" field in struct
ipfrag_skb_cb...

I probably need to wakeup this monday morning ?

Untested patch follows, only compiled.

[PATCH] frags: Remove unecessary bits

While trying to move 'offset' to the beginning of frag CB, I found
inet_skb_parm field was unused.

Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index 75347ea..0f51ae0 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -55,7 +55,6 @@ static int sysctl_ipfrag_max_dist __read_mostly = 64;

struct ipfrag_skb_cb
{
- struct inet_skb_parm h;
int offset;
};

diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 6d4292f..122e0be 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -57,7 +57,6 @@

struct ip6frag_skb_cb
{
- struct inet6_skb_parm h;
int offset;
};