kernel: cake: skb hash backport to 419

Commit 7b4877c2040a63332cd1f10023c51698fec2ed98 backported to 5.4 only,
backport to 4.19 as well.

Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

diff --git a/target/linux/generic/backport-4.19/395-v5.8-net-sch_cake-Take-advantage-of-skb-hash-where-appropriate.patch b/target/linux/generic/backport-4.19/395-v5.8-net-sch_cake-Take-advantage-of-skb-hash-where-appropriate.patch
new file mode 100644 (file)
index 0000000..7b3396c
--- /dev/null
+++ b/target/linux/generic/backport-4.19/395-v5.8-net-sch_cake-Take-advantage-of-skb-hash-where-appropriate.patch
@@ -0,0 +1,170 @@
+From b0c19ed6088ab41dd2a727b60594b7297c15d6ce Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Toke=20H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
+Date: Fri, 29 May 2020 14:43:44 +0200
+Subject: [PATCH] sch_cake: Take advantage of skb->hash where appropriate
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+While the other fq-based qdiscs take advantage of skb->hash and doesn't
+recompute it if it is already set, sch_cake does not.
+
+This was a deliberate choice because sch_cake hashes various parts of the
+packet header to support its advanced flow isolation modes. However,
+foregoing the use of skb->hash entirely loses a few important benefits:
+
+- When skb->hash is set by hardware, a few CPU cycles can be saved by not
+  hashing again in software.
+
+- Tunnel encapsulations will generally preserve the value of skb->hash from
+  before the encapsulation, which allows flow-based qdiscs to distinguish
+  between flows even though the outer packet header no longer has flow
+  information.
+
+It turns out that we can preserve these desirable properties in many cases,
+while still supporting the advanced flow isolation properties of sch_cake.
+This patch does so by reusing the skb->hash value as the flow_hash part of
+the hashing procedure in cake_hash() only in the following conditions:
+
+- If the skb->hash is marked as covering the flow headers (skb->l4_hash is
+  set)
+
+AND
+
+- NAT header rewriting is either disabled, or did not change any values
+  used for hashing. The latter is important to match local-origin packets
+  such as those of a tunnel endpoint.
+
+The immediate motivation for fixing this was the recent patch to WireGuard
+to preserve the skb->hash on encapsulation. As such, this is also what I
+tested against; with this patch, added latency under load for competing
+flows drops from ~8 ms to sub-1ms on an RRUL test over a WireGuard tunnel
+going through a virtual link shaped to 1Gbps using sch_cake. This matches
+the results we saw with a similar setup using sch_fq_codel when testing the
+WireGuard patch.
+
+Fixes: 046f6fd5daef ("sched: Add Common Applications Kept Enhanced (cake) qdisc")
+Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
+Signed-off-by: David S. Miller <davem@davemloft.net>
+Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
+---
+ net/sched/sch_cake.c | 65 ++++++++++++++++++++++++++++++++++----------
+ 1 file changed, 51 insertions(+), 14 deletions(-)
+
+--- a/net/sched/sch_cake.c
++++ b/net/sched/sch_cake.c
+@@ -585,26 +585,48 @@ static bool cobalt_should_drop(struct co
+       return drop;
+ }
+ 
+-static void cake_update_flowkeys(struct flow_keys *keys,
++static bool cake_update_flowkeys(struct flow_keys *keys,
+                                const struct sk_buff *skb)
+ {
+ #if IS_ENABLED(CONFIG_NF_CONNTRACK)
+       struct nf_conntrack_tuple tuple = {};
+-      bool rev = !skb->_nfct;
++      bool rev = !skb->_nfct, upd = false;
++      __be32 ip;
+ 
+       if (tc_skb_protocol(skb) != htons(ETH_P_IP))
+-              return;
++              return false;
+ 
+       if (!nf_ct_get_tuple_skb(&tuple, skb))
+-              return;
++              return false;
+ 
+-      keys->addrs.v4addrs.src = rev ? tuple.dst.u3.ip : tuple.src.u3.ip;
+-      keys->addrs.v4addrs.dst = rev ? tuple.src.u3.ip : tuple.dst.u3.ip;
++      ip = rev ? tuple.dst.u3.ip : tuple.src.u3.ip;
++      if (ip != keys->addrs.v4addrs.src) {
++              keys->addrs.v4addrs.src = ip;
++              upd = true;
++      }
++      ip = rev ? tuple.src.u3.ip : tuple.dst.u3.ip;
++      if (ip != keys->addrs.v4addrs.dst) {
++              keys->addrs.v4addrs.dst = ip;
++              upd = true;
++      }
+ 
+       if (keys->ports.ports) {
+-              keys->ports.src = rev ? tuple.dst.u.all : tuple.src.u.all;
+-              keys->ports.dst = rev ? tuple.src.u.all : tuple.dst.u.all;
++              __be16 port;
++
++              port = rev ? tuple.dst.u.all : tuple.src.u.all;
++              if (port != keys->ports.src) {
++                      keys->ports.src = port;
++                      upd = true;
++              }
++              port = rev ? tuple.src.u.all : tuple.dst.u.all;
++              if (port != keys->ports.dst) {
++                      port = keys->ports.dst;
++                      upd = true;
++              }
+       }
++      return upd;
++#else
++      return false;
+ #endif
+ }
+ 
+@@ -625,23 +647,36 @@ static bool cake_ddst(int flow_mode)
+ static u32 cake_hash(struct cake_tin_data *q, const struct sk_buff *skb,
+                    int flow_mode, u16 flow_override, u16 host_override)
+ {
++      bool hash_flows = (!flow_override && !!(flow_mode & CAKE_FLOW_FLOWS));
++      bool hash_hosts = (!host_override && !!(flow_mode & CAKE_FLOW_HOSTS));
++      bool nat_enabled = !!(flow_mode & CAKE_FLOW_NAT_FLAG);
+       u32 flow_hash = 0, srchost_hash = 0, dsthost_hash = 0;
+       u16 reduced_hash, srchost_idx, dsthost_idx;
+       struct flow_keys keys, host_keys;
++      bool use_skbhash = skb->l4_hash;
+ 
+       if (unlikely(flow_mode == CAKE_FLOW_NONE))
+               return 0;
+ 
+-      /* If both overrides are set we can skip packet dissection entirely */
+-      if ((flow_override || !(flow_mode & CAKE_FLOW_FLOWS)) &&
+-          (host_override || !(flow_mode & CAKE_FLOW_HOSTS)))
++      /* If both overrides are set, or we can use the SKB hash and nat mode is
++       * disabled, we can skip packet dissection entirely. If nat mode is
++       * enabled there's another check below after doing the conntrack lookup.
++       */
++      if ((!hash_flows || (use_skbhash && !nat_enabled)) && !hash_hosts)
+               goto skip_hash;
+ 
+       skb_flow_dissect_flow_keys(skb, &keys,
+                                  FLOW_DISSECTOR_F_STOP_AT_FLOW_LABEL);
+ 
+-      if (flow_mode & CAKE_FLOW_NAT_FLAG)
+-              cake_update_flowkeys(&keys, skb);
++      /* Don't use the SKB hash if we change the lookup keys from conntrack */
++      if (nat_enabled && cake_update_flowkeys(&keys, skb))
++              use_skbhash = false;
++
++      /* If we can still use the SKB hash and don't need the host hash, we can
++       * skip the rest of the hashing procedure
++       */
++      if (use_skbhash && !hash_hosts)
++              goto skip_hash;
+ 
+       /* flow_hash_from_keys() sorts the addresses by value, so we have
+        * to preserve their order in a separate data structure to treat
+@@ -680,12 +715,14 @@ static u32 cake_hash(struct cake_tin_dat
+       /* This *must* be after the above switch, since as a
+        * side-effect it sorts the src and dst addresses.
+        */
+-      if (flow_mode & CAKE_FLOW_FLOWS)
++      if (hash_flows && !use_skbhash)
+               flow_hash = flow_hash_from_keys(&keys);
+ 
+ skip_hash:
+       if (flow_override)
+               flow_hash = flow_override - 1;
++      else if (use_skbhash)
++              flow_hash = skb->hash;
+       if (host_override) {
+               dsthost_hash = host_override - 1;
+               srchost_hash = host_override - 1;