> On 13 Aug, 2018, at 12:34 am, Jonathan Morton <chromati...@gmail.com> wrote: > >> On 12 Aug, 2018, at 10:42 pm, Toke Høiland-Jørgensen <t...@toke.dk> wrote: >> >> Yes it does; setting tc_classid is one way to set the flow class that is >> read by the call to tcf_classify(). I guess the problem is that >> cake_hash() is bypassed, so the *host_refcnt variables are not >> incremented. >> >> There's been another report of the same issue on github; haven't had >> time to look into it yet, but I guess this is the reason... > > I'll see what I can do tonight, now that I understand the problem.
Oh, this is a mess. Ultimately the problem stems from having previously factored out choosing tin and flow to two distinct functions, but now there's an external mechanism for overriding both at once, which can only hook into one or the other. The easiest fix is to just remove the broken support for setting the flow ID via a filter, but leave in the support for setting the tin. I think that's the most useful "simple" fix. It's perhaps worth noting that this was the first thing I *removed* when reworking fq_codel into the first version of Cake, because I couldn't see a valid use for it. The next simplest fix is to ignore the flow ID override unless we're in "flows" mode. We can then make valid assumptions about what should go into the host tables. The *right* fix, if we want to maximise functionality, would be to pass the result struct by reference into cake_hash(), where it can override the *host* IDs (not the flow ID). Users can then choose between using the override as a flow ID (by setting "hosts" mode instead of "flows"), or retaining the default host-isolation semantics with a revised definition of "host". I feel compelled to point out that baggage like this is probably at least some of why Cake is no longer faster than HTB+fq_codel, as it used to be. - Jonathan Morton _______________________________________________ Cake mailing list Cake@lists.bufferbloat.net https://lists.bufferbloat.net/listinfo/cake
