netdev - Re: [PATCH 2.6.27] tcp_htcp: last

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4917B0E9.2030304@room52.net>
Date:	Mon, 10 Nov 2008 14:56:25 +1100
From:	Lawrence Stewart <lstewart@...m52.net>
To:	Douglas Leith <Doug.Leith@...m.ie>
CC:	Netdev <netdev@...r.kernel.org>,
	Stephen Hemminger <shemminger@...tta.com>,
	Lachlan Andrew <lachlan.andrew@...il.com>,
	grenville armitage <garmitage@...n.edu.au>
Subject: Re: [PATCH 2.6.27] tcp_htcp: last_cong bug fix

Hi Doug,

Thanks very much for looking at this.

My familiarity with the Linux TCP stack is not up to scratch as you 
know. I'm curious about the "if (ca->undo_last_cong)" condition you 
added to htcp_cwnd_undo(). Is there a possibility of the stack calling 
into this function multiple times without at least one congestion 
control related state change in between?

I will test your patch and confirm it resolves the issue we reported.

Also, while we're at it, any chance you might consider adding the 
functionality in the attached patch so that we can control adaptive 
backoff via a module tunable?

Cheers,
Lawrence



Douglas Leith wrote:
> This patch fixes a minor bug in tcp_htcp.c which has been highlighted by 
> Lachlan Andrew and Lawrence Stewart.   Currently, the time since the 
> last congestion event, which is stored in variable last_cong, is reset 
> whenever there is a state change into TCP_CA_Open.  This includes 
> transitions of the type TCP_CA_Open->TCP_CA_Disorder->TCP_CA_Open which 
> are not associated with backoff of cwnd.  The patch changes last_cong to 
> be updated only on transitions into TCP_CA_Open that occur after 
> experiencing the congestion-related states TCP_CA_Loss, TCP_CA_Recovery, 
> TCP_CA_CWR.
> 
> Doug
> 
> --- tcp_htcp.c   2008-11-05 15:50:18.000000000 +0000
> +++ tcp_htcp.c.new       2008-11-08 07:11:18.000000000 +0000
> @@ -69,9 +69,12 @@
>         const struct tcp_sock *tp = tcp_sk(sk);
>         struct htcp *ca = inet_csk_ca(sk);
> 
> -       ca->last_cong = ca->undo_last_cong;
> -       ca->maxRTT = ca->undo_maxRTT;
> -       ca->old_maxB = ca->undo_old_maxB;
> +       if (ca->undo_last_cong) {
> +               ca->last_cong = ca->undo_last_cong;
> +               ca->maxRTT = ca->undo_maxRTT;
> +               ca->old_maxB = ca->undo_old_maxB;
> +               ca->undo_last_cong=0; // flag that ca->last_cong is not 
> to be reset when enter TCP_CA_Open state
> +       }
> 
>         return max(tp->snd_cwnd, (tp->snd_ssthresh << 7) / ca->beta);
>  }
> @@ -265,12 +268,15 @@
>  static void htcp_state(struct sock *sk, u8 new_state)
>  {
>         switch (new_state) {
> -       case TCP_CA_Open:
> +       case TCP_CA_Open:
>                 {
>                         struct htcp *ca = inet_csk_ca(sk);
> -                       ca->last_cong = jiffies;
> +                       if (ca->undo_last_cong) {
> +                               ca->last_cong=jiffies;
> +                               ca->undo_last_cong=0;
> +                       }
>                 }
> -               break;
> +               break;
>         case TCP_CA_CWR:
>         case TCP_CA_Recovery:
>         case TCP_CA_Loss:
> 
> 


View attachment "switch_adaptive_backoff.patch" of type "text/plain" (850 bytes)