Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Toke Høiland-Jørgensen  writes:

> Kevin Darbyshire-Bryant  writes:
>
>>> On 4 Mar 2019, at 17:36, Toke Høiland-Jørgensen  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.
>
> No comment ;)
>
>> @@ -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 & 0x4000) {
> 
> 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.
>
> Well, we have plenty of time to try out things; the whole point of the
> rc cycle is testing. But sure, if we don't settle on something, we can
> just revert it :)

And, well, the simple thing to do is just keep the current behaviour, or
add a mask of 0xff, and use the tc action for everything that's more
advanced than this...

>>> 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 guess this could be an option as well?
>
>> 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.
>
> Not sure what you mean here?
>
>> 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!
>
> I think I would start with `cp act_connmark.c act_dscp.c`, adding the
> new file to the Makefile and Kconfig, and working from there. Then rip
> out everything not needed, and copy over what you already added to
> cake.

Or even simpler, just add new options to act_connmark...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 4 Mar 2019, at 17:36, Toke Høiland-Jørgensen  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.

No comment ;)

> @@ -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 & 0x4000) {
 
 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.

Well, we have plenty of time to try out things; the whole point of the
rc cycle is testing. But sure, if we don't settle on something, we can
just revert it :)

>> 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 guess this could be an option as well?

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

Not sure what you mean here?

> 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!

I think I would start with `cp act_connmark.c act_dscp.c`, adding the
new file to the Makefile and Kconfig, and working from there. Then rip
out everything not needed, and copy over what you already added to cake.

Happy to help you work out the details; but I think we'll make more
progress on this if you are driving it :)

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Kevin Darbyshire-Bryant]

> On 4 Mar 2019, at 17:36, Toke Høiland-Jørgensen  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 & 0x4000) {
>>> 
>>> 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

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 4 Mar 2019, at 16:39, Toke Høiland-Jørgensen  wrote:
>> 
>> Kevin Darbyshire-Bryant  writes:
>> 
>> [ ... snipping a bit of context here ... ]
>> 
>>> +void cake_update_ct_mark(struct sk_buff *skb, u8 dscp)
>>> +{
>>> +   enum ip_conntrack_info ctinfo;
>>> +   struct nf_conn *ct;
>>> +
>>> +   ct = nf_ct_get(skb, );
>>> +   if (!ct)
>>> +   return;
>>> +
>>> +   ct->mark &= 0x80ff;
>>> +   ct->mark |= (0x40 | dscp) << 24;
>> 
>> Right, so we *might* have an argument that putting the *tin* into the
>> fwmark is CAKE's business, but copying over the dscp mark is not
>> something a qdisc should be doing…
> 
> Why ever not?  It’s not the DSCP, it’s a lookup value into the cake
> priority table, it just happens to look like the DSCP ;-)
 
 If it quacks like a duck…
>>> 
>>> I honestly don’t know where to go from here. I’m clearly trying to do
>>> something that the kernel doesn’t want to do.
>> 
>> I'm not disputing that what you're trying to do (moving DSCP field into
>> connmark) is useful. I'm just questioning whether CAKE is the right
>> place to do this. I think it would fit better in a TC action; either
>> modify act_connmark, or create a new action to sync fwmarks and DSCP
>> marks…
>
> Interesting.  Thinks out loud - Two actions - ‘act_storedscp’,
> ‘act_restoredscp'

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

>>> @@ -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 & 0x4000) {
>> 
>> 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?

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

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Kevin Darbyshire-Bryant]

> On 4 Mar 2019, at 16:39, Toke Høiland-Jørgensen  wrote:
> 
> Kevin Darbyshire-Bryant  writes:
> 
> [ ... snipping a bit of context here ... ]
> 
>> +void cake_update_ct_mark(struct sk_buff *skb, u8 dscp)
>> +{
>> +enum ip_conntrack_info ctinfo;
>> +struct nf_conn *ct;
>> +
>> +ct = nf_ct_get(skb, );
>> +if (!ct)
>> +return;
>> +
>> +ct->mark &= 0x80ff;
>> +ct->mark |= (0x40 | dscp) << 24;
> 
> Right, so we *might* have an argument that putting the *tin* into the
> fwmark is CAKE's business, but copying over the dscp mark is not
> something a qdisc should be doing…
 
 Why ever not?  It’s not the DSCP, it’s a lookup value into the cake
 priority table, it just happens to look like the DSCP ;-)
>>> 
>>> If it quacks like a duck…
>> 
>> I honestly don’t know where to go from here. I’m clearly trying to do
>> something that the kernel doesn’t want to do.
> 
> I'm not disputing that what you're trying to do (moving DSCP field into
> connmark) is useful. I'm just questioning whether CAKE is the right
> place to do this. I think it would fit better in a TC action; either
> modify act_connmark, or create a new action to sync fwmarks and DSCP
> marks…

Interesting.  Thinks out loud - Two actions - ‘act_storedscp’, ‘act_restoredscp'

As I said earlier I couldn't work out how m_conntrack did… anything at all to 
be honest!

