[PATCH 0/10] [RFC]: CCID 3 updates

[PATCH 0/10] [RFC]: CCID 3 updates

[Gerrit Renker]

I have cut this into 10 small pieces to make reviewing easier. 

If everyone is happy with it, I can re-send it as one piece.

I have compile-tested all patches but I think CCID 3 needs some more
(other) work and then some performance-testing. 

Patch  1: Avoid congestion control on zero-sized data packets.
          Sending zero-sized Data/DataAck packets is theoretically possible, 
          but just makes trouble for CCID 3. This patch changes the error
          code and removes one message which always gets printed.

Patch  2: Remove redundant statements in ccid3_hc_tx_packet_sent.
          An analysis showed that one statement can be removed.

Patch  3: Resolves a small FIXME (Acks when nothing has been sent).

Patch  4: Consolidate timer resets.
          This considers calculating the nofeedback timeout when no feedback
          has previously been received. It applies common sense, since RFC 3448
          is not clear about this case (it recommends to use max(4*R, 2*s/X), but
          R is still undefined due to section 4.2)

Patch  5: Consistently update t_nom, t_ipi, t_delta.
          This, like the other patches, results from the earlier analysis and
          exploits the dependencies among variables.

Patch  6: Consolidate handling of t_RTO.
	  This updates several things related to t_RTO and nofeedback timeout,
          exploiting yet another dependency.
	  And it adds a FIXME: currently the way it is calculated is non-standard.
          The code does t_RTO              = max(4*R, 1 second)
                        nofeedback_timeout = max(t_RTO, 2 * s/X)
          (RFC 3448:    nofeedback_timeout = max(4 * R, 2 * s/X) )
          I have asked my colleague about it - is it good or bad to keep it this way???
        
