On Aug 21, 2018, at 1:25 PM, Toke Høiland-Jørgensen <toke@toke.dk> wrote:

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".

Ah, making it possible to override both host and flow mode is a great
idea! I guess we could use the major/minor distinction in the class to
steer this. I'll see if I can't integrate this.

So, I implemented this; in the latest commit on github it is again
possible to override the flow hashing by setting the class ID with a TC
filter; and the host hash can be overridden by setting the major number
of the class ID. In my testing the hangs from before are gone, but if
anyone else wants to test, please do!

I'll write up a description of the filter overrides in the man page, and
submit the change upstream as well...

Well that’s good timing for me as I’m wrapping up a small utility/eBPF to classify an arbitrary username to either MAC or IP. Here’s the work in progress, which is not done yet as flow fairness is still under construction, and I haven’t gotten my IPv6 support to pass the rather stubborn eBPF verifier: https://github.com/heistp/tc-users

With your new code Toke:

- I so far haven’t seen my VM either crash or suddenly fill its disk with logs, which is a bonus. :)

- With the new major/minor ID distinction, I’d probably use major for the user and minor for the flow hash?

Another thing I haven’t looked into yet is that when fq_codel is the qdisc, the eBPF action is only called "once in a while” (start of a new flow?) With cake it’s called for every single packet, which is what I expected to happen, but very different behavior.

Lastly, if anyone has time to review even just a little code for what is or is not good or idiomatic C, post an issue and I’d appreciate it. Yes, I yield to the ‘goto’ proponents when it comes to error handling and resource de-allocation. :)