> 
> This would both sidestep the whole conntrack dependency issue, and make
> the same functionality available outside of CAKE (for an HTB-based
> setup, for instance).
> 
>> v2 addressing some of the comments attached.  Is it best to keep the
>> in progress patches here or should they be github PRs ?
> 
> Patches on the mailing list is fine by me, and it seems there are people
> reading the list, but not github, so let's keep it here for now at
> least. However, I'll hold off on more detailed comments on the patch
> until we've resolved the point above. With one exception:

There’s certainly been some response here that’s for sure :-)

> 
>> @@ -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 & 0x4000) {
> 
> 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.

#if bit 6 0 then not been marked by cake - go & mangle the DSCP ready for cake 
to find & set
#the mark.
ipt -t mangle -A PREROUTING  -i $IFACE -m mark --mark 0x00/0x4000 -g 
QOS_MARK_${IFACE}
ipt -t mangle -A POSTROUTING -o $IFACE -m mark --mark 0x00/0x4000 -g 
QOS_MARK_${IFACE}



Cheers,

Kevin D-B

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

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

[ ... snipping a bit of context here ... ]

> +void cake_update_ct_mark(struct sk_buff *skb, u8 dscp)
> +{
> + enum ip_conntrack_info ctinfo;
> + struct nf_conn *ct;
> +
> + ct = nf_ct_get(skb, );
> + if (!ct)
> + return;
> +
> + ct->mark &= 0x80ff;
> + ct->mark |= (0x40 | dscp) << 24;
 
 Right, so we *might* have an argument that putting the *tin* into the
 fwmark is CAKE's business, but copying over the dscp mark is not
 something a qdisc should be doing…
>>> 
>>> Why ever not?  It’s not the DSCP, it’s a lookup value into the cake
>>> priority table, it just happens to look like the DSCP ;-)
>> 
>> If it quacks like a duck…
>
> I honestly don’t know where to go from here. I’m clearly trying to do
> something that the kernel doesn’t want to do.

I'm not disputing that what you're trying to do (moving DSCP field into
connmark) is useful. I'm just questioning whether CAKE is the right
place to do this. I think it would fit better in a TC action; either
modify act_connmark, or create a new action to sync fwmarks and DSCP
marks...

This would both sidestep the whole conntrack dependency issue, and make
the same functionality available outside of CAKE (for an HTB-based
setup, for instance).

> v2 addressing some of the comments attached.  Is it best to keep the
> in progress patches here or should they be github PRs ?

Patches on the mailing list is fine by me, and it seems there are people
reading the list, but not github, so let's keep it here for now at
least. However, I'll hold off on more detailed comments on the patch
until we've resolved the point above. With one exception:

> @@ -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 & 0x4000) {

I think there's something odd with this mask?  There's only one bit set
in it...

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Kevin Darbyshire-Bryant]

> On 4 Mar 2019, at 12:44, Toke Høiland-Jørgensen  wrote:
> 
> Kevin Darbyshire-Bryant  writes:
> 
>>> On 4 Mar 2019, at 11:17, Toke Høiland-Jørgensen  wrote:
>>> 
>>> Kevin Darbyshire-Bryant  writes:
>>> 
> On 4 Mar 2019, at 08:39, Pete Heist  wrote:
> 
> 
>> On Mar 3, 2019, at 12:52 PM, Kevin Darbyshire-Bryant 
>>  wrote:
>> 
>> The very bad idea:
>> 
>> And it’s bad ‘cos it’s sort of incompatible with the existing fwmark
>> implementation as described above. So an awful lot of our
>> shenanigans above is due to DSCP not traversing the internet
>> particularly well. The solution above abstracts DSCP into ’tins’
>> which we put into fwmarks. Another approach would be to put the DSCP
>> *into* the fwmark. CAKE could (optionally) copy the FWMARK contained
>> DSCP into the diffserv field onto the actual packets. Voila DSCP
>> traversal across ’tinternet with tin/bandwidth allocation in our
>> local domain preserved.
> 
> If I understand it right, another use case for this “very bad idea”
> is preserving DSCP locally while traversing upstream WiFi links as
> besteffort, which avoids airtime efficiency problems that can occur
> with 802.11e (WMM). In cases where the router config can’t be changed
> (802.11e is mandatory after all) I’ve used IPIP tunnels for this, as
> it hides DSCP from the WiFi stack while preserving the values through
> the tunnel, but this would be easier. Neat… :)
 
 Everyone has understood the intent & maybe the implementation
 correctly. 2 patches attached, one for cake, one for tc.
 
 They are naively coded and some of it undoes Toke’s recent tidying up
 (sorry!)
