Re: [Cake] openwrt build with latest cake and other qdiscs

[Dave Taht <dave.taht@gmail.com>]

Well, there is a lot of redundant work being done in cake_enqueue in
this case - the diffserv lookup (which might not be redundant but
usually is), the hashing (which will be), grabbing the timestamp
(probably won't hurt to just grab it once for the whole aggregate),
and so on down through cake_enqueue

I do not have sufficient expertise to evaluate the rest. But would
certainly like to benchmark it as is.

On Thu, May 14, 2015 at 3:06 PM, Jonathan Morton <chromatix99@gmail.com> wrote:
>
>> On 14 May, 2015, at 21:15, Sebastian Moeller <moeller0@gmx.de> wrote:
>>
>> I guess your idea of peeling for ATM carrier and for >1ms wire time aggregates sounds like a decent idea…
>
> Code review time, then:
>
>         /*
>          * We tolerate GSO aggregates if they occupy < 1ms of wire time
>          * AND we don't need to perform ATM cell-framing.  We're unlikely
>          * to need the latter above 24Mbps (best ADSL downlink), where
>          * handling individual packets is still cheap.
>          *
>          * But if those conditions aren't met, we need to split it.
>          */
>         if(unlikely(len > (q->rate_bps >> 10) ||
>                         (q->rate_flags & CAKE_FLAG_ATM)) &&
>                         skb_is_gso(skb))
>         {
>                 struct sk_buff *segs, *nskb;
>                 netdev_features_t features = netif_skb_features(skb);
>                 int ret = NET_XMIT_DROP;
>
>                 segs = skb_gso_segment(skb, features & ~NETIF_F_GSO_MASK);
>
>                 if (IS_ERR_OR_NULL(segs))
>                         return qdisc_reshape_fail(skb, sch);
>
>                 while (segs) {
>                         nskb = segs->next;
>                         segs->next = NULL;
>                         qdisc_skb_cb(segs)->pkt_len = segs->len;
>
>                         switch(cake_enqueue(segs, sch)) {
>                                 case NET_XMIT_CN:
>                                         ret = NET_XMIT_CN;
>                                         /* fall */
>
>                                 case NET_XMIT_SUCCESS:
>                                         if(ret == NET_XMIT_DROP)
>                                                 ret = NET_XMIT_SUCCESS;
>                                         qdisc_tree_decrease_qlen(sch, -1);
>                                         /* fall */
>
>                                 default:;
>                         }
>                         segs = nskb;
>                 }
>
>                 qdisc_tree_decrease_qlen(sch, 1);
>                 consume_skb(skb);
>                 return ret;
>         }
>
> I’m thinking of inserting the above into the beginning of cake_enqueue().  This of course makes it a recursive function (but only by one level).
>
> A bit of lateral thinking was necessary to convert the equivalent code in TBF (which relies on a child qdisc to do the actual queuing) into a form that cake could use, so I’m not pushing it to git until someone can spot what, if anything, I’ve got horribly wrong.  However, it does compile and doesn’t seem to crash when I run it on a box with GSO turned on, so it’s probably worth a cautious try.
>
> One quirk is that, in its current form, it’ll *always* peel GSO aggregates in unlimited mode.  That’s because rate_bps is zero in that case.  I could clean up that test by having a separate “peeling threshold” calculated at configure time.
>
>  - Jonathan Morton
>
> _______________________________________________
> Cake mailing list
> Cake@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/cake



-- 
Dave Täht
Open Networking needs **Open Source Hardware**

https://plus.google.com/u/0/+EricRaymond/posts/JqxCe2pFr67