[Cake] act_connmark + dscp

[Toke Høiland-Jørgensen]

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

>> On 11 Mar 2019, at 13:00, Toke Høiland-Jørgensen <toke at redhat.com> wrote:
>> 
>> 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…
>
> I’ll try to remember that if I’m ever in the eBPF vicinity.
>
>
>> 
>>>> 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.
>
> Agreed.  Masking should have been in the original patch.  Had I known
> about (and been amused by) ffs it would have been done :-)

Indeed, I also had a chuckle over the function name :D

>> 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.
>
> You’ve already clearly demonstrated that the marking/flagging can be
> done using iptables, so it is doable outside of CAKE.  The iptables
> rules to do it outside of CAKE are…an unpleasant way of implementing a
> lookup table which also requires knowledge of the inner workings of
> CAKE (lookup table & tin order). Personally I feel that’s ugly, but
> you already know that :-)

Yeah, you basically summed up the argument I am planning to use to get
it accepted :)

>> 3. We submit whatever we come up with. If it gets accepted, great,
>>   otherwise we iterate based on feedback.
>
> I await the tarring & feathering followed by “You’re trying to set a
> conntrack mark……from a QDISC?!!!!!! get outta town” :-)

Haha, yup, should be fun!

-Toke