>>> 
>>> Heh. First comment: Don't do that ;)
>> 
>> I did say naively coded.
>> 
>>> 
>>> A few more below.
>>> 
 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
 diff --git a/pkt_sched.h b/pkt_sched.h
 index a2f570c..d1f288d 100644
 --- a/pkt_sched.h
 +++ b/pkt_sched.h
 @@ -879,6 +879,7 @@ enum {
TCA_CAKE_ACK_FILTER,
TCA_CAKE_SPLIT_GSO,
TCA_CAKE_FWMARK,
 +  TCA_CAKE_ICING,
__TCA_CAKE_MAX
 };
 #define TCA_CAKE_MAX   (__TCA_CAKE_MAX - 1)
 diff --git a/sch_cake.c b/sch_cake.c
 index 733b897..5aca0f3 100644
 --- a/sch_cake.c
 +++ b/sch_cake.c
 @@ -270,7 +270,8 @@ enum {
CAKE_FLAG_INGRESS  = BIT(2),
CAKE_FLAG_WASH = BIT(3),
CAKE_FLAG_SPLIT_GSO= BIT(4),
 -  CAKE_FLAG_FWMARK   = BIT(5)
 +  CAKE_FLAG_FWMARK   = BIT(5),
 +  CAKE_FLAG_ICING= BIT(6)
>>> 
>>> This implies that icing and fwmark can be enabled completely
>>> independently of each other. Are you sure about the semantics for that?
>> 
>> No, I’m not.  I sent the patches so others could see my lunacy in action and 
>> hopefully improve it.
>> 
>> The actual operation links FWMARK, INGRESS & ICING in a variety of
>> combinations.
> 
> Right, so obviously this needs to be thought through…

Yes.  Volunteers with thoughts & ideally code sought.

> 
 };
 
 /* COBALT operates the Codel and BLUE algorithms in parallel, in order to
 @@ -333,7 +334,7 @@ static const u8 diffserv8[] = {
 };
 
 static const u8 diffserv4[] = {
 -  0, 2, 0, 0, 2, 0, 0, 0,
 +  0, 1, 0, 0, 2, 0, 0, 0,
1, 0, 0, 0, 0, 0, 0, 0,
2, 0, 2, 0, 2, 0, 2, 0,
2, 0, 2, 0, 2, 0, 2, 0,
 @@ -344,7 +345,7 @@ static const u8 diffserv4[] = {
 };
 
 static const u8 diffserv3[] = {
 -  0, 0, 0, 0, 2, 0, 0, 0,
 +  0, 1, 0, 0, 2, 0, 0, 0,
>>> 
>>> Why are you messing with the diffserv mappings in this patch?
>> 
>> This is a combination patch of Dave’s new LE coding and the
>> fwmark/dscp mangling.
> 
> Ah. Well let's keep that separate from this patch/discussion…

Done
> 
>>> 
1, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
0, 0, 0, 0, 0, 0, 0, 0,
 @@ -1618,7 +1619,24 @@ static unsigned int cake_drop(struct Qdisc *sch, 
 struct sk_buff **to_free)
return idx + (tin << 16);
 }
 
 -static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
 +void cake_update_diffserv(struct sk_buff *skb, u8 dscp)
 +{
 +  switch (skb->protocol) {
 +  case htons(ETH_P_IP):
 +  if ((ipv4_get_dsfield(ip_hdr(skb)) & ~INET_ECN_MASK) != dscp)
 +  ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, dscp);
 +  break;
 +  case htons(ETH_P_IPV6):
 +  if ((ipv6_get_dsfield(ipv6_hdr(skb)) & ~INET_ECN_MASK) != dscp)
 +  ipv6_change_dsfield(ipv6_hdr(skb), INET_ECN_MASK, dscp);
 +  break;
 +  default:
 +  break;
 +  }
 +
 +}
>>> 
>>> So washing is just a special case of this (wash is
>>> cake_update_diffserv(skb,0)). So you shouldn't need to add another
>>> function, just augment the 

Re: [Cake] Upstream submission of dual-mode fairness patch

[Georgios Amanakis]

Agreed then! Thank you all for your insights!
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Upstream submission of dual-mode fairness patch

[Pete Heist]

I added some text for the ingress keyword and related fairness behavior to the 
cake man page, in case there’s feedback or changes:

https://github.com/dtaht/tc-adv/pull/25/commits/9a7905a8a768fc57926b1875e8be445865e08627
 


___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 4 Mar 2019, at 11:17, Toke Høiland-Jørgensen  wrote:
>> 
>> Kevin Darbyshire-Bryant  writes:
>> 
 On 4 Mar 2019, at 08:39, Pete Heist  wrote:
 
 
> On Mar 3, 2019, at 12:52 PM, Kevin Darbyshire-Bryant 
>  wrote:
> 
> The very bad idea:
> 
> And it’s bad ‘cos it’s sort of incompatible with the existing fwmark
> implementation as described above. So an awful lot of our
> shenanigans above is due to DSCP not traversing the internet
> particularly well. The solution above abstracts DSCP into ’tins’
> which we put into fwmarks. Another approach would be to put the DSCP
> *into* the fwmark. CAKE could (optionally) copy the FWMARK contained
> DSCP into the diffserv field onto the actual packets. Voila DSCP
> traversal across ’tinternet with tin/bandwidth allocation in our
> local domain preserved.
 
 If I understand it right, another use case for this “very bad idea”
 is preserving DSCP locally while traversing upstream WiFi links as
 besteffort, which avoids airtime efficiency problems that can occur
 with 802.11e (WMM). In cases where the router config can’t be changed
 (802.11e is mandatory after all) I’ve used IPIP tunnels for this, as
 it hides DSCP from the WiFi stack while preserving the values through
 the tunnel, but this would be easier. Neat… :)
>>> 
>>> Everyone has understood the intent & maybe the implementation
>>> correctly. 2 patches attached, one for cake, one for tc.
>>> 
>>> They are naively coded and some of it undoes Toke’s recent tidying up
>>> (sorry!)
>> 
>> Heh. First comment: Don't do that ;)
>
> I did say naively coded.
>
>> 
>> A few more below.
>> 
>>> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
>>> diff --git a/pkt_sched.h b/pkt_sched.h
>>> index a2f570c..d1f288d 100644
>>> --- a/pkt_sched.h
>>> +++ b/pkt_sched.h
>>> @@ -879,6 +879,7 @@ enum {
>>> TCA_CAKE_ACK_FILTER,
>>> TCA_CAKE_SPLIT_GSO,
>>> TCA_CAKE_FWMARK,
>>> +   TCA_CAKE_ICING,
>>> __TCA_CAKE_MAX
>>> };
>>> #define TCA_CAKE_MAX(__TCA_CAKE_MAX - 1)
>>> diff --git a/sch_cake.c b/sch_cake.c
>>> index 733b897..5aca0f3 100644
>>> --- a/sch_cake.c
>>> +++ b/sch_cake.c
>>> @@ -270,7 +270,8 @@ enum {
>>> CAKE_FLAG_INGRESS  = BIT(2),
>>> CAKE_FLAG_WASH = BIT(3),
>>> CAKE_FLAG_SPLIT_GSO= BIT(4),
>>> -   CAKE_FLAG_FWMARK   = BIT(5)
>>> +   CAKE_FLAG_FWMARK   = BIT(5),
>>> +   CAKE_FLAG_ICING= BIT(6)
>> 
>> This implies that icing and fwmark can be enabled completely
>> independently of each other. Are you sure about the semantics for that?
>
> No, I’m not.  I sent the patches so others could see my lunacy in action and 
> hopefully improve it.
>
> The actual operation links FWMARK, INGRESS & ICING in a variety of
> combinations.

Right, so obviously this needs to be thought through...

>>> };
>>> 
>>> /* COBALT operates the Codel and BLUE algorithms in parallel, in order to
>>> @@ -333,7 +334,7 @@ static const u8 diffserv8[] = {
>>> };
>>> 
>>> static const u8 diffserv4[] = {
>>> -   0, 2, 0, 0, 2, 0, 0, 0,
>>> +   0, 1, 0, 0, 2, 0, 0, 0,
>>> 1, 0, 0, 0, 0, 0, 0, 0,
>>> 2, 0, 2, 0, 2, 0, 2, 0,
>>> 2, 0, 2, 0, 2, 0, 2, 0,
>>> @@ -344,7 +345,7 @@ static const u8 diffserv4[] = {
>>> };
>>> 
>>> static const u8 diffserv3[] = {
>>> -   0, 0, 0, 0, 2, 0, 0, 0,
>>> +   0, 1, 0, 0, 2, 0, 0, 0,
>> 
>> Why are you messing with the diffserv mappings in this patch?
>
> This is a combination patch of Dave’s new LE coding and the
> fwmark/dscp mangling.

Ah. Well let's keep that separate from this patch/discussion...

>> 
>>> 1, 0, 0, 0, 0, 0, 0, 0,
>>> 0, 0, 0, 0, 0, 0, 0, 0,
>>> 0, 0, 0, 0, 0, 0, 0, 0,
>>> @@ -1618,7 +1619,24 @@ static unsigned int cake_drop(struct Qdisc *sch, 
>>> struct sk_buff **to_free)
>>> return idx + (tin << 16);
>>> }
>>> 
>>> -static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
>>> +void cake_update_diffserv(struct sk_buff *skb, u8 dscp)
>>> +{
>>> +   switch (skb->protocol) {
>>> +   case htons(ETH_P_IP):
>>> +   if ((ipv4_get_dsfield(ip_hdr(skb)) & ~INET_ECN_MASK) != dscp)
>>> +   ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, dscp);
>>> +   break;
>>> +   case htons(ETH_P_IPV6):
>>> +   if ((ipv6_get_dsfield(ipv6_hdr(skb)) & ~INET_ECN_MASK) != dscp)
>>> +   ipv6_change_dsfield(ipv6_hdr(skb), INET_ECN_MASK, dscp);
>>> +   break;
>>> +   default:
>>> +   break;
>>> +   }
>>> +
>>> +}
>> 
>> So washing is just a special case of this (wash is
>> cake_update_diffserv(skb,0)). So you shouldn't need to add another
>> function, just augment the existing handling code.
>
> Erm, that’s exactly what I’ve done.

Ah, right; I guess it's just the reverting of the cleanup patch that is
the issue, then :)

>>> +static u8 cake_handle_diffserv(struct sk_buff *skb, bool wash)
>>> {
>>> u8 

Re: [Cake] Using firewall connmarks as tin selectors

[Kevin Darbyshire-Bryant]

> On 4 Mar 2019, at 11:17, Toke Høiland-Jørgensen  wrote:
> 
> Kevin Darbyshire-Bryant  writes:
> 
>>> On 4 Mar 2019, at 08:39, Pete Heist  wrote:
>>> 
>>> 
 On Mar 3, 2019, at 12:52 PM, Kevin Darbyshire-Bryant 
  wrote:
 
 The very bad idea:
 
 And it’s bad ‘cos it’s sort of incompatible with the existing fwmark
 implementation as described above. So an awful lot of our
 shenanigans above is due to DSCP not traversing the internet
 particularly well. The solution above abstracts DSCP into ’tins’
 which we put into fwmarks. Another approach would be to put the DSCP
 *into* the fwmark. CAKE could (optionally) copy the FWMARK contained
 DSCP into the diffserv field onto the actual packets. Voila DSCP
 traversal across ’tinternet with tin/bandwidth allocation in our
 local domain preserved.
>>> 
>>> If I understand it right, another use case for this “very bad idea”
>>> is preserving DSCP locally while traversing upstream WiFi links as
>>> besteffort, which avoids airtime efficiency problems that can occur
>>> with 802.11e (WMM). In cases where the router config can’t be changed
>>> (802.11e is mandatory after all) I’ve used IPIP tunnels for this, as
>>> it hides DSCP from the WiFi stack while preserving the values through
>>> the tunnel, but this would be easier. Neat… :)
>> 
>> Everyone has understood the intent & maybe the implementation
>> correctly. 2 patches attached, one for cake, one for tc.
>> 
>> They are naively coded and some of it undoes Toke’s recent tidying up
>> (sorry!)
> 
> Heh. First comment: Don't do that ;)

I did say naively coded.

> 
> A few more below.
> 
>> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
>> diff --git a/pkt_sched.h b/pkt_sched.h
>> index a2f570c..d1f288d 100644
>> --- a/pkt_sched.h
>> +++ b/pkt_sched.h
>> @@ -879,6 +879,7 @@ enum {
>>  TCA_CAKE_ACK_FILTER,
>>  TCA_CAKE_SPLIT_GSO,
>>  TCA_CAKE_FWMARK,
>> +TCA_CAKE_ICING,
>>  __TCA_CAKE_MAX
>> };
>> #define TCA_CAKE_MAX (__TCA_CAKE_MAX - 1)
>> diff --git a/sch_cake.c b/sch_cake.c
>> index 733b897..5aca0f3 100644
>> --- a/sch_cake.c
>> +++ b/sch_cake.c
>> @@ -270,7 +270,8 @@ enum {
>>  CAKE_FLAG_INGRESS  = BIT(2),
>>  CAKE_FLAG_WASH = BIT(3),
>>  CAKE_FLAG_SPLIT_GSO= BIT(4),
>> -CAKE_FLAG_FWMARK   = BIT(5)
>> +CAKE_FLAG_FWMARK   = BIT(5),
>> +CAKE_FLAG_ICING= BIT(6)
> 
> This implies that icing and fwmark can be enabled completely
> independently of each other. Are you sure about the semantics for that?

No, I’m not.  I sent the patches so others could see my lunacy in action and 
hopefully improve it.

The actual operation links FWMARK, INGRESS & ICING in a variety of combinations.

>> };
>> 
>> /* COBALT operates the Codel and BLUE algorithms in parallel, in order to
>> @@ -333,7 +334,7 @@ static const u8 diffserv8[] = {
>> };
>> 
>> static const u8 diffserv4[] = {
>> -0, 2, 0, 0, 2, 0, 0, 0,
>> +0, 1, 0, 0, 2, 0, 0, 0,
>>  1, 0, 0, 0, 0, 0, 0, 0,
>>  2, 0, 2, 0, 2, 0, 2, 0,
>>  2, 0, 2, 0, 2, 0, 2, 0,
>> @@ -344,7 +345,7 @@ static const u8 diffserv4[] = {
>> };
>> 
>> static const u8 diffserv3[] = {
>> -0, 0, 0, 0, 2, 0, 0, 0,
>> +0, 1, 0, 0, 2, 0, 0, 0,
> 
> Why are you messing with the diffserv mappings in this patch?

This is a combination patch of Dave’s new LE coding and the fwmark/dscp 
mangling.

> 
>>  1, 0, 0, 0, 0, 0, 0, 0,
>>  0, 0, 0, 0, 0, 0, 0, 0,
>>  0, 0, 0, 0, 0, 0, 0, 0,
>> @@ -1618,7 +1619,24 @@ static unsigned int cake_drop(struct Qdisc *sch, 
>> struct sk_buff **to_free)
>>  return idx + (tin << 16);
>> }
>> 
>> -static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
>> +void cake_update_diffserv(struct sk_buff *skb, u8 dscp)
>> +{
>> +switch (skb->protocol) {
>> +case htons(ETH_P_IP):
>> +if ((ipv4_get_dsfield(ip_hdr(skb)) & ~INET_ECN_MASK) != dscp)
>> +ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, dscp);
>> +break;
>> +case htons(ETH_P_IPV6):
>> +if ((ipv6_get_dsfield(ipv6_hdr(skb)) & ~INET_ECN_MASK) != dscp)
>> +ipv6_change_dsfield(ipv6_hdr(skb), INET_ECN_MASK, dscp);
>> +break;
>> +default:
>> +break;
>> +}
>> +
>> +}
> 
> So washing is just a special case of this (wash is
> cake_update_diffserv(skb,0)). So you shouldn't need to add another
> function, just augment the existing handling code.

Erm, that’s exactly what I’ve done.

> 
>> +static u8 cake_handle_diffserv(struct sk_buff *skb, bool wash)
>> {
>>  u8 dscp;
>> 
>> @@ -1644,37 +1662,70 @@ static u8 cake_handle_diffserv(struct sk_buff *skb, 
>> u16 wash)
>>  }
>> }
>> 
>> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK)
> 
> Save an ifdef below by moving the ifdef inside the function definition.
> 
>> +void cake_update_ct_mark(struct sk_buff *skb, u8 dscp)
>> +{
>> +enum ip_conntrack_info ctinfo;
>> +

Re: [Cake] Using firewall connmarks as tin selectors

[John Sager]

Let action connmark continue to do that. You still need mirred anyway.

John


On 4 March 2019 11:04:39 GMT, "Toke Høiland-Jørgensen"  wrote:
>Kevin Darbyshire-Bryant  writes:
>
>>> On 3 Mar 2019, at 12:22, John Sager  wrote:
>>> 
>>> If you are going to do that, I would suggest using a few of the
>upper bits
>>> of the 32-bit fwmark/connmark space available, rather than the
>lowest bits.
>>> Then that would allow to use fwmarks other purposes, and to use the
>lowest
>>> bits, as well in the future. As iptables allows a mask before
>comparison,
>>> then choose a specific mask for the bits you use both for setting
>and testing.
>>
>> That’s a good point and I’m sort of hoping upstream reject the
>current
>> submission on that basis.  I think the ‘use of fwmarks’ needs more
>> thought as to how it’s done for the future - too keen to get
>something
>> out.  Maybe it’s sufficient as is, I don’t know.
>
>https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=0b5c7efdfc6e389ec6840579fe90bdb6f42b08dc
>
>This means it'll be in 5.1; so we have until that is released (~8 weeks
>or so) to set the behaviour in stone.
>
>I do think we at least need to add masking of the mark before using it
>for tin selection; the question is just which bits to use from it.
>
>As for setting the fwmark back in conntrack, I'm not sure I agree that
>this is something CAKE should be doing. Mostly because it means even
>tighter coupling between CAKE and the conntrack subsystem. However, I
>may be convinced by a sufficiently neat implementation, and anyway this
>is definitely something that will need to wait for 5.2 for upstream.
>
>So I think the priority is to agree on semantics for masking the fwmark
>when reading, and getting that implemented in a way that is compatible
>with both other uses of marks, and with anything we else we might want
>to do down the road.
>
>-Toke

-- 
Sent from the Aether.___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 4 Mar 2019, at 08:39, Pete Heist  wrote:
>> 
>> 
>>> On Mar 3, 2019, at 12:52 PM, Kevin Darbyshire-Bryant 
>>>  wrote:
>>> 
>>> The very bad idea:
>>> 
>>> And it’s bad ‘cos it’s sort of incompatible with the existing fwmark
>>> implementation as described above. So an awful lot of our
>>> shenanigans above is due to DSCP not traversing the internet
>>> particularly well. The solution above abstracts DSCP into ’tins’
>>> which we put into fwmarks. Another approach would be to put the DSCP
>>> *into* the fwmark. CAKE could (optionally) copy the FWMARK contained
>>> DSCP into the diffserv field onto the actual packets. Voila DSCP
>>> traversal across ’tinternet with tin/bandwidth allocation in our
>>> local domain preserved.
>> 
>> If I understand it right, another use case for this “very bad idea”
>> is preserving DSCP locally while traversing upstream WiFi links as
>> besteffort, which avoids airtime efficiency problems that can occur
>> with 802.11e (WMM). In cases where the router config can’t be changed
>> (802.11e is mandatory after all) I’ve used IPIP tunnels for this, as
>> it hides DSCP from the WiFi stack while preserving the values through
>> the tunnel, but this would be easier. Neat… :)
>
> Everyone has understood the intent & maybe the implementation
> correctly. 2 patches attached, one for cake, one for tc.
>
> They are naively coded and some of it undoes Toke’s recent tidying up
> (sorry!)

Heh. First comment: Don't do that ;)

A few more below.

> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
> diff --git a/pkt_sched.h b/pkt_sched.h
> index a2f570c..d1f288d 100644
> --- a/pkt_sched.h
> +++ b/pkt_sched.h
> @@ -879,6 +879,7 @@ enum {
>   TCA_CAKE_ACK_FILTER,
>   TCA_CAKE_SPLIT_GSO,
>   TCA_CAKE_FWMARK,
> + TCA_CAKE_ICING,
>   __TCA_CAKE_MAX
>  };
>  #define TCA_CAKE_MAX (__TCA_CAKE_MAX - 1)
> diff --git a/sch_cake.c b/sch_cake.c
> index 733b897..5aca0f3 100644
> --- a/sch_cake.c
> +++ b/sch_cake.c
> @@ -270,7 +270,8 @@ enum {
>   CAKE_FLAG_INGRESS  = BIT(2),
>   CAKE_FLAG_WASH = BIT(3),
>   CAKE_FLAG_SPLIT_GSO= BIT(4),
> - CAKE_FLAG_FWMARK   = BIT(5)
> + CAKE_FLAG_FWMARK   = BIT(5),
> + CAKE_FLAG_ICING= BIT(6)

This implies that icing and fwmark can be enabled completely
independently of each other. Are you sure about the semantics for that?
>  };
>  
>  /* COBALT operates the Codel and BLUE algorithms in parallel, in order to
> @@ -333,7 +334,7 @@ static const u8 diffserv8[] = {
>  };
>  
>  static const u8 diffserv4[] = {
> - 0, 2, 0, 0, 2, 0, 0, 0,
> + 0, 1, 0, 0, 2, 0, 0, 0,
>   1, 0, 0, 0, 0, 0, 0, 0,
>   2, 0, 2, 0, 2, 0, 2, 0,
>   2, 0, 2, 0, 2, 0, 2, 0,
> @@ -344,7 +345,7 @@ static const u8 diffserv4[] = {
>  };
>  
>  static const u8 diffserv3[] = {
> - 0, 0, 0, 0, 2, 0, 0, 0,
> + 0, 1, 0, 0, 2, 0, 0, 0,

Why are you messing with the diffserv mappings in this patch?

>   1, 0, 0, 0, 0, 0, 0, 0,
>   0, 0, 0, 0, 0, 0, 0, 0,
>   0, 0, 0, 0, 0, 0, 0, 0,
> @@ -1618,7 +1619,24 @@ static unsigned int cake_drop(struct Qdisc *sch, 
> struct sk_buff **to_free)
>   return idx + (tin << 16);
>  }
>  
> -static u8 cake_handle_diffserv(struct sk_buff *skb, u16 wash)
> +void cake_update_diffserv(struct sk_buff *skb, u8 dscp)
> +{
> + switch (skb->protocol) {
> + case htons(ETH_P_IP):
> + if ((ipv4_get_dsfield(ip_hdr(skb)) & ~INET_ECN_MASK) != dscp)
> + ipv4_change_dsfield(ip_hdr(skb), INET_ECN_MASK, dscp);
> + break;
> + case htons(ETH_P_IPV6):
> + if ((ipv6_get_dsfield(ipv6_hdr(skb)) & ~INET_ECN_MASK) != dscp)
> + ipv6_change_dsfield(ipv6_hdr(skb), INET_ECN_MASK, dscp);
> + break;
> + default:
> + break;
> + }
> +
> +}

So washing is just a special case of this (wash is
cake_update_diffserv(skb,0)). So you shouldn't need to add another
function, just augment the existing handling code.

> +static u8 cake_handle_diffserv(struct sk_buff *skb, bool wash)
>  {
>   u8 dscp;
>  
> @@ -1644,37 +1662,70 @@ static u8 cake_handle_diffserv(struct sk_buff *skb, 
> u16 wash)
>   }
>  }
>  
> +#if IS_REACHABLE(CONFIG_NF_CONNTRACK)

Save an ifdef below by moving the ifdef inside the function definition.

> +void cake_update_ct_mark(struct sk_buff *skb, u8 dscp)
> +{
> + enum ip_conntrack_info ctinfo;
> + struct nf_conn *ct;
> +
> + ct = nf_ct_get(skb, );
> + if (!ct)
> + return;
> +
> + ct->mark &= 0x80ff;
> + ct->mark |= (0x40 | dscp) << 24;

Right, so we *might* have an argument that putting the *tin* into the
fwmark is CAKE's business, but copying over the dscp mark is not
something a qdisc should be doing...

> + nf_conntrack_event_cache(IPCT_MARK, ct);
> +}
> +#endif

Also, are you sure this will work in all permutations of conntrack being
a module vs 

Re: [Cake] Using firewall connmarks as tin selectors

[Toke Høiland-Jørgensen]

Kevin Darbyshire-Bryant  writes:

>> On 3 Mar 2019, at 12:22, John Sager  wrote:
>> 
>> If you are going to do that, I would suggest using a few of the upper bits
>> of the 32-bit fwmark/connmark space available, rather than the lowest bits.
>> Then that would allow to use fwmarks other purposes, and to use the lowest
>> bits, as well in the future. As iptables allows a mask before comparison,
>> then choose a specific mask for the bits you use both for setting and 
>> testing.
>
> That’s a good point and I’m sort of hoping upstream reject the current
> submission on that basis.  I think the ‘use of fwmarks’ needs more
> thought as to how it’s done for the future - too keen to get something
> out.  Maybe it’s sufficient as is, I don’t know.

https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git/commit/?id=0b5c7efdfc6e389ec6840579fe90bdb6f42b08dc

This means it'll be in 5.1; so we have until that is released (~8 weeks
or so) to set the behaviour in stone.

I do think we at least need to add masking of the mark before using it
for tin selection; the question is just which bits to use from it.

As for setting the fwmark back in conntrack, I'm not sure I agree that
this is something CAKE should be doing. Mostly because it means even
tighter coupling between CAKE and the conntrack subsystem. However, I
may be convinced by a sufficiently neat implementation, and anyway this
is definitely something that will need to wait for 5.2 for upstream.

So I think the priority is to agree on semantics for masking the fwmark
when reading, and getting that implemented in a way that is compatible
with both other uses of marks, and with anything we else we might want
to do down the road.

-Toke
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

[Cake] suggest not to use udp with iperf3 versions prior to 3.1.5

[Pete Heist]

Just spent an hour chasing down a strange problem with gradually increasing UDP 
packet loss with cake (but not fq_codel), which happens when testing with 
iperf3 3.1.3, but not iperf or irtt. After compiling iperf3 from HEAD, poof, 
the problem disappeared. It’s not just iperf3’s -l flag as I know it’s default 
was too high in 3.1.3 and causing fragmentation, but some other UDP handling 
issue with iperf3.

I still don’t know the reason why it would only happen with cake+iperf3 3.1.3 
and not fq_codel+iperf3 3.1.3 (and go away when ‘cake interplanetary’ is used), 
but I’ll rather leave it alone now and suggest not to use iperf3 < 3.1.5 for 
UDP...

Pete

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Kevin Darbyshire-Bryant]

> On 4 Mar 2019, at 08:39, Pete Heist  wrote:
> 
> 
>> On Mar 3, 2019, at 12:52 PM, Kevin Darbyshire-Bryant 
>>  wrote:
>> 
>> The very bad idea:
>> 
>> And it’s bad ‘cos it’s sort of incompatible with the existing fwmark 
>> implementation as described above.  So an awful lot of our shenanigans above 
>> is due to DSCP not traversing the internet particularly well.  The solution 
>> above abstracts DSCP into ’tins’ which we put into fwmarks.  Another 
>> approach would be to put the DSCP *into* the fwmark.  CAKE could 
>> (optionally) copy the FWMARK contained DSCP into the diffserv field onto the 
>> actual packets.  Voila DSCP traversal across ’tinternet with tin/bandwidth 
>> allocation in our local domain preserved.
> 
> If I understand it right, another use case for this “very bad idea” is 
> preserving DSCP locally while traversing upstream WiFi links as besteffort, 
> which avoids airtime efficiency problems that can occur with 802.11e (WMM). 
> In cases where the router config can’t be changed (802.11e is mandatory after 
> all) I’ve used IPIP tunnels for this, as it hides DSCP from the WiFi stack 
> while preserving the values through the tunnel, but this would be easier. 
> Neat… :)

Everyone has understood the intent & maybe the implementation correctly.  2 
patches attached, one for cake, one for tc.

They are naively coded and some of it undoes Toke’s recent tidying up (sorry!)

The ingress path is dependent upon tc actions having restored the fwmark.  I 
cannot for the life of me work out how that actually happens (ie. the code that 
does it) to see if that’s all it does, or if cake could do it itself and so on.

Anyway, the code warts and all….



Cheers,

Kevin D-B

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


cake-add-dscp-copy-and-icing.patch
Description: cake-add-dscp-copy-and-icing.patch


my_layer_cake.qos
Description: my_layer_cake.qos


tc-cake-add-fwmark-icing-options.patch
Description: tc-cake-add-fwmark-icing-options.patch
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Using firewall connmarks as tin selectors

[Pete Heist]

> On Mar 3, 2019, at 12:52 PM, Kevin Darbyshire-Bryant 
>  wrote:
> 
> The very bad idea:
> 
> And it’s bad ‘cos it’s sort of incompatible with the existing fwmark 
> implementation as described above.  So an awful lot of our shenanigans above 
> is due to DSCP not traversing the internet particularly well.  The solution 
> above abstracts DSCP into ’tins’ which we put into fwmarks.  Another approach 
> would be to put the DSCP *into* the fwmark.  CAKE could (optionally) copy the 
> FWMARK contained DSCP into the diffserv field onto the actual packets.  Voila 
> DSCP traversal across ’tinternet with tin/bandwidth allocation in our local 
> domain preserved.

If I understand it right, another use case for this “very bad idea” is 
preserving DSCP locally while traversing upstream WiFi links as besteffort, 
which avoids airtime efficiency problems that can occur with 802.11e (WMM). In 
cases where the router config can’t be changed (802.11e is mandatory after all) 
I’ve used IPIP tunnels for this, as it hides DSCP from the WiFi stack while 
preserving the values through the tunnel, but this would be easier. Neat… :)
___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake

Re: [Cake] Upstream submission of dual-mode fairness patch

[Pete Heist]

> On Mar 4, 2019, at 5:22 AM, Ryan Mounce  wrote:
> 
> On Mon, 4 Mar 2019 at 13:47, Jonathan Morton  wrote:
>> 
>>> On 4 Mar, 2019, at 4:55 am, Georgios Amanakis  wrote:
>>> 
>>> …the fairness logic wouldn't account for that "ingress traffic" and would 
>>> yield fairer results.
>> 
>> Well there we have a quandary, since presently it enforces fairness of 
>> *load* on the bottleneck link, but the change would alter that to fairness 
>> of *delivered* traffic.  The current setup is arguably more robust against 
>> adversarial traffic, don't you think?

I think that’s the best argument for the current behavior.

> And it provides an incentive to use ECN so that congestion signals can
> be sent "for free" without dropping packets that have traversed the
> bottleneck. I'm firmly in favour of the current setup

Agreed. I was going to provide test results of aggressive UDP vs TCP with and 
without the change, but I’m seeing some odd behavior with UDP that I’ll 
investigate more and start in a separate thread if needed. :)

___
Cake mailing list
Cake@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/cake