[Cake] act_connmark + dscp

[Toke Høiland-Jørgensen]

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

>> On 6 Mar 2019, at 15:21, Toke Høiland-Jørgensen <toke at redhat.com> wrote:
>> 
>> Kevin Darbyshire-Bryant <kevin at darbyshire-bryant.me.uk> writes:
>> 
>>> Before I go too far down this road (and to avoid the horror of
>>> actually trying to code it) here’s what I’m trying to achieve.
>>> 
>>> 
>>> act_connmark + dscp is designed to copy a DSCP code to/from conntrack mark.  It uses 8 bits of the mark field, currently the most significant byte.
>>> 
>>> Bits 31-26: DSCP
>>> Bit 25: Spare/Future
>>> Bit 24: Valid DSCP set
>>> 
>>> The valid bit is set when the ‘getdscp’ function has written a DSCP
>>> value into the conntrack (& hence skb) mark. This allows us & other
>>> skb->mark/ct->mark applications (eg iptables, cake qdisc) to know that
>>> a DSCP value has been placed in the field. We cannot simply use a
>>> non-zero DSCP because zero is a valid DSCP.
>> 
>> If someone installs the action, the field is supposedly always copied;
>> so why do we need another flag?
>
> I’m trying to limit the number of times expensive iptables mangle
> rules have to run.

Right, I see your point, but I'm worried that this can risk becoming a
source of hard-to-debug bugs if this bit happens to get set by accident
in other places. So, I would suggest to at least make it optional (and
configurable). So how about the following configuration options:

- fwmark mask (32-bit value)
  specifies a mask to apply to the fwmark field before all operations

- fwmark shift (valid values 0-32)
  specifies how many bits to left-shift the DSCP values before putting
  them into the fwmark (and how many bits to right-shift the value read
  from fwmark before writing it to the DSCP field); this could also be
  inferred from the mask rather than be a separate option (shift =
  lowest set bit of mask)

- get_dscp (boolean; cannot be set along with set_dscp)
  if set, the DSCP field will be copied to the fwmark field, subject to
  mask and shift

- set_dscp (boolean; cannot be set along with get_dscp)
  if set, the value in fwmark will be copied to the DSCP field, subject
  to mask and shift

- state mask (32-bit value; probably needs better name)
  if set: the get_dscp action will OR the resulting fwmark before
  storing it. the set_dscp value will AND the fwmark with this value
  before doing anything, and abort if the result is false.

I think this would allow you to implement what you described, without
hard-coding any behaviour. Right?

Does anyone else have any opinions / objections to the above API?
  

> The reality is that I enjoyed doing this in the cake codebase.  I
> cannot say the same for act_connmark in fact I hate it so much I’m
> stopping.  The mental effort for a non-programmer and more importantly
> a non-kernel programmer is exhausting & I’m completely disillusioned.
> I really need to concentrate on the job that means I can pay the
> mortgage, which isn’t bashing my head against the kernel.

Fair enough; no reason to do this if you're not enjoying it! We can
iterate on the API, and I guess I can write the code at some point in
the future if no one else beats me to it. No promises on when, though,
so if someone else feels like tackling it, please go ahead :)

-Toke