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] Update kernel version check in cake
Date: Mon, 28 Sep 2015 17:25:32 +0100	[thread overview]
Message-ID: <560969FC.3040107@darbyshire-bryant.me.uk> (raw)
In-Reply-To: <CAA93jw5ju-N2dgb6yLH2vuJ2ie7mbW2T423cknTYn=47YZVqmA@mail.gmail.com>

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

On 28/09/15 13:09, Dave Taht wrote:
> so nice to get some actual work done. I've been so buried in this FCC stuff.
>
> kevin, could you take a look at
>
>  https://github.com/dtaht/sch_cake/blob/master/sch_cake.c#L426
>
> and figure out the right thing?
>
> Ideally this is where I wanted to put the "squash" option as well,
> wiping the non-ecn bits. squashing could be made as a new number in
> the CAKE_MODE_BESTEFFORT enum, and treated basically the same
> elsewhere as CAKE_MODE_BESTEFFORT.
>
Hi Dave :-)

This makes me laugh so much.  Today I somehow appear to have convinced
Seb that I understand German (I don't) and you think that I understand
linux kernel network internals whereas in reality I barely know on end
of a C compiler from the other.  Maybe I should try politics? :-)

But I've had a quick look at the code and from what I glean from a bit
of googling, guesswork & intuition, mostly confirms the comment in the
code namely we don't need the 'cow'.

1) unlikely (blah blah blah) - hint the compiler that the following call
is unlikely to fail for optimisation purposes.
2) skb_cow_head copies an skb with a certain amount of buffer headroom,
the implication being we're going to stuff some more data into it.  We
actually request another sizeof(ip_hdr) bytes and for the life of me I
cannot work out why we need more space.....though see point 3.


I can't help feel this could be replaced with:

static inline unsigned int cake_get_diffserv(struct sk_buff *skb)
{
    /* borrowed from sch_dsmark */
    switch (skb->protocol) {
    case htons(ETH_P_IP):

        return ipv4_get_dsfield(ip_hdr(skb)) >> 2;

    case htons(ETH_P_IPV6):
        return ipv6_get_dsfield(ipv6_hdr(skb)) >> 2;

    default:
        /* If there is no Diffserv field, treat as bulk */
        return 0;
    };
}

Maybe if I'm daring, I'll try compiling this and see how my router blows
up :-)


3) I think the real clue is your comment that this is where you wanted
to do squashing, which sounds more like altering packets and to me much
more dangerous and also I'm beginning to guess a *LOT* more here.  I
suspect a skb_cow_head (cow=copy on write) is more likely to be needed
here, but I still fail to see the need to allocate extra space for an
ip_hdr struct...we're going to mangle an existing header.

The thought of writing to an skb fills me with terror!  I really don't
know the rules.  Some here know this stuff upside down, backwards,
inside out *and* forwards.

I'm guessing, but I'd love to know how well I guessed ;-)

Kevin 


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

  reply	other threads:[~2015-09-28 16:25 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-28 11:52 Kevin Darbyshire-Bryant
2015-09-28 12:09 ` Dave Taht
2015-09-28 16:25   ` Kevin Darbyshire-Bryant [this message]
2015-09-28 23:01     ` Jonathan Morton
2015-09-29  9:42       ` Kevin Darbyshire-Bryant
2015-09-29  9:55         ` Dave Taht
2015-09-29 11:10           ` Dave Taht
2015-09-29 15:10             ` Sebastian Moeller

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=560969FC.3040107@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