[Cake] Using firewall connmarks as tin selectors

[Kevin Darbyshire-Bryant]

> On 4 Mar 2019, at 17:36, Toke Høiland-Jørgensen <toke at redhat.com> wrote:
> < snips >
> 
> Or act_dscp with 'get' and 'set' options :)
> 
>> As I said earlier I couldn't work out how m_conntrack did… anything at
>> all to be honest!
> 
> act_connmark is pretty straight forward:
> https://elixir.bootlin.com/linux/latest/source/net/sched/act_connmark.c#L34

Oh bloody hell I’m an idiot - I was looking in user space tc code for something that obviously lives in kernel land.  Yes *now* I can see what it does.

> 
>>>> @@ -1661,13 +1695,14 @@ static struct cake_tin_data *cake_select_tin(struct Qdisc *sch,
>>>> 		tin = 0;
>>>> 
>>>> 	else if (q->rate_flags & CAKE_FLAG_FWMARK && /* use fw mark */
>>>> -		 skb->mark &&
>>>> -		 skb->mark <= q->tin_cnt)
>>>> -		tin = q->tin_order[skb->mark - 1];
>>>> -
>>>> -	else if (TC_H_MAJ(skb->priority) == sch->handle &&
>>>> -		 TC_H_MIN(skb->priority) > 0 &&
>>>> -		 TC_H_MIN(skb->priority) <= q->tin_cnt)
>>>> +		   skb->mark & 0x40000000) {
>>> 
>>> I think there's something odd with this mask?  There's only one bit set
>>> in it…
>> 
>> I use the single bit as a flag to indicate cake has stored the  DSCP
>> in the lower 6 bits of the byte.  Otherwise with a DSCP of 0 (the
>> default) it’s rather difficult to know if a connection has been
>> through the cake ’save dscp to fwmark’ process or not.  That also
>> indicates to user space whether it should consider mangling packets or
>> not e.g.
> 
> Ah, right. But that breaks the previous use case where someone just
> wants to set fwmarks that get turned into CAKE tins?

Yes, which is why I was hoping for upstream to bounce it, not least because  of the unmasked use of fwmark field.  Personally I’d like to see that change reverted before we get trapped into something we’ll regret.

> I think this definitely is leaning towards decoupling the
> fw-mark-to-DSCP settings to an action. And probably making the shift and
> mask configurable rather than hard-coding something…

I think so too though I think the mechanism of copying the DSCP bits and adding a ‘I did this’ flag bit should be retained so that other user space tools (iptables etc) can detect when a connmark based DSCP has been set/applied.  I think cake ‘fwmark’ should have the smarts to look for the act_dscp DSCP value if nothing else so we don’t have to have the overhead of act_dscp set restoring DSCP to all the packets if we don’t want to.

I’m right at the limit of my coding ability with what I’ve sent in so far - the kernel space bits of act_connmark leave me mostly confused - really not sure where to start with act_dscp!



Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A