[Cake] act_connmark + dscp

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant <kevin at darbyshire-bryant.me.uk> writes:

>>> The ugliness of doing the diffserv parsing is what prompted the idea
>>> of storing the DSCP directly and I felt the stored tin selection was
>>> effectively abstracting the diffserv field anyway.
>> 
>> Right, but that means that the CAKE interpretation of the fwmark would
>> have to change from something that selects the tin, to something that is
>> treated as a DSCP mark. I think this was the part that I was missing
>> before. I don't think this is a good idea, as that means we tie the
>> marks to one particular use case.
>
> I did say it was incompatible with the existing tin storing idea right
> from day 1 and why I was so keen/worried by the fact it had already
> gone upstream.

Yeah, but I didn't connect the dots for how this related to the CAKE use
case until I saw your full example...

>>> Storing the DSCP is more compatible with differing egress v ingress
>>> mappings (eg. egress diffserv4, ingress diffserv3 though I can’t
>>> really think of a use case for that)
>> 
>> I think that if someone wants to do something like that, we are way out
>> of "simple use case that we want to actively support" territory, and can
>> legitimately ask people to go write a BPF filter or something :)
>> 
>>> Of course using fwmark as tin number selector in cake doesn’t preclude
>>> some other mechanism of storing & recovering DSCP to/from firewall
>>> mark e.g. the previously discussed act-connmark+dscp which would help
>>> anyone who wanted to do such ‘link traversing’ DSCP shenanigans.  That
>>> of course makes you happier since cake doesn’t embed itself further
>>> into conntrack.
>> 
>> Yeah, I definitely don't think CAKE has any business writing DSCP values
>> into the mark. However, as I said before, there may be a case for adding
>
> I’m going forward on the basis that *cake* ’storing DSCP within
> fwmark’ idea has died and will be trying to cobble something together
> within act_connmark as that has a wider potential client base than
> just cake.

Yup, sounds good.

BTW, for future reference, while digging through the TC eBPF code, I
discovered that it is (I think) possible to indirectly pass information
to the eBPF program: If you add a classid when adding the bpf filter,
the kernel will put that into skb->tc_classid before executing the eBPF
program. This serves as a default if you don't do any classification
from the eBPF program, but presumably you can also just read and modify
it from the eBPF program...

>> an option to write the tin selection back to conntrack. Something like
>> the patch below would do it (with an option to control it, of course),
>> but it does incur a dependency on another conntrack header, so I'm not
>> sure if it will be acceptable to upstream. Also, we would need to figure
>> out how the option should work.
>
> I think before expending any further mental effort on that, the
> question should be asked of the kernel people.  I don’t see the point
> in working out the semantics of something that ultimately won’t be
> accepted.

Unfortunately that is not how upstream development works. If we want
this feature, we're going to have to do the work of defining and
implementing it and make our case that it should be included.

I think a possible way forward would be:

1. I'll submit the masking patch for the existing fwmark feature to
   upstream. That should get in this cycle, and will form the basis for
   anything else we want to do in the future.

2. For the next cycle (i.e., upstream 5.2) we can hash out what we
   really need for this use case, and how it should work from a user
   perspective; we're going to need that anyway even if we end up
   implementing it differently. This also includes deciding on whether
   this is doable outside of CAKE, or if we need another feature flag.

3. We submit whatever we come up with. If it gets accepted, great,
   otherwise we iterate based on feedback.

Any objections? :)

-Toke