From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-il1-x136.google.com (mail-il1-x136.google.com [IPv6:2607:f8b0:4864:20::136]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.bufferbloat.net (Postfix) with ESMTPS id A7E403B29E for ; Fri, 29 Oct 2021 11:22:28 -0400 (EDT) Received: by mail-il1-x136.google.com with SMTP id s3so11087268ild.0 for ; Fri, 29 Oct 2021 08:22:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :content-transfer-encoding; bh=J1ehUKYAUTuJwx741bSPTWosQWTmnlRKhvPFP2xYjoM=; b=aFPdCKqQ/qD22VKinSJ6a+VET9FDhMcMYrvafFQowVSpoLV8YOyheLQktGqoq83Nvj eKUlmanGKQ/Y8tKEZaYz62TfdZbjyV/iYcmZa0dcr2GfD47v5VwOeP5jqt4TNvXtVHQu VeGKzO3/X46pblN9RIFImlhngK0wBy67JgmsDUxW0vwbBhdPAWQ1iDc4Xk9Q3jNCKyNx 7D31i86Bu6nB7IfGddArN91VtAHNyBibfpeku2eKyAm1xUNbQgl5w9O6AwbFQvhg7Xym 4tVRCTqy8v+4FV/XQBJacI6rAXbQMSAziNuBLBZDPdZW7cG5IlSqlETXLLTzVDzgZ9bY 14xA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:content-transfer-encoding; bh=J1ehUKYAUTuJwx741bSPTWosQWTmnlRKhvPFP2xYjoM=; b=wK7IWfAXzmNkH4LZEH7zT4SfWedD/dmyDVaY0nCAnq8HeK/26mX4W9TI3DUTCOwi/i WDdOB/XnfZ7GqFga1QOAVPdIuAK6Xt0RIXSsCM1LzuFkx4KuW+n7YGSTZJynbpw49UWP h1IJmRIrbE0v4sAGSJuY3c2mdheKgTDc09ZlihKP84XIHU7Zu9iOrP/FvrxRVCyrChAO i/tSWATz4YIUEJ0fXIHkARaSWGjFFPnYVeHGyA0pp+CWVxQ4tT7aoFpW146RTKW0T1g8 rSjQbv0kVFFfDWMelKwG9glZIflxbHEL29MibAOvjade7qCHzPKHcs8Pp/x1uDNUDBnx 7y6g== X-Gm-Message-State: AOAM5323rrLQjrubY3Ohmz+FQISUgZwJe9Ghj+eglCjgMjyTqnBv7Q85 KoSlyt/BLG8Gp5aQexooj92OMZKvw6hkvDFK8LbWa84L X-Google-Smtp-Source: ABdhPJyKxI4vv+ScnWh7yPwM8WIGv2As1KNGxI7aLvK1sSTWh9dWgPRZJQKmZxhX9gMUXV3Tn9o3ITlu8meG/gyoepQ= X-Received: by 2002:a92:c245:: with SMTP id k5mr5621238ilo.274.1635520947783; Fri, 29 Oct 2021 08:22:27 -0700 (PDT) MIME-Version: 1.0 References: <20211028191500.47377-1-asadsa@ifi.uio.no> In-Reply-To: From: Dave Taht Date: Fri, 29 Oct 2021 08:22:15 -0700 Message-ID: To: ECN-Sane Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: [Ecn-sane] Fwd: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates X-BeenThere: ecn-sane@lists.bufferbloat.net X-Mailman-Version: 2.1.20 Precedence: list List-Id: Discussion of explicit congestion notification's impact on the Internet List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Oct 2021 15:22:28 -0000 ---------- Forwarded message --------- From: Eric Dumazet Date: Fri, Oct 29, 2021 at 7:53 AM Subject: Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates To: Neal Cardwell Cc: Asad Sajjad Ahmed , David S. Miller , Jakub Kicinski , netdev , Toke H=C3=B8iland-J=C3=B8rgensen , Ingemar Johansson S , Tom Henderson , Bob Briscoe , Olga Albisser On Fri, Oct 29, 2021 at 6:54 AM Neal Cardwell wrote: > > On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed wro= te: > > > > Commit "fq_codel: generalise ce_threshold marking for subset of traffic= " > > [1] enables ce_threshold to be used in the Internet, not just in data > > centres. > > > > Because ce_threshold is in time units, it can cause poor utilization at > > low link rates when it represents <1 packet. > > E.g., if link rate <12Mb/s ce_threshold=3D1ms is <1500B packet. > > > > So, suppress ECN marking unless the backlog is also > 1 MTU. > > > > A similar patch to [1] was tested on an earlier kernel, and a similar > > one-packet check prevented poor utilization at low link rates [2]. > > > > [1] commit dfcb63ce1de6 ("fq_codel: generalise ce_threshold marking for= subset of traffic") > > > > [2] See right hand column of plots at the end of: > > https://bobbriscoe.net/projects/latency/dctth_journal_draft20190726.pdf > > > > Signed-off-by: Asad Sajjad Ahmed > > Signed-off-by: Olga Albisser > > --- > > include/net/codel_impl.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/include/net/codel_impl.h b/include/net/codel_impl.h > > index 137d40d8cbeb..4e3e8473e776 100644 > > --- a/include/net/codel_impl.h > > +++ b/include/net/codel_impl.h > > @@ -248,7 +248,8 @@ static struct sk_buff *codel_dequeue(void *ctx, > > vars->rec_inv_sqrt)= ; > > } > > end: > > - if (skb && codel_time_after(vars->ldelay, params->ce_threshold)= ) { > > + if (skb && codel_time_after(vars->ldelay, params->ce_threshold)= && > > + *backlog > params->mtu) { I think this idea would apply to codel quite well. (This helper is common to codel and fq_codel) But with fq_codel my thoughts are: *backlog is the backlog of the qdisc, not the backlog for the flow, and it includes the packet currently being removed from the queue. Setting ce_threshold to 1ms while the link rate is 12Mbs sounds misconfiguration to me. Even if this flow has to transmit one tiny packet every minute, it will get CE mark just because at least one packet from an elephant flow is currently being sent to the wire. BQL won't prevent that at least one packet is being processed while the tiny packet is coming into fq_codel qdisc. vars->ldelay =3D now - skb_time_func(skb); For tight ce_threshold, vars->ldelay would need to be replaced by now - (time of first codel_dequeue() after this skb has been queued). This seems a bit hard to implement cheaply. > > bool set_ce =3D true; > > > > if (params->ce_threshold_mask) { > > -- > > Sounds like a good idea, and looks good to me. > > Acked-by: Neal Cardwell > > Eric, what do you think? > > neal --=20 Fixing Starlink's Latencies: https://www.youtube.com/watch?v=3Dc9gLo6Xrwgw Dave T=C3=A4ht CEO, TekLibre, LLC