Patch  7: Avoid `division by zero' errors.
          This adds the protection against dividing a zero RTT for the sender.
          I believe that this could be further improved - with regard to malfunctioning
          timestamps.
          
Patch  8: Larger initial windows.
          This implements the larger-initial-windows feature for CCID 3, as per [RFC 4342, 5.]

Patch  9: Add documentation for the TFRC structure fields (in include/linux/tfrc.h).
          ==> Here I have a suggestion. 
               The only, single source code file 
               which currently includes          include/linux/tfrc.h
               is                                net/dccp/ccids/lib/tfrc_equation.c
               Maybe it would be a good
               idea to merge                     include/linux/tfrc.h
               with                              net/dccp/ccids/lib/tfrc.h
               (also included by                 net/dccp/ccids/lib/tfrc_equation.c ),
               since                             net/dccp/ccids/lib/tfrc.h
               is currently almost empty ???

Patch 10: Simplify dccp_rcv_state_process.
          This adds a minor simplification in the control flow.


-- Gerrit

Re: [PATCH 0/10] [RFC]: CCID 3 updates

[Arnaldo Carvalho de Melo]

On Fri, Nov 24, 2006 at 04:19:04PM +0000, Gerrit Renker wrote:
> I have cut this into 10 small pieces to make reviewing easier. 
> 
> If everyone is happy with it, I can re-send it as one piece.

I'll look at it probably tomorrow (saturday), but please, please don't
combine patches (even less the ones you already splitted), keep it as
small and doing just one thing, that is always best for reviewing when
merging and, more importantly, afterwards, when we have to look back :-)

- Arnaldo

Re: [PATCH 0/10] [RFC]: CCID 3 updates

[Ian McDonald]

On 11/25/06, Arnaldo Carvalho de Melo <acme@mandriva.com> wrote:
> On Fri, Nov 24, 2006 at 04:19:04PM +0000, Gerrit Renker wrote:
> > I have cut this into 10 small pieces to make reviewing easier.
> >
> > If everyone is happy with it, I can re-send it as one piece.
>
> I'll look at it probably tomorrow (saturday), but please, please don't
> combine patches (even less the ones you already splitted), keep it as
> small and doing just one thing, that is always best for reviewing when
> merging and, more importantly, afterwards, when we have to look back :-)
>
I'm looking at these and signing them off when obviously correct.
Others I'm acking to say they look ok.

I'll do some regression testing hopefully later in the week but I
don't want to hold things up in case people want my opinion (as if I
am that important... ha ha)

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

Re: [PATCH 0/10] [RFC]: CCID 3 updates

[Gerrit Renker]

Quoting Arnaldo Carvalho de Melo:
|  On Fri, Nov 24, 2006 at 04:19:04PM +0000, Gerrit Renker wrote:
|  > I have cut this into 10 small pieces to make reviewing easier. 
|  > 
|  > If everyone is happy with it, I can re-send it as one piece.
|  
|  I'll look at it probably tomorrow (saturday), but please, please don't
|  combine patches (even less the ones you already splitted), keep it as
|  small and doing just one thing, that is always best for reviewing when
|  merging and, more importantly, afterwards, when we have to look back :-)
Thank you, I wasn't clear about that one and it is necessary. I have had some
time to go over what I have sent and over the CCID 3 code and I can find more
things that need tending to. Ian has done a lot of work getting the scaled-integer
operations right, but --horror-- there are more subtle bugs in this. For instance,
usecs_div is not used for calculating the one packet per second threshold, s/t_mbi
For instance,
  hctx->ccid3hctx_x = max_t(u32, ... 
					       (hctx->ccid3hctx_s /
					        TFRC_MAX_BACK_OFF_TIME));

TFRC_MAX_BACK_OFF_TIME is 64. If hctx->ccid3hctx_s < 64 bytes (which is frequent on
audio or VoIP packets, then integer division says `send at most 0 bytes per second'.
So I think it is best to go through the entire code and check each of these cases; maybe
use a set of `fixpoint' computation routines as used by the Kame patch.

Thank you and Ian for your feedback - I will check through this first and update patches
accordingly.

Re: [PATCH 0/10] [RFC]: CCID 3 updates

[Arnaldo Carvalho de Melo]

On 11/26/06, Ian McDonald <ian.mcdonald@jandi.co.nz> wrote:
> On 11/25/06, Arnaldo Carvalho de Melo <acme@mandriva.com> wrote:
> > On Fri, Nov 24, 2006 at 04:19:04PM +0000, Gerrit Renker wrote:
> > > I have cut this into 10 small pieces to make reviewing easier.
> > >
> > > If everyone is happy with it, I can re-send it as one piece.
> >
> > I'll look at it probably tomorrow (saturday), but please, please don't
> > combine patches (even less the ones you already splitted), keep it as
> > small and doing just one thing, that is always best for reviewing when
> > merging and, more importantly, afterwards, when we have to look back :-)
> >
> I'm looking at these and signing them off when obviously correct.
> Others I'm acking to say they look ok.
>
> I'll do some regression testing hopefully later in the week but I
> don't want to hold things up in case people want my opinion (as if I
> am that important... ha ha)

Much, much valued, I'm so pleased that now there is discussion about
dccp patches, more people involved, we must keep it that way and
atract more people to get DCCP on Linux (and elsewhere) useful as
designed.

- Arnaldo

Re: [PATCH 0/10] [RFC]: CCID 3 updates

[Gerrit Renker]

|  I'm looking at these and signing them off when obviously correct.
|  Others I'm acking to say they look ok.
Thank you very much for checking these. 
I have added all your Signed-off/Acked-by lines where provided.

|  I'll do some regression testing hopefully later in the week but I
|  don't want to hold things up
This would definitively be a great help. And in no way a hold-up, since
if you can spot things which aren't right, then they can be fixed now
so that we don't have to return to these. 
Please do, feedback and checking in all form is more than welcome.

Gerrit

Re: [PATCH 0/10] [RFC]: CCID 3 updates

[Arnaldo Carvalho de Melo]

On Mon, Nov 27, 2006 at 12:46:00PM +0000, Gerrit Renker wrote:
> |  I'm looking at these and signing them off when obviously correct.
> |  Others I'm acking to say they look ok.

> Thank you very much for checking these. 
> I have added all your Signed-off/Acked-by lines where provided.
> 
> |  I'll do some regression testing hopefully later in the week but I
> |  don't want to hold things up

> This would definitively be a great help. And in no way a hold-up, since
> if you can spot things which aren't right, then they can be fixed now
> so that we don't have to return to these. 
> Please do, feedback and checking in all form is more than welcome.

Yes, and its not like DCCP in Linux is rock solid and we're doing risky
changes, we're still in the development phase, where we know that what
is there still requires mucho love and care, if something we do proves
wrong we just fix it, business as usual :-)

- Arnaldo