Re: [Cake] Update kernel version check in cake

[Kevin Darbyshire-Bryant <kevin@darbyshire-bryant.me.uk>]

[-- 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 --]