Cake - FQ_codel the next generation
 help / color / mirror / Atom feed
From: Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>
To: Dave Taht <dave.taht@gmail.com>
Cc: cake@lists.bufferbloat.net
Subject: Re: [Cake] NET_XMIT_CN question
Date: Fri, 27 Nov 2015 15:44:02 +0000	[thread overview]
Message-ID: <56587A42.7030905@darbyshire-bryant.me.uk> (raw)
In-Reply-To: <CAA93jw7j_Wba3oNGS36eZixoZ9CMCemp-XxfBB9CrAA2ScENEA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2214 bytes --]



On 27/11/15 15:16, Dave Taht wrote:
> We decreased the size of the queue, regardless of what flow we dropped
> from... so why do we not need to decrease the queue size when using
> NET_XMIT_CN? Where is that done by the caller? I have grepped and
> googled...
>
> As best as I recall the semantics of NET_XMIT_CN changed over time, it used to
> only work on enqueue on a tail drop queue, where it would push back on
> the stack to not, actually, drop the packet, but to halve the window
> and requeue.
>
> Staring at the code now, I don't see how it could be right... here OR
> in the existing fq_codel code in the mainline.
>
>         /* Return Congestion Notification only if we dropped a packet
>          * from this flow.
>          */
>         if (fq_codel_drop(sch) == idx)
>                 return NET_XMIT_CN;
>
>         /* As we dropped a packet, better let upper stack know this */
>         qdisc_tree_decrease_qlen(sch, 1);
>         return NET_XMIT_SUCCESS;
>
>
> (I am not sure NET_XMIT_CN works right when there is a hash collision,
> for example, although the updated doc does seem to indicate that,
>
> ... still, then, where does (an equivalent for)
> qdisc_tree_decrease_qlen kick in?)
>
> So I ripped it out. Otherwise... the code does look correct, now that
> I've reviewed it further.
> But does putting it back in have any measurable effect?
I've no realistic way of testing & finding out.  I'd say that if that
bit of code ever gets executed then things are in an unfortunate place
already, however if there's a way of telling a 'very active talker' (and
they must be 'cos they've just queued a packet *and* cake_drop() has
chosen them as having the biggest b->backlogs[i] - and that may help
answer one of your todo items as well) to back off, then we should take it.

Spaghetti monster knows what *actually* happens further up the stack :-)

>
> ...
>
> Also I note there was a bunch of commits to sch_fq_codel mainline since july,
> notably this one.
>
> c0afd9ce4d6a646fb6433536f95a418bb348fab1
>
>
> Dave Täht
> Let's go make home routers and wifi faster! With better software!
> https://www.gofundme.com/savewifi
>



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4816 bytes --]

  reply	other threads:[~2015-11-27 15:44 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-27 15:16 Dave Taht
2015-11-27 15:44 ` Kevin Darbyshire-Bryant [this message]
2015-12-17 21:39   ` Kevin Darbyshire-Bryant

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.bufferbloat.net/postorius/lists/cake.lists.bufferbloat.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56587A42.7030905@darbyshire-bryant.me.uk \
    --to=kevin@darbyshire-bryant.me.uk \
    --cc=cake@lists.bufferbloat.net \
    --cc=dave.taht@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox