Re: [PATCH 2/5]: DCCP Recalc on non-loss intervals

[Gerrit Renker <gerrit@erg.abdn.ac.uk>]

Hi Ian,

I appreciate the work you have put into this patch and have no doubt that
it may have cost a lot of time, but the problem here is one of perspective.

Let's separate the issues:

|  The RFC is a little confusing but I think I have carried out it's
|  intention correctly. I'll quote verbatim here the relevant parts:
|  
|     When calculating the average loss interval we need to decide whether
|     to include the interval since the most recent packet loss event.  We
|     only do this if it is sufficiently large to increase the average loss
|     interval.
|  
|     Thus if the most recent loss intervals are I_0 to I_n, with I_0 being
|     the interval since the most recent loss event, then we calculate the
|     average loss interval I_mean as:
|  
|  Notice here the "interval since the most recent packet loss event".
|  This implies (but would help if explicit) that this is an interval
|  with no loss. We only use if it would increase the average.
|  
|  Backing up my implications is that we have intervals from I_0 to I_n
|  which is actually n+1 intervals so therefore I read I_0 to be the
|  interval of non-loss. Does this help? Once you see that it all falls
|  into place.
|  
|  > If you mean this, then the patch is not necessary. The reason is that this statement
|  > refers to I_tot0 versus I_tot1. The statement
|  >
|  >   I_tot = max(I_tot0, I_tot1);
|  >
|  > takes care of using the `interval since the most recent packet loss event' only if
|  > it is sufficiently large. It is not explicit to see, but is implicitly contained in the
|  > way the two weighted averages are compared against each other: it boils down to
|  > comparing I_0 against the oldest 5 loss intervals.
|  >
|  > [ For a more detailed explanation, cf. the MSc thesis by Widmer, I also have a set of
|  >   notes on this case ]
|  >
|  I didn't mean that but someone had tried to implement it and had done
|  it incorrectly. The intent as I read is not to compare the calculation
|  with and without the most recent loss interval but to compare with and
|  without the non-loss interval.
 1) see previous posting - I think we are still having a misunderstanding here and
    I do think that what you cite above is already covered by the condition 
    I_tot = max(I_tot0, I_tot1)". That would mean, as soon as we implement [RFC 3448, 5.4],
    we have this sorted out (and this method has been tested for 7 years).

|  The implementation in the kernel, which was wrong logic, also had
|  wrong implementation! The code was taken over from FreeBSD (relicensed
|  under GPL) and we actually didn't analyse properly and were excluding
|  the wrong item from the loop as linked lists ran the opposite
|  direction in FreeBSD to Linux in this case!!
 2) if you think this has not been implemented correctly, then can you please point out
    > where <. The situation at the moment is: you are saying "X has been implemented
    wrongly, so I use Y to solve the problem". Y however has not much in common with X, so
    we are left with a broken method X and a non-standard method Y. 

|  > As far as I can see, it is non-standard.
|  
|  I believe it is standard as per earlier. It does boost performance but
|  correctly so. Search the archives for Burak and you will see what it
|  solves. It is particularly applicable to links like satellite with
|  fixed bandwidth. Think about say a 128 Kbits link and you get a loss
|  when you are at 40 Kbits per second. If you get no more loss then you
|  can never achieve your link speed as i_mean is not recalculated.
 3) I have a suggestion: you seem to be sure of the merits of this solution and report on 
    test results. My suggestion would be to offer using this method via a configuration
    option (same as per the nofeedback timer threshold). You could put detailed advice into
    the kernel configuration menu, and we would have the best of both worlds - a standards-
    compliant TFRC implementation and a configurable Ian's nifty solution. Would that settle
    things?


|  All this is is a method of not having to call calc_i_mean for EVERY
|  packet which would be very expensive on CPU. It is new but is a very
 4) This code is only invoked when ccid3_hc_rx_detect_loss() returns true, hence it is not
    invoked for every packet.