netdev - Re: [PATCH net-next] fq_codel: avoid under-utilization with ce

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: <CADVnQykDUB4DgUaV0rd6-OKafO+F6w=BRfxviuZ_MJLY3xMV+Q@mail.gmail.com>
Date:   Fri, 29 Oct 2021 09:54:09 -0400
From:   Neal Cardwell <ncardwell@...gle.com>
To:     Asad Sajjad Ahmed <asadsa@....uio.no>
Cc:     "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        netdev <netdev@...r.kernel.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Toke Høiland-Jørgensen <toke@...hat.com>,
        Ingemar Johansson S <ingemar.s.johansson@...csson.com>,
        Tom Henderson <tomh@...h.org>,
        Bob Briscoe <research@...briscoe.net>,
        Olga Albisser <olga@...isser.org>
Subject: Re: [PATCH net-next] fq_codel: avoid under-utilization with
 ce_threshold at low link rates

On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed <asadsa@....uio.no> wrote:
>
> 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=1ms 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 <asadsa@....uio.no>
> Signed-off-by: Olga Albisser <olga@...isser.org>
> ---
>  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) {
>                 bool set_ce = true;
>
>                 if (params->ce_threshold_mask) {
> --

Sounds like a good idea, and looks good to me.

Acked-by: Neal Cardwell <ncardwell@...gle.com>

Eric, what do you think?

neal