Re: [PATCH] fragment: add fast path

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

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