Both the IPFIX and SFLOW modules are thread safe, so there's no
particular reason to pass them up to the main thread.  Eliminating
this step significantly simplifies the code.

Signed-off-by: Ethan Jackson <et...@nicira.com>
---
 ofproto/ofproto-dpif-upcall.c |  250 +++++++++++++++++++++++++++--------------
 ofproto/ofproto-dpif-upcall.h |   32 ------
 ofproto/ofproto-dpif-xlate.c  |   38 +++++++
 ofproto/ofproto-dpif-xlate.h  |    5 +
 ofproto/ofproto-dpif.c        |   95 ----------------
 5 files changed, 206 insertions(+), 214 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index d75c61b..bfd01b5 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -29,6 +29,8 @@
 #include "list.h"
 #include "netlink.h"
 #include "ofpbuf.h"
+#include "ofproto-dpif-ipfix.h"
+#include "ofproto-dpif-sflow.h"
 #include "ofproto-dpif.h"
 #include "packets.h"
 #include "poll-loop.h"
@@ -83,7 +85,6 @@ struct udpif {
 
     /* Queues to pass up to ofproto-dpif. */
     struct guarded_list drop_keys; /* "struct drop key"s. */
-    struct guarded_list upcalls;   /* "struct upcall"s. */
     struct guarded_list fmbs;      /* "struct flow_miss_batch"es. */
 
     /* Number of times udpif_revalidate() has been called. */
@@ -94,6 +95,29 @@ struct udpif {
     struct latch exit_latch; /* Tells child threads to exit. */
 };
 
+enum upcall_type {
+    /* Handled internally by udpif code.  Not returned by upcall_next().*/
+    BAD_UPCALL,                 /* Some kind of bug somewhere. */
+    MISS_UPCALL,                /* A flow miss.  */
+
+    /* Require main thread's involvement.  May be returned by upcall_next(). */
+    SFLOW_UPCALL,               /* sFlow sample. */
+    FLOW_SAMPLE_UPCALL,         /* Per-flow sampling. */
+    IPFIX_UPCALL                /* Per-bridge sampling. */
+};
+
+struct upcall {
+    struct list list_node;          /* For queuing upcalls. */
+    struct flow_miss *flow_miss;    /* This upcall's flow_miss. */
+
+    /* Raw upcall plus data for keeping track of the memory backing it. */
+    struct dpif_upcall dpif_upcall; /* As returned by dpif_recv() */
+    struct ofpbuf upcall_buf;       /* Owns some data in 'dpif_upcall'. */
+    uint64_t upcall_stub[512 / 8];  /* Buffer to reduce need for malloc(). */
+};
+
+static void upcall_destroy(struct upcall *);
+
 static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
 
 static void recv_upcalls(struct udpif *);
@@ -113,7 +137,6 @@ udpif_create(struct dpif_backer *backer, struct dpif *dpif)
     udpif->wait_seq = seq_create();
     latch_init(&udpif->exit_latch);
     guarded_list_init(&udpif->drop_keys);
-    guarded_list_init(&udpif->upcalls);
     guarded_list_init(&udpif->fmbs);
     atomic_init(&udpif->reval_seq, 0);
 
@@ -125,7 +148,6 @@ udpif_destroy(struct udpif *udpif)
 {
     struct flow_miss_batch *fmb;
     struct drop_key *drop_key;
-    struct upcall *upcall;
 
     udpif_recv_set(udpif, 0, false);
 
@@ -133,16 +155,11 @@ udpif_destroy(struct udpif *udpif)
         drop_key_destroy(drop_key);
     }
 
-    while ((upcall = upcall_next(udpif))) {
-        upcall_destroy(upcall);
-    }
-
     while ((fmb = flow_miss_batch_next(udpif))) {
         flow_miss_batch_destroy(fmb);
     }
 
     guarded_list_destroy(&udpif->drop_keys);
-    guarded_list_destroy(&udpif->upcalls);
     guarded_list_destroy(&udpif->fmbs);
     latch_destroy(&udpif->exit_latch);
     seq_destroy(udpif->wait_seq);
@@ -221,7 +238,6 @@ udpif_wait(struct udpif *udpif)
 {
     uint64_t seq = seq_read(udpif->wait_seq);
     if (!guarded_list_is_empty(&udpif->drop_keys) ||
-        !guarded_list_is_empty(&udpif->upcalls) ||
         !guarded_list_is_empty(&udpif->fmbs)) {
         poll_immediate_wake();
     } else {
@@ -254,18 +270,8 @@ udpif_revalidate(struct udpif *udpif)
     udpif_drop_key_clear(udpif);
 }
 
-/* Retreives the next upcall which ofproto-dpif is responsible for handling.
- * The caller is responsible for destroying the returned upcall with
- * upcall_destroy(). */
-struct upcall *
-upcall_next(struct udpif *udpif)
-{
-    struct list *next = guarded_list_pop_front(&udpif->upcalls);
-    return next ? CONTAINER_OF(next, struct upcall, list_node) : NULL;
-}
-
 /* Destroys and deallocates 'upcall'. */
-void
+static void
 upcall_destroy(struct upcall *upcall)
 {
     if (upcall) {
@@ -475,11 +481,129 @@ classify_upcall(const struct upcall *upcall)
 }
 
 static void
+handle_sflow_upcall(struct udpif *udpif, struct upcall *upcall)
+{
+    struct dpif_upcall *dupcall = &upcall->dpif_upcall;
+    union user_action_cookie cookie;
+    struct ofproto_dpif *ofproto;
+    struct dpif_sflow *sflow;
+    odp_port_t odp_in_port;
+    struct flow flow;
+
+    if (xlate_receive(udpif->backer, dupcall->packet, dupcall->key,
+                      dupcall->key_len, &flow, NULL, &ofproto, &odp_in_port)) {
+        return;
+    }
+
+    sflow = xlate_get_sflow(ofproto);
+    if (!sflow) {
+        return;
+    }
+
+    memset(&cookie, 0, sizeof cookie);
+    memcpy(&cookie, nl_attr_get(dupcall->userdata), sizeof cookie.sflow);
+    dpif_sflow_received(sflow, dupcall->packet, &flow, odp_in_port, &cookie);
+    dpif_sflow_unref(sflow);
+}
+
+static void
+handle_ipfix_upcall(struct udpif *udpif, struct upcall *upcall,
+                    bool flow_sample)
+{
+    struct dpif_upcall *dupcall = &upcall->dpif_upcall;
+    struct ofproto_dpif *ofproto;
+    struct dpif_ipfix *ipfix;
+    struct flow flow;
+
+    if (xlate_receive(udpif->backer, dupcall->packet, dupcall->key,
+                      dupcall->key_len, &flow, NULL, &ofproto, NULL)) {
+        return;
+    }
+
+    ipfix = xlate_get_ipfix(ofproto);
+    if (!ipfix) {
+        return;
+    }
+
+    if (flow_sample) {
+        union user_action_cookie cookie;
+
+        memset(&cookie, 0, sizeof cookie);
+        memcpy(&cookie, nl_attr_get(dupcall->userdata),
+               sizeof cookie.flow_sample);
+
+        /* The flow reflects exactly the contents of the packet.  Sample
+         * the packet using it. */
+        dpif_ipfix_flow_sample(ipfix, dupcall->packet, &flow,
+                               cookie.flow_sample.collector_set_id,
+                               cookie.flow_sample.probability,
+                               cookie.flow_sample.obs_domain_id,
+                               cookie.flow_sample.obs_point_id);
+    } else {
+        /* The flow reflects exactly the contents of the packet.  Sample
+         * the packet using it. */
+        dpif_ipfix_bridge_sample(ipfix, dupcall->packet, &flow);
+    }
+    dpif_ipfix_unref(ipfix);
+}
+
+static void
+dispatch_miss_upcall(struct udpif *udpif, struct upcall *upcall)
+{
+    struct dpif_upcall *dupcall = &upcall->dpif_upcall;
+    uint32_t hash = udpif->secret;
+    struct handler *handler;
+    size_t n_bytes, left;
+    struct nlattr *nla;
+
+    n_bytes = 0;
+    NL_ATTR_FOR_EACH (nla, left, dupcall->key, dupcall->key_len) {
+        enum ovs_key_attr type = nl_attr_type(nla);
+        if (type == OVS_KEY_ATTR_IN_PORT
+            || type == OVS_KEY_ATTR_TCP
+            || type == OVS_KEY_ATTR_UDP) {
+            if (nl_attr_get_size(nla) == 4) {
+                ovs_be32 attr = nl_attr_get_be32(nla);
+                hash = mhash_add(hash, (OVS_FORCE uint32_t) attr);
+                n_bytes += 4;
+            } else {
+                VLOG_WARN("Netlink attribute with incorrect size.");
+            }
+        }
+    }
+    hash =  mhash_finish(hash, n_bytes);
+
+    handler = &udpif->handlers[hash % udpif->n_handlers];
+
+    ovs_mutex_lock(&handler->mutex);
+    if (handler->n_upcalls < MAX_QUEUE_LENGTH) {
+        list_push_back(&handler->upcalls, &upcall->list_node);
+        handler->n_new_upcalls = ++handler->n_upcalls;
+
+        if (handler->n_new_upcalls >= FLOW_MISS_MAX_BATCH) {
+            xpthread_cond_signal(&handler->wake_cond);
+        }
+        ovs_mutex_unlock(&handler->mutex);
+        if (!VLOG_DROP_DBG(&rl)) {
+            struct ds ds = DS_EMPTY_INITIALIZER;
+
+            odp_flow_key_format(upcall->dpif_upcall.key,
+                                upcall->dpif_upcall.key_len,
+                                &ds);
+            VLOG_DBG("dispatcher: miss enqueue (%s)", ds_cstr(&ds));
+            ds_destroy(&ds);
+        }
+    } else {
+        ovs_mutex_unlock(&handler->mutex);
+        COVERAGE_INC(miss_queue_overflow);
+        upcall_destroy(upcall);
+    }
+}
+
+static void
 recv_upcalls(struct udpif *udpif)
 {
-    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(60, 60);
     size_t n_udpif_new_upcalls = 0;
-    struct handler *handler;
     int n;
 
     for (;;) {
@@ -496,75 +620,27 @@ recv_upcalls(struct udpif *udpif)
             break;
         }
 
-        upcall->type = classify_upcall(upcall);
-        if (upcall->type == BAD_UPCALL) {
+        switch (classify_upcall(upcall)) {
+        case MISS_UPCALL:
+            dispatch_miss_upcall(udpif, upcall);
+            break;
+        case SFLOW_UPCALL:
+            handle_sflow_upcall(udpif, upcall);
+            break;
+        case IPFIX_UPCALL:
+            handle_ipfix_upcall(udpif, upcall, false);
+            break;
+        case FLOW_SAMPLE_UPCALL:
+            handle_ipfix_upcall(udpif, upcall, true);
+            break;
+        case BAD_UPCALL:
             upcall_destroy(upcall);
-        } else if (upcall->type == MISS_UPCALL) {
-            struct dpif_upcall *dupcall = &upcall->dpif_upcall;
-            uint32_t hash = udpif->secret;
-            struct nlattr *nla;
-            size_t n_bytes, left;
-
-            n_bytes = 0;
-            NL_ATTR_FOR_EACH (nla, left, dupcall->key, dupcall->key_len) {
-                enum ovs_key_attr type = nl_attr_type(nla);
-                if (type == OVS_KEY_ATTR_IN_PORT
-                    || type == OVS_KEY_ATTR_TCP
-                    || type == OVS_KEY_ATTR_UDP) {
-                    if (nl_attr_get_size(nla) == 4) {
-                        ovs_be32 attr = nl_attr_get_be32(nla);
-                        hash = mhash_add(hash, (OVS_FORCE uint32_t) attr);
-                        n_bytes += 4;
-                    } else {
-                        VLOG_WARN("Netlink attribute with incorrect size.");
-                    }
-                }
-            }
-            hash =  mhash_finish(hash, n_bytes);
-
-            handler = &udpif->handlers[hash % udpif->n_handlers];
-
-            ovs_mutex_lock(&handler->mutex);
-            if (handler->n_upcalls < MAX_QUEUE_LENGTH) {
-                list_push_back(&handler->upcalls, &upcall->list_node);
-                handler->n_new_upcalls = ++handler->n_upcalls;
-
-                if (handler->n_new_upcalls >= FLOW_MISS_MAX_BATCH) {
-                    xpthread_cond_signal(&handler->wake_cond);
-                }
-                ovs_mutex_unlock(&handler->mutex);
-                if (!VLOG_DROP_DBG(&rl)) {
-                    struct ds ds = DS_EMPTY_INITIALIZER;
-
-                    odp_flow_key_format(upcall->dpif_upcall.key,
-                                        upcall->dpif_upcall.key_len,
-                                        &ds);
-                    VLOG_DBG("dispatcher: miss enqueue (%s)", ds_cstr(&ds));
-                    ds_destroy(&ds);
-                }
-            } else {
-                ovs_mutex_unlock(&handler->mutex);
-                COVERAGE_INC(miss_queue_overflow);
-                upcall_destroy(upcall);
-            }
-        } else {
-            size_t len;
-
-            len = guarded_list_push_back(&udpif->upcalls, &upcall->list_node,
-                                         MAX_QUEUE_LENGTH);
-            if (len > 0) {
-                n_udpif_new_upcalls = len;
-                if (n_udpif_new_upcalls >= FLOW_MISS_MAX_BATCH) {
-                    seq_change(udpif->wait_seq);
-                }
-            } else {
-                COVERAGE_INC(upcall_queue_overflow);
-                upcall_destroy(upcall);
-            }
+            break;
         }
     }
     for (n = 0; n < udpif->n_handlers; ++n) {
-        handler = &udpif->handlers[n];
+        struct handler *handler = &udpif->handlers[n];
+
         if (handler->n_new_upcalls) {
             handler->n_new_upcalls = 0;
             ovs_mutex_lock(&handler->mutex);
diff --git a/ofproto/ofproto-dpif-upcall.h b/ofproto/ofproto-dpif-upcall.h
index 9bd19ad..99bbedf 100644
--- a/ofproto/ofproto-dpif-upcall.h
+++ b/ofproto/ofproto-dpif-upcall.h
@@ -40,38 +40,6 @@ void udpif_wait(struct udpif *);
 
 void udpif_revalidate(struct udpif *);
 
-/* udpif can handle some upcalls on its own.  Others need the main ofproto_dpif
- * code to handle them.  This interface passes upcalls not handled by udpif up
- * to the ofproto_dpif main thread. */
-
-/* Type of an upcall. */
-enum upcall_type {
-    /* Handled internally by udpif code.  Not returned by upcall_next().*/
-    BAD_UPCALL,                 /* Some kind of bug somewhere. */
-    MISS_UPCALL,                /* A flow miss.  */
-
-    /* Require main thread's involvement.  May be returned by upcall_next(). */
-    SFLOW_UPCALL,               /* sFlow sample. */
-    FLOW_SAMPLE_UPCALL,         /* Per-flow sampling. */
-    IPFIX_UPCALL                /* Per-bridge sampling. */
-};
-
-/* An upcall. */
-struct upcall {
-    struct list list_node;          /* For queuing upcalls. */
-    struct flow_miss *flow_miss;    /* This upcall's flow_miss. */
-
-    enum upcall_type type;          /* Classification. */
-
-    /* Raw upcall plus data for keeping track of the memory backing it. */
-    struct dpif_upcall dpif_upcall; /* As returned by dpif_recv() */
-    struct ofpbuf upcall_buf;       /* Owns some data in 'dpif_upcall'. */
-    uint64_t upcall_stub[512 / 8];  /* Buffer to reduce need for malloc(). */
-};
-
-struct upcall *upcall_next(struct udpif *);
-void upcall_destroy(struct upcall *);
-
 /* udpif figures out how to forward packets, and does forward them, but it
  * can't set up datapath flows on its own.  This interface passes packet
  * forwarding data from udpif to the higher level ofproto_dpif to allow the
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index a5b6814..b4a237c 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -2462,6 +2462,44 @@ xlate_out_copy(struct xlate_out *dst, const struct 
xlate_out *src)
     ofpbuf_put(&dst->odp_actions, src->odp_actions.data,
                src->odp_actions.size);
 }
+
+/* Returns a reference to the sflow handled associated with ofproto, or NULL if
+ * there is none.  The caller is responsible for decrementing the results ref
+ * count with dpif_sflow_unref(). */
+struct dpif_sflow *
+xlate_get_sflow(const struct ofproto_dpif *ofproto)
+{
+    struct dpif_sflow *sflow = NULL;
+    struct xbridge *xbridge;
+
+    ovs_rwlock_rdlock(&xlate_rwlock);
+    xbridge = xbridge_lookup(ofproto);
+    if (xbridge) {
+        sflow = dpif_sflow_ref(xbridge->sflow);
+    }
+    ovs_rwlock_unlock(&xlate_rwlock);
+
+    return sflow;
+}
+
+/* Returns a reference to the ipfix handled associated with ofproto, or NULL if
+ * there is none.  The caller is responsible for decrementing the results ref
+ * count with dpif_ipfix_unref(). */
+struct dpif_ipfix *
+xlate_get_ipfix(const struct ofproto_dpif *ofproto)
+{
+    struct xbridge *xbridge = NULL;
+    struct dpif_ipfix *ipfix;
+
+    ovs_rwlock_rdlock(&xlate_rwlock);
+    xbridge = xbridge_lookup(ofproto);
+    if (xbridge) {
+        ipfix = dpif_ipfix_ref(xbridge->ipfix);
+    }
+    ovs_rwlock_unlock(&xlate_rwlock);
+
+    return ipfix;
+}
 
 static struct skb_priority_to_dscp *
 get_skb_priority(const struct xport *xport, uint32_t skb_priority)
diff --git a/ofproto/ofproto-dpif-xlate.h b/ofproto/ofproto-dpif-xlate.h
index a54a9e4..8bd669b 100644
--- a/ofproto/ofproto-dpif-xlate.h
+++ b/ofproto/ofproto-dpif-xlate.h
@@ -154,4 +154,9 @@ void xlate_in_init(struct xlate_in *, struct ofproto_dpif *,
 void xlate_out_uninit(struct xlate_out *);
 void xlate_actions_for_side_effects(struct xlate_in *);
 void xlate_out_copy(struct xlate_out *dst, const struct xlate_out *src);
+
+struct dpif_sflow * xlate_get_sflow(const struct ofproto_dpif *)
+    OVS_EXCLUDED(xlate_rwlock);
+struct dpif_ipfix * xlate_get_ipfix(const struct ofproto_dpif *)
+    OVS_EXCLUDED(xlate_rwlock);
 #endif /* ofproto-dpif-xlate.h */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 80874b8..d4d0ad6 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3428,107 +3428,12 @@ handle_flow_misses(struct dpif_backer *backer, struct 
flow_miss_batch *fmb)
 }
 
 static void
-handle_sflow_upcall(struct dpif_backer *backer,
-                    const struct dpif_upcall *upcall)
-{
-    struct ofproto_dpif *ofproto;
-    union user_action_cookie cookie;
-    struct flow flow;
-    odp_port_t odp_in_port;
-
-    if (xlate_receive(backer, upcall->packet, upcall->key, upcall->key_len,
-                      &flow, NULL, &ofproto, &odp_in_port)
-        || !ofproto->sflow) {
-        return;
-    }
-
-    memset(&cookie, 0, sizeof cookie);
-    memcpy(&cookie, nl_attr_get(upcall->userdata), sizeof cookie.sflow);
-    dpif_sflow_received(ofproto->sflow, upcall->packet, &flow,
-                        odp_in_port, &cookie);
-}
-
-static void
-handle_flow_sample_upcall(struct dpif_backer *backer,
-                          const struct dpif_upcall *upcall)
-{
-    struct ofproto_dpif *ofproto;
-    union user_action_cookie cookie;
-    struct flow flow;
-
-    if (xlate_receive(backer, upcall->packet, upcall->key, upcall->key_len,
-                      &flow, NULL, &ofproto, NULL)
-        || !ofproto->ipfix) {
-        return;
-    }
-
-    memset(&cookie, 0, sizeof cookie);
-    memcpy(&cookie, nl_attr_get(upcall->userdata), sizeof cookie.flow_sample);
-
-    /* The flow reflects exactly the contents of the packet.  Sample
-     * the packet using it. */
-    dpif_ipfix_flow_sample(ofproto->ipfix, upcall->packet, &flow,
-                           cookie.flow_sample.collector_set_id,
-                           cookie.flow_sample.probability,
-                           cookie.flow_sample.obs_domain_id,
-                           cookie.flow_sample.obs_point_id);
-}
-
-static void
-handle_ipfix_upcall(struct dpif_backer *backer,
-                    const struct dpif_upcall *upcall)
-{
-    struct ofproto_dpif *ofproto;
-    struct flow flow;
-
-    if (xlate_receive(backer, upcall->packet, upcall->key, upcall->key_len,
-                      &flow, NULL, &ofproto, NULL)
-        || !ofproto->ipfix) {
-        return;
-    }
-
-    /* The flow reflects exactly the contents of the packet.  Sample
-     * the packet using it. */
-    dpif_ipfix_bridge_sample(ofproto->ipfix, upcall->packet, &flow);
-}
-
-static void
 handle_upcalls(struct dpif_backer *backer)
 {
     struct flow_miss_batch *fmb;
     int n_processed;
 
     for (n_processed = 0; n_processed < FLOW_MISS_MAX_BATCH; n_processed++) {
-        struct upcall *upcall = upcall_next(backer->udpif);
-
-        if (!upcall) {
-            break;
-        }
-
-        switch (upcall->type) {
-        case SFLOW_UPCALL:
-            handle_sflow_upcall(backer, &upcall->dpif_upcall);
-            break;
-
-        case FLOW_SAMPLE_UPCALL:
-            handle_flow_sample_upcall(backer, &upcall->dpif_upcall);
-            break;
-
-        case IPFIX_UPCALL:
-            handle_ipfix_upcall(backer, &upcall->dpif_upcall);
-            break;
-
-        case BAD_UPCALL:
-            break;
-
-        case MISS_UPCALL:
-            NOT_REACHED();
-        }
-
-        upcall_destroy(upcall);
-    }
-
-    for (n_processed = 0; n_processed < FLOW_MISS_MAX_BATCH; n_processed++) {
         struct drop_key *drop_key = drop_key_next(backer->udpif);
         if (!drop_key) {
             break;
-- 
1.7.9.5

_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to