| How can I get a complete copy of the DCCP source checked out, including | your/Ian's patches? Where is there something on line that explains the git | process? Thanks for your/anyone's help. I found the following tutorial useful: http://www.kernel.org/pub/software/scm/git/docs/tutorial.html In addition, Ian's links are also very helpful - they have helped me in the past. In the git sources there are a few summary documents as well, I found Documentation/everyday.txt good Both my patches on http://www.erg.abdn.ac.uk/users/gerrit/dccp/patch-backlog/the_whole_lot.tar.gz and Ian's patches on http://wand.net.nz/~iam4/dccp/patches/ are `stacked': if you have the quilt tool then it is best to fork a new git branch and apply them in order. (If not, one can use for p in `cat series`; do patch -p1 < $p; done). There is overlap between Ian's and my patch set; I usually copy those patches from the list where we have finished discussion. | > ( I just found out that there is another | > PROBLEM#1: For the first loss interval (as per [RFC 3448, 6.3.1]), the timestamp | > and sequence number are not stored, only the interval is. ) I just noted that I was wrong here - timestamp and sequence number are stored, but by a different routine (dccp_li_hist_interval_new) - it would be great if we could perform the storing of timestamp/sequence number always in the same function (dccp_li_update_li). | > | > For all other loss events, timestamp and sequence number are stored. This also holds | > for I_0; it could be that the interval is 0 here due to (seq_temp becomes `interval') | > seq_temp = dccp_delta_seqno(head->dccplih_seqno, seq_nonloss); | > but I am not entirely sure about that. And it is probably safer to encode this | > case more explicitly. | > | > In any way, this clarification leads to | > PROBLEM#2: Since I_0 always sits at the head of the list, there are at most n=7 | > completed loss interval entries (not 8 as the RFC recommends). | | I find this difficult to follow, but is there still confusion? According to | my reading of the patches, and Ian confirmed this, what RFC3448 means by I_0 | **is not stored on the list at all**. The list stores I_1...I_8. Remember | that I_0 is the incomplete loss interval. ACK - I_0 is the incomplete loss interval. The way the code is currently organised, I_0 sits always on top of the list, which is due to: * in ccid3_hc_rx_packet_recv(), the call to ccid3_hc_rx_detect_loss() comes first * ccid3_hc_rx_detect_loss in turn calls dccp_li_update_li() * control then resumes in ccid3_hc_rx_packet_recv() Hence the detection of a loss event always triggers insertion of the sequence number and timestamp at the head of the list. With regard to your above comment, we should probably change this so that I_0 is not stored on top of the list - or adapt the list handling so that I_0 can be distinguished from I_1 ... I_k (k <= 8). | > | Your comments about Ian's patch are also wrong, for the same reason. I | > | believe Ian's patch calculates I_tot0 and I_tot1 correctly, although with | > | swapped names. Certainly if there is a problem it is not the one you have | > | identified. | > If the above assumption that the interval of I_0 is 0 holds then in Ian's patch | > (where I_0' is the interval called `non_loss' in Ian's patch) we have that | > - I_tot0 is the sum I_1 * w_1 + ... I_7 * w_7 | > - I_tot1 is the sum I_0' * w_0 + I_1 * w_2 + I_2 * w3 + ... + I_6 * w_7 | > This can't be right. | | Your analysis is wrong, I think. Ian's patch sent 12/27 calculates | | I_tot0 = I_1 * w_0 + ... + I_8 * w_7 | I_tot1 = I_0 * w_0 + I_1 * w_1 + ... + I_7 * w_7. | | Because "i" starts at 0. The relevant part of the patch. | | i_tot0 += li_entry->dccplih_interval * dccp_li_hist_w[i]; | w_tot += dccp_li_hist_w[i]; | + if (i != 7) | + i_tot1 += li_entry->dccplih_interval | + * dccp_li_hist_w[i + 1]; | + if (i == 0) { | + non_loss = dccp_hdr_seq(skb); | + sub48(&non_loss, li_entry->dccplih_seqno); | + } | | When "i == 0", the loop is examining I_1. Then i_tot0 += I_1*w[0], and i_tot1 | += I_1*w[1], exactly as one would like. The case "i==7" (that is, I_8) | affects i_tot0 but not i_tot1, exactly as one would like. I think it is different - that when 'i == 0' then we are looking at I_0. This is confirmed by the statement if (i == 0) { non_loss = dccp_hdr_seq(skb); sub48(&non_loss, li_entry->dccplih_seqno); } which shows how the code uses the last recorded loss sequence number li_entry->dccplih_seqno to compute the interval length for non_loss = I_0. Hence we have here that i_tot0 += I_0' * w[0] and i_tot1 += I_0' * w[1], where I_0' is the interval stored in li_entry. As per earlier email this could be 0 but I am still not entirely sure it is. In the second iteration of the loop we have i_tot0 += I_1 * w[1] i_tot1 += I_1 * w[2] and in the third we have i_tot0 += I_2 * w[2] i_tot1 += I_2 * w[3] but the RFC says I_tot0 = I_0 * w[0] + I_1 * w[1] + I_2 * w[2] + I_3 * w[3] + I_4 * w[4] + I_5 * w[5] + I_6 * w[6] + I_7 * w[7] I_tot1 = I_1 * w[0] + I_2 * w[1] + I_3 * w[2] + I_4 * w[3] + I_5 * w[4] + I_6 * w[5] + I_7 * w[6] + I_8 * w[7] which is different from what the above code does. So I think what we can take home as `todo' items from this is * correct the list handling with regard to I_0 * ensure that I_1 ... I_8 all fit in the list * compute the interval length I_0 as done for `non_loss' in Ian's patch * change the summation loop so that it matches the one in the RFC I am still trying to compile all this into (yet another :) summary, thanks for your comments. Gerrit - To unsubscribe from this list: send the line "unsubscribe dccp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
