* [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates @ 2021-10-28 19:15 Asad Sajjad Ahmed 2021-10-29 13:54 ` Neal Cardwell 0 siblings, 1 reply; 7+ messages in thread From: Asad Sajjad Ahmed @ 2021-10-28 19:15 UTC (permalink / raw) To: David S. Miller, Jakub Kicinski Cc: netdev, Eric Dumazet, Toke Høiland-Jørgensen, Neal Cardwell, Ingemar Johansson S, Tom Henderson, Bob Briscoe, Asad Sajjad Ahmed, Olga Albisser 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@ifi.uio.no> Signed-off-by: Olga Albisser <olga@albisser.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) { -- 2.30.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates 2021-10-28 19:15 [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates Asad Sajjad Ahmed @ 2021-10-29 13:54 ` Neal Cardwell 2021-10-29 14:53 ` Eric Dumazet 0 siblings, 1 reply; 7+ messages in thread From: Neal Cardwell @ 2021-10-29 13:54 UTC (permalink / raw) To: Asad Sajjad Ahmed Cc: David S. Miller, Jakub Kicinski, netdev, Eric Dumazet, Toke Høiland-Jørgensen, Ingemar Johansson S, Tom Henderson, Bob Briscoe, Olga Albisser On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed <asadsa@ifi.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@ifi.uio.no> > Signed-off-by: Olga Albisser <olga@albisser.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@google.com> Eric, what do you think? neal ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates 2021-10-29 13:54 ` Neal Cardwell @ 2021-10-29 14:53 ` Eric Dumazet 2021-10-29 14:57 ` Eric Dumazet ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Eric Dumazet @ 2021-10-29 14:53 UTC (permalink / raw) To: Neal Cardwell Cc: Asad Sajjad Ahmed, David S. Miller, Jakub Kicinski, netdev, Toke Høiland-Jørgensen, Ingemar Johansson S, Tom Henderson, Bob Briscoe, Olga Albisser On Fri, Oct 29, 2021 at 6:54 AM Neal Cardwell <ncardwell@google.com> wrote: > > On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed <asadsa@ifi.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@ifi.uio.no> > > Signed-off-by: Olga Albisser <olga@albisser.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) { 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 = 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 = true; > > > > if (params->ce_threshold_mask) { > > -- > > Sounds like a good idea, and looks good to me. > > Acked-by: Neal Cardwell <ncardwell@google.com> > > Eric, what do you think? > > neal ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates 2021-10-29 14:53 ` Eric Dumazet @ 2021-10-29 14:57 ` Eric Dumazet 2021-10-31 20:31 ` Dave Taht 2021-10-31 21:50 ` Bob Briscoe 2 siblings, 0 replies; 7+ messages in thread From: Eric Dumazet @ 2021-10-29 14:57 UTC (permalink / raw) To: Neal Cardwell Cc: Asad Sajjad Ahmed, David S. Miller, Jakub Kicinski, netdev, Toke Høiland-Jørgensen, Ingemar Johansson S, Tom Henderson, Bob Briscoe, Olga Albisser On Fri, Oct 29, 2021 at 7:53 AM Eric Dumazet <edumazet@google.com> wrote: > Setting ce_threshold to 1ms while the link rate is 12Mbs sounds > misconfiguration to me. > It is like using a ce_threshold of 1usec on a 10Gbit link. About all packets would get CE marked. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates 2021-10-29 14:53 ` Eric Dumazet 2021-10-29 14:57 ` Eric Dumazet @ 2021-10-31 20:31 ` Dave Taht 2021-10-31 20:34 ` Dave Taht 2021-10-31 21:50 ` Bob Briscoe 2 siblings, 1 reply; 7+ messages in thread From: Dave Taht @ 2021-10-31 20:31 UTC (permalink / raw) To: Eric Dumazet Cc: Neal Cardwell, Asad Sajjad Ahmed, David S. Miller, Jakub Kicinski, netdev, Toke Høiland-Jørgensen, Ingemar Johansson S, Tom Henderson, Bob Briscoe, Olga Albisser On Fri, Oct 29, 2021 at 7:53 AM Eric Dumazet <edumazet@google.com> wrote: > > On Fri, Oct 29, 2021 at 6:54 AM Neal Cardwell <ncardwell@google.com> wrote: > > > > On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed <asadsa@ifi.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@ifi.uio.no> > > > Signed-off-by: Olga Albisser <olga@albisser.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) { > > 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 = 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 = true; > > > > > > if (params->ce_threshold_mask) { > > > -- > > > > Sounds like a good idea, and looks good to me. > > > > Acked-by: Neal Cardwell <ncardwell@google.com> > > > > Eric, what do you think? > > > > neal My 2c: I would prefer this entire patch series be reverted and put back into the l4s and BBRv2 trees where it can be thoroughly evaluated, and appropriate parameterizations found for bare metal DCs, virtualized environments, hosts and routers at a variety of bandwidths. I'm not huge on arbitrarily cluttering up the fq_codel hot path, or the uapi, before that happens. Also, if anyone has any suggestions as to how to apply this correctly to the wifi users in the kernel, I'm all ears. I think a safer path forward in linux mainline would be to remove ECT0 from consideration as RFC3168 ECN in the INET_macros (which all the existing qdiscs will then pick up) and have the unconfigured linux hosts and vm substrates across the internet revert to drop in that case. Prague is defined to revert to reno in that case. Throughput, if not reductions in local queuing, will be good. A good test of the ECT0 concept across the wire might be to then have the in-kernel dctcp instance start using that instead of ECT1 and see what breaks. I would appreciate a set of before/after benchmarks on a future submission. It's otherwise not my call, I do wish more folk would read this ( https://github.com/heistp/l4s-tests#key-findings) before committing seemingly untested code. -- I tried to build a better future, a few times: https://wayforward.archive.org/?site=https%3A%2F%2Fwww.icei.org Dave Täht CEO, TekLibre, LLC ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates 2021-10-31 20:31 ` Dave Taht @ 2021-10-31 20:34 ` Dave Taht 0 siblings, 0 replies; 7+ messages in thread From: Dave Taht @ 2021-10-31 20:34 UTC (permalink / raw) To: Eric Dumazet Cc: Neal Cardwell, Asad Sajjad Ahmed, David S. Miller, Jakub Kicinski, netdev, Toke Høiland-Jørgensen, Ingemar Johansson S, Tom Henderson, Bob Briscoe, Olga Albisser On Sun, Oct 31, 2021 at 1:31 PM Dave Taht <dave.taht@gmail.com> wrote: > > On Fri, Oct 29, 2021 at 7:53 AM Eric Dumazet <edumazet@google.com> wrote: > > > > On Fri, Oct 29, 2021 at 6:54 AM Neal Cardwell <ncardwell@google.com> wrote: > > > > > > On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed <asadsa@ifi.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@ifi.uio.no> > > > > Signed-off-by: Olga Albisser <olga@albisser.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) { > > > > 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 = 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 = true; > > > > > > > > if (params->ce_threshold_mask) { > > > > -- > > > > > > Sounds like a good idea, and looks good to me. > > > > > > Acked-by: Neal Cardwell <ncardwell@google.com> > > > > > > Eric, what do you think? > > > > > > neal > > My 2c: sigh, I got my bits flipped. > > I would prefer this entire patch series be reverted and put back into > the l4s and BBRv2 trees where it can be thoroughly evaluated, and > appropriate parameterizations found for bare metal DCs, virtualized > environments, hosts and routers at a variety of bandwidths. > > I'm not huge on arbitrarily cluttering up the fq_codel hot path, or > the uapi, before that happens. > > Also, if anyone has any suggestions as to how to apply this correctly > to the wifi users in the kernel, I'm all ears. > > I think a safer path forward in linux mainline would be to remove ECT0 > from consideration as RFC3168 ECN in the INET_macros (which all the > existing qdiscs will then pick up) and have the unconfigured linux > hosts and vm substrates across the internet revert to drop in that > case. Prague is defined to revert to reno in that case. Throughput, if > not reductions in local queuing, will be good. > > A good test of the ECT0 concept across the wire might be to then have > the in-kernel dctcp instance start using that instead of ECT1 and see > what breaks. > > I would appreciate a set of before/after benchmarks on a future submission. > > It's otherwise not my call, I do wish more folk would read this ( > https://github.com/heistp/l4s-tests#key-findings) before committing > seemingly untested code. > > -- > I tried to build a better future, a few times: > https://wayforward.archive.org/?site=https%3A%2F%2Fwww.icei.org > > Dave Täht CEO, TekLibre, LLC -- I tried to build a better future, a few times: https://wayforward.archive.org/?site=https%3A%2F%2Fwww.icei.org Dave Täht CEO, TekLibre, LLC ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates 2021-10-29 14:53 ` Eric Dumazet 2021-10-29 14:57 ` Eric Dumazet 2021-10-31 20:31 ` Dave Taht @ 2021-10-31 21:50 ` Bob Briscoe 2 siblings, 0 replies; 7+ messages in thread From: Bob Briscoe @ 2021-10-31 21:50 UTC (permalink / raw) To: Eric Dumazet, Neal Cardwell Cc: Asad Sajjad Ahmed, David S. Miller, Jakub Kicinski, netdev, Toke Høiland-Jørgensen, Ingemar Johansson S, Tom Henderson, Olga Albisser Eric, On 29/10/2021 15:53, Eric Dumazet wrote: > On Fri, Oct 29, 2021 at 6:54 AM Neal Cardwell <ncardwell@google.com> wrote: >> On Thu, Oct 28, 2021 at 3:15 PM Asad Sajjad Ahmed <asadsa@ifi.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@ifi.uio.no> >>> Signed-off-by: Olga Albisser <olga@albisser.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) { > 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, [BB] Ah. Hadn't appreciated that. Thanks. We were modelling this on a check of (*backlog <= params->mtu) in codel_should_drop(), which I thought was similarly checking for low link rate in fq_codel and codel. But didn't do our homework properly... > 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. [BB] The idea was meant to be that you don't have to know the drain rate of the queue. This additional check was meant to suppress marking if (ldelay > ce_threshold) && !(qlen > 1). ce_threshold = 1ms was only an example, nonetheless we had tested that config down to 4Mb/s with two flows. We AND'd ce_threshold with a check for qlen >1 packet and we still got >95% link utilization, as shown in the plots referenced via [2], e.g. Fig 4. But checking qlen would have disrupted the code somewhat, so we had hoped to be able to add a check of the /queue/'s backlog instead, thinking it was conveniently already available. [2] See right hand column of plots at the end of: https://bobbriscoe.net/projects/latency/dctth_journal_draft20190726.pdf We were conscious that this could have suppressed marking of a queue of more than one small packet, as long as ldelay also exceeded ce_threshold, but we figured that would do no great harm. > 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. [BB] Yes, now we understand it's the backlog of the whole qdisc, this isn't the behaviour we intended. > vars->ldelay = 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. [BB] We'll think whether we can do the qlen check less disruptively. Bob > > > > >>> bool set_ce = true; >>> >>> if (params->ce_threshold_mask) { >>> -- >> Sounds like a good idea, and looks good to me. >> >> Acked-by: Neal Cardwell <ncardwell@google.com> >> >> Eric, what do you think? >> >> neal -- ________________________________________________________________ Bob Briscoe http://bobbriscoe.net/ ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-10-31 22:30 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-28 19:15 [PATCH net-next] fq_codel: avoid under-utilization with ce_threshold at low link rates Asad Sajjad Ahmed 2021-10-29 13:54 ` Neal Cardwell 2021-10-29 14:53 ` Eric Dumazet 2021-10-29 14:57 ` Eric Dumazet 2021-10-31 20:31 ` Dave Taht 2021-10-31 20:34 ` Dave Taht 2021-10-31 21:50 ` Bob Briscoe
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.