[ovs-dev] [PATCH 10/18] lib/cfm: Use relaxed atomics and optimize cfm_should_process_flow().

[Jarno Rajahalme]

The atomics here do not synchronize the state of any other variables,
so we can use relaxed atomics.

cfm_should_process_flow() is rearranged to set the megaflow mask bits
only if necessary, and to avoid the atomic operation on non-BFD
packets.

Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
 lib/cfm.c |   42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/lib/cfm.c b/lib/cfm.c
index d83a68c..c6657c3 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -128,6 +128,9 @@ struct cfm {
                                  recomputed. */
     long long int last_tx;    /* Last CCM transmission time. */
 
+    /* These bools are atomic to allow readers to check their values
+     * without taking 'mutex'.  Such readers do not assume the values they
+     * read are synchronized with any other members. */
     atomic_bool check_tnl_key; /* Verify the tunnel key of inbound packets? */
     atomic_bool extended;      /* Extended mode. */
     struct ovs_refcount ref_cnt;
@@ -184,7 +187,9 @@ static const uint8_t *
 cfm_ccm_addr(struct cfm *cfm)
 {
     bool extended;
-    atomic_read(&cfm->extended, &extended);
+
+    atomic_read_relaxed(&cfm->extended, &extended);
+
     return extended ? eth_addr_ccm_x : eth_addr_ccm;
 }
 
@@ -578,6 +583,8 @@ cfm_compose_ccm(struct cfm *cfm, struct ofpbuf *packet,
         eth_push_vlan(packet, htons(ETH_TYPE_VLAN), htons(tci));
     }
 
+    atomic_read_relaxed(&cfm->extended, &extended);
+
     ccm = ofpbuf_l3(packet);
     ccm->mdlevel_version = 0;
     ccm->opcode = CCM_OPCODE;
@@ -588,7 +595,6 @@ cfm_compose_ccm(struct cfm *cfm, struct ofpbuf *packet,
     memset(ccm->zero, 0, sizeof ccm->zero);
     ccm->end_tlv = 0;
 
-    atomic_read(&cfm->extended, &extended);
     if (extended) {
         ccm->mpid = htons(hash_mpid(cfm->mpid));
         ccm->mpid64 = htonll(cfm->mpid);
@@ -664,8 +670,8 @@ cfm_configure(struct cfm *cfm, const struct cfm_settings *s)
     interval = ms_to_ccm_interval(s->interval);
     interval_ms = ccm_interval_to_ms(interval);
 
-    atomic_store(&cfm->check_tnl_key, s->check_tnl_key);
-    atomic_store(&cfm->extended, s->extended);
+    atomic_store_relaxed(&cfm->check_tnl_key, s->check_tnl_key);
+    atomic_store_relaxed(&cfm->extended, s->extended);
 
     cfm->ccm_vlan = s->ccm_vlan;
     cfm->ccm_pcp = s->ccm_pcp & (VLAN_PCP_MASK >> VLAN_PCP_SHIFT);
@@ -717,14 +723,25 @@ cfm_should_process_flow(const struct cfm *cfm_, const struct flow *flow,
     struct cfm *cfm = CONST_CAST(struct cfm *, cfm_);
     bool check_tnl_key;
 
-    atomic_read(&cfm->check_tnl_key, &check_tnl_key);
+    /* Most packets are not CFM. */
+    if (OVS_LIKELY(ntohs(flow->dl_type) != ETH_TYPE_CFM)) {
+        return false;
+    }
+
+    atomic_read_relaxed(&cfm->check_tnl_key, &check_tnl_key);
+
+    /* Atomic read is always a memory access, do some work
+     * before we need the result. */
     memset(&wc->masks.dl_dst, 0xff, sizeof wc->masks.dl_dst);
+    if (OVS_UNLIKELY(!eth_addr_equals(flow->dl_dst, cfm_ccm_addr(cfm)))) {
+        return false;
+    }
+
     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 (ntohs(flow->dl_type) == ETH_TYPE_CFM
-            && eth_addr_equals(flow->dl_dst, cfm_ccm_addr(cfm))
-            && (!check_tnl_key || flow->tunnel.tun_id == htonll(0)));
+    return true;
 }
 
 /* Updates internal statistics relevant to packet 'p'.  Should be called on
@@ -736,9 +753,12 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
 {
     struct ccm *ccm;
     struct eth_header *eth;
+    bool extended;
 
     ovs_mutex_lock(&mutex);
 
+    atomic_read_relaxed(&cfm->extended, &extended);
+
     eth = ofpbuf_l2(p);
     ccm = ofpbuf_at(p, (uint8_t *)ofpbuf_l3(p) - (uint8_t *)ofpbuf_data(p),
                     CCM_ACCEPT_LEN);
@@ -777,10 +797,8 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
         uint64_t ccm_mpid;
         uint32_t ccm_seq;
         bool ccm_opdown;
-        bool extended;
         enum cfm_fault_reason cfm_fault = 0;
 
-        atomic_read(&cfm->extended, &extended);
         if (extended) {
             ccm_mpid = ntohll(ccm->mpid64);
             ccm_opdown = ccm->opdown;
@@ -931,7 +949,7 @@ cfm_get_opup__(const struct cfm *cfm_) OVS_REQUIRES(mutex)
     struct cfm *cfm = CONST_CAST(struct cfm *, cfm_);
     bool extended;
 
-    atomic_read(&cfm->extended, &extended);
+    atomic_read_relaxed(&cfm->extended, &extended);
 
     return extended ? cfm->remote_opup : -1;
 }
@@ -1006,7 +1024,7 @@ cfm_print_details(struct ds *ds, struct cfm *cfm) OVS_REQUIRES(mutex)
     bool extended;
     int fault;
 
-    atomic_read(&cfm->extended, &extended);
+    atomic_read_relaxed(&cfm->extended, &extended);
 
     ds_put_format(ds, "---- %s ----\n", cfm->name);
     ds_put_format(ds, "MPID %"PRIu64":%s%s\n", cfm->mpid,
-- 
1.7.10.4