[Codel] [RFC PATCH] Don't give malicious CE senders a free ride

[Codel] [RFC PATCH] Don't give malicious CE senders a free ride

[Dave Taht]

After doing myself in with BBR not respecting CE marks, I'm now thinking
that perhaps the right thing for red/fq_codel/cake/pie/etc to do is actually
*drop* stuff that it sees that is already CE marked, when it too wants
to mark, instead of giving it a free ride.

It's an unfriendly world out there.

Not happy with the name of the function... and to me - it looks like
the existing  call sites for IP_ECN_set_ce would actually be saner if
they always returned 0 instead of !ecn.

diff --git a/include/net/inet_ecn.h b/include/net/inet_ecn.h
index dce2d58..3742d84 100644
--- a/include/net/inet_ecn.h
+++ b/include/net/inet_ecn.h
@@ -71,6 +71,36 @@ static inline void INET_ECN_dontxmit(struct sock *sk)
                (label) |= htonl(INET_ECN_ECT_0 << 20);                 \
     } while (0)


+/* When CE is already asserted, sometimes it is saner to drop the packet
+   in order to not give malicious CE senders as much of a free ride. */
+
+static inline int IP_ECN_safe_set_ce(struct iphdr *iph)
+{
+       u32 check = (__force u32)iph->check;
+       u32 ecn = (iph->tos + 1) & INET_ECN_MASK;
+
+       /*
+        * After the last operation we have (in binary):
+        * INET_ECN_NOT_ECT => 01
+        * INET_ECN_ECT_1   => 10
+        * INET_ECN_ECT_0   => 11
+        * INET_ECN_CE      => 00
+        */
+       if (!(ecn & 2))
+               return 0;
+
+       /*
+        * The following gives us:
+        * INET_ECN_ECT_1 => check += htons(0xFFFD)
+        * INET_ECN_ECT_0 => check += htons(0xFFFE)
+        */
+       check += (__force u16)htons(0xFFFB) + (__force u16)htons(ecn);
+
+       iph->check = (__force __sum16)(check + (check>=0xFFFF));
+       iph->tos |= INET_ECN_CE;
+       return 1;
+}
+

-- 
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org

Re: [Codel] [Cake] [RFC PATCH] Don't give malicious CE senders a free ride

[Jonathan Morton]

> On 6 Oct, 2016, at 07:28, Dave Taht <dave.taht@gmail.com> wrote:
> 
> After doing myself in with BBR not respecting CE marks, I'm now thinking
> that perhaps the right thing for red/fq_codel/cake/pie/etc to do is actually
> *drop* stuff that it sees that is already CE marked, when it too wants
> to mark, instead of giving it a free ride.
> 
> It's an unfriendly world out there.

I’m consciously relying on FQ behaviour and the BLUE component of COBALT to take care of flows unresponsive to CE marks.  They harm their own latency and don’t get (much) improvement in throughput, FQ keeps them from impacting other flows, and BLUE starts dropping if their window growth is unbounded.

 - Jonathan Morton