* Re: [BUG] Performance issues if DCCP packets are routed
From: Ian McDonald @ 2006-03-09 1:41 UTC (permalink / raw)
To: dccp
In-Reply-To: <cbec11ac0603081740o2004650dr8a91de3dd9418189@mail.gmail.com>
What I neglected to say is that TCP works fine and it is very low latency...
On 3/9/06, Ian McDonald <imcdnzl@gmail.com> wrote:
> I've been testing DCCP using netem for quite a while now - just about
> always with loss and delay. This works really well.
>
> A few weeks ago I noticed that if netem isn't turned on but traffic
> still goes between boxes through a router the performance is abysmal -
> can be as low as 20K per second instead of 100 Mbit and there is much
> data loss.
>
> I had assumed that this was a recent regression so parked it until I
> had time to test. However I just tested with a clean 2.6.14 and the
> problem persists....
>
> The test setup is
>
> 10.0.1.31 ----- 10.0.1.33/10.0.2.33 ------- 10.0.2.32
>
> and the command is just:
> (from 10.0.1.31) ./ttcp_acme -c -l500 -n10000 -t 10.0.2.32
> and
> (from 10.0.2.32) ./ttcp_acme -c -l500 -n10000 -r
>
> the following setup works fine:
> 10.0.1.31 ----- 10.0.1.33
> ./ttcp_acme -c -l500 -n10000 -t 10.0.1.33
> and
> ./ttcp_acme -c -l500 -n10000 -r
>
> I'll add this to my list of bugs to look at at some stage (along with
> updating tx buffering, CCID2 not working with loss/delay) but
> interested in seeing if others have similar problems?
>
> 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
>
--
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
* [BUG] Performance issues if DCCP packets are routed
From: Ian McDonald @ 2006-03-09 1:40 UTC (permalink / raw)
To: dccp
I've been testing DCCP using netem for quite a while now - just about
always with loss and delay. This works really well.
A few weeks ago I noticed that if netem isn't turned on but traffic
still goes between boxes through a router the performance is abysmal -
can be as low as 20K per second instead of 100 Mbit and there is much
data loss.
I had assumed that this was a recent regression so parked it until I
had time to test. However I just tested with a clean 2.6.14 and the
problem persists....
The test setup is
10.0.1.31 ----- 10.0.1.33/10.0.2.33 ------- 10.0.2.32
and the command is just:
(from 10.0.1.31) ./ttcp_acme -c -l500 -n10000 -t 10.0.2.32
and
(from 10.0.2.32) ./ttcp_acme -c -l500 -n10000 -r
the following setup works fine:
10.0.1.31 ----- 10.0.1.33
./ttcp_acme -c -l500 -n10000 -t 10.0.1.33
and
./ttcp_acme -c -l500 -n10000 -r
I'll add this to my list of bugs to look at at some stage (along with
updating tx buffering, CCID2 not working with loss/delay) but
interested in seeing if others have similar problems?
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: Report: feat negot state
From: Eddie Kohler @ 2006-03-08 19:18 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
Hey Andrea!
Just a couple things:
On Mar 8, 2006, at 11:01 AM, Andrea Bittau wrote:
>
>> It's not that bad. (1) All feature negotiation options are
>> limited to 252
>> bytes of feature value at most. (2) All current SP features are
>> one-byte
>> valued. So you can do this.
>
> yea, but I was thinking if you chain many change X in the same
> packet. I'm not
> sure that's legal. But anyway, an implementation may process
> options from the
> tail of the packet to front, and discard any previously seen
> feature negos for
> the same option.
(1) Implementations MUST process options from front to back.
"Options MUST be processed sequentially, starting at the first option
in the packet header."
(2) "A packet MAY contain more than one feature negotiation option,
possibly including two options that refer to the same feature; as
usual, the options are processed sequentially."
> yea, in fact i got "scared" when I wrote the multi-byte patch. the:
> if (c = s)
> became a:
> if (memcmp(c, s, len) = 0)
Yep :)
Eddie
^ permalink raw reply
* Re: Report: feat negot state
From: Andrea Bittau @ 2006-03-08 19:01 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
On Wed, Mar 08, 2006 at 08:08:53AM -0800, Eddie Kohler wrote:
> >Basically, because i coded server priority features first, and then
> >non-negotiable. With NN features, you need to send confirms even for
> >stuff you
> >don't know about [or something like that] so i didn't know where to shove
> >the
> >confirm =D
>
> I don't quite understand this. If you don't know about the feature, then
> how can you tell whether it is NN?
OK the issue was this. With SP, you have a preference list, so whenever you
confirm X, you can shove the confirm in the preference list data structure for
X.
For NN, it might be possible to not have a preference for X. [?] e.g. say you
get CHANGE_L ACK_RATIO, maybe you don't even care about ACK_RATIO yourself, so
you might not have a feature structure list for your own ack ratio, where you
could put the confirm.
This is what happens when coding without thinking / designing. But hey, time =D
> >2) I'll try to re-write the multi-byte feature support patch. I believe
> >this is
> > important functionality which is missing.
>
> For NN features, yes.
yea, in fact i initially wrote the patch solely for ack ratios. I wanted to see
how CCID2 would behave when ack ratios really did change =D
> The Init Cookie option lets the server package up all that state, send it
> to the client, and forget it. The server might still have to load a CCID,
> but that doesn't worry me -- we only have 3 CCIDs.
I remember seeing the init cookie in the spec. I just re-read it, yes it solves
the problem. I didn't know it was so versatile [that you can encode ~200 bytes
of data.] I thought it was just a 32bit number or something.
> It's not that bad. (1) All feature negotiation options are limited to 252
> bytes of feature value at most. (2) All current SP features are one-byte
> valued. So you can do this.
yea, but I was thinking if you chain many change X in the same packet. I'm not
sure that's legal. But anyway, an implementation may process options from the
tail of the packet to front, and discard any previously seen feature negos for
the same option.
yea, in fact i got "scared" when I wrote the multi-byte patch. the:
if (c = s)
became a:
if (memcmp(c, s, len) = 0)
anyway, yes, i agree with you that there is no real dos issue. What got me
thinking about this was mereley "when will my code suck?" [apart from "in the
common case"] =D
As Arnaldo said, the goal is to get this working properly as quickly as
possible. I don't think we are far. From what I can see, only two big things
are missing [in feat nego]:
* Timers & Feature negotation.
Both have been written in some patch, it's just a matter of putting the puzzle
together and framing it =D
^ permalink raw reply
* Re: Report: feat negot state
From: Arnaldo Carvalho de Melo @ 2006-03-08 16:39 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
On 3/8/06, Eddie Kohler <kohler@cs.ucla.edu> wrote:
> Hi Arnaldo,
>
> Arnaldo Carvalho de Melo wrote:
> >>The Init Cookie option lets the server package up all that state, send it to
> >>the client, and forget it. The server might still have to load a CCID, but
> >>that doesn't worry me -- we only have 3 CCIDs.
> >
> >
> > Sure, Init Cookie will be eventually implemented, but I want to
> > support everything
> > that is specified in the draft, and furthermore, reworking the core Linux INET
> > infrastructure to have TCP and DCCP sharing _everything_ that is not specific to
> > one of them, that has been my goal since day one, if I decided just to implement
> > Init Cookies I would have a much easier ride, but I'm stubborn on supporting all
> > the things specified in the draft to the best of my capacity use this
> > implementation
> > to fully validate the draft in the Real World(tm), even if takes more time 8)
> >
> > If one looks at work Eric Dumazet, Ben LaHaise and others are doing on improving
> > TCP that _already_ are helping DCCP due to all this code sharing, its
> > wonderful, I
> > really, really didn't wanted to go the SCTP way where few things is shared with
> > other INET connection oriented protocols (well, at the time there was
> > only TCP :P).
>
> This is a great approach! But TCP implements syn cookies, which is very
> similar to the Init Cookie. So Init Cookies are not totally DCCP-only. I'm
> just saying the DOS need not be such a big deal.
Sure, my idea is to abstract TCP syncookies/DCCP Init Cookies at the
struct inet_request_sock level, then both will have the same code flow :-)
So stay tuned as after we get the feature negotiation mostly working I'll attack
this new syncookies/init cookie abstraction.
Ah, another thing in the backburner is to support DCCPv6 only, i.e. right now we
have dccp_ipv4 and dccp_ipv6 modules, but dccp_ipv6 still requires dccp_ipv4
due to v6 mapped v4 addresses, that is not selectable, but I'll rework this code
at the inet_connection_sock level to make it possible to have a IPv6 only setup,
paving the way for IPv4 retirement as I promised to DaveM 8-) :-P
- Arnaldo
^ permalink raw reply
* Re: Report: feat negot state
From: Eddie Kohler @ 2006-03-08 16:28 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
Hi Arnaldo,
Arnaldo Carvalho de Melo wrote:
>>The Init Cookie option lets the server package up all that state, send it to
>>the client, and forget it. The server might still have to load a CCID, but
>>that doesn't worry me -- we only have 3 CCIDs.
>
>
> Sure, Init Cookie will be eventually implemented, but I want to
> support everything
> that is specified in the draft, and furthermore, reworking the core Linux INET
> infrastructure to have TCP and DCCP sharing _everything_ that is not specific to
> one of them, that has been my goal since day one, if I decided just to implement
> Init Cookies I would have a much easier ride, but I'm stubborn on supporting all
> the things specified in the draft to the best of my capacity use this
> implementation
> to fully validate the draft in the Real World(tm), even if takes more time 8)
>
> If one looks at work Eric Dumazet, Ben LaHaise and others are doing on improving
> TCP that _already_ are helping DCCP due to all this code sharing, its
> wonderful, I
> really, really didn't wanted to go the SCTP way where few things is shared with
> other INET connection oriented protocols (well, at the time there was
> only TCP :P).
This is a great approach! But TCP implements syn cookies, which is very
similar to the Init Cookie. So Init Cookies are not totally DCCP-only. I'm
just saying the DOS need not be such a big deal.
Eddie
^ permalink raw reply
* Re: Report: feat negot state
From: Arnaldo Carvalho de Melo @ 2006-03-08 16:19 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
On 3/8/06, Eddie Kohler <kohler@cs.ucla.edu> wrote:
> Hi guys!
Hi Eddie!
> > * The server must remember which CONFIRMs it sent. [In fact, the previous code
> > was bugged because of this I think---it would remember the features of ALL
> > clients in a single socket... anyway.]
> >
> > This means that the server must not trash memory, it has to hold some state,
> > and potentially even setup those options. E.g. it might have to load a CCID,
> > etc etc.
>
> The Init Cookie option lets the server package up all that state, send it to
> the client, and forget it. The server might still have to load a CCID, but
> that doesn't worry me -- we only have 3 CCIDs.
Sure, Init Cookie will be eventually implemented, but I want to
support everything
that is specified in the draft, and furthermore, reworking the core Linux INET
infrastructure to have TCP and DCCP sharing _everything_ that is not specific to
one of them, that has been my goal since day one, if I decided just to implement
Init Cookies I would have a much easier ride, but I'm stubborn on supporting all
the things specified in the draft to the best of my capacity use this
implementation
to fully validate the draft in the Real World(tm), even if takes more time 8)
If one looks at work Eric Dumazet, Ben LaHaise and others are doing on improving
TCP that _already_ are helping DCCP due to all this code sharing, its
wonderful, I
really, really didn't wanted to go the SCTP way where few things is shared with
other INET connection oriented protocols (well, at the time there was
only TCP :P).
> > * The server must actually calculate the CONFIRMS. With server priority, the
> > calculation works as follows: Given preference list C and S of the client and
> > server respectively, the winning value is the first value which appears in S
> > and also in C.
> >
> > Basically:
> >
> > for each s in S
> > for each c in C
> > if (s = c)
> > w00t();
>
> It's not that bad. (1) All feature negotiation options are limited to 252
> bytes of feature value at most. (2) All current SP features are one-byte
> valued. So you can do this.
>
> uint32_t bitmap[8] = { 0, 0, 0, 0, 0, 0, 0, 0 }; // 256 bits
> for each c in C
> turn on corresponding bit in bitmap;
> for each s in S
> if corresponding bit in bitmap is true
> w00t();
>
> O(n^2) -> O(n).
>
> For multi-byte SP features (if they ever exist), this trick doesn't work, but
> the DOS is less serious, since the maximum preference list length is 1/2 (1/3,
> 1/4...) as long.
Interesting idea, will get back to this after getting what we have now
working as
a first step.
- Arnaldo
^ permalink raw reply
* Re: Report: feat negot state
From: Eddie Kohler @ 2006-03-08 16:08 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
Hi guys!
Andrea Bittau wrote:
> good. only 2 queues: one for "pending changes" and one for confirms. You
> might have wondered why i had a "confirm" inside the pending change queue
> itself, and then a seperate confirm queue.
>
> Basically, because i coded server priority features first, and then
> non-negotiable. With NN features, you need to send confirms even for stuff you
> don't know about [or something like that] so i didn't know where to shove the
> confirm =D
I don't quite understand this. If you don't know about the feature, then how
can you tell whether it is NN?
> Anyway, it's much better like this [i was planning to do it at some point, but i
> just "died"]
>
>
> /* Check if nothing is being changed. */
> - if (ccid_nr = new_ccid_nr)
> + /* XXX handle multibyte values */
> + if (ccid_nr = *val)
> return 0;
>
>
> ccid can only be 1 byte. kill comment. assert(len = 1);
It is worth pointing out here that ALL server-priority features specified in
DCCP so far use one byte values. So I'd just hold off implementing multi-byte
SP features.
> + rc = dccp_feat_change(dmsk, DCCPO_CHANGE_L, DCCPF_ACK_RATIO,
> + &dmsk->dccpms_ack_ratio, 1, gfp);
>
> ack ratio is 2 bytes i think.
Yep.
> 1) Merge it
> 2) I'll try to re-write the multi-byte feature support patch. I believe this is
> important functionality which is missing.
For NN features, yes.
> So now feature negotiation is moved into the minisock stuff. I don't know what
> it means exactly, but i think the minisock stuff is the socket created upon
> receiving a SYN... i.e. a half open connection. The idea is to keep as little
> state as possible in order to avoid SYN floods and other lame dos attacks.
>
> While I was writing feature negotiation, I spotted quite a nice dos attack.
> Actually, probably it sucks, but here it is.
>
> The idea is that when you send a REQUEST, you can include your CHANGE options.
> The server will RESPOND with the various CONFIRM options. Here are the nice
> implications of this:
>
> * The server must remember which CONFIRMs it sent. [In fact, the previous code
> was bugged because of this I think---it would remember the features of ALL
> clients in a single socket... anyway.]
>
> This means that the server must not trash memory, it has to hold some state,
> and potentially even setup those options. E.g. it might have to load a CCID,
> etc etc.
The Init Cookie option lets the server package up all that state, send it to
the client, and forget it. The server might still have to load a CCID, but
that doesn't worry me -- we only have 3 CCIDs.
> * The server must actually calculate the CONFIRMS. With server priority, the
> calculation works as follows: Given preference list C and S of the client and
> server respectively, the winning value is the first value which appears in S
> and also in C.
>
> Basically:
>
> for each s in S
> for each c in C
> if (s = c)
> w00t();
It's not that bad. (1) All feature negotiation options are limited to 252
bytes of feature value at most. (2) All current SP features are one-byte
valued. So you can do this.
uint32_t bitmap[8] = { 0, 0, 0, 0, 0, 0, 0, 0 }; // 256 bits
for each c in C
turn on corresponding bit in bitmap;
for each s in S
if corresponding bit in bitmap is true
w00t();
O(n^2) -> O(n).
For multi-byte SP features (if they ever exist), this trick doesn't work, but
the DOS is less serious, since the maximum preference list length is 1/2 (1/3,
1/4...) as long.
E
^ permalink raw reply
* Re: Report: feat negot state
From: Arnaldo Carvalho de Melo @ 2006-03-08 15:51 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
On 3/8/06, Andrea Bittau <a.bittau@cs.ucl.ac.uk> wrote:
> On Tue, Mar 07, 2006 at 12:05:51AM -0300, Arnaldo Carvalho de Melo wrote:
> > With the current patch, available at:
> >
> > http://oops.ghostprotocols.net:81/acme/dccp_minisock.patch
>
>
> Great work. I see you've been busy lately =D Anyway, I'll comment inline, and
> then write a bunch of random thoughts at the end.
>
>
> + struct list_head dccpms_pend;
> + struct list_head dccpms_conf;
>
> good. only 2 queues: one for "pending changes" and one for confirms. You
> might have wondered why i had a "confirm" inside the pending change queue
> itself, and then a seperate confirm queue.
> Basically, because i coded server priority features first, and then
> non-negotiable. With NN features, you need to send confirms even for stuff you
> don't know about [or something like that] so i didn't know where to shove the
> confirm =D
:-)
> Anyway, it's much better like this [i was planning to do it at some point, but i
> just "died"]
Ressurrected now? :-)
>
> /* Check if nothing is being changed. */
> - if (ccid_nr = new_ccid_nr)
> + /* XXX handle multibyte values */
> + if (ccid_nr = *val)
> return 0;
>
>
> ccid can only be 1 byte. kill comment. assert(len = 1);
>
> -
> - new_ccid = ccid_new(new_ccid_nr, sk, rx, GFP_ATOMIC);
> +#if 0
> + new_ccid = ccid_new(*val, sk, rx, GFP_ATOMIC);
> if (new_ccid = NULL)
> return -ENOMEM;
>
>
> that if 0 is only for your debug right?
Well, this part is incomplete, read on, I'll address this on my next comments
>
> switch (feat) {
> case DCCPF_CCID:
> - return dccp_feat_update_ccid(sk, type, val);
> + return dccp_feat_update_ccid(dmsk, type, val, len);
>
> update_ccid() shouldn't care about len. it knows the length of ccid. Well I
> guess it depends on who you leave it to do sanity checks. But by the time the
> option is "negotiated", errors would have been spotted by upper layers. [For
> example when doing server priority reconciliation.]
Read on...
> + feat->dccpf_conf_idx = sidx;
> + feat->dccpf_conf_len = rlen;
>
> conf_len should be the length of the option itself, not the length of the
> feature list. I guess this will be sorted when we work on the multi-byte patch.
OK, the reconcile stuff needs to be reconciled with the drafts, its
buggy right now :)
>
> /* update the option on our side [we are about to send the confirm] */
> - rc = dccp_feat_update(sk, opt->dccpop_type, opt->dccpop_feat, *res);
> + rc = dccp_feat_update(dmsk, feat->dccpf_type, feat->dccpf_feat,
> + spref + sidx, 1);
>
> similar problem. Hardcoded feature length of 1. perhaps it should be
> feat->dccpf_conf_len.
ditto
>
>
> + feat->dccpf_conf_idx = 0;
> + feat->dccpf_conf_len = 1;
>
> should the index be sidx? Also, same problem with legnth.
ditto
>
> - /* generate the confirm [if required] */
> - dccp_feat_flush_confirm(sk);
> -
> + /* Confirms will be sent on the next packet */
>
> yea but what if the next packet is never sent? I.e. uni-directional connection.
> There should be a mechanism to force an ack after X amount of time if the dude
> doesn't talk.
We need a timer on the minisock perhaps, its still experimental stuff, trying to
figure out how to do feature negotiation at the earliest possible moment at the
server side, i.e. when creating the dccp_request_sock minisock and sending the
RESPONSE packet (first packet sent by the server) without holding too much
state on the request_sock.
> + list_for_each_entry(feat, &dmsk->dccpms_pend, dccpf_node) {
> + if (!dccp_feat_negotiated(feat) &&
> + feat->dccpf_type = t &&
> + feat->dccpf_feat = feature) {
> + feat->dccpf_conf_idx = 0;
> + feat->dccpf_conf_len = len;
>
> is the index correct? Also, len should be length of option not of preference
> list. Confirms will contain confirmed value + pref list.
what I propose to sort out these things is to merge it but not push to Dave, we
work on it, then rework the sequence of csets and push to Dave, agreed?
> + /* We got a confirmation: change the option */
> + dccp_feat_update(dmsk, feat->dccpf_type,
> + feat->dccpf_feat, val, len);
>
> i'm not sure feat_update should care about length [unless IT sanity checks]. If
> so, len should be of value, not pref list.
ditto
> - if (all_confirmed) {
> - dccp_pr_debug("clear feat negotiation timer %p\n", sk);
> - inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
> }
>
> so what's the story with timers? Did you kill them? Will changes be
> re-transmitted?
Well, here we have to think a bit more, REQUEST packets are already
reliably transmitted by dccp_connect using sk_send_head and ICSK_TIME_RETRANS,
and dccpms_pend feature changes are retransmitted till they are negotiated,
but not on DATA/DATAACK packets, i.e. we're almost with the infrastructure that
is needed for reliable changes, but we don't have a timer on the
minisock, it seems,
have to revisit the inet_request_sock stuff to see how to fit this.
>
> + rc = dccp_feat_change(dmsk, DCCPO_CHANGE_L, DCCPF_ACK_RATIO,
> + &dmsk->dccpms_ack_ratio, 1, gfp);
>
> ack ratio is 2 bytes i think.
yeah, this needs more work
>
> + case DCCPF_CCID:
> + return len >= sizeof(__u8);
>
> should it also have an && <= number_of_ccids at this point? Breaks modular
> architecture =D But you hardcoded MAX_FEATURE_VALUE or whatever it's called
> anyway =D
Well, that also needs more work
> /* confirm any options [NN opts] */
> - list_for_each_entry_safe(opt, next, &dp->dccps_options.dccpo_conf,
> - dccpop_node) {
> - dccp_insert_feat_opt(skb, opt->dccpop_type,
> - opt->dccpop_feat, opt->dccpop_val,
> - opt->dccpop_len);
> - /* fear empty confirms */
> - if (opt->dccpop_val)
> - kfree(opt->dccpop_val);
> - kfree(opt);
>
> I belive the "confirm queue" now contains confirms for SP and NN options... so
> you can probably kill the [] in the comment.
will do
>
> - inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
> - inet_csk(sk)->icsk_rto, DCCP_RTO_MAX);
> - }
>
> again... change requests are reliable.
yup
>
> ==========>
> OK I think it's a great tidy up and addition of very good code. I would suggest
> the following:
>
> 1) Merge it
> 2) I'll try to re-write the multi-byte feature support patch. I believe this is
> important functionality which is missing.
>
>
> So now feature negotiation is moved into the minisock stuff. I don't know what
> it means exactly, but i think the minisock stuff is the socket created upon
> receiving a SYN... i.e. a half open connection. The idea is to keep as little
> state as possible in order to avoid SYN floods and other lame dos attacks.
yes, and dccp_minisock_move moves dreq_minisock to dccps_minisock
> While I was writing feature negotiation, I spotted quite a nice dos attack.
> Actually, probably it sucks, but here it is.
>
> The idea is that when you send a REQUEST, you can include your CHANGE options.
> The server will RESPOND with the various CONFIRM options. Here are the nice
> implications of this:
>
> * The server must remember which CONFIRMs it sent. [In fact, the previous code
> was bugged because of this I think---it would remember the features of ALL
> clients in a single socket... anyway.]
yes, we can't store the client state in the DCCP_ROLE_LISTEN socket, that is the
reason for dccp_minisock
> This means that the server must not trash memory, it has to hold some state,
> and potentially even setup those options. E.g. it might have to load a CCID,
> etc etc.
yup, I'm stuck now at this, i.e. ccid_hc_rx_init can't touch the full
socket, so we
perhaps need to instantiate the CCID in the three way handshake, where we
have just a dccp_request_sock and later, when we complete the handshake we'd
pass the full sized sock (dccp_sock) to the CCID instantiated
previously, kinda like
making what now is called ccid_hc_rx_init to ccid_hc_rx_new and reintroducing
ccid_hc_rx_init to pass the full sized sock (dccp_sock), ccid_hc_tx
doesn't needs
this as in the client side we create dccp_sock from the beggining, but
to make it
simetric with the rx case we do the same (ccid_hc_tx_new + ccid_hc_tx_init).
> * The server must actually calculate the CONFIRMS. With server priority, the
> calculation works as follows: Given preference list C and S of the client and
> server respectively, the winning value is the first value which appears in S
> and also in C.
>
> Basically:
>
> for each s in S
> for each c in C
> if (s = c)
> w00t();
>
> Therefore, would the following DoS DCCP:
> * Send a bunch of change options for a SP option in the REQUEST. Provide an
> ultra long feature list which will not mathc anything on the server, or maybe,
> the last byte will match or something like that.
>
> * The server will have to scan the whole list and compare it against its own
> stuff.
>
> I'm not sure you can write a generic "fast calculation" algorithm as it is not
> mandatory to sort the preference lists in any way.
Yeah, that is the main reason why I think we can't use a doubly linked
list for the
features, on 64bit arches we'd be using 16 bytes only for the list
links! So I guess
we should use an array with a sensible initial size that would expand if needed,
but somehow we must limit the number of acceptable features to grok while doing
the 3way handshake.
- Arnaldo
^ permalink raw reply
* Re: Report: feat negot state
From: Andrea Bittau @ 2006-03-08 12:51 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
On Tue, Mar 07, 2006 at 12:05:51AM -0300, Arnaldo Carvalho de Melo wrote:
> With the current patch, available at:
>
> http://oops.ghostprotocols.net:81/acme/dccp_minisock.patch
Great work. I see you've been busy lately =D Anyway, I'll comment inline, and
then write a bunch of random thoughts at the end.
+ struct list_head dccpms_pend;
+ struct list_head dccpms_conf;
good. only 2 queues: one for "pending changes" and one for confirms. You
might have wondered why i had a "confirm" inside the pending change queue
itself, and then a seperate confirm queue.
Basically, because i coded server priority features first, and then
non-negotiable. With NN features, you need to send confirms even for stuff you
don't know about [or something like that] so i didn't know where to shove the
confirm =D
Anyway, it's much better like this [i was planning to do it at some point, but i
just "died"]
/* Check if nothing is being changed. */
- if (ccid_nr = new_ccid_nr)
+ /* XXX handle multibyte values */
+ if (ccid_nr = *val)
return 0;
ccid can only be 1 byte. kill comment. assert(len = 1);
-
- new_ccid = ccid_new(new_ccid_nr, sk, rx, GFP_ATOMIC);
+#if 0
+ new_ccid = ccid_new(*val, sk, rx, GFP_ATOMIC);
if (new_ccid = NULL)
return -ENOMEM;
that if 0 is only for your debug right?
switch (feat) {
case DCCPF_CCID:
- return dccp_feat_update_ccid(sk, type, val);
+ return dccp_feat_update_ccid(dmsk, type, val, len);
update_ccid() shouldn't care about len. it knows the length of ccid. Well I
guess it depends on who you leave it to do sanity checks. But by the time the
option is "negotiated", errors would have been spotted by upper layers. [For
example when doing server priority reconciliation.]
+ feat->dccpf_conf_idx = sidx;
+ feat->dccpf_conf_len = rlen;
conf_len should be the length of the option itself, not the length of the
feature list. I guess this will be sorted when we work on the multi-byte patch.
/* update the option on our side [we are about to send the confirm] */
- rc = dccp_feat_update(sk, opt->dccpop_type, opt->dccpop_feat, *res);
+ rc = dccp_feat_update(dmsk, feat->dccpf_type, feat->dccpf_feat,
+ spref + sidx, 1);
similar problem. Hardcoded feature length of 1. perhaps it should be
feat->dccpf_conf_len.
+ feat->dccpf_conf_idx = 0;
+ feat->dccpf_conf_len = 1;
should the index be sidx? Also, same problem with legnth.
- /* generate the confirm [if required] */
- dccp_feat_flush_confirm(sk);
-
+ /* Confirms will be sent on the next packet */
yea but what if the next packet is never sent? I.e. uni-directional connection.
There should be a mechanism to force an ack after X amount of time if the dude
doesn't talk.
+ list_for_each_entry(feat, &dmsk->dccpms_pend, dccpf_node) {
+ if (!dccp_feat_negotiated(feat) &&
+ feat->dccpf_type = t &&
+ feat->dccpf_feat = feature) {
+ feat->dccpf_conf_idx = 0;
+ feat->dccpf_conf_len = len;
is the index correct? Also, len should be length of option not of preference
list. Confirms will contain confirmed value + pref list.
+ /* We got a confirmation: change the option */
+ dccp_feat_update(dmsk, feat->dccpf_type,
+ feat->dccpf_feat, val, len);
i'm not sure feat_update should care about length [unless IT sanity checks]. If
so, len should be of value, not pref list.
- if (all_confirmed) {
- dccp_pr_debug("clear feat negotiation timer %p\n", sk);
- inet_csk_clear_xmit_timer(sk, ICSK_TIME_RETRANS);
}
so what's the story with timers? Did you kill them? Will changes be
re-transmitted?
+ rc = dccp_feat_change(dmsk, DCCPO_CHANGE_L, DCCPF_ACK_RATIO,
+ &dmsk->dccpms_ack_ratio, 1, gfp);
ack ratio is 2 bytes i think.
+ case DCCPF_CCID:
+ return len >= sizeof(__u8);
should it also have an && <= number_of_ccids at this point? Breaks modular
architecture =D But you hardcoded MAX_FEATURE_VALUE or whatever it's called
anyway =D
/* confirm any options [NN opts] */
- list_for_each_entry_safe(opt, next, &dp->dccps_options.dccpo_conf,
- dccpop_node) {
- dccp_insert_feat_opt(skb, opt->dccpop_type,
- opt->dccpop_feat, opt->dccpop_val,
- opt->dccpop_len);
- /* fear empty confirms */
- if (opt->dccpop_val)
- kfree(opt->dccpop_val);
- kfree(opt);
I belive the "confirm queue" now contains confirms for SP and NN options... so
you can probably kill the [] in the comment.
- inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
- inet_csk(sk)->icsk_rto, DCCP_RTO_MAX);
- }
again... change requests are reliable.
==========
OK I think it's a great tidy up and addition of very good code. I would suggest
the following:
1) Merge it
2) I'll try to re-write the multi-byte feature support patch. I believe this is
important functionality which is missing.
So now feature negotiation is moved into the minisock stuff. I don't know what
it means exactly, but i think the minisock stuff is the socket created upon
receiving a SYN... i.e. a half open connection. The idea is to keep as little
state as possible in order to avoid SYN floods and other lame dos attacks.
While I was writing feature negotiation, I spotted quite a nice dos attack.
Actually, probably it sucks, but here it is.
The idea is that when you send a REQUEST, you can include your CHANGE options.
The server will RESPOND with the various CONFIRM options. Here are the nice
implications of this:
* The server must remember which CONFIRMs it sent. [In fact, the previous code
was bugged because of this I think---it would remember the features of ALL
clients in a single socket... anyway.]
This means that the server must not trash memory, it has to hold some state,
and potentially even setup those options. E.g. it might have to load a CCID,
etc etc.
* The server must actually calculate the CONFIRMS. With server priority, the
calculation works as follows: Given preference list C and S of the client and
server respectively, the winning value is the first value which appears in S
and also in C.
Basically:
for each s in S
for each c in C
if (s = c)
w00t();
Therefore, would the following DoS DCCP:
* Send a bunch of change options for a SP option in the REQUEST. Provide an
ultra long feature list which will not mathc anything on the server, or maybe,
the last byte will match or something like that.
* The server will have to scan the whole list and compare it against its own
stuff.
I'm not sure you can write a generic "fast calculation" algorithm as it is not
mandatory to sort the preference lists in any way.
^ permalink raw reply
* Re: Report: feat negot state
From: Ian McDonald @ 2006-03-07 4:11 UTC (permalink / raw)
To: dccp
In-Reply-To: <39e6f6c70603061905i1db54071m6cc2776edd3828bb@mail.gmail.com>
On 3/7/06, Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
> With the current patch, available at:
>
> http://oops.ghostprotocols.net:81/acme/dccp_minisock.patch
>
Looks good from having a quick read of code and I like some of the
other little tidyups too (int to u32 for example, checking sk value).
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
* Report: feat negot state
From: Arnaldo Carvalho de Melo @ 2006-03-07 3:05 UTC (permalink / raw)
To: dccp
With the current patch, available at:
http://oops.ghostprotocols.net:81/acme/dccp_minisock.patch
Simple ttcp session transmitting 5 packets:
127.0.0.1.4496 > 127.0.0.1.5001: request <change_l ack_ratio 2,
change_r ccid 2, change_l ccid 2>
127.0.0.1.5001 > 127.0.0.1.4496: response <change_l ack_ratio 2,
confirm_l ccid 2, confirm_r ccid 2, confirm_r ack_ratio 2>
127.0.0.1.4496 > 127.0.0.1.5001: ack <nop, confirm_r ack_ratio 2,
ack_vector0 0x00, elapsed_time 108>
127.0.0.1.4496 > 127.0.0.1.5001: dataack <nop, nop, ack_vector0 0x00,
elapsed_time 16857, ndp_count 1>
127.0.0.1.5001 > 127.0.0.1.4496: ack <nop, ack_vector0 0x01,
elapsed_time 20>
127.0.0.1.4496 > 127.0.0.1.5001: dataack <nop, ack_vector0 0x00,
elapsed_time 2495>
127.0.0.1.5001 > 127.0.0.1.4496: ack <nop, nop, ack_vector0 0x00,
elapsed_time 15, ndp_count 1>
127.0.0.1.4496 > 127.0.0.1.5001: dataack <nop, ack_vector0 0x00,
elapsed_time 2404>
127.0.0.1.5001 > 127.0.0.1.4496: ack <nop, nop, ack_vector0 0x00,
elapsed_time 13, ndp_count 2>
127.0.0.1.4496 > 127.0.0.1.5001: dataack <nop, ack_vector0 0x00,
elapsed_time 2327>
127.0.0.1.5001 > 127.0.0.1.4496: ack <nop, nop, ack_vector0 0x00,
elapsed_time 14, ndp_count 3>
127.0.0.1.4496 > 127.0.0.1.5001: dataack <nop, ack_vector0 0x00,
elapsed_time 2077>
127.0.0.1.5001 > 127.0.0.1.4496: ack <nop, nop, ack_vector0 0x00,
elapsed_time 13, ndp_count 4>
127.0.0.1.4496 > 127.0.0.1.5001: close <nop, ack_vector0 0x00,
elapsed_time 8425>
127.0.0.1.5001 > 127.0.0.1.4496: reset <nop, nop, ack_vector0 0x00,
elapsed_time 1121, ndp_count 5>
Still work in progress, requires more testing, but gets feature
negotiation down to
dccp_request_sock level, still need to possibly move more stuff to
dccp_minisock, that
is (or should) be the bare minimum needed to do feature negotiation
when we still don't
have a full blown socket (struct dccp_sock), the previous code is
wrong as we can't use
the listening sock to do feature negotiation at all, so we have to
store the feat negot
state in the minisock that represents the embryonic connection (struct
dccp_request_sock),
and later move this state (in dccp_create_openreq_child) to the newly
inet_csk_cloned
full socket.
And I don't like the many allocations still present, even having
reduced some of the
previous ones I think perhaps we should use a buffer like when we encode the
options in a dccp packet, will try this later.
I'm going to have some sleep now, so this is just for you guys to take
a look at the
current status, more work is needed and the patch has unrelated
changes, will split
when I get satisfied with this stuff.
G'night,
- Arnaldo
^ permalink raw reply
* DCCP with iperf was Re: Question on iperf versions
From: Ian McDonald @ 2006-03-06 20:14 UTC (permalink / raw)
To: dccp
On 3/7/06, John S. Estabrook <jestabro@ncsa.uiuc.edu> wrote:
>
> Connie, 2.0.2 is the recommended version, which addresses several bugs in
> 2.0.1; the patch for 2.0.2, if you're refering to the one mentionied at
> the bottom of the iperf page, was submitted but is not necessary---it's a
> feature addition (re DCCP), not a bug fix.
>
I wondered who had also written an iperf patch for DCCP besides me and
I see that my code has been posted up there :-)
Just a note for those looking in future - the iperf works for Linux
2.6.14 but not on the latest development kernels. I will work on
fixing this at some point but if anybody feels brave I think it is
because of the introduction of service codes. It is also worth adding
multiple CCID support as well I believe...
I also intend to start merging some of the changes into the iperf
codebase over time (probably a long time with my current workload!).
Also this code was originally written by others and I've just tidied
it up a little bit.
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: net-2.6.17 rebased...
From: David S. Miller @ 2006-03-04 1:55 UTC (permalink / raw)
To: dccp
In-Reply-To: <cbec11ac0603011628r662dbe19oc1abf02a1b6b0a2@mail.gmail.com>
From: "Ian McDonald" <imcdnzl@gmail.com>
Date: Thu, 2 Mar 2006 13:48:18 +1300
> If you get a chance can you push the ccid3 divide by zero fix upstream
> to Linus for 2.6.16 as it has no functionality changed and eliminates
> a nasty little bug...
> The commit for this is b6da19617f4ab610d3d90bcbdf65fa7e2b3d7b53 in your tree
Done, thanks.
^ permalink raw reply
* Re: net-2.6.17 rebased...
From: Ian McDonald @ 2006-03-02 0:48 UTC (permalink / raw)
To: dccp
In-Reply-To: <cbec11ac0603011628r662dbe19oc1abf02a1b6b0a2@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1010 bytes --]
And *** again... someone pointed out my mailer is wrapping lines. I'll
go hide in the corner and beat myself up. This time attached as I
can't get gmail to defeat line wrapping. I promise I'll get it right
next patch so I don't humiliate myself quite so much next time
Dave,
If you get a chance can you push the ccid3 divide by zero fix upstream
to Linus for 2.6.16 as it has no functionality changed and eliminates
a nasty little bug...
The commit for this is b6da19617f4ab610d3d90bcbdf65fa7e2b3d7b53 in your tree
I have also put at end of this e-mail after reapplying on linus tree
so above commit doesn't have fuzz...
[DCCP] ccid3: Divide by zero fix
In rare circumstances 0 is returned by dccp_li_hist_calc_i_mean which leads to
a divide by zero in ccid3_hc_rx_packet_recv. Explicitly check for zero return
now. Update copyright notice at same time.
Found by Arnaldo.
Signed-off-by: Ian McDonald <imcdnzl@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: div.diff --]
[-- Type: text/x-patch; name="div.diff", Size: 1007 bytes --]
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index aa68e0a..35d1d34 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -2,7 +2,7 @@
* net/dccp/ccids/ccid3.c
*
* Copyright (c) 2005 The University of Waikato, Hamilton, New Zealand.
- * Copyright (c) 2005 Ian McDonald <iam4@cs.waikato.ac.nz>
+ * Copyright (c) 2005-6 Ian McDonald <imcdnzl@gmail.com>
*
* An implementation of the DCCP protocol
*
@@ -1033,9 +1033,13 @@ static void ccid3_hc_rx_packet_recv(stru
p_prev = hcrx->ccid3hcrx_p;
/* Calculate loss event rate */
- if (!list_empty(&hcrx->ccid3hcrx_li_hist))
+ if (!list_empty(&hcrx->ccid3hcrx_li_hist)) {
+ u32 i_mean = dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+
/* Scaling up by 1000000 as fixed decimal */
- hcrx->ccid3hcrx_p = 1000000 / dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+ if (i_mean != 0)
+ hcrx->ccid3hcrx_p = 1000000 / i_mean;
+ }
if (hcrx->ccid3hcrx_p > p_prev) {
ccid3_hc_rx_send_feedback(sk);
^ permalink raw reply related
* Re: net-2.6.17 rebased...
From: Ian McDonald @ 2006-03-02 0:32 UTC (permalink / raw)
To: dccp
In-Reply-To: <cbec11ac0603011628r662dbe19oc1abf02a1b6b0a2@mail.gmail.com>
F**k - just pasted in the wrong file. Trying again
On 3/2/06, Ian McDonald <imcdnzl@gmail.com> wrote:
> On 3/2/06, David S. Miller <davem@davemloft.net> wrote:
> >
> > This tree was getting crufty, so I rebased it today.
> > It was actually a lot easier than I had anticipated.
> >
> > master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.17.git
> >
> Dave,
>
> If you get a chance can you push the ccid3 divide by zero fix upstream
> to Linus for 2.6.16 as it has no functionality changed and eliminates
> a nasty little bug...
>
> The commit for this is b6da19617f4ab610d3d90bcbdf65fa7e2b3d7b53 in your tree
>
I have also put at end of this e-mail after reapplying on linus tree
so above commit doesn't have fuzz...
[DCCP] ccid3: Divide by zero fix
In rare circumstances 0 is returned by dccp_li_hist_calc_i_mean which leads to
a divide by zero in ccid3_hc_rx_packet_recv. Explicitly check for zero return
now. Update copyright notice at same time.
Found by Arnaldo.
Signed-off-by: Ian McDonald <imcdnzl@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index aa68e0a..35d1d34 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -2,7 +2,7 @@
* net/dccp/ccids/ccid3.c
*
* Copyright (c) 2005 The University of Waikato, Hamilton, New Zealand.
- * Copyright (c) 2005 Ian McDonald <iam4@cs.waikato.ac.nz>
+ * Copyright (c) 2005-6 Ian McDonald <imcdnzl@gmail.com>
*
* An implementation of the DCCP protocol
*
@@ -1033,9 +1033,13 @@ static void ccid3_hc_rx_packet_recv(stru
p_prev = hcrx->ccid3hcrx_p;
/* Calculate loss event rate */
- if (!list_empty(&hcrx->ccid3hcrx_li_hist))
+ if (!list_empty(&hcrx->ccid3hcrx_li_hist)) {
+ u32 i_mean = dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+
/* Scaling up by 1000000 as fixed decimal */
- hcrx->ccid3hcrx_p = 1000000 /
dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+ if (i_mean != 0)
+ hcrx->ccid3hcrx_p = 1000000 / i_mean;
+ }
if (hcrx->ccid3hcrx_p > p_prev) {
ccid3_hc_rx_send_feedback(sk);
^ permalink raw reply related
* Re: net-2.6.17 rebased...
From: Ian McDonald @ 2006-03-02 0:28 UTC (permalink / raw)
To: dccp
On 3/2/06, David S. Miller <davem@davemloft.net> wrote:
>
> This tree was getting crufty, so I rebased it today.
> It was actually a lot easier than I had anticipated.
>
> master.kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.17.git
>
Dave,
If you get a chance can you push the ccid3 divide by zero fix upstream
to Linus for 2.6.16 as it has no functionality changed and eliminates
a nasty little bug...
The commit for this is b6da19617f4ab610d3d90bcbdf65fa7e2b3d7b53 in your tree
I have also put at end of this e-mail after reapplying on linus tree
so above commit doesn't have fuzz...
[DCCP] ccid3: Divide by zero fix
In rare circumstances 0 is returned by dccp_li_hist_calc_i_mean which leads to
a divide by zero in ccid3_hc_rx_packet_recv. Explicitly check for zero return
now. Update copyright notice at same time.
Found by Arnaldo.
Signed-off-by: Ian McDonald <imcdnzl@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
--- 2b82f96f1291c42ee9485465801f5f51897bec64
+++ ff426a9009993445a15cfcac6c88be1e39e07913
@@ -2,7 +2,7 @@
* net/dccp/ccids/ccid3.c
*
* Copyright (c) 2005 The University of Waikato, Hamilton, New Zealand.
- * Copyright (c) 2005 Ian McDonald <iam4@cs.waikato.ac.nz>
+ * Copyright (c) 2005-6 Ian McDonald <imcdnzl@gmail.com>
*
* An implementation of the DCCP protocol
*
@@ -1014,9 +1014,13 @@ static void ccid3_hc_rx_packet_recv(stru
p_prev = hcrx->ccid3hcrx_p;
/* Calculate loss event rate */
- if (!list_empty(&hcrx->ccid3hcrx_li_hist))
+ if (!list_empty(&hcrx->ccid3hcrx_li_hist)) {
+ u32 i_mean = dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+
/* Scaling up by 1000000 as fixed decimal */
- hcrx->ccid3hcrx_p = 1000000 /
dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+ if (i_mean != 0)
+ hcrx->ccid3hcrx_p = 1000000 / i_mean;
+ }
if (hcrx->ccid3hcrx_p > p_prev) {
ccid3_hc_rx_send_feedback(sk);
^ permalink raw reply
* Re: [BUG] DCCP : Suspected race in ackvec code
From: Arnaldo Carvalho de Melo @ 2006-02-28 23:52 UTC (permalink / raw)
To: dccp
In-Reply-To: <cbec11ac0602281518uc14fe5fl73eda47b211a7d9f@mail.gmail.com>
On 2/28/06, Arnaldo Carvalho de Melo <acme@ghostprotocols.net> wrote:
> On 2/28/06, Ian McDonald <imcdnzl@gmail.com> wrote:
> > When send_ackvec sysctl is on DCCP locks the machine. Unfortunately it
> > doesn't send any output to the kernel.
>
> I'm getting below results by just stopping the syslog daemon and setting
> printk log level to 9:
>
> [root@qemu ~]# service syslog stop
> [root@qemu ~]# echo 9 > /proc/sys/kernel/printk
>
> With this the printk messages goes to the console, allowing us to see the
> dccp_pr_debug messages till the very moment when a soft lockup happens
> at dccp_ackvec_parse.
>
> [root@qemu ~]# modprobe dccp dccp_debug=1
> [root@qemu ~]# echo 3 > /proc/sys/net/dccp/default/rx_ccid
> [root@qemu ~]# echo 3 > /proc/sys/net/dccp/default/tx_ccid
>
> Oh well, not reproducing it right now, try with these settings to see you
> reproduce it in your setup.
Ok, here it is, I guess the problem is some leak or bad handling of OOM in
the ackvec code, as it is more easily triggered when I boot qemu using
only 16M:
[42949433.470000] dccp_ackvec_add: <7>dccp_insert_option_elapsed_time:
CLIENT TX opt: ELAPSED_TIME\x1012265, len=6, seqnoX78985321
[42949433.480000] dccp_insert_option_ackvec: CLIENT tx: ACK Vector 0,
len=1, ack_seqnoX78985321, ack_acknoX29316757
[42949433.480000] dccp_v4_rcv: DATAACK src\x127.0.0.1@3408
dst\x127.0.0.1@5001 seqX78985321, ackX29316757
[42949433.480000] dccp_parse_options: server rx opt: ELAPSED_TIME\x1012265
[42949433.480000] dccp_ackvec_add: <7>dccp_insert_option_elapsed_time:
CLIENT TX opt: ELAPSED_TIME\x1013363, len=6, seqnoX78985322
[42949433.490000] dccp_insert_option_ackvec: CLIENT tx: ACK Vector 0,
len=1, ack_seqnoX78985322, ack_acknoX29316757
[42949433.490000] dccp_v4_rcv: DATAACK src\x127.0.0.1@3408
dst\x127.0.0.1@5001 seqX78985322, ackX29316757
[42949433.490000] dccp_parse_options: server rx opt: ELAPSED_TIME\x1013363
[42949433.490000] dccp_ackvec_add: <7>dccp_insert_option_elapsed_time:
CLIENT TX opt: ELAPSED_TIME\x1014216, len=6, seqnoX78985323
[42949433.500000] dccp_insert_option_ackvec: CLIENT tx: ACK Vector 0,
len=1, ack_seqnoX78985323, ack_acknoX29316757
[42949433.500000] dccp_v4_rcv: DATAACK src\x127.0.0.1@3408
dst\x127.0.0.1@5001 seqX78985323, ackX29316757
[42949433.500000] dccp_parse_options: server rx opt: ELAPSED_TIME\x1014216
[42949433.500000] dccp_ackvec_add: <7>dccp_insert_option_elapsed_time:
CLIENT TX opt: ELAPSED_TIME\x1015700, len=6, seqnoX78985324
[42949433.510000] dccp_insert_option_ackvec: CLIENT tx: ACK Vector 0,
len=1, ack_seqnoX78985324, ack_acknoX29316757
[42949433.510000] dccp_v4_rcv: DATAACK src\x127.0.0.1@3408
dst\x127.0.0.1@5001 seqX78985324, ackX29316757
[42949433.510000] dccp_parse_options: server rx opt: ELAPSED_TIME\x1015700
[42949433.510000] dccp_ackvec_add: <7>dccp_insert_option_elapsed_time:
CLIENT TX opt: ELAPSED_TIME\x1016233, len=6, seqnoX78985325
[42949433.520000] dccp_insert_option_ackvec: CLIENT tx: ACK Vector 0,
len=1, ack_seqnoX78985325, ack_acknoX29316757
[42949433.520000] dccp_v4_rcv: DATAACK src\x127.0.0.1@3408
dst\x127.0.0.1@5001 seqX78985325, ackX29316757
[42949433.520000] dccp_parse_options: server rx opt: ELAPSED_TIME\x1016233
[42949433.520000] dccp_ackvec_add: ttcp-t: 256000 bytes in 10.81 real
seconds = 23.14 KB/sec +++
ttcp-t: 1000 I/O calls, msec/call = 11.06, calls/sec = 92.54
ttcp-t: 0.0user 0.5sys 0:10real 5% 0i+0d 0maxrss 0+1pf 1011+40csw
<7>dccp_set_state: client(c0858380) OPEN -> CLOSING
[42949433.530000] dccp_insert_option_elapsed_time: CLIENT TX opt:
ELAPSED_TIME\x1017569, len=6, seqnoX78985326
[42949433.530000] dccp_insert_option_ackvec: CLIENT tx: ACK Vector 0,
len=1, ack_seqnoX78985326, ack_acknoX29316757
[42949433.530000] dccp_v4_rcv: CLOSE src\x127.0.0.1@3408
dst\x127.0.0.1@5001 seqX78985326, ackX29316757
[42949433.530000] dccp_parse_options: server rx opt: ELAPSED_TIME\x1017569
[42949433.530000] dccp_ackvec_add: <7>dccp_insert_option_elapsed_time:
server TX opt: ELAPSED_TIME"0, len=4, seqnoX29316758
[42949433.530000] dccp_insert_option_ackvec: server tx: ACK Vector 0,
len\x15, ack_seqnoX29316758, ack_acknoX78985326
[42949433.530000] dccp_set_state: server(c0858040) OPEN -> CLOSED
[42949433.540000] dccp_v4_rcv: RESET src\x127.0.0.1@5001
dst\x127.0.0.1@3408 seqX29316758, ackX78985326
[42949433.540000] dccp_ackvec_check_rcv_ackvector: CLIENT rx ack: ACK
vector 0, len\x15, ack_seqnoX78985326, ack_acknoX29316757, ACKED!
[42949442.920000] BUG: soft lockup detected on CPU#0!
[42949442.920000]
[42949442.920000] Pid: 576, comm: ttcp
[42949442.920000] EIP: 0060:[<c1827582>] CPU: 0
[42949442.920000] EIP is at dccp_ackvec_parse+0x1b2/0x270 [dccp]
[42949442.920000] EFLAGS: 00000286 Not tainted (2.6.16-rc4acme #1)
[42949442.920000] EAX: ffc30000 EBX: 00015e6a ECX: 326e0000 EDX: ffffffff
[42949442.920000] ESI: 32310000 EDI: 00015e6a EBP: c06a52c0 DS: 007b ES: 007b
[42949442.920000] CR0: 8005003b CR2: b7e84039 CR3: 0064c000 CR4: 00000690
[42949442.920000] [<c18235dd>] dccp_parse_options+0x43d/0x770 [dccp]
[42949442.920000] [<c0113935>] local_bh_enable+0x75/0x80
[42949442.920000] [<c1821d1c>] dccp_rcv_state_process+0x7c/0x760 [dccp]
[42949442.920000] [<c02142c2>] ip_queue_xmit+0x192/0x4a0
[42949442.920000] [<c010f968>] vprintk+0x258/0x2d0
[42949442.920000] [<c182f389>] dccp_v4_do_rcv+0x49/0x290 [dccp_ipv4]
[42949442.920000] [<c18244dc>] dccp_transmit_skb+0x27c/0x410 [dccp]
[42949442.920000] [<c01f2e59>] release_sock+0x59/0xa0
[42949442.920000] [<c1825baf>] dccp_close+0xaf/0x200 [dccp]
[42949442.920000] [<c02366bf>] inet_release+0x3f/0x70
[42949442.920000] [<c01f0cef>] sock_release+0x1f/0x90
[42949442.920000] [<c01f0d91>] sock_close+0x31/0x50
[42949442.920000] [<c01448fd>] __fput+0x9d/0x150
[42949442.920000] [<c0141972>] filp_close+0x52/0x90
[42949442.920000] [<c010feeb>] put_files_struct+0x7b/0xd0
[42949442.920000] [<c0110ea4>] do_exit+0x104/0x7b0
[42949442.920000] [<c02c4ba4>] pcibios_fixup_bus+0xc4/0x120
[42949442.920000] [<c02c4ba0>] pcibios_fixup_bus+0xc0/0x120
[42949442.920000] [<c02c4bd0>] pcibios_fixup_bus+0xf0/0x120
[42949442.920000] [<c0111584>] do_group_exit+0x34/0x70
[42949442.920000] [<c0102755>] syscall_call+0x7/0xb
^ permalink raw reply
* Re: [BUG] DCCP : Suspected race in ackvec code
From: Arnaldo Carvalho de Melo @ 2006-02-28 23:38 UTC (permalink / raw)
To: dccp
In-Reply-To: <cbec11ac0602281518uc14fe5fl73eda47b211a7d9f@mail.gmail.com>
On 2/28/06, Ian McDonald <imcdnzl@gmail.com> wrote:
> When send_ackvec sysctl is on DCCP locks the machine. Unfortunately it
> doesn't send any output to the kernel.
I'm getting below results by just stopping the syslog daemon and setting
printk log level to 9:
[root@qemu ~]# service syslog stop
[root@qemu ~]# echo 9 > /proc/sys/kernel/printk
With this the printk messages goes to the console, allowing us to see the
dccp_pr_debug messages till the very moment when a soft lockup happens
at dccp_ackvec_parse.
[root@qemu ~]# modprobe dccp dccp_debug=1
[root@qemu ~]# echo 3 > /proc/sys/net/dccp/default/rx_ccid
[root@qemu ~]# echo 3 > /proc/sys/net/dccp/default/tx_ccid
Oh well, not reproducing it right now, try with these settings to see you
reproduce it in your setup.
Regards,
- Arnaldo
^ permalink raw reply
* [BUG] DCCP : Suspected race in ackvec code
From: Ian McDonald @ 2006-02-28 23:18 UTC (permalink / raw)
To: dccp
When send_ackvec sysctl is on DCCP locks the machine. Unfortunately it
doesn't send any output to the kernel.
Test cases:
On receive end (10.0.2.2):
./ttcp_acme -c -l500 -r
On send end (10.0.7.7):
./ttcp_acme -c -l500 -n1000 -t 10.0.2.2
And the setup on 10.0.5.5 (which provides a route between 10.0.2.2 and
10.0.7.7) is:
tc qdisc add dev eth0 root netem delay 50ms loss 10%
tc qdisc add dev eth1 root netem delay 50ms loss 10%
Passes:
echo 0 > /proc/sys/net/dccp/default/send_ackvec
echo 3 > /proc/sys/net/dccp/default/rx_ccid
echo 3 > /proc/sys/net/dccp/default/tx_ccid
Locks the machine:
echo 1 > /proc/sys/net/dccp/default/send_ackvec
echo 3 > /proc/sys/net/dccp/default/rx_ccid
echo 3 > /proc/sys/net/dccp/default/tx_ccid
Locks the machine:
echo 1 > /proc/sys/net/dccp/default/send_ackvec
echo 2 > /proc/sys/net/dccp/default/rx_ccid
echo 2 > /proc/sys/net/dccp/default/tx_ccid
Doesn't lock the machine but doesn't complete the test:
echo 0 > /proc/sys/net/dccp/default/send_ackvec
echo 2 > /proc/sys/net/dccp/default/rx_ccid
echo 2 > /proc/sys/net/dccp/default/tx_ccid
The only reason I say that there is a race is that the problems are
similar to when I had races without proper locking when implementing
tx buffering. When you inject delay/loss with netem it triggers a lot
more code coverage because timers get fired, data is missing etc.
I've had a quick look around but can't find it yet - I find these
races quite nasty to find but will keep looking at it over as time
permits...
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 0/4][DCCP]: Fixes and cleanups
From: David S. Miller @ 2006-02-28 22:54 UTC (permalink / raw)
To: dccp
In-Reply-To: <11411645423360-git-send-email-acme@mandriva.com>
From: Arnaldo Carvalho de Melo <acme@mandriva.com>
Date: Tue, 28 Feb 2006 19:09:02 -0300
> Please consider pulling from:
>
> master.kernel.org:/pub/scm/linux/kernel/git/acme/net-2.6.17.git
All looks good, pulled.
Thanks a lot.
^ permalink raw reply
* [PATCH 2/4] [DCCP] ccid2: coding style cleanups
From: Arnaldo Carvalho de Melo @ 2006-02-28 22:09 UTC (permalink / raw)
To: dccp
No changes in the logic where made.
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
---
net/dccp/ccids/ccid2.c | 76 ++++++++++++++++--------------------------------
1 files changed, 26 insertions(+), 50 deletions(-)
1c6ddda9288aba6a761b45357d119a47df165d4a
diff --git a/net/dccp/ccids/ccid2.c b/net/dccp/ccids/ccid2.c
index 11c73ab..e00e225 100644
--- a/net/dccp/ccids/ccid2.c
+++ b/net/dccp/ccids/ccid2.c
@@ -37,10 +37,7 @@
static int ccid2_debug;
-#if 0
-#define CCID2_DEBUG
-#endif
-
+#undef CCID2_DEBUG
#ifdef CCID2_DEBUG
#define ccid2_pr_debug(format, a...) \
do { if (ccid2_debug) \
@@ -56,10 +53,8 @@ static const int ccid2_seq_len = 128;
static void ccid2_hc_tx_check_sanity(const struct ccid2_hc_tx_sock *hctx)
{
int len = 0;
- struct ccid2_seq *seqp;
int pipe = 0;
-
- seqp = hctx->ccid2hctx_seqh;
+ struct ccid2_seq *seqp = hctx->ccid2hctx_seqh;
/* there is data in the chain */
if (seqp != hctx->ccid2hctx_seqt) {
@@ -69,9 +64,8 @@ static void ccid2_hc_tx_check_sanity(con
pipe++;
while (seqp != hctx->ccid2hctx_seqt) {
- struct ccid2_seq *prev;
+ struct ccid2_seq *prev = seqp->ccid2s_prev;
- prev = seqp->ccid2s_prev;
len++;
if (!prev->ccid2s_acked)
pipe++;
@@ -92,7 +86,7 @@ static void ccid2_hc_tx_check_sanity(con
seqp = seqp->ccid2s_prev;
len++;
BUG_ON(len > ccid2_seq_len);
- } while(seqp != hctx->ccid2hctx_seqh);
+ } while (seqp != hctx->ccid2hctx_seqh);
BUG_ON(len != ccid2_seq_len);
ccid2_pr_debug("total len=%d\n", len);
@@ -141,7 +135,7 @@ static void ccid2_change_l_ack_ratio(str
* -sorbo.
*/
if (val != 2) {
- struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
+ const struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
int max = hctx->ccid2hctx_cwnd / 2;
/* round up */
@@ -179,9 +173,6 @@ static void ccid2_hc_tx_rto_expire(unsig
struct ccid2_hc_tx_sock *hctx = ccid2_hc_tx_sk(sk);
long s;
- /* XXX I don't think i'm locking correctly
- * -sorbo.
- */
bh_lock_sock(sk);
if (sock_owned_by_user(sk)) {
sk_reset_timer(sk, &hctx->ccid2hctx_rtotimer,
@@ -278,36 +269,33 @@ static void ccid2_hc_tx_packet_sent(stru
/* We had an ack loss in this window... */
if (hctx->ccid2hctx_ackloss) {
if (hctx->ccid2hctx_arsent >= hctx->ccid2hctx_cwnd) {
- hctx->ccid2hctx_arsent = 0;
- hctx->ccid2hctx_ackloss = 0;
+ hctx->ccid2hctx_arsent = 0;
+ hctx->ccid2hctx_ackloss = 0;
}
- }
- /* No acks lost up to now... */
- else {
+ } else {
+ /* No acks lost up to now... */
/* decrease ack ratio if enough packets were sent */
if (dp->dccps_l_ack_ratio > 1) {
/* XXX don't calculate denominator each time */
- int denom;
+ int denom = dp->dccps_l_ack_ratio * dp->dccps_l_ack_ratio -
+ dp->dccps_l_ack_ratio;
- denom = dp->dccps_l_ack_ratio * dp->dccps_l_ack_ratio -
- dp->dccps_l_ack_ratio;
denom = hctx->ccid2hctx_cwnd * hctx->ccid2hctx_cwnd / denom;
if (hctx->ccid2hctx_arsent >= denom) {
ccid2_change_l_ack_ratio(sk, dp->dccps_l_ack_ratio - 1);
hctx->ccid2hctx_arsent = 0;
}
- }
- /* we can't increase ack ratio further [1] */
- else {
+ } else {
+ /* we can't increase ack ratio further [1] */
hctx->ccid2hctx_arsent = 0; /* or maybe set it to cwnd*/
}
}
/* setup RTO timer */
- if (!timer_pending(&hctx->ccid2hctx_rtotimer)) {
+ if (!timer_pending(&hctx->ccid2hctx_rtotimer))
ccid2_start_rto_timer(sk);
- }
+
#ifdef CCID2_DEBUG
ccid2_pr_debug("pipe=%d\n", hctx->ccid2hctx_pipe);
ccid2_pr_debug("Sent: seq=%llu\n", seq);
@@ -320,7 +308,7 @@ static void ccid2_hc_tx_packet_sent(stru
seqp->ccid2s_sent);
seqp = seqp->ccid2s_next;
}
- } while(0);
+ } while (0);
ccid2_pr_debug("=====\n");
ccid2_hc_tx_check_sanity(hctx);
#endif
@@ -378,7 +366,6 @@ static int ccid2_ackvector(struct sock *
*vec = value;
*veclen = len;
return offset + (opt_ptr - options);
- break;
}
}
@@ -416,13 +403,11 @@ static inline void ccid2_new_ack(struct
hctx->ccid2hctx_ssacks = 0;
*maxincr = *maxincr - 1;
}
- }
- /* increased cwnd enough for this single ack */
- else {
+ } else {
+ /* increased cwnd enough for this single ack */
hctx->ccid2hctx_ssacks = 0;
}
- }
- else {
+ } else {
hctx->ccid2hctx_ssacks = 0;
hctx->ccid2hctx_acks++;
@@ -444,8 +429,7 @@ static inline void ccid2_new_ack(struct
r, jiffies, seqp->ccid2s_seq);
hctx->ccid2hctx_srtt = r;
hctx->ccid2hctx_rttvar = r >> 1;
- }
- else {
+ } else {
/* RTTVAR */
long tmp = hctx->ccid2hctx_srtt - r;
if (tmp < 0)
@@ -528,12 +512,10 @@ static void ccid2_hc_tx_packet_recv(stru
if (hctx->ccid2hctx_rpdupack = -1) {
hctx->ccid2hctx_rpdupack = 0;
hctx->ccid2hctx_rpseq = seqno;
- }
- else {
+ } else {
/* check if packet is consecutive */
- if ((hctx->ccid2hctx_rpseq + 1) = seqno) {
+ if ((hctx->ccid2hctx_rpseq + 1) = seqno)
hctx->ccid2hctx_rpseq++;
- }
/* it's a later packet */
else if (after48(seqno, hctx->ccid2hctx_rpseq)) {
hctx->ccid2hctx_rpdupack++;
@@ -541,7 +523,6 @@ static void ccid2_hc_tx_packet_recv(stru
/* check if we got enough dupacks */
if (hctx->ccid2hctx_rpdupack > hctx->ccid2hctx_numdupack) {
-
hctx->ccid2hctx_rpdupack = -1; /* XXX lame */
hctx->ccid2hctx_rpseq = 0;
@@ -559,7 +540,6 @@ static void ccid2_hc_tx_packet_recv(stru
case DCCP_PKT_ACK:
case DCCP_PKT_DATAACK:
break;
-
default:
return;
}
@@ -612,11 +592,9 @@ static void ccid2_hc_tx_packet_recv(stru
if (state =
DCCP_ACKVEC_STATE_ECN_MARKED) {
loss = 1;
- }
- else {
+ } else
ccid2_new_ack(sk, seqp,
&maxincr);
- }
seqp->ccid2s_acked = 1;
ccid2_pr_debug("Got ack for %llu\n",
@@ -648,13 +626,11 @@ static void ccid2_hc_tx_packet_recv(stru
while (1) {
if (seqp->ccid2s_acked) {
done++;
- if (done = hctx->ccid2hctx_numdupack) {
+ if (done = hctx->ccid2hctx_numdupack)
break;
- }
}
- if (seqp = hctx->ccid2hctx_seqt) {
+ if (seqp = hctx->ccid2hctx_seqt)
break;
- }
seqp = seqp->ccid2s_prev;
}
@@ -798,6 +774,6 @@ static __exit void ccid2_module_exit(voi
module_exit(ccid2_module_exit);
MODULE_AUTHOR("Andrea Bittau <a.bittau@cs.ucl.ac.uk>");
-MODULE_DESCRIPTION("DCCP TCP CCID2 CCID");
+MODULE_DESCRIPTION("DCCP TCP-Like (CCID2) CCID");
MODULE_LICENSE("GPL");
MODULE_ALIAS("net-dccp-ccid-2");
--
1.2.2.gd27d
^ permalink raw reply related
* [PATCH 1/4] [DCCP] ipv6: cleanups
From: Arnaldo Carvalho de Melo @ 2006-02-28 22:09 UTC (permalink / raw)
To: dccp
No changes in the logic were made, just removing trailing whitespaces, etc.
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
---
net/dccp/ipv6.c | 256 ++++++++++++++++++++++++++++---------------------------
1 files changed, 129 insertions(+), 127 deletions(-)
1cb5220969e0f2ceef2ad5f0d280e8a98221ed63
diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c
index 6bd9979..f28f38f 100644
--- a/net/dccp/ipv6.c
+++ b/net/dccp/ipv6.c
@@ -1,6 +1,6 @@
/*
* DCCP over IPv6
- * Linux INET6 implementation
+ * Linux INET6 implementation
*
* Based on net/dccp6/ipv6.c
*
@@ -66,8 +66,8 @@ static void dccp_v6_hash(struct sock *sk
}
static inline u16 dccp_v6_check(struct dccp_hdr *dh, int len,
- struct in6_addr *saddr,
- struct in6_addr *daddr,
+ struct in6_addr *saddr,
+ struct in6_addr *daddr,
unsigned long base)
{
return csum_ipv6_magic(saddr, daddr, len, IPPROTO_DCCP, base);
@@ -82,17 +82,17 @@ static __u32 dccp_v6_init_sequence(struc
skb->nh.ipv6h->saddr.s6_addr32,
dh->dccph_dport,
dh->dccph_sport);
- else
- return secure_dccp_sequence_number(skb->nh.iph->daddr,
- skb->nh.iph->saddr,
- dh->dccph_dport,
- dh->dccph_sport);
+
+ return secure_dccp_sequence_number(skb->nh.iph->daddr,
+ skb->nh.iph->saddr,
+ dh->dccph_dport,
+ dh->dccph_sport);
}
-static int dccp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
+static int dccp_v6_connect(struct sock *sk, struct sockaddr *uaddr,
int addr_len)
{
- struct sockaddr_in6 *usin = (struct sockaddr_in6 *) uaddr;
+ struct sockaddr_in6 *usin = (struct sockaddr_in6 *)uaddr;
struct inet_connection_sock *icsk = inet_csk(sk);
struct inet_sock *inet = inet_sk(sk);
struct ipv6_pinfo *np = inet6_sk(sk);
@@ -105,10 +105,10 @@ static int dccp_v6_connect(struct sock *
dp->dccps_role = DCCP_ROLE_CLIENT;
- if (addr_len < SIN6_LEN_RFC2133)
+ if (addr_len < SIN6_LEN_RFC2133)
return -EINVAL;
- if (usin->sin6_family != AF_INET6)
+ if (usin->sin6_family != AF_INET6)
return -EAFNOSUPPORT;
memset(&fl, 0, sizeof(fl));
@@ -125,17 +125,15 @@ static int dccp_v6_connect(struct sock *
fl6_sock_release(flowlabel);
}
}
-
/*
- * connect() to INADDR_ANY means loopback (BSD'ism).
- */
-
- if (ipv6_addr_any(&usin->sin6_addr))
- usin->sin6_addr.s6_addr[15] = 0x1;
+ * connect() to INADDR_ANY means loopback (BSD'ism).
+ */
+ if (ipv6_addr_any(&usin->sin6_addr))
+ usin->sin6_addr.s6_addr[15] = 1;
addr_type = ipv6_addr_type(&usin->sin6_addr);
- if(addr_type & IPV6_ADDR_MULTICAST)
+ if (addr_type & IPV6_ADDR_MULTICAST)
return -ENETUNREACH;
if (addr_type & IPV6_ADDR_LINKLOCAL) {
@@ -160,9 +158,8 @@ static int dccp_v6_connect(struct sock *
np->flow_label = fl.fl6_flowlabel;
/*
- * DCCP over IPv4
+ * DCCP over IPv4
*/
-
if (addr_type = IPV6_ADDR_MAPPED) {
u32 exthdrlen = icsk->icsk_ext_hdr_len;
struct sockaddr_in sin;
@@ -180,7 +177,6 @@ static int dccp_v6_connect(struct sock *
sk->sk_backlog_rcv = dccp_v4_do_rcv;
err = dccp_v4_connect(sk, (struct sockaddr *)&sin, sizeof(sin));
-
if (err) {
icsk->icsk_ext_hdr_len = exthdrlen;
icsk->icsk_af_ops = &dccp_ipv6_af_ops;
@@ -206,8 +202,9 @@ static int dccp_v6_connect(struct sock *
fl.fl_ip_dport = usin->sin6_port;
fl.fl_ip_sport = inet->sport;
- if (np->opt && np->opt->srcrt) {
- struct rt0_hdr *rt0 = (struct rt0_hdr *)np->opt->srcrt;
+ if (np->opt != NULL && np->opt->srcrt != NULL) {
+ const struct rt0_hdr *rt0 = (struct rt0_hdr *)np->opt->srcrt;
+
ipv6_addr_copy(&final, &fl.fl6_dst);
ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
final_p = &final;
@@ -216,10 +213,12 @@ static int dccp_v6_connect(struct sock *
err = ip6_dst_lookup(sk, &dst, &fl);
if (err)
goto failure;
+
if (final_p)
ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
+ err = xfrm_lookup(&dst, &fl, sk, 0);
+ if (err < 0)
goto failure;
if (saddr = NULL) {
@@ -234,7 +233,7 @@ static int dccp_v6_connect(struct sock *
ip6_dst_store(sk, dst, NULL);
icsk->icsk_ext_hdr_len = 0;
- if (np->opt)
+ if (np->opt != NULL)
icsk->icsk_ext_hdr_len = (np->opt->opt_flen +
np->opt->opt_nflen);
@@ -308,7 +307,6 @@ static void dccp_v6_err(struct sk_buff *
/* icmp should have updated the destination cache entry */
dst = __sk_dst_check(sk, np->dst_cookie);
-
if (dst = NULL) {
struct inet_sock *inet = inet_sk(sk);
struct flowi fl;
@@ -325,16 +323,17 @@ static void dccp_v6_err(struct sk_buff *
fl.fl_ip_dport = inet->dport;
fl.fl_ip_sport = inet->sport;
- if ((err = ip6_dst_lookup(sk, &dst, &fl))) {
+ err = ip6_dst_lookup(sk, &dst, &fl);
+ if (err) {
sk->sk_err_soft = -err;
goto out;
}
- if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0) {
+ err = xfrm_lookup(&dst, &fl, sk, 0);
+ if (err < 0) {
sk->sk_err_soft = -err;
goto out;
}
-
} else
dst_hold(dst);
@@ -358,11 +357,12 @@ static void dccp_v6_err(struct sk_buff *
req = inet6_csk_search_req(sk, &prev, dh->dccph_dport,
&hdr->daddr, &hdr->saddr,
inet6_iif(skb));
- if (!req)
+ if (req = NULL)
goto out;
- /* ICMPs are not backlogged, hence we cannot get
- * an established socket here.
+ /*
+ * ICMPs are not backlogged, hence we cannot get an established
+ * socket here.
*/
BUG_TRAP(req->sk = NULL);
@@ -376,7 +376,7 @@ static void dccp_v6_err(struct sk_buff *
case DCCP_REQUESTING:
case DCCP_RESPOND: /* Cannot happen.
- It can, it SYNs are crossed. --ANK */
+ It can, it SYNs are crossed. --ANK */
if (!sock_owned_by_user(sk)) {
DCCP_INC_STATS_BH(DCCP_MIB_ATTEMPTFAILS);
sk->sk_err = err;
@@ -385,7 +385,6 @@ static void dccp_v6_err(struct sk_buff *
* (see connect in sock.c)
*/
sk->sk_error_report(sk);
-
dccp_done(sk);
} else
sk->sk_err_soft = err;
@@ -431,14 +430,16 @@ static int dccp_v6_send_response(struct
ireq6->pktopts) {
struct sk_buff *pktopts = ireq6->pktopts;
struct inet6_skb_parm *rxopt = IP6CB(pktopts);
+
if (rxopt->srcrt)
opt = ipv6_invert_rthdr(sk,
(struct ipv6_rt_hdr *)(pktopts->nh.raw +
rxopt->srcrt));
}
- if (opt && opt->srcrt) {
- struct rt0_hdr *rt0 = (struct rt0_hdr *)opt->srcrt;
+ if (opt != NULL && opt->srcrt != NULL) {
+ const struct rt0_hdr *rt0 = (struct rt0_hdr *)opt->srcrt;
+
ipv6_addr_copy(&final, &fl.fl6_dst);
ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
final_p = &final;
@@ -447,15 +448,19 @@ static int dccp_v6_send_response(struct
err = ip6_dst_lookup(sk, &dst, &fl);
if (err)
goto done;
+
if (final_p)
ipv6_addr_copy(&fl.fl6_dst, final_p);
- if ((err = xfrm_lookup(&dst, &fl, sk, 0)) < 0)
+
+ err = xfrm_lookup(&dst, &fl, sk, 0);
+ if (err < 0)
goto done;
}
skb = dccp_make_response(sk, dst, req);
if (skb != NULL) {
struct dccp_hdr *dh = dccp_hdr(skb);
+
dh->dccph_checksum = dccp_v6_check(dh, skb->len,
&ireq6->loc_addr,
&ireq6->rmt_addr,
@@ -469,7 +474,7 @@ static int dccp_v6_send_response(struct
}
done:
- if (opt && opt != np->opt)
+ if (opt != NULL && opt != np->opt)
sock_kfree_s(sk, opt, opt->tot_len);
dst_release(dst);
return err;
@@ -500,7 +505,7 @@ static void dccp_v6_send_check(struct so
struct dccp_hdr *dh = dccp_hdr(skb);
dh->dccph_checksum = csum_ipv6_magic(&np->saddr, &np->daddr,
- len, IPPROTO_DCCP,
+ len, IPPROTO_DCCP,
csum_partial((char *)dh,
dh->dccph_doff << 2,
skb->csum));
@@ -508,7 +513,7 @@ static void dccp_v6_send_check(struct so
static void dccp_v6_ctl_send_reset(struct sk_buff *rxskb)
{
- struct dccp_hdr *rxdh = dccp_hdr(rxskb), *dh;
+ struct dccp_hdr *rxdh = dccp_hdr(rxskb), *dh;
const int dccp_hdr_reset_len = sizeof(struct dccp_hdr) +
sizeof(struct dccp_hdr_ext) +
sizeof(struct dccp_hdr_reset);
@@ -520,7 +525,7 @@ static void dccp_v6_ctl_send_reset(struc
return;
if (!ipv6_unicast_destination(rxskb))
- return;
+ return;
/*
* We need to grab some memory, and put together an RST,
@@ -529,7 +534,7 @@ static void dccp_v6_ctl_send_reset(struc
skb = alloc_skb(MAX_HEADER + sizeof(struct ipv6hdr) +
dccp_hdr_reset_len, GFP_ATOMIC);
- if (skb = NULL)
+ if (skb = NULL)
return;
skb_reserve(skb, MAX_HEADER + sizeof(struct ipv6hdr) +
@@ -608,7 +613,7 @@ static void dccp_v6_ctl_send_ack(struct
dh->dccph_dport = rxdh->dccph_sport;
dh->dccph_doff = dccp_hdr_ack_len / 4;
dh->dccph_x = 1;
-
+
dccp_hdr_set_seq(dh, DCCP_SKB_CB(rxskb)->dccpd_ack_seq);
dccp_hdr_set_ack(dccp_hdr_ack_bits(skb),
DCCP_SKB_CB(rxskb)->dccpd_seq);
@@ -660,7 +665,6 @@ static struct sock *dccp_v6_hnd_req(stru
&iph->saddr, dh->dccph_sport,
&iph->daddr, ntohs(dh->dccph_dport),
inet6_iif(skb));
-
if (nsk != NULL) {
if (nsk->sk_state != DCCP_TIME_WAIT) {
bh_lock_sock(nsk);
@@ -689,17 +693,17 @@ static int dccp_v6_conn_request(struct s
return dccp_v4_conn_request(sk, skb);
if (!ipv6_unicast_destination(skb))
- goto drop;
+ goto drop;
if (dccp_bad_service_code(sk, service)) {
reset_code = DCCP_RESET_CODE_BAD_SERVICE_CODE;
goto drop;
}
/*
- * There are no SYN attacks on IPv6, yet...
+ * There are no SYN attacks on IPv6, yet...
*/
if (inet_csk_reqsk_queue_is_full(sk))
- goto drop;
+ goto drop;
if (sk_acceptq_is_full(sk) && inet_csk_reqsk_queue_young(sk) > 1)
goto drop;
@@ -733,7 +737,7 @@ static int dccp_v6_conn_request(struct s
ipv6_addr_type(&ireq6->rmt_addr) & IPV6_ADDR_LINKLOCAL)
ireq6->iif = inet6_iif(skb);
- /*
+ /*
* Step 3: Process LISTEN state
*
* Set S.ISR, S.GSR, S.SWL, S.SWH from packet or Init Cookie
@@ -777,9 +781,8 @@ static struct sock *dccp_v6_request_recv
/*
* v6 mapped
*/
-
newsk = dccp_v4_request_recv_sock(sk, skb, req, dst);
- if (newsk = NULL)
+ if (newsk = NULL)
return NULL;
newdp6 = (struct dccp6_sock *)newsk;
@@ -825,9 +828,9 @@ static struct sock *dccp_v6_request_recv
if (sk_acceptq_is_full(sk))
goto out_overflow;
- if (np->rxopt.bits.osrcrt = 2 &&
- opt = NULL && ireq6->pktopts) {
- struct inet6_skb_parm *rxopt = IP6CB(ireq6->pktopts);
+ if (np->rxopt.bits.osrcrt = 2 && opt = NULL && ireq6->pktopts) {
+ const struct inet6_skb_parm *rxopt = IP6CB(ireq6->pktopts);
+
if (rxopt->srcrt)
opt = ipv6_invert_rthdr(sk,
(struct ipv6_rt_hdr *)(ireq6->pktopts->nh.raw +
@@ -841,8 +844,9 @@ static struct sock *dccp_v6_request_recv
memset(&fl, 0, sizeof(fl));
fl.proto = IPPROTO_DCCP;
ipv6_addr_copy(&fl.fl6_dst, &ireq6->rmt_addr);
- if (opt && opt->srcrt) {
- struct rt0_hdr *rt0 = (struct rt0_hdr *) opt->srcrt;
+ if (opt != NULL && opt->srcrt != NULL) {
+ const struct rt0_hdr *rt0 = (struct rt0_hdr *)opt->srcrt;
+
ipv6_addr_copy(&final, &fl.fl6_dst);
ipv6_addr_copy(&fl.fl6_dst, rt0->addr);
final_p = &final;
@@ -860,7 +864,7 @@ static struct sock *dccp_v6_request_recv
if ((xfrm_lookup(&dst, &fl, sk, 0)) < 0)
goto out;
- }
+ }
newsk = dccp_create_openreq_child(sk, req, skb);
if (newsk = NULL)
@@ -873,9 +877,8 @@ static struct sock *dccp_v6_request_recv
*/
ip6_dst_store(newsk, dst, NULL);
- newsk->sk_route_caps = dst->dev->features &
- ~(NETIF_F_IP_CSUM | NETIF_F_TSO);
-
+ newsk->sk_route_caps = dst->dev->features & ~(NETIF_F_IP_CSUM |
+ NETIF_F_TSO);
newdp6 = (struct dccp6_sock *)newsk;
newinet = inet_sk(newsk);
newinet->pinet6 = &newdp6->inet6;
@@ -889,7 +892,7 @@ static struct sock *dccp_v6_request_recv
ipv6_addr_copy(&newnp->rcv_saddr, &ireq6->loc_addr);
newsk->sk_bound_dev_if = ireq6->iif;
- /* Now IPv6 options...
+ /* Now IPv6 options...
First: no IPv4 options.
*/
@@ -911,20 +914,20 @@ static struct sock *dccp_v6_request_recv
newnp->mcast_oif = inet6_iif(skb);
newnp->mcast_hops = skb->nh.ipv6h->hop_limit;
- /* Clone native IPv6 options from listening socket (if any)
-
- Yes, keeping reference count would be much more clever,
- but we make one more one thing there: reattach optmem
- to newsk.
+ /*
+ * Clone native IPv6 options from listening socket (if any)
+ *
+ * Yes, keeping reference count would be much more clever, but we make
+ * one more one thing there: reattach optmem to newsk.
*/
- if (opt) {
+ if (opt != NULL) {
newnp->opt = ipv6_dup_options(newsk, opt);
if (opt != np->opt)
sock_kfree_s(sk, opt, opt->tot_len);
}
inet_csk(newsk)->icsk_ext_hdr_len = 0;
- if (newnp->opt)
+ if (newnp->opt != NULL)
inet_csk(newsk)->icsk_ext_hdr_len = (newnp->opt->opt_nflen +
newnp->opt->opt_flen);
@@ -941,7 +944,7 @@ out_overflow:
NET_INC_STATS_BH(LINUX_MIB_LISTENOVERFLOWS);
out:
NET_INC_STATS_BH(LINUX_MIB_LISTENDROPS);
- if (opt && opt != np->opt)
+ if (opt != NULL && opt != np->opt)
sock_kfree_s(sk, opt, opt->tot_len);
dst_release(dst);
return NULL;
@@ -975,8 +978,8 @@ static int dccp_v6_do_rcv(struct sock *s
goto discard;
/*
- * socket locking is here for SMP purposes as backlog rcv
- * is currently called with bh processing disabled.
+ * socket locking is here for SMP purposes as backlog rcv is currently
+ * called with bh processing disabled.
*/
/* Do Stevens' IPV6_PKTOPTIONS.
@@ -1001,20 +1004,20 @@ static int dccp_v6_do_rcv(struct sock *s
return 0;
}
- if (sk->sk_state = DCCP_LISTEN) {
+ if (sk->sk_state = DCCP_LISTEN) {
struct sock *nsk = dccp_v6_hnd_req(sk, skb);
- if (!nsk)
- goto discard;
+ if (nsk = NULL)
+ goto discard;
/*
* Queue it on the new socket if the new socket is active,
* otherwise we just shortcircuit this and continue with
* the new socket..
*/
- if(nsk != sk) {
+ if (nsk != sk) {
if (dccp_child_process(sk, nsk, skb))
goto reset;
- if (opt_skb)
+ if (opt_skb != NULL)
__kfree_skb(opt_skb);
return 0;
}
@@ -1027,7 +1030,7 @@ static int dccp_v6_do_rcv(struct sock *s
reset:
dccp_v6_ctl_send_reset(skb);
discard:
- if (opt_skb)
+ if (opt_skb != NULL)
__kfree_skb(opt_skb);
kfree_skb(skb);
return 0;
@@ -1060,7 +1063,7 @@ static int dccp_v6_rcv(struct sk_buff **
dh->dccph_sport,
&skb->nh.ipv6h->daddr, ntohs(dh->dccph_dport),
inet6_iif(skb));
- /*
+ /*
* Step 2:
* If no socket ...
* Generate Reset(No Connection) unless P.type = Reset
@@ -1069,15 +1072,14 @@ static int dccp_v6_rcv(struct sk_buff **
if (sk = NULL)
goto no_dccp_socket;
- /*
+ /*
* Step 2:
* ... or S.state = TIMEWAIT,
* Generate Reset(No Connection) unless P.type = Reset
* Drop packet and return
*/
-
if (sk->sk_state = DCCP_TIME_WAIT)
- goto do_time_wait;
+ goto do_time_wait;
if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb))
goto discard_and_relse;
@@ -1116,32 +1118,32 @@ do_time_wait:
}
static struct inet_connection_sock_af_ops dccp_ipv6_af_ops = {
- .queue_xmit = inet6_csk_xmit,
- .send_check = dccp_v6_send_check,
- .rebuild_header = inet6_sk_rebuild_header,
- .conn_request = dccp_v6_conn_request,
- .syn_recv_sock = dccp_v6_request_recv_sock,
- .net_header_len = sizeof(struct ipv6hdr),
- .setsockopt = ipv6_setsockopt,
- .getsockopt = ipv6_getsockopt,
- .addr2sockaddr = inet6_csk_addr2sockaddr,
- .sockaddr_len = sizeof(struct sockaddr_in6)
+ .queue_xmit = inet6_csk_xmit,
+ .send_check = dccp_v6_send_check,
+ .rebuild_header = inet6_sk_rebuild_header,
+ .conn_request = dccp_v6_conn_request,
+ .syn_recv_sock = dccp_v6_request_recv_sock,
+ .net_header_len = sizeof(struct ipv6hdr),
+ .setsockopt = ipv6_setsockopt,
+ .getsockopt = ipv6_getsockopt,
+ .addr2sockaddr = inet6_csk_addr2sockaddr,
+ .sockaddr_len = sizeof(struct sockaddr_in6)
};
/*
* DCCP over IPv4 via INET6 API
*/
static struct inet_connection_sock_af_ops dccp_ipv6_mapped = {
- .queue_xmit = ip_queue_xmit,
- .send_check = dccp_v4_send_check,
- .rebuild_header = inet_sk_rebuild_header,
- .conn_request = dccp_v6_conn_request,
- .syn_recv_sock = dccp_v6_request_recv_sock,
- .net_header_len = sizeof(struct iphdr),
- .setsockopt = ipv6_setsockopt,
- .getsockopt = ipv6_getsockopt,
- .addr2sockaddr = inet6_csk_addr2sockaddr,
- .sockaddr_len = sizeof(struct sockaddr_in6)
+ .queue_xmit = ip_queue_xmit,
+ .send_check = dccp_v4_send_check,
+ .rebuild_header = inet_sk_rebuild_header,
+ .conn_request = dccp_v6_conn_request,
+ .syn_recv_sock = dccp_v6_request_recv_sock,
+ .net_header_len = sizeof(struct iphdr),
+ .setsockopt = ipv6_setsockopt,
+ .getsockopt = ipv6_getsockopt,
+ .addr2sockaddr = inet6_csk_addr2sockaddr,
+ .sockaddr_len = sizeof(struct sockaddr_in6)
};
/* NOTE: A lot of things set to zero explicitly by call to
@@ -1168,35 +1170,35 @@ static int dccp_v6_destroy_sock(struct s
}
static struct proto dccp_v6_prot = {
- .name = "DCCPv6",
- .owner = THIS_MODULE,
- .close = dccp_close,
- .connect = dccp_v6_connect,
- .disconnect = dccp_disconnect,
- .ioctl = dccp_ioctl,
- .init = dccp_v6_init_sock,
- .setsockopt = dccp_setsockopt,
- .getsockopt = dccp_getsockopt,
- .sendmsg = dccp_sendmsg,
- .recvmsg = dccp_recvmsg,
- .backlog_rcv = dccp_v6_do_rcv,
- .hash = dccp_v6_hash,
- .unhash = dccp_unhash,
- .accept = inet_csk_accept,
- .get_port = dccp_v6_get_port,
- .shutdown = dccp_shutdown,
- .destroy = dccp_v6_destroy_sock,
- .orphan_count = &dccp_orphan_count,
- .max_header = MAX_DCCP_HEADER,
- .obj_size = sizeof(struct dccp6_sock),
- .rsk_prot = &dccp6_request_sock_ops,
- .twsk_prot = &dccp6_timewait_sock_ops,
+ .name = "DCCPv6",
+ .owner = THIS_MODULE,
+ .close = dccp_close,
+ .connect = dccp_v6_connect,
+ .disconnect = dccp_disconnect,
+ .ioctl = dccp_ioctl,
+ .init = dccp_v6_init_sock,
+ .setsockopt = dccp_setsockopt,
+ .getsockopt = dccp_getsockopt,
+ .sendmsg = dccp_sendmsg,
+ .recvmsg = dccp_recvmsg,
+ .backlog_rcv = dccp_v6_do_rcv,
+ .hash = dccp_v6_hash,
+ .unhash = dccp_unhash,
+ .accept = inet_csk_accept,
+ .get_port = dccp_v6_get_port,
+ .shutdown = dccp_shutdown,
+ .destroy = dccp_v6_destroy_sock,
+ .orphan_count = &dccp_orphan_count,
+ .max_header = MAX_DCCP_HEADER,
+ .obj_size = sizeof(struct dccp6_sock),
+ .rsk_prot = &dccp6_request_sock_ops,
+ .twsk_prot = &dccp6_timewait_sock_ops,
};
static struct inet6_protocol dccp_v6_protocol = {
- .handler = dccp_v6_rcv,
- .err_handler = dccp_v6_err,
- .flags = INET6_PROTO_NOPOLICY | INET6_PROTO_FINAL,
+ .handler = dccp_v6_rcv,
+ .err_handler = dccp_v6_err,
+ .flags = INET6_PROTO_NOPOLICY | INET6_PROTO_FINAL,
};
static struct proto_ops inet6_dccp_ops = {
--
1.2.2.gd27d
^ permalink raw reply related
* [PATCH 3/4] [DCCP] options: Fix some aspects of mandatory option processing
From: Arnaldo Carvalho de Melo @ 2006-02-28 22:09 UTC (permalink / raw)
To: dccp
According to dccp draft (draft-ietf-dccp-spec-13.txt) section 5.8.2 (Mandatory
Option) the following patch correct the handling of the following cases:
1) "... and any Mandatory options received on DCCP-Data packets MUST be
ignored."
2) "The connection is in error and should be reset with Reset Code 5, ...
if option O is absent (Mandatory was the last byte of the option list), or
if option O equals Mandatory."
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
Signed-off-by: Hagen Paul Pfeifer <hagen@jauu.net>
---
net/dccp/options.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletions(-)
61e2a0fe55ae1e6b2d1f7f10660065de8c606e5c
diff --git a/net/dccp/options.c b/net/dccp/options.c
index 79d228e..8867b6f 100644
--- a/net/dccp/options.c
+++ b/net/dccp/options.c
@@ -109,7 +109,8 @@ int dccp_parse_options(struct sock *sk,
case DCCPO_MANDATORY:
if (mandatory)
goto out_invalid_option;
- mandatory = 1;
+ if (pkt_type != DCCP_PKT_DATA)
+ mandatory = 1;
break;
case DCCPO_NDP_COUNT:
if (len > 3)
@@ -249,6 +250,10 @@ int dccp_parse_options(struct sock *sk,
mandatory = 0;
}
+ /* mandatory was the last byte in option list -> reset connection */
+ if (mandatory)
+ goto out_invalid_option;
+
return 0;
out_invalid_option:
--
1.2.2.gd27d
^ permalink raw reply related
* [PATCH 4/4] [DCCP] ccid3: Divide by zero fix
From: Arnaldo Carvalho de Melo @ 2006-02-28 22:09 UTC (permalink / raw)
To: dccp
In rare circumstances 0 is returned by dccp_li_hist_calc_i_mean which leads to
a divide by zero in ccid3_hc_rx_packet_recv. Explicitly check for zero return
now. Update copyright notice at same time.
Found by Arnaldo.
Signed-off-by: Ian McDonald <imcdnzl@gmail.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@mandriva.com>
---
net/dccp/ccids/ccid3.c | 10 +++++++---
1 files changed, 7 insertions(+), 3 deletions(-)
8135f5c98f27d04094f7931495a917d856192f6a
diff --git a/net/dccp/ccids/ccid3.c b/net/dccp/ccids/ccid3.c
index 2b82f96..ff426a9 100644
--- a/net/dccp/ccids/ccid3.c
+++ b/net/dccp/ccids/ccid3.c
@@ -2,7 +2,7 @@
* net/dccp/ccids/ccid3.c
*
* Copyright (c) 2005 The University of Waikato, Hamilton, New Zealand.
- * Copyright (c) 2005 Ian McDonald <iam4@cs.waikato.ac.nz>
+ * Copyright (c) 2005-6 Ian McDonald <imcdnzl@gmail.com>
*
* An implementation of the DCCP protocol
*
@@ -1014,9 +1014,13 @@ static void ccid3_hc_rx_packet_recv(stru
p_prev = hcrx->ccid3hcrx_p;
/* Calculate loss event rate */
- if (!list_empty(&hcrx->ccid3hcrx_li_hist))
+ if (!list_empty(&hcrx->ccid3hcrx_li_hist)) {
+ u32 i_mean = dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+
/* Scaling up by 1000000 as fixed decimal */
- hcrx->ccid3hcrx_p = 1000000 / dccp_li_hist_calc_i_mean(&hcrx->ccid3hcrx_li_hist);
+ if (i_mean != 0)
+ hcrx->ccid3hcrx_p = 1000000 / i_mean;
+ }
if (hcrx->ccid3hcrx_p > p_prev) {
ccid3_hc_rx_send_feedback(sk);
--
1.2.2.gd27d
^ 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