[ovs-dev] [PATCH] bfd: Make the tp_dst masking megaflow-friendly.

Ben Pfaff blp at ovn.org
Wed Oct 3 21:05:30 UTC 2018


On Mon, Oct 01, 2018 at 02:12:29PM -0700, Han Zhou wrote:
> From: Han Zhou <hzhou8 at ebay.com>
> 
> When there are tunnel ports with BFD enabled, all UDP flows will have
> dst port as match condition in datapath, which causes unnecessarily
> high flow miss for all UDP traffic, and results in latency increase.
> For more details, see [1].
> 
> This patch solves the problem by masking tp_dst only for the leading
> bits that is enough to tell the mismatch when it is not BFD traffic.
> 
> [1] https://mail.openvswitch.org/pipermail/ovs-discuss/2018-September/047360.html
> 
> Signed-off-by: Han Zhou <hzhou8 at ebay.com>

I think that we only need to "un-wildcard" one bit, not all the leading
bits.

What do you think of the following slight generalization?

Thanks,

Ben.

diff --git a/lib/bfd.c b/lib/bfd.c
index 530826240c71..cc8c6857afa4 100644
--- a/lib/bfd.c
+++ b/lib/bfd.c
@@ -672,19 +672,18 @@ bfd_should_process_flow(const struct bfd *bfd_, const struct flow *flow,
 
     if (flow->dl_type == htons(ETH_TYPE_IP)) {
         memset(&wc->masks.nw_proto, 0xff, sizeof wc->masks.nw_proto);
-        if (flow->nw_proto == IPPROTO_UDP && !(flow->nw_frag & FLOW_NW_FRAG_LATER)) {
-            memset(&wc->masks.tp_dst, 0xff, sizeof wc->masks.tp_dst);
-            if (flow->tp_dst == htons(BFD_DEST_PORT)) {
-                bool check_tnl_key;
-
-                atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key);
-                if (check_tnl_key) {
-                    memset(&wc->masks.tunnel.tun_id, 0xff,
-                           sizeof wc->masks.tunnel.tun_id);
-                    return flow->tunnel.tun_id == htonll(0);
-                }
-                return true;
+        if (flow->nw_proto == IPPROTO_UDP
+            && !(flow->nw_frag & FLOW_NW_FRAG_LATER)
+            && tp_dst_equals(flow, BFD_DEST_PORT, wc)) {
+            bool check_tnl_key;
+
+            atomic_read_relaxed(&bfd->check_tnl_key, &check_tnl_key);
+            if (check_tnl_key) {
+                memset(&wc->masks.tunnel.tun_id, 0xff,
+                       sizeof wc->masks.tunnel.tun_id);
+                return flow->tunnel.tun_id == htonll(0);
             }
+            return true;
         }
     }
     return false;
diff --git a/lib/flow.h b/lib/flow.h
index 1f6c1d6ae30d..902ab1bad5f1 100644
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -1188,4 +1188,26 @@ static inline bool is_stp(const struct flow *flow)
             && eth_addr_equals(flow->dl_dst, eth_addr_stp));
 }
 
+/* Returns true if flow->tp_dst equals 'port'.  If 'wc' is nonnull, sets
+ * appropriate bits in wc->masks.tp_dst to account for the test.
+ *
+ * The caller must already have ensured that 'flow' is a protocol for which
+ * tp_dst is relevant. */
+static inline bool tp_dst_equals(const struct flow *flow, uint16_t port,
+                                 struct flow_wildcards *wc)
+{
+    uint16_t diff = port ^ ntohs(flow->tp_dst);
+    if (wc) {
+        if (diff) {
+            /* Set mask for the most significant mismatching bit. */
+            int ofs = raw_clz64((uint64_t) diff << 48); /* range [0,15] */
+            wc->masks.tp_dst |= htons(0x8000 >> ofs);
+        } else {
+            /* Must match all bits. */
+            wc->masks.tp_dst = OVS_BE16_MAX;
+        }
+    }
+    return !diff;
+}
+
 #endif /* flow.h */


More information about the dev mailing list