From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bk0-f43.google.com (mail-bk0-f43.google.com [209.85.214.43]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id 10E29202224 for ; Mon, 25 Jun 2012 08:20:22 -0700 (PDT) Received: by bkty5 with SMTP id y5so6846695bkt.16 for ; Mon, 25 Jun 2012 08:20:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; bh=pPXmcgDuUDW5ybC5Xu73//lXvhcxYYHZ264dz06pmvo=; b=xfjV0cF+TJ+I4PUCC9URkB1Wyc2zjeIWgGIPahUL5AoiITpp71biQ899Dqm0o8+3UJ LayFq3GuRKmfkMZPIk8XQfwNQJm2+Z0ZW8KRo/4WcDN1Y/SOK2gfVmXTYaRAPQ1ZydCv ot9lr9MogwCmSp/szj6vuPbVn7Hh1qZntuRlJVZt2+Ghy9o+gG4ovGx8xmglwcdXbcfC NyQuMn2LFDf4wpORMglxCM6acKwH7+z5t0d530gcvsl+JYPXa9ohIF5aMFJkOZL880dw 4f3LQkvJtVVKsOr2BknU6z0VnJjooDnDZZ9UdGhw9mi3G/IVhvlEuv3sSWusU+1pouGw LY5Q== Received: by 10.204.153.21 with SMTP id i21mr3927600bkw.38.1340637620323; Mon, 25 Jun 2012 08:20:20 -0700 (PDT) Received: from [172.28.89.78] ([74.125.122.49]) by mx.google.com with ESMTPS id h18sm47010393bkh.8.2012.06.25.08.20.18 (version=SSLv3 cipher=OTHER); Mon, 25 Jun 2012 08:20:19 -0700 (PDT) From: Eric Dumazet To: Dave Taht In-Reply-To: References: <1340600422-1806-1-git-send-email-dave.taht@bufferbloat.net> <1340600422-1806-2-git-send-email-dave.taht@bufferbloat.net> <1340601724.23933.16.camel@edumazet-glaptop> Content-Type: text/plain; charset="UTF-8" Date: Mon, 25 Jun 2012 17:20:17 +0200 Message-ID: <1340637617.10893.56.camel@edumazet-glaptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Content-Transfer-Encoding: 8bit Cc: codel@lists.bufferbloat.net, Dave =?ISO-8859-1?Q?T=E4ht?= Subject: Re: [Codel] [RFCv2 PATCH] codel: add ecn_target for when to drop rather than mark ecn packets X-BeenThere: codel@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: CoDel AQM discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 25 Jun 2012 15:20:23 -0000 On Mon, 2012-06-25 at 07:50 -0700, Dave Taht wrote: > On Sun, Jun 24, 2012 at 10:22 PM, Eric Dumazet wrote: > > On Sun, 2012-06-24 at 22:00 -0700, Dave Täht wrote: > >> From: Dave Taht > >> codel_Newton_step(vars); > >> - if (params->ecn && INET_ECN_set_ce(skb)) { > >> + if (params->ecn && > >> + params->ecn_target > vars->ldelay && > > > > Wrong test ? > > > > We want ECN if delay < ecn_target, not if delay > ecn_target > > > > (unresponsive flows will make delay being above ecn_target, while > > responsive ones should make delay more like target ( < en_target) > > > > if (params->ecn && > > vars->ldelay <= params->ecn_target && > > INET_ECN_set_ce(skb)) { > > The orig is the same test, as params->ecn_target > vars->ldelay > is equivalent to vars->ldelay <= params->ecn_target Yes, I must say I dont like the testing of a variable against a LIMIT using if (LIMIT > variable) I prefer for readability if (variable < LIMIT) If you take a look at linux code, your form is very seldom used. > My own mental debate was whether to switch ecn from a bool to being a > codel_time_t, > and use a value of 0 for noecn and whatever for the ecn target value. Well, why not.