Re: Sequence Number Validation Bug Fixes 0/2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gerrit,

> | This tool converts DCCP CCID 2 connections to TCP connections which can
> | then be analyzed with Tcptrace (http://tcptrace.org). (I can provide a
> | link to the tool if people are interested).
> | 
> Please do -- such a tool has been missing for a long time, and the idea to hook
> it up with tcptrace is both original and efficient. Analysing Ack Vectors using
> tcpdump/wireshark is very tedious, so anything that can help in debugging Ack
> Vector traces is a great help.

The tool can be downloaded from here:
http://jeroserver.homelinux.net/software/dccp/files/dccp2tcp.tar.gz
or here: http://oak.cats.ohiou.edu/~sj323707/dccp2tcp.tar.gz

At the moment I'm calling it dccp2tcp and releasing it under the GNU
GPL. Right now, the tool only understands DCCP over IPv4 over Ethernet
and only compiles on Linux.

I've included a README file that explains the commandline args and what
the various parts of the Tcptrace graphs represent. The short story is
you probably want to run it like this: dccp2tcp -s dccp_file tcp_file

> 
> | It turns out this behavior is due to a nasty bug in the dccp sequence
> | number validation code (dccp_check_seqno in input.c). What happens is
> | that the receiver legitimately stops incrementing it's ack number
> | because packets from the sender are outside the valid Ack window (why
> | this happens is another problem I'm still working on). DCCP should send
> | a sync when this happens, but this sync needs to be rate-limited. If it
> | isn't time to send a sync, the DCCP code currently returns 0 which
> | indicates that the packet is good!! 
> | 
> Thanks a lot, you have found a real bug. I have edited your patch since
> Evolution squeezed tabs into whitespaces, and added your Signed-off --
> please check the attached result, as I would like to submit this soon.
>
Looks good. You have my okay to submit it.
> 
> With regard to not updating the Ack Number, I remember having seen something
> related earlier. There is a relationship between the current sending rate, the
> Ack Ratio, the cwnd, and the Sequence Window. The Ack window is advanced by the
> GSS and depends on the local value of the Sequence Window. RFC 4340, 7.5.2
> recommends 5 * max_packets_per_RTT as guideline for Sequence Window.
> 
> When I looked at this earlier I noted that the dynamic update of Ack Ratio,
> cwnd, and Sequence Window (which are all inter-related, but are controlled
> by different code paths) does not produce the expected interaction, therefore
> Ack Ratio is currently always 1 and Sequence Window is also static and does
> not change throughout the connection. 
> 
> It may be that both these shortcuts cause the behaviour you observed. The
> Ack Ratio update is disabled in net/dccp/feat.c:dccp_hdlr_ack_ratio(), but
> there is currently no code to predict (guess) what the sending rate will
> be for the next 5 RTTs -- if such a heuristic is possible.

Interesting, but I don't think that's what I'm seeing.

What I'm noticing is that the DCCP sender doesn't update it's ack
numbers very often. In examining packet captures, it is typical for DCCP
to send 20-60 packets with the same ack number (If I increase the ack
and seq validity windows to 1000, I've seen 200 packets with the same
ack number).

Since the receiver should ack every packet it gets, I would think that
we should see very few packets (1-2) sent before a new ack packet comes
and updates the GSR and hence the outgoing packet's ack numbers.

Some debugging output (printk at every send and receive) seems to
indicate that the sending and receiving are happening in large chunks
and not in small slices (we send 50 packets, then receive 20 packets,
etc).

At the moment I don't have a clue why that would happen. Thoughts/ideas
would be appreciated.

> 
> I am interested in suggestions to improve this.
> 
> | In examining this section of code, I also noticed that the the GSR is
> | updated by any packet in the valid sequence number window. I believe
> | this represents a minor bug, because it would allow the GSR to move
> | backward as a result of an out of order packet that is still in the
> | valid window. I am also sending a patch for that as a separate email.
> |
> Excellent catch, there is a second bug here. Apart from the clerical edits
> (please consult Documentation/SubmittingPatches), there is a small
> modification, sent in response to the other patch.

Thanks for the reference to Documentation/SubmittingPatches. Good
information to know.

Comments on the other patch following in a separate email.

Samuel Jero
Internetworking Research Group
Ohio University

Attachment: signature.asc
Description: This is a digitally signed message part


[Index of Archives]     [Linux Kernel]     [IETF DCCP]     [Linux Networking]     [Git]     [Security]     [Linux Assembly]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]

  Powered by Linux