* [PATCH 1/1] DCCP Ackvec: fix soft lockup in ackvec handling code
From: Andrea Bittau @ 2006-06-09 18:26 UTC (permalink / raw)
To: dccp
A soft lockup existed in the handling of ack vector records. Specifically, when
a tail of the list of ack vector records was removed, it was possible to end up
iterating infinitely on an element of the tail.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index 2349078..a856d72 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -453,6 +453,7 @@ #endif
(unsigned long long)
avr->dccpavr_ack_ackno);
dccp_ackvec_throw_record(av, avr);
+ break;
}
/*
* If it wasn't received, continue scanning... we might
^ permalink raw reply related
* Re: [PATCH 1/1] DCCP Ackvec: fix soft lockup in ackvec handling code
From: Ian McDonald @ 2006-06-10 8:28 UTC (permalink / raw)
To: dccp
In-Reply-To: <20060609182649.GA20777@shorty.sorbonet.org>
[-- Attachment #1: Type: text/plain, Size: 2098 bytes --]
Dave,
I have tested this and wonder whether it can be pushed upstream for
Linus for 2.6.17 if at all possible. I know Arnaldo is the maintainer
but as you are aware he is very busy with other projects at present.
This fix makes CCID2 usable - previously it was unusable for most
users. I had about five users report it as broken to me and this fixes
it for me. I am able to make it crash reproducibly without it and it
doesn't with the patch.
I've also put inline and attached (as gmail mailer mangles sometimes)
the patch redone against Linus' tree as Andrea's patch didn't apply
cleanly for me.
Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
Thanks,
Ian
--
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index b5981e5..8c211c5 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -452,6 +452,7 @@ #endif
(unsigned long long)
avr->dccpavr_ack_ackno);
dccp_ackvec_throw_record(av, avr);
+ break;
}
/*
* If it wasn't received, continue scanning... we might
On 6/10/06, Andrea Bittau <a.bittau@cs.ucl.ac.uk> wrote:
> A soft lockup existed in the handling of ack vector records. Specifically, when
> a tail of the list of ack vector records was removed, it was possible to end up
> iterating infinitely on an element of the tail.
>
> Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
>
> ---
>
> diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
> index 2349078..a856d72 100644
> --- a/net/dccp/ackvec.c
> +++ b/net/dccp/ackvec.c
> @@ -453,6 +453,7 @@ #endif
> (unsigned long long)
> avr->dccpavr_ack_ackno);
> dccp_ackvec_throw_record(av, avr);
> + break;
> }
> /*
> * If it wasn't received, continue scanning... we might
> -
[-- Attachment #2: ackvecfix.diff --]
[-- Type: text/x-patch, Size: 351 bytes --]
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index b5981e5..8c211c5 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -452,6 +452,7 @@ #endif
(unsigned long long)
avr->dccpavr_ack_ackno);
dccp_ackvec_throw_record(av, avr);
+ break;
}
/*
* If it wasn't received, continue scanning... we might
^ permalink raw reply related
* Re: [PATCH 1/1] DCCP Ackvec: fix soft lockup in ackvec handling
From: David Miller @ 2006-06-10 8:31 UTC (permalink / raw)
To: dccp
In-Reply-To: <20060609182649.GA20777@shorty.sorbonet.org>
From: "Ian McDonald" <ian.mcdonald@jandi.co.nz>
Date: Sat, 10 Jun 2006 20:28:39 +1200
> I have tested this and wonder whether it can be pushed upstream for
> Linus for 2.6.17 if at all possible. I know Arnaldo is the maintainer
> but as you are aware he is very busy with other projects at present.
I'll push this upstream over the weekend, it's in my patch
log :-)
^ permalink raw reply
* Re: [PATCH 1/1] DCCP Ackvec: fix soft lockup in ackvec handling code
From: Arnaldo Carvalho de Melo @ 2006-06-11 23:18 UTC (permalink / raw)
To: dccp
In-Reply-To: <20060609182649.GA20777@shorty.sorbonet.org>
David Miller wrote:
> From: "Ian McDonald" <ian.mcdonald@jandi.co.nz>
> Date: Sat, 10 Jun 2006 20:28:39 +1200
>
>> I have tested this and wonder whether it can be pushed upstream for
>> Linus for 2.6.17 if at all possible. I know Arnaldo is the maintainer
>> but as you are aware he is very busy with other projects at present.
>
> I'll push this upstream over the weekend, it's in my patch
> log :-)
Thanks everybody, real one liner, huh? :-)
- Arnaldo
^ permalink raw reply
* [PATCH 1/1] DCCP fix calculation of ackvec length
From: Andrea Bittau @ 2006-06-12 17:56 UTC (permalink / raw)
To: dccp
The length of the ack vector was incorrectly calculated upon the reception of an
"ack-of-an-ack". This would cause the length to become "negative" [i.e. the
integer would wrap] causing an incorrect ack vector which was too long.
Consequently, the ackvector could not be inserted into packets as an option [it
semply was too long] and all packets transmitted would get dropped. This
includes dropping pure ACKs. Eventually, even the sender on the other side
would stop because ACKs are not being sent by the receiver.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index 2349078..6e7d91c 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -354,11 +373,14 @@ static void dccp_ackvec_throw_record(str
{
struct dccp_ackvec_record *next;
- av->dccpav_buf_tail = avr->dccpavr_ack_ptr - 1;
- if (av->dccpav_buf_tail = 0)
- av->dccpav_buf_tail = DCCP_MAX_ACKVEC_LEN - 1;
-
- av->dccpav_vec_len -= avr->dccpavr_sent_len;
+ /* sort out vector length */
+ if (av->dccpav_buf_head <= avr->dccpavr_ack_ptr) {
+ av->dccpav_vec_len = avr->dccpavr_ack_ptr - av->dccpav_buf_head;
+ }
+ else {
+ av->dccpav_vec_len = DCCP_MAX_ACKVEC_LEN - 1 - av->dccpav_buf_head
+ + avr->dccpavr_ack_ptr;
+ }
/* free records */
list_for_each_entry_safe_from(avr, next, &av->dccpav_records,
^ permalink raw reply related
* Locking validator output on DCCP
From: Ian McDonald @ 2006-06-20 23:40 UTC (permalink / raw)
To: dccp
Folks,
I am getting this when I am using DCCP with 2.6.17-rc6-mm2 with Ingo's
lock dependency patch:
Jun 21 09:38:58 localhost kernel: [ 102.068588]
Jun 21 09:38:58 localhost kernel: [ 102.068592]
======================Jun 21 09:38:58 localhost kernel: [ 102.068602] [ INFO: possible
recursive locking detected ]
Jun 21 09:38:58 localhost kernel: [ 102.068608]
---------------------------------------------
Jun 21 09:38:58 localhost kernel: [ 102.068615] idle/0 is trying to
acquire lock:
Jun 21 09:38:58 localhost kernel: [ 102.068620]
(&sk->sk_lock.slock#3){-+..}, at: [<c0245511>] sk_clone+0x5a/0x190
Jun 21 09:38:58 localhost kernel: [ 102.068644]
Jun 21 09:38:58 localhost kernel: [ 102.068646] but task is already
holding lock:
Jun 21 09:38:58 localhost kernel: [ 102.068651]
(&sk->sk_lock.slock#3){-+..}, at: [<c024650d>]
sk_receive_skb+0xe6/0xfe
Jun 21 09:38:58 localhost kernel: [ 102.068668]
Jun 21 09:38:58 localhost kernel: [ 102.068670] other info that might
help us debug this:
Jun 21 09:38:58 localhost kernel: [ 102.068676] 2 locks held by idle/0:
Jun 21 09:38:58 localhost kernel: [ 102.068679] #0:
(&tp->rx_lock){-+..}, at: [<e087d915>] rtl8139_poll+0x42/0x41c
[8139too]
Jun 21 09:38:58 localhost kernel: [ 102.068722] #1:
(&sk->sk_lock.slock#3){-+..}, at: [<c024650d>]
sk_receive_skb+0xe6/0xfe
Jun 21 09:38:58 localhost kernel: [ 102.068739]
Jun 21 09:38:58 localhost kernel: [ 102.068741] stack backtrace:
Jun 21 09:38:58 localhost kernel: [ 102.069053] [<c0103a2a>]
show_trace_log_lvl+0x53/0xff
Jun 21 09:38:58 localhost kernel: [ 102.069091] [<c0104078>]
show_trace+0x16/0x19
Jun 21 09:38:58 localhost kernel: [ 102.069121] [<c010411e>]
dump_stack+0x1a/0x1f
Jun 21 09:38:58 localhost kernel: [ 102.069151] [<c012d6cb>]
__lock_acquire+0x8e6/0x902
Jun 21 09:38:58 localhost kernel: [ 102.069363] [<c012d879>]
lock_acquire+0x4e/0x66
Jun 21 09:38:58 localhost kernel: [ 102.069562] [<c029779d>]
_spin_lock+0x24/0x32
Jun 21 09:38:58 localhost kernel: [ 102.069777] [<c0245511>]
sk_clone+0x5a/0x190
Jun 21 09:38:58 localhost kernel: [ 102.071244] [<c026cc93>]
inet_csk_clone+0xf/0x67
Jun 21 09:38:58 localhost kernel: [ 102.072932] [<e0cc7f31>]
dccp_create_openreq_child+0x17/0x2fe [dccp]
Jun 21 09:38:58 localhost kernel: [ 102.072993] [<e0b48c9b>]
dccp_v4_request_recv_sock+0x47/0x260 [dccp_ipv4]
Jun 21 09:38:58 localhost kernel: [ 102.073020] [<e0cc8340>]
dccp_check_req+0x128/0x264 [dccp]
Jun 21 09:38:58 localhost kernel: [ 102.073049] [<e0b48a38>]
dccp_v4_do_rcv+0x74/0x290 [dccp_ipv4]
Jun 21 09:38:58 localhost kernel: [ 102.073067] [<c0246492>]
sk_receive_skb+0x6b/0xfe
Jun 21 09:38:58 localhost kernel: [ 102.074607] [<e0b49e9c>]
dccp_v4_rcv+0x4ea/0x66e [dccp_ipv4]
Jun 21 09:38:58 localhost kernel: [ 102.074651] [<c0265540>]
ip_local_deliver+0x159/0x1f1
Jun 21 09:38:58 localhost kernel: [ 102.076322] [<c02653ba>]
ip_rcv+0x3e9/0x416
Jun 21 09:38:58 localhost kernel: [ 102.077995] [<c024b768>]
netif_receive_skb+0x287/0x317
Jun 21 09:38:58 localhost kernel: [ 102.079562] [<e087db67>]
rtl8139_poll+0x294/0x41c [8139too]
Jun 21 09:38:58 localhost kernel: [ 102.079610] [<c024d149>]
net_rx_action+0x8b/0x17c
Jun 21 09:38:58 localhost kernel: [ 102.081181] [<c011adf6>]
__do_softirq+0x54/0xb3
Jun 21 09:38:58 localhost kernel: [ 102.081357] [<c011ae84>]
do_softirq+0x2f/0x47
Jun 21 09:38:58 localhost kernel: [ 102.081482] [<c011b0a5>]
irq_exit+0x39/0x46
Jun 21 09:38:58 localhost kernel: [ 102.081608] [<c0104f73>] do_IRQ+0x77/0x84
Jun 21 09:38:58 localhost kernel: [ 102.081644] [<c0103561>]
common_interrupt+0x25/0x2c
Jun 21 09:38:58 localhost kernel: [ 154.463644] CCID: Registered CCID 3 (ccid3)
The code of sk_clone (net/core/sock.c) is:
struct sock *sk_clone(const struct sock *sk, const gfp_t priority)
{
struct sock *newsk = sk_alloc(sk->sk_family, priority, sk->sk_prot, 0);
if (newsk != NULL) {
struct sk_filter *filter;
memcpy(newsk, sk, sk->sk_prot->obj_size);
/* SANITY */
sk_node_init(&newsk->sk_node);
sock_lock_init(newsk);
The relevant code is the sock_lock_init
The code of sk_receive_skb (net/core/sock.c) is:
int sk_receive_skb(struct sock *sk, struct sk_buff *skb)
{
int rc = NET_RX_SUCCESS;
if (sk_filter(sk, skb, 0))
goto discard_and_relse;
skb->dev = NULL;
bh_lock_sock(sk);
The relevant code is the bh_lock_sock.
As I read this it is not a recursive lock as sk_clone is occurring
second and is actually creating a new socket so they are trying to
lock on different sockets.
Can someone tell me whether I am correct in my thinking or not? If I
am then I will work out how to tell the lock validator not to worry
about it.
Thanks,
Ian
--
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand
^ permalink raw reply
* Re: Locking validator output on DCCP
From: Herbert Xu @ 2006-06-21 0:34 UTC (permalink / raw)
To: dccp
In-Reply-To: <5640c7e00606201640wa1ad17fl46a32ceb49513867@mail.gmail.com>
On Tue, Jun 20, 2006 at 11:40:05PM +0000, Ian McDonald wrote:
>
> I am getting this when I am using DCCP with 2.6.17-rc6-mm2 with Ingo's
> lock dependency patch:
>
> Jun 21 09:38:58 localhost kernel: [ 102.068588]
> Jun 21 09:38:58 localhost kernel: [ 102.068592]
> ======================> Jun 21 09:38:58 localhost kernel: [ 102.068602] [ INFO: possible
> recursive locking detected ]
> Jun 21 09:38:58 localhost kernel: [ 102.068608]
> ---------------------------------------------
> Jun 21 09:38:58 localhost kernel: [ 102.068615] idle/0 is trying to
> acquire lock:
> Jun 21 09:38:58 localhost kernel: [ 102.068620]
> (&sk->sk_lock.slock#3){-+..}, at: [<c0245511>] sk_clone+0x5a/0x190
> Jun 21 09:38:58 localhost kernel: [ 102.068644]
> Jun 21 09:38:58 localhost kernel: [ 102.068646] but task is already
> holding lock:
> Jun 21 09:38:58 localhost kernel: [ 102.068651]
> (&sk->sk_lock.slock#3){-+..}, at: [<c024650d>]
> sk_receive_skb+0xe6/0xfe
...
> As I read this it is not a recursive lock as sk_clone is occurring
> second and is actually creating a new socket so they are trying to
> lock on different sockets.
>
> Can someone tell me whether I am correct in my thinking or not? If I
> am then I will work out how to tell the lock validator not to worry
> about it.
I agree, this looks bogus. Ingo, could you please take a look?
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
-
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: Locking validator output on DCCP
From: Arjan van de Ven @ 2006-06-21 10:18 UTC (permalink / raw)
To: dccp
In-Reply-To: <5640c7e00606201640wa1ad17fl46a32ceb49513867@mail.gmail.com>
On Wed, 2006-06-21 at 10:34 +1000, Herbert Xu wrote:
> > As I read this it is not a recursive lock as sk_clone is occurring
> > second and is actually creating a new socket so they are trying to
> > lock on different sockets.
> >
> > Can someone tell me whether I am correct in my thinking or not? If I
> > am then I will work out how to tell the lock validator not to worry
> > about it.
>
> I agree, this looks bogus. Ingo, could you please take a look?
Fix is relatively easy:
sk_clone creates a new socket, and thus can never deadlock, and in fact
can be called with the original socket locked. This therefore is a
legitimate nesting case; mark it as such.
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
net/core/sock.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.17-rc6-mm2/net/core/sock.c
=================================--- linux-2.6.17-rc6-mm2.orig/net/core/sock.c
+++ linux-2.6.17-rc6-mm2/net/core/sock.c
@@ -846,7 +846,7 @@ struct sock *sk_clone(const struct sock
/* SANITY */
sk_node_init(&newsk->sk_node);
sock_lock_init(newsk);
- bh_lock_sock(newsk);
+ bh_lock_sock_nested(newsk);
atomic_set(&newsk->sk_rmem_alloc, 0);
atomic_set(&newsk->sk_wmem_alloc, 0);
^ permalink raw reply
* Re: Locking validator output on DCCP
From: Ingo Molnar @ 2006-06-21 10:52 UTC (permalink / raw)
To: dccp
In-Reply-To: <5640c7e00606201640wa1ad17fl46a32ceb49513867@mail.gmail.com>
* Herbert Xu <herbert@gondor.apana.org.au> wrote:
> > Can someone tell me whether I am correct in my thinking or not? If I
> > am then I will work out how to tell the lock validator not to worry
> > about it.
>
> I agree, this looks bogus. Ingo, could you please take a look?
sure - Ian, could you try Arjan's fix below?
Ingo
------------------------------------------------------------
Subject: lock validator: annotate vlan "master" device locks
From: Arjan van de Ven <arjan@linux.intel.com>
vlan devices are "masters" of normal "slave" devices and thus need their
own lock key. (this will be switched to the reinit_key APIs once they
are available)
Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
net/8021q/vlan.c | 2 ++
1 file changed, 2 insertions(+)
Index: linux-2.6.17-rc6-mm2/net/8021q/vlan.c
=================================--- linux-2.6.17-rc6-mm2.orig/net/8021q/vlan.c
+++ linux-2.6.17-rc6-mm2/net/8021q/vlan.c
@@ -463,6 +463,8 @@ static struct net_device *register_vlan_
if (new_dev = NULL)
goto out_unlock;
+ spin_lock_init(&dev->xmit_lock);
+
#ifdef VLAN_DEBUG
printk(VLAN_DBG "Allocated new name -:%s:-\n", new_dev->name);
#endif
^ permalink raw reply
* [PATCH 1/1] [DCCP]: Fix default sequence window size
From: Ian McDonald @ 2006-06-22 3:42 UTC (permalink / raw)
To: dccp
When using the default sequence window size (100) I got the following in
my logs:
Jun 22 14:24:09 localhost kernel: [ 1492.114775] DCCP: Step 6 failed for
DATA packet, (LSWL(6279674225) <= P.seqno(6279674749) <=
S.SWH(6279674324)) and (P.ackno doesn't exist or LAWL(18798206530) <=
P.ackno(1125899906842620) <= S.AWH(18798206548), sending SYNC...
Jun 22 14:24:09 localhost kernel: [ 1492.115147] DCCP: Step 6 failed for
DATA packet, (LSWL(6279674225) <= P.seqno(6279674750) <=
S.SWH(6279674324)) and (P.ackno doesn't exist or LAWL(18798206530) <=
P.ackno(1125899906842620) <= S.AWH(18798206549), sending SYNC...
I went to alter the default sysctl and it didn't take for new sockets.
Below patch fixes this.
I think the default is too low but it is what the DCCP spec specifies.
As a side effect of this my rx speed using iperf goes from about 2.8
Mbits/sec to 3.5. This is still far too slow but it is a step in the
right direction.
Compile tested only for IPv6 but not particularly complex change.
Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
---
diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
index f2c011f..3119804 100644
--- a/net/dccp/ipv4.c
+++ b/net/dccp/ipv4.c
@@ -32,6 +32,8 @@ #include "ccid.h"
#include "dccp.h"
#include "feat.h"
+extern int dccp_feat_default_sequence_window;
+
/*
* This is the global socket data structure used for responding to
* the Out-of-the-blue (OOTB) packets. A control sock will be created
@@ -505,8 +507,7 @@ int dccp_v4_conn_request(struct sock *sk
ireq = inet_rsk(req);
ireq->loc_addr = daddr;
ireq->rmt_addr = saddr;
- req->rcv_wnd = 100; /* Fake, option parsing will get the
- right value */
+ req->rcv_wnd = dccp_feat_default_sequence_window;
ireq->opt = NULL;
/*
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 65e2ab0..aa147f4 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -33,6 +33,8 @@ #include <net/xfrm.h>
#include "dccp.h"
#include "ipv6.h"
+extern int dccp_feat_default_sequence_window;
+
/* Socket used for sending RSTs and ACKs */
static struct socket *dccp_v6_ctl_socket;
@@ -708,8 +710,7 @@ static int dccp_v6_conn_request(struct s
ireq = inet_rsk(req);
ipv6_addr_copy(&ireq6->rmt_addr, &skb->nh.ipv6h->saddr);
ipv6_addr_copy(&ireq6->loc_addr, &skb->nh.ipv6h->daddr);
- req->rcv_wnd = 100; /* Fake, option parsing will get the
- right value */
+ req->rcv_wnd = dccp_feat_default_sequence_window;
ireq6->pktopts = NULL;
if (ipv6_opt_accepted(sk, skb) ||
diff --git a/net/dccp/options.c b/net/dccp/options.c
index e9feb2a..6409561 100644
--- a/net/dccp/options.c
+++ b/net/dccp/options.c
@@ -30,6 +30,8 @@ int dccp_feat_default_ack_ratio int dccp_feat_default_send_ack_vector = DCCPF_INITIAL_SEND_ACK_VECTOR;
int dccp_feat_default_send_ndp_count = DCCPF_INITIAL_SEND_NDP_COUNT;
+EXPORT_SYMBOL_GPL(dccp_feat_default_sequence_window);
+
void dccp_minisock_init(struct dccp_minisock *dmsk)
{
dmsk->dccpms_sequence_window = dccp_feat_default_sequence_window;
^ permalink raw reply related
* Re: [PATCH 1/1] [DCCP]: Fix default sequence window size
From: Arnaldo Carvalho de Melo @ 2006-06-22 3:48 UTC (permalink / raw)
To: dccp
In-Reply-To: <449A1199.1080404@jandi.co.nz>
On 6/22/06, Ian McDonald <ian.mcdonald@jandi.co.nz> wrote:
> When using the default sequence window size (100) I got the following in
> my logs:
> Jun 22 14:24:09 localhost kernel: [ 1492.114775] DCCP: Step 6 failed for
> DATA packet, (LSWL(6279674225) <= P.seqno(6279674749) <> S.SWH(6279674324)) and (P.ackno doesn't exist or LAWL(18798206530) <> P.ackno(1125899906842620) <= S.AWH(18798206548), sending SYNC...
> Jun 22 14:24:09 localhost kernel: [ 1492.115147] DCCP: Step 6 failed for
> DATA packet, (LSWL(6279674225) <= P.seqno(6279674750) <> S.SWH(6279674324)) and (P.ackno doesn't exist or LAWL(18798206530) <> P.ackno(1125899906842620) <= S.AWH(18798206549), sending SYNC...
>
> I went to alter the default sysctl and it didn't take for new sockets.
> Below patch fixes this.
>
> I think the default is too low but it is what the DCCP spec specifies.
>
> As a side effect of this my rx speed using iperf goes from about 2.8
> Mbits/sec to 3.5. This is still far too slow but it is a step in the
> right direction.
Agreed, minor nitpick below...
> Compile tested only for IPv6 but not particularly complex change.
>
> Signed-off-by: Ian McDonald <ian.mcdonald@jandi.co.nz>
>
> ---
>
> diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c
> index f2c011f..3119804 100644
> --- a/net/dccp/ipv4.c
> +++ b/net/dccp/ipv4.c
> @@ -32,6 +32,8 @@ #include "ccid.h"
> #include "dccp.h"
> #include "feat.h"
>
> +extern int dccp_feat_default_sequence_window;
> +
Could you please add this one in feat.h? Then I'll be OK with this one
and will ask Dave to merge it.
Thanks,
- Arnaldo
> /*
> * This is the global socket data structure used for responding to
> * the Out-of-the-blue (OOTB) packets. A control sock will be created
> @@ -505,8 +507,7 @@ int dccp_v4_conn_request(struct sock *sk
> ireq = inet_rsk(req);
> ireq->loc_addr = daddr;
> ireq->rmt_addr = saddr;
> - req->rcv_wnd = 100; /* Fake, option parsing will get the
> - right value */
> + req->rcv_wnd = dccp_feat_default_sequence_window;
> ireq->opt = NULL;
>
> /*
> diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
> index 65e2ab0..aa147f4 100644
> --- a/net/dccp/ipv6.c
> +++ b/net/dccp/ipv6.c
> @@ -33,6 +33,8 @@ #include <net/xfrm.h>
> #include "dccp.h"
> #include "ipv6.h"
>
> +extern int dccp_feat_default_sequence_window;
> +
See? :-)
^ permalink raw reply
* Re: Locking validator output on DCCP
From: Ian McDonald @ 2006-06-22 4:51 UTC (permalink / raw)
To: dccp
In-Reply-To: <5640c7e00606201640wa1ad17fl46a32ceb49513867@mail.gmail.com>
On 6/21/06, Arjan van de Ven <arjan@linux.intel.com> wrote:
> On Wed, 2006-06-21 at 10:34 +1000, Herbert Xu wrote:
> > > As I read this it is not a recursive lock as sk_clone is occurring
> > > second and is actually creating a new socket so they are trying to
> > > lock on different sockets.
> > >
> > > Can someone tell me whether I am correct in my thinking or not? If I
> > > am then I will work out how to tell the lock validator not to worry
> > > about it.
> >
> > I agree, this looks bogus. Ingo, could you please take a look?
>
> Fix is relatively easy:
>
>
> sk_clone creates a new socket, and thus can never deadlock, and in fact
> can be called with the original socket locked. This therefore is a
> legitimate nesting case; mark it as such.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
>
>
> ---
> net/core/sock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Index: linux-2.6.17-rc6-mm2/net/core/sock.c
> =================================> --- linux-2.6.17-rc6-mm2.orig/net/core/sock.c
> +++ linux-2.6.17-rc6-mm2/net/core/sock.c
> @@ -846,7 +846,7 @@ struct sock *sk_clone(const struct sock
> /* SANITY */
> sk_node_init(&newsk->sk_node);
> sock_lock_init(newsk);
> - bh_lock_sock(newsk);
> + bh_lock_sock_nested(newsk);
>
> atomic_set(&newsk->sk_rmem_alloc, 0);
> atomic_set(&newsk->sk_wmem_alloc, 0);
>
>
When I do this it now shifts around. I'll investigate further
(probably tomorrow).
Now get
Jun 22 14:20:48 localhost kernel: [ 1276.424531]
======================Jun 22 14:20:48 localhost kernel: [ 1276.424541] [ INFO: possible
recursive locking detected ]
Jun 22 14:20:48 localhost kernel: [ 1276.424546]
---------------------------------------------
Jun 22 14:20:48 localhost kernel: [ 1276.424553] idle/0 is trying to
acquire lock:
Jun 22 14:20:48 localhost kernel: [ 1276.424559]
(&sk->sk_lock.slock#5/1){-+..}, at: [<c024594e>] sk_clone+0x5f/0x195
Jun 22 14:20:48 localhost kernel: [ 1276.424585]
Jun 22 14:20:48 localhost kernel: [ 1276.424587] but task is already
holding lock:
Jun 22 14:20:48 localhost kernel: [ 1276.424592]
(&sk->sk_lock.slock#5/1){-+..}, at: [<c027cd87>]
tcp_v4_rcv+0x42e/0x9b3
Jun 22 14:20:48 localhost kernel: [ 1276.424616]
Jun 22 14:20:48 localhost kernel: [ 1276.424618] other info that might
help us debug this:
Jun 22 14:20:48 localhost kernel: [ 1276.424624] 2 locks held by idle/0:
Jun 22 14:20:48 localhost kernel: [ 1276.424628] #0:
(&tp->rx_lock){-+..}, at: [<e0898915>] rtl8139_poll+0x42/0x41c
[8139too]
Jun 22 14:20:48 localhost kernel: [ 1276.424666] #1:
(&sk->sk_lock.slock#5/1){-+..}, at: [<c027cd87>]
tcp_v4_rcv+0x42e/0x9b3
Jun 22 14:20:48 localhost kernel: [ 1276.424685]
Jun 22 14:20:48 localhost kernel: [ 1276.424686] stack backtrace:
Jun 22 14:20:48 localhost kernel: [ 1276.425002] [<c0103a2a>]
show_trace_log_lvl+0x53/0xff
Jun 22 14:20:48 localhost kernel: [ 1276.425038] [<c0104078>]
show_trace+0x16/0x19
Jun 22 14:20:48 localhost kernel: [ 1276.425068] [<c010411e>]
dump_stack+0x1a/0x1f
Jun 22 14:20:48 localhost kernel: [ 1276.425099] [<c012d6cb>]
__lock_acquire+0x8e6/0x902
Jun 22 14:20:48 localhost kernel: [ 1276.425311] [<c012d879>]
lock_acquire+0x4e/0x66
Jun 22 14:20:48 localhost kernel: [ 1276.425510] [<c02989e1>]
_spin_lock_nested+0x26/0x36
Jun 22 14:20:48 localhost kernel: [ 1276.425726] [<c024594e>]
sk_clone+0x5f/0x195
Jun 22 14:20:48 localhost kernel: [ 1276.427191] [<c026d10f>]
inet_csk_clone+0xf/0x67
Jun 22 14:20:48 localhost kernel: [ 1276.428879] [<c027d3d0>]
tcp_create_openreq_child+0x15/0x32b
Jun 22 14:20:48 localhost kernel: [ 1276.430598] [<c027b383>]
tcp_v4_syn_recv_sock+0x47/0x29c
Jun 22 14:20:48 localhost kernel: [ 1276.432313] [<e0fcf440>]
tcp_v6_syn_recv_sock+0x37/0x534 [ipv6]
Jun 22 14:20:48 localhost kernel: [ 1276.432482] [<c027d886>]
tcp_check_req+0x1a0/0x2db
Jun 22 14:20:48 localhost kernel: [ 1276.434198] [<c027aecc>]
tcp_v4_do_rcv+0x9f/0x2fe
Jun 22 14:20:48 localhost kernel: [ 1276.435911] [<c027d28b>]
tcp_v4_rcv+0x932/0x9b3
Jun 22 14:20:48 localhost kernel: [ 1276.437632] [<c0265980>]
ip_local_deliver+0x159/0x1f1
Jun 22 14:20:48 localhost kernel: [ 1276.439305] [<c02657fa>]
ip_rcv+0x3e9/0x416
Jun 22 14:20:48 localhost kernel: [ 1276.440977] [<c024bba4>]
netif_receive_skb+0x287/0x317
Jun 22 14:20:48 localhost kernel: [ 1276.442542] [<e0898b67>]
rtl8139_poll+0x294/0x41c [8139too]
Jun 22 14:20:48 localhost kernel: [ 1276.442590] [<c024d585>]
net_rx_action+0x8b/0x17c
Jun 22 14:20:48 localhost kernel: [ 1276.444160] [<c011adf6>]
__do_softirq+0x54/0xb3
Jun 22 14:20:48 localhost kernel: [ 1276.444335] [<c011ae84>]
do_softirq+0x2f/0x47
Jun 22 14:20:48 localhost kernel: [ 1276.444460] [<c011b0a5>]
irq_exit+0x39/0x46
Jun 22 14:20:48 localhost kernel: [ 1276.444585] [<c0104f73>] do_IRQ+0x77/0x84
Jun 22 14:20:48 localhost kernel: [ 1276.444621] [<c0103561>]
common_interrupt+0x25/0x2c
--
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand
^ permalink raw reply
* Re: Locking validator output on DCCP
From: Ian McDonald @ 2006-06-22 4:55 UTC (permalink / raw)
To: dccp
In-Reply-To: <5640c7e00606201640wa1ad17fl46a32ceb49513867@mail.gmail.com>
On 6/21/06, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> > > Can someone tell me whether I am correct in my thinking or not? If I
> > > am then I will work out how to tell the lock validator not to worry
> > > about it.
> >
> > I agree, this looks bogus. Ingo, could you please take a look?
>
> sure - Ian, could you try Arjan's fix below?
>
> Ingo
>
> ------------------------------------------------------------
> Subject: lock validator: annotate vlan "master" device locks
> From: Arjan van de Ven <arjan@linux.intel.com>
>
The fix you sent here was the incorrect one but I did test Arjan's as
per previous e-mail.
Real dumb question time. The lock validator is testing for recursive
lock holding. Given that this is a lock at a different address can we
eliminate all such cases? Or are you trying to detect code here that
keeps on locking same type of lock in case of error and we should
explicitly flag...
Ian
--
Ian McDonald
Web: http://wand.net.nz/~iam4
Blog: http://imcdnzl.blogspot.com
WAND Network Research Group
Department of Computer Science
University of Waikato
New Zealand
^ permalink raw reply
* Re: [PATCH 1/1] [DCCP]: Fix default sequence window size
From: Andrea Bittau @ 2006-06-22 7:54 UTC (permalink / raw)
To: dccp
In-Reply-To: <449A1199.1080404@jandi.co.nz>
> As a side effect of this my rx speed using iperf goes from about 2.8
> Mbits/sec to 3.5. This is still far too slow but it is a step in the
> right direction.
I got a bunch of fixes i need to commit. I'll try to send out patches this
week. They should make things go faster. What is your test setup like?
I was able to do 1Gb/s with 0rtt. As soon as I add delay with netem, I could
only do ~75Mbit/s. Still investigating why.
Also make sure you change the socket buffer wmem sysctl.
^ permalink raw reply
* mail voor linux mailinglist
From: Johannes Deleu @ 2006-06-22 12:41 UTC (permalink / raw)
To: dccp
Hi,
for a project, we're interested in using DCCP. As such, I did some
performance measurements with the latest version of the DCCP
implementation. To my opinion, they are not really what I expected.
Could you give some ideas or maybe some information about the current
status of the implementation (e.g. is packet loss detection planned for
implementation, when ?) ?
Results of similar tests are also interesting to see if they compare.
For the tests, I used an impairment node (based on linux, which
introduced some packet loss).
Measurements where performed with the patched iperf on linux-2.6.16.20
and I came to the following table:
TCP DCCP-CCID3 (Send/Recv)
No drop 94 Mbps 93.3 Mbps 93.2 Mbps
Drop 1% 83.4 Mbps 93.3 Mbps 92.3 Mbps
Drop 5% 16.6 Mbps 93.3 Mbps 88.6 Mbps
Drop 10% 3.25 Mbps 93.3 Mbps 83.9 Mbps
The transmitter does not detect the packet loss and doesnt slow down,
instead it keeps sending <CCID option 192 4294967295>.
As CCID3 is TFRC, I expected similar results for TCP and DCCP-CCID3.
When i tried out linux-2.6.17.1 (no packet drop) the receiver seems to
stop sending feedback packets after a while and the transmitter slows
down. The receiving process doesnt receive any data (recv doesnt return).
[ 3] local 10.0.1.3 port 3743 connected with 10.0.1.1 port 5001
[ 3] 0.0- 1.0 sec 11.0 MBytes 92.5 Mbits/sec
[ 3] 1.0- 2.0 sec 10.9 MBytes 91.6 Mbits/sec
[ 3] 2.0- 3.0 sec 10.9 MBytes 91.6 Mbits/sec
[ 3] 3.0- 4.0 sec 10.9 MBytes 91.6 Mbits/sec
[ 3] 4.0- 5.0 sec 10.9 MBytes 91.6 Mbits/sec
[ 3] 5.0- 6.0 sec 10.9 MBytes 91.6 Mbits/sec
[ 3] 6.0- 7.0 sec 6.86 MBytes 57.5 Mbits/sec
[ 3] 7.0- 8.0 sec 194 KBytes 1.59 Mbits/sec
[ 3] 8.0- 9.0 sec 100 KBytes 819 Kbits/sec
[ 3] 9.0-10.0 sec 100 KBytes 819 Kbits/sec
[ 3] 10.0-11.0 sec 100 KBytes 819 Kbits/sec
[ 3] 11.0-12.0 sec 100 KBytes 819 Kbits/sec
[ 3] 12.0-13.0 sec 100 KBytes 819 Kbits/sec
best regards,
Johannes
^ permalink raw reply
* [PATCH 01/11] ACKVEC: fix ackvector length calculation
From: Andrea Bittau @ 2006-06-22 17:51 UTC (permalink / raw)
To: dccp
Fix ackvector length calculation upon receiving an "ack-of-ack". This patch
aviods the ackvector from growing too large which causes it to not be inserted
into packets.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index 8c211c5..8dab723 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -353,11 +353,13 @@ static void dccp_ackvec_throw_record(str
{
struct dccp_ackvec_record *next;
- av->dccpav_buf_tail = avr->dccpavr_ack_ptr - 1;
- if (av->dccpav_buf_tail = 0)
- av->dccpav_buf_tail = DCCP_MAX_ACKVEC_LEN - 1;
-
- av->dccpav_vec_len -= avr->dccpavr_sent_len;
+ /* sort out vector length */
+ if (av->dccpav_buf_head <= avr->dccpavr_ack_ptr)
+ av->dccpav_vec_len = avr->dccpavr_ack_ptr - av->dccpav_buf_head;
+ else
+ av->dccpav_vec_len = DCCP_MAX_ACKVEC_LEN - 1
+ - av->dccpav_buf_head
+ + avr->dccpavr_ack_ptr;
/* free records */
list_for_each_entry_safe_from(avr, next, &av->dccpav_records,
^ permalink raw reply related
* [PATCH 02/11] DCCP: Fix how DCCP_ACKVEC_STATE_NOT_RECEIVED is used
From: Andrea Bittau @ 2006-06-22 17:53 UTC (permalink / raw)
To: dccp
Fix the way state is masked out. DCCP_ACKVEC_STATE_NOT_RECEIVED is defined as
appears in the packet, therefore bit shifting is not required. This fix allows
CCID2 to correctly detect losses.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index 8dab723..bc5ff12 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -436,8 +436,7 @@ static void dccp_ackvec_check_rcv_ackvec
break;
found:
if (between48(avr->dccpavr_ack_seqno, ackno_end_rl, ackno)) {
- const u8 state = (*vector &
- DCCP_ACKVEC_STATE_MASK) >> 6;
+ const u8 state = *vector & DCCP_ACKVEC_STATE_MASK;
if (state != DCCP_ACKVEC_STATE_NOT_RECEIVED) {
#ifdef CONFIG_IP_DCCP_DEBUG
struct dccp_sock *dp = dccp_sk(sk);
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index d4f9e2d..6fe9cb9 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -583,8 +583,8 @@ static void ccid2_hc_tx_packet_recv(stru
* run length
*/
while (between48(seqp->ccid2s_seq,ackno_end_rl,ackno)) {
- const u8 state = (*vector &
- DCCP_ACKVEC_STATE_MASK) >> 6;
+ const u8 state = *vector &
+ DCCP_ACKVEC_STATE_MASK;
/* new packet received or marked */
if (state != DCCP_ACKVEC_STATE_NOT_RECEIVED &&
^ permalink raw reply related
* [PATCH 03/11] ACKVEC: remove unused variables
From: Andrea Bittau @ 2006-06-22 17:53 UTC (permalink / raw)
To: dccp
Get rid of unused variables in ackvector state.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ackvec.c b/net/dccp/ackvec.c
index bc5ff12..4d176d3 100644
--- a/net/dccp/ackvec.c
+++ b/net/dccp/ackvec.c
@@ -142,14 +142,13 @@ struct dccp_ackvec *dccp_ackvec_alloc(co
struct dccp_ackvec *av = kmem_cache_alloc(dccp_ackvec_slab, priority);
if (av != NULL) {
- av->dccpav_buf_head - av->dccpav_buf_tail = DCCP_MAX_ACKVEC_LEN - 1;
+ av->dccpav_buf_head = DCCP_MAX_ACKVEC_LEN - 1;
av->dccpav_buf_ackno = DCCP_MAX_SEQNO + 1;
av->dccpav_buf_nonce = av->dccpav_buf_nonce = 0;
av->dccpav_ack_ptr = 0;
av->dccpav_time.tv_sec = 0;
av->dccpav_time.tv_usec = 0;
- av->dccpav_sent_len = av->dccpav_vec_len = 0;
+ av->dccpav_vec_len = 0;
INIT_LIST_HEAD(&av->dccpav_records);
}
diff --git a/net/dccp/ackvec.h b/net/dccp/ackvec.h
index ec7a89b..ec9a988 100644
--- a/net/dccp/ackvec.h
+++ b/net/dccp/ackvec.h
@@ -55,9 +55,7 @@ struct dccp_ackvec {
struct list_head dccpav_records;
struct timeval dccpav_time;
u8 dccpav_buf_head;
- u8 dccpav_buf_tail;
u8 dccpav_ack_ptr;
- u8 dccpav_sent_len;
u8 dccpav_vec_len;
u8 dccpav_buf_nonce;
u8 dccpav_ack_nonce;
@@ -108,7 +106,7 @@ extern int dccp_insert_option_ackvec(str
static inline int dccp_ackvec_pending(const struct dccp_ackvec *av)
{
- return av->dccpav_sent_len != av->dccpav_vec_len;
+ return av->dccpav_vec_len;
}
#else /* CONFIG_IP_DCCP_ACKVEC */
static inline int dccp_ackvec_init(void)
^ permalink raw reply related
* [PATCH 04/11] CCID2: Fix jiffie wrap issues
From: Andrea Bittau @ 2006-06-22 17:54 UTC (permalink / raw)
To: dccp
Jiffies are now handled correctly (I hope) in CCID2. If they wrap, no problem.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index 6fe9cb9..f953016 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -27,7 +27,6 @@
*
* BUGS:
* - sequence number wrapping
- * - jiffies wrapping
*/
#include <linux/config.h>
@@ -72,7 +71,8 @@ static void ccid2_hc_tx_check_sanity(con
/* packets are sent sequentially */
BUG_ON(seqp->ccid2s_seq <= prev->ccid2s_seq);
- BUG_ON(seqp->ccid2s_sent < prev->ccid2s_sent);
+ BUG_ON(time_before(seqp->ccid2s_sent,
+ prev->ccid2s_sent));
BUG_ON(len > ccid2_seq_len);
seqp = prev;
@@ -419,8 +419,9 @@ static inline void ccid2_new_ack(struct
/* update RTO */
if (hctx->ccid2hctx_srtt = -1 ||
- (jiffies - hctx->ccid2hctx_lastrtt) >= hctx->ccid2hctx_srtt) {
- unsigned long r = jiffies - seqp->ccid2s_sent;
+ ((long)jiffies - (long)hctx->ccid2hctx_lastrtt) >+ hctx->ccid2hctx_srtt) {
+ unsigned long r = (long)jiffies - (long)seqp->ccid2s_sent;
int s;
/* first measurement */
^ permalink raw reply related
* [PATCH 05/11] CCID2: Allocate seq records on demand
From: Andrea Bittau @ 2006-06-22 17:55 UTC (permalink / raw)
To: dccp
Allocate more sequence state on demand. Each time a packet is sent out by
CCID2, a record of it needs to be kept. This list of records grows
proportionally to cwnd. Previously, the length of this list was hardcored and
therefore the cwnd could only grow to this value (of 128). Now, records are
allocated on demand as necessary---cwnd may grow as it wishes. The exceptional
case of when memory is not available is not handled gracefully. Perhaps, cwnd
should be capped at that point.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index f953016..72fdc26 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -46,8 +46,6 @@ #else
#define ccid2_pr_debug(format, a...)
#endif
-static const int ccid2_seq_len = 128;
-
#ifdef CCID2_DEBUG
static void ccid2_hc_tx_check_sanity(const struct ccid2_hc_tx_sock *hctx)
{
@@ -73,7 +71,6 @@ static void ccid2_hc_tx_check_sanity(con
BUG_ON(seqp->ccid2s_seq <= prev->ccid2s_seq);
BUG_ON(time_before(seqp->ccid2s_sent,
prev->ccid2s_sent));
- BUG_ON(len > ccid2_seq_len);
seqp = prev;
}
@@ -85,16 +82,56 @@ static void ccid2_hc_tx_check_sanity(con
do {
seqp = seqp->ccid2s_prev;
len++;
- BUG_ON(len > ccid2_seq_len);
} while (seqp != hctx->ccid2hctx_seqh);
- BUG_ON(len != ccid2_seq_len);
ccid2_pr_debug("total len=%d\n", len);
+ BUG_ON(len != hctx->ccid2hctx_seqbufc*CCID2_SEQBUF_LEN);
}
#else
#define ccid2_hc_tx_check_sanity(hctx) do {} while (0)
#endif
+static int ccid2_hc_tx_alloc_seq(struct ccid2_hc_tx_sock *hctx, int num)
+{
+ struct ccid2_seq *seqp;
+ int i;
+
+ /* check if we have space to preserve the pointer to the buffer */
+ if (hctx->ccid2hctx_seqbufc >= (sizeof(hctx->ccid2hctx_seqbuf) /
+ sizeof(struct ccid2_seq*)))
+ return -ENOMEM;
+
+ /* allocate buffer and initialize linked list */
+ seqp = kmalloc(sizeof(*seqp) * num, gfp_any());
+ if (seqp = NULL)
+ return -ENOMEM;
+
+ for (i = 0; i < (num - 1); i++) {
+ seqp[i].ccid2s_next = &seqp[i + 1];
+ seqp[i + 1].ccid2s_prev = &seqp[i];
+ }
+ seqp[num - 1].ccid2s_next = seqp;
+ seqp->ccid2s_prev = &seqp[num - 1];
+
+ /* This is the first allocation. Initiate the head and tail. */
+ if (hctx->ccid2hctx_seqbufc = 0)
+ hctx->ccid2hctx_seqh = hctx->ccid2hctx_seqt = seqp;
+ else {
+ /* link the existing list with the one we just created */
+ hctx->ccid2hctx_seqh->ccid2s_next = seqp;
+ seqp->ccid2s_prev = hctx->ccid2hctx_seqh;
+
+ hctx->ccid2hctx_seqt->ccid2s_prev = &seqp[num - 1];
+ seqp[num - 1].ccid2s_next = hctx->ccid2hctx_seqt;
+ }
+
+ /* store the original pointer to the buffer so we can free it */
+ hctx->ccid2hctx_seqbuf[hctx->ccid2hctx_seqbufc] = seqp;
+ hctx->ccid2hctx_seqbufc++;
+
+ return 0;
+}
+
static int ccid2_hc_tx_send_packet(struct sock *sk,
struct sk_buff *skb, int len)
{
@@ -233,6 +270,7 @@ static void ccid2_hc_tx_packet_sent(stru
{
struct dccp_sock *dp = dccp_sk(sk);
struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
+ struct ccid2_seq *next;
u64 seq;
ccid2_hc_tx_check_sanity(hctx);
@@ -252,16 +290,24 @@ static void ccid2_hc_tx_packet_sent(stru
hctx->ccid2hctx_seqh->ccid2s_seq = seq;
hctx->ccid2hctx_seqh->ccid2s_acked = 0;
hctx->ccid2hctx_seqh->ccid2s_sent = jiffies;
- hctx->ccid2hctx_seqh = hctx->ccid2hctx_seqh->ccid2s_next;
+
+ next = hctx->ccid2hctx_seqh->ccid2s_next;
+ /* check if we need to alloc more space */
+ if (next = hctx->ccid2hctx_seqt) {
+ int rc;
+
+ ccid2_pr_debug("allocating more space in history\n");
+ rc = ccid2_hc_tx_alloc_seq(hctx, CCID2_SEQBUF_LEN);
+ BUG_ON(rc); /* XXX what do we do? */
+
+ next = hctx->ccid2hctx_seqh->ccid2s_next;
+ BUG_ON(next = hctx->ccid2hctx_seqt);
+ }
+ hctx->ccid2hctx_seqh = next;
ccid2_pr_debug("cwnd=%d pipe=%d\n", hctx->ccid2hctx_cwnd,
hctx->ccid2hctx_pipe);
- if (hctx->ccid2hctx_seqh = hctx->ccid2hctx_seqt) {
- /* XXX allocate more space */
- WARN_ON(1);
- }
-
hctx->ccid2hctx_sent++;
/* Ack Ratio. Need to maintain a concept of how many windows we sent */
@@ -677,33 +723,17 @@ static void ccid2_hc_tx_packet_recv(stru
static int ccid2_hc_tx_init(struct ccid *ccid, struct sock *sk)
{
struct ccid2_hc_tx_sock *hctx = ccid_priv(ccid);
- int seqcount = ccid2_seq_len;
- int i;
/* XXX init variables with proper values */
hctx->ccid2hctx_cwnd = 1;
hctx->ccid2hctx_ssthresh = 10;
hctx->ccid2hctx_numdupack = 3;
+ hctx->ccid2hctx_seqbufc = 0;
/* XXX init ~ to window size... */
- hctx->ccid2hctx_seqbuf = kmalloc(sizeof(*hctx->ccid2hctx_seqbuf) *
- seqcount, gfp_any());
- if (hctx->ccid2hctx_seqbuf = NULL)
+ if (ccid2_hc_tx_alloc_seq(hctx, CCID2_SEQBUF_LEN))
return -ENOMEM;
- for (i = 0; i < (seqcount - 1); i++) {
- hctx->ccid2hctx_seqbuf[i].ccid2s_next - &hctx->ccid2hctx_seqbuf[i + 1];
- hctx->ccid2hctx_seqbuf[i + 1].ccid2s_prev - &hctx->ccid2hctx_seqbuf[i];
- }
- hctx->ccid2hctx_seqbuf[seqcount - 1].ccid2s_next - hctx->ccid2hctx_seqbuf;
- hctx->ccid2hctx_seqbuf->ccid2s_prev - &hctx->ccid2hctx_seqbuf[seqcount - 1];
-
- hctx->ccid2hctx_seqh = hctx->ccid2hctx_seqbuf;
- hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh;
hctx->ccid2hctx_sent = 0;
hctx->ccid2hctx_rto = 3 * HZ;
hctx->ccid2hctx_srtt = -1;
@@ -722,10 +752,13 @@ static int ccid2_hc_tx_init(struct ccid
static void ccid2_hc_tx_exit(struct sock *sk)
{
struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
+ int i;
ccid2_hc_tx_kill_rto_timer(sk);
- kfree(hctx->ccid2hctx_seqbuf);
- hctx->ccid2hctx_seqbuf = NULL;
+
+ for (i = 0; i < hctx->ccid2hctx_seqbufc; i++)
+ kfree(hctx->ccid2hctx_seqbuf[i]);
+ hctx->ccid2hctx_seqbufc = 0;
}
static void ccid2_hc_rx_packet_recv(struct sock *sk, struct sk_buff *skb)
diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h
index 451a874..1a7904e 100644
--- a/net/dccp/ccids/ccid2.h
+++ b/net/dccp/ccids/ccid2.h
@@ -35,6 +35,9 @@ struct ccid2_seq {
struct ccid2_seq *ccid2s_next;
};
+#define CCID2_SEQBUF_LEN 256
+#define CCID2_SEQBUF_MAX 128
+
/** struct ccid2_hc_tx_sock - CCID2 TX half connection
*
* @ccid2hctx_ssacks - ACKs recv in slow start
@@ -53,7 +56,8 @@ struct ccid2_hc_tx_sock {
int ccid2hctx_ssthresh;
int ccid2hctx_pipe;
int ccid2hctx_numdupack;
- struct ccid2_seq *ccid2hctx_seqbuf;
+ struct ccid2_seq *ccid2hctx_seqbuf[CCID2_SEQBUF_MAX];
+ int ccid2hctx_seqbufc;
struct ccid2_seq *ccid2hctx_seqh;
struct ccid2_seq *ccid2hctx_seqt;
long ccid2hctx_rto;
^ permalink raw reply related
* [PATCH 06/11] CCID2: Initialize ssthresh to "infinity"
From: Andrea Bittau @ 2006-06-22 17:56 UTC (permalink / raw)
To: dccp
Initialize the slow-start threshold to "infinity". This way, upon connection
initiation, slow-start will be exited only upon a packet loss. This patch will
allow connections to quickly gain speed.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index 72fdc26..c11201b 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -724,9 +724,8 @@ static int ccid2_hc_tx_init(struct ccid
{
struct ccid2_hc_tx_sock *hctx = ccid_priv(ccid);
- /* XXX init variables with proper values */
hctx->ccid2hctx_cwnd = 1;
- hctx->ccid2hctx_ssthresh = 10;
+ hctx->ccid2hctx_ssthresh = 666666; /* "infinite" [for a while] */
hctx->ccid2hctx_numdupack = 3;
hctx->ccid2hctx_seqbufc = 0;
^ permalink raw reply related
* [PATCH 07/11] CCID2: Tell DCCP to quickly check whether cwnd is available
From: Andrea Bittau @ 2006-06-22 17:57 UTC (permalink / raw)
To: dccp
If not enough cwnd is available, tell the sender to check again as soon as
possible. This will increase CPU utilization (polling frequently for cwnd) but
will improve network performance. That is, the sender will need to wait less
before detecting the increase of cwnd. A better architecture would be for the
CCID to call-back (or dequeue) from DCCP when it is able to transmit
traffic---not the other way around as it currently occurs.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index c11201b..b2d140f 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -160,7 +160,7 @@ static int ccid2_hc_tx_send_packet(struc
}
}
- return 100; /* XXX */
+ return 1; /* XXX CCID should dequeue when ready instead of polling */
}
static void ccid2_change_l_ack_ratio(struct sock *sk, int val)
^ permalink raw reply related
* [PATCH 08/11] CCID2: Halve cwnd once upon multiple losses in a single RTT
From: Andrea Bittau @ 2006-06-22 17:58 UTC (permalink / raw)
To: dccp
When multiple losses occur in one RTT, the window should be halved only once [a
single "congestion event"]. This is now implemented, although not perfectly.
Slightly changed the interface for changing the cwnd: pass hctx instead of dp.
This is required in order to allow for change_cwnd to be called from _init().
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index b2d140f..f0bc0b6 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -188,10 +188,8 @@ static void ccid2_change_l_ack_ratio(str
dp->dccps_l_ack_ratio = val;
}
-static void ccid2_change_cwnd(struct sock *sk, int val)
+static void ccid2_change_cwnd(struct ccid2_hc_tx_sock *hctx, int val)
{
- struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
-
if (val = 0)
val = 1;
@@ -235,7 +233,7 @@ static void ccid2_hc_tx_rto_expire(unsig
hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd >> 1;
if (hctx->ccid2hctx_ssthresh < 2)
hctx->ccid2hctx_ssthresh = 2;
- ccid2_change_cwnd(sk, 1);
+ ccid2_change_cwnd(hctx, 1);
/* clear state about stuff we sent */
hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqh;
@@ -445,7 +443,7 @@ static inline void ccid2_new_ack(struct
/* increase every 2 acks */
hctx->ccid2hctx_ssacks++;
if (hctx->ccid2hctx_ssacks = 2) {
- ccid2_change_cwnd(sk, hctx->ccid2hctx_cwnd + 1);
+ ccid2_change_cwnd(hctx, hctx->ccid2hctx_cwnd+1);
hctx->ccid2hctx_ssacks = 0;
*maxincr = *maxincr - 1;
}
@@ -458,7 +456,7 @@ static inline void ccid2_new_ack(struct
hctx->ccid2hctx_acks++;
if (hctx->ccid2hctx_acks >= hctx->ccid2hctx_cwnd) {
- ccid2_change_cwnd(sk, hctx->ccid2hctx_cwnd + 1);
+ ccid2_change_cwnd(hctx, hctx->ccid2hctx_cwnd + 1);
hctx->ccid2hctx_acks = 0;
}
}
@@ -534,6 +532,22 @@ static void ccid2_hc_tx_dec_pipe(struct
ccid2_hc_tx_kill_rto_timer(sk);
}
+static void ccid2_congestion_event(struct ccid2_hc_tx_sock *hctx,
+ struct ccid2_seq *seqp)
+{
+ if (time_before(seqp->ccid2s_sent, hctx->ccid2hctx_last_cong)) {
+ dccp_pr_debug("Multiple losses in one RTT---treating as one\n");
+ return;
+ }
+
+ hctx->ccid2hctx_last_cong = jiffies;
+
+ ccid2_change_cwnd(hctx, hctx->ccid2hctx_cwnd >> 1);
+ hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd;
+ if (hctx->ccid2hctx_ssthresh < 2)
+ hctx->ccid2hctx_ssthresh = 2;
+}
+
static void ccid2_hc_tx_packet_recv(struct sock *sk, struct sk_buff *skb)
{
struct dccp_sock *dp = dccp_sk(sk);
@@ -544,7 +558,6 @@ static void ccid2_hc_tx_packet_recv(stru
unsigned char veclen;
int offset = 0;
int done = 0;
- int loss = 0;
unsigned int maxincr = 0;
ccid2_hc_tx_check_sanity(hctx);
@@ -638,7 +651,8 @@ static void ccid2_hc_tx_packet_recv(stru
!seqp->ccid2s_acked) {
if (state =
DCCP_ACKVEC_STATE_ECN_MARKED) {
- loss = 1;
+ ccid2_congestion_event(hctx,
+ seqp);
} else
ccid2_new_ack(sk, seqp,
&maxincr);
@@ -690,7 +704,13 @@ static void ccid2_hc_tx_packet_recv(stru
/* check for lost packets */
while (1) {
if (!seqp->ccid2s_acked) {
- loss = 1;
+ ccid2_pr_debug("Packet lost: %llu\n",
+ seqp->ccid2s_seq);
+ /* XXX need to traverse from tail -> head in
+ * order to detect multiple congestion events in
+ * one ack vector.
+ */
+ ccid2_congestion_event(hctx, seqp);
ccid2_hc_tx_dec_pipe(sk);
}
if (seqp = hctx->ccid2hctx_seqt)
@@ -709,14 +729,6 @@ static void ccid2_hc_tx_packet_recv(stru
hctx->ccid2hctx_seqt = hctx->ccid2hctx_seqt->ccid2s_next;
}
- if (loss) {
- /* XXX do bit shifts guarantee a 0 as the new bit? */
- ccid2_change_cwnd(sk, hctx->ccid2hctx_cwnd >> 1);
- hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd;
- if (hctx->ccid2hctx_ssthresh < 2)
- hctx->ccid2hctx_ssthresh = 2;
- }
-
ccid2_hc_tx_check_sanity(hctx);
}
@@ -724,7 +736,7 @@ static int ccid2_hc_tx_init(struct ccid
{
struct ccid2_hc_tx_sock *hctx = ccid_priv(ccid);
- hctx->ccid2hctx_cwnd = 1;
+ ccid2_change_cwnd(hctx, 1);
hctx->ccid2hctx_ssthresh = 666666; /* "infinite" [for a while] */
hctx->ccid2hctx_numdupack = 3;
hctx->ccid2hctx_seqbufc = 0;
@@ -739,6 +751,7 @@ static int ccid2_hc_tx_init(struct ccid
hctx->ccid2hctx_rttvar = -1;
hctx->ccid2hctx_lastrtt = 0;
hctx->ccid2hctx_rpdupack = -1;
+ hctx->ccid2hctx_last_cong = jiffies;
hctx->ccid2hctx_rtotimer.function = &ccid2_hc_tx_rto_expire;
hctx->ccid2hctx_rtotimer.data = (unsigned long)sk;
diff --git a/net/dccp/ccids/ccid2.h b/net/dccp/ccids/ccid2.h
index 1a7904e..3bc4b76 100644
--- a/net/dccp/ccids/ccid2.h
+++ b/net/dccp/ccids/ccid2.h
@@ -71,6 +71,7 @@ struct ccid2_hc_tx_sock {
u64 ccid2hctx_rpseq;
int ccid2hctx_rpdupack;
int ccid2hctx_sendwait;
+ unsigned long ccid2hctx_last_cong;
};
struct ccid2_hc_rx_sock {
^ permalink raw reply related
* [PATCH 09/11] CCID2: Add Kconfig option for CCID2 debug
From: Andrea Bittau @ 2006-06-22 17:59 UTC (permalink / raw)
To: dccp
Allow the user to choose whether or not to enable CCID2 debugging via Kconfig.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/Kconfig b/net/dccp/ccids/Kconfig
index ca00191..25fb4f6 100644
--- a/net/dccp/ccids/Kconfig
+++ b/net/dccp/ccids/Kconfig
@@ -30,6 +30,14 @@ config IP_DCCP_CCID2
If in doubt, say M.
+config IP_DCCP_CCID2_DEBUG
+ bool "CCID2 debug"
+ depends on IP_DCCP_CCID2
+ ---help---
+ Enable CCID2 debug messages.
+
+ If in doubt, say N.
+
config IP_DCCP_CCID3
tristate "CCID3 (TCP-Friendly) (EXPERIMENTAL)"
depends on IP_DCCP
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index f0bc0b6..db0f1f1 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -36,8 +36,7 @@ #include "ccid2.h"
static int ccid2_debug;
-#undef CCID2_DEBUG
-#ifdef CCID2_DEBUG
+#ifdef CONFIG_IP_DCCP_CCID2_DEBUG
#define ccid2_pr_debug(format, a...) \
do { if (ccid2_debug) \
printk(KERN_DEBUG "%s: " format, __FUNCTION__, ##a); \
@@ -46,7 +45,7 @@ #else
#define ccid2_pr_debug(format, a...)
#endif
-#ifdef CCID2_DEBUG
+#ifdef CONFIG_IP_DCCP_CCID2_DEBUG
static void ccid2_hc_tx_check_sanity(const struct ccid2_hc_tx_sock *hctx)
{
int len = 0;
@@ -340,7 +339,7 @@ static void ccid2_hc_tx_packet_sent(stru
if (!timer_pending(&hctx->ccid2hctx_rtotimer))
ccid2_start_rto_timer(sk);
-#ifdef CCID2_DEBUG
+#ifdef CONFIG_IP_DCCP_CCID2_DEBUG
ccid2_pr_debug("pipe=%d\n", hctx->ccid2hctx_pipe);
ccid2_pr_debug("Sent: seq=%llu\n", seq);
do {
^ permalink raw reply related
* [PATCH 10/11] CCID2: Add helper functions for changing important CCID2 state
From: Andrea Bittau @ 2006-06-22 18:00 UTC (permalink / raw)
To: dccp
Introduce methods which manipulate interesting congestion control state such as
pipe and rtt estimate. This is useful for people wishing to monitor the
variables of CCID and instrument the code [perhaps using Kprobes]. Personally,
I am a fan of encapsulation---that justifies this change =D.
Signed-off-by: Andrea Bittau <a.bittau@cs.ucl.ac.uk>
---
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index db0f1f1..b6f6681 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -199,6 +199,21 @@ static void ccid2_change_cwnd(struct cci
hctx->ccid2hctx_cwnd = val;
}
+static void ccid2_change_srtt(struct ccid2_hc_tx_sock *hctx, long val)
+{
+ ccid2_pr_debug("change SRTT to %ld\n", val);
+
+ if (hctx->ccid2hctx_srtt = val)
+ return;
+
+ hctx->ccid2hctx_srtt = val;
+}
+
+static void ccid2_change_pipe(struct ccid2_hc_tx_sock *hctx, long val)
+{
+ hctx->ccid2hctx_pipe = val;
+}
+
static void ccid2_start_rto_timer(struct sock *sk);
static void ccid2_hc_tx_rto_expire(unsigned long data)
@@ -228,7 +243,7 @@ static void ccid2_hc_tx_rto_expire(unsig
ccid2_start_rto_timer(sk);
/* adjust pipe, cwnd etc */
- hctx->ccid2hctx_pipe = 0;
+ ccid2_change_pipe(hctx, 0);
hctx->ccid2hctx_ssthresh = hctx->ccid2hctx_cwnd >> 1;
if (hctx->ccid2hctx_ssthresh < 2)
hctx->ccid2hctx_ssthresh = 2;
@@ -274,7 +289,7 @@ static void ccid2_hc_tx_packet_sent(stru
BUG_ON(!hctx->ccid2hctx_sendwait);
hctx->ccid2hctx_sendwait = 0;
- hctx->ccid2hctx_pipe++;
+ ccid2_change_pipe(hctx, hctx->ccid2hctx_pipe+1);
BUG_ON(hctx->ccid2hctx_pipe < 0);
/* There is an issue. What if another packet is sent between
@@ -471,11 +486,13 @@ static inline void ccid2_new_ack(struct
if (hctx->ccid2hctx_srtt = -1) {
ccid2_pr_debug("R: %lu Time=%lu seq=%llu\n",
r, jiffies, seqp->ccid2s_seq);
- hctx->ccid2hctx_srtt = r;
+ ccid2_change_srtt(hctx, r);
hctx->ccid2hctx_rttvar = r >> 1;
} else {
/* RTTVAR */
long tmp = hctx->ccid2hctx_srtt - r;
+ long srtt;
+
if (tmp < 0)
tmp *= -1;
@@ -485,10 +502,12 @@ static inline void ccid2_new_ack(struct
hctx->ccid2hctx_rttvar += tmp;
/* SRTT */
- hctx->ccid2hctx_srtt *= 7;
- hctx->ccid2hctx_srtt >>= 3;
+ srtt = hctx->ccid2hctx_srtt;
+ srtt *= 7;
+ srtt >>= 3;
tmp = r >> 3;
- hctx->ccid2hctx_srtt += tmp;
+ srtt += tmp;
+ ccid2_change_srtt(hctx, srtt);
}
s = hctx->ccid2hctx_rttvar << 2;
/* clock granularity is 1 when based on jiffies */
@@ -524,7 +543,7 @@ static void ccid2_hc_tx_dec_pipe(struct
{
struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
- hctx->ccid2hctx_pipe--;
+ ccid2_change_pipe(hctx, hctx->ccid2hctx_pipe-1);
BUG_ON(hctx->ccid2hctx_pipe < 0);
if (hctx->ccid2hctx_pipe = 0)
@@ -746,7 +765,7 @@ static int ccid2_hc_tx_init(struct ccid
hctx->ccid2hctx_sent = 0;
hctx->ccid2hctx_rto = 3 * HZ;
- hctx->ccid2hctx_srtt = -1;
+ ccid2_change_srtt(hctx, -1);
hctx->ccid2hctx_rttvar = -1;
hctx->ccid2hctx_lastrtt = 0;
hctx->ccid2hctx_rpdupack = -1;
^ permalink raw reply related
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