[ovs-dev] [PATCH] vswitchd: Process special packets more aggressively.

Ethan Jackson ethan at nicira.com
Fri Feb 4 23:02:37 UTC 2011


Before this patch, special packets such as LACP and CFM messages
were only processed if they had NORMAL open flow actions.  With
this patch these messages are always processed unless originated in
ofproto_send_packet().
---
 ofproto/ofproto.c |   26 +++++++++++++++++++++++++-
 ofproto/ofproto.h |    2 ++
 vswitchd/bridge.c |   23 +++++++++++++++++++----
 3 files changed, 46 insertions(+), 5 deletions(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index c980f69..07e89a3 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -124,6 +124,11 @@ struct action_xlate_ctx {
      * calling action_xlate_ctx_init(). */
     void (*resubmit_hook)(struct action_xlate_ctx *, const struct rule *);
 
+    /* If true, the admissibility of 'flow' should be checked before executing
+     * its actions.  If flow is not admissible it will be rendered
+     * uninstallable and no actions will be executed. */
+    bool check_admissible;
+
 /* xlate_actions() initializes and uses these members.  The client might want
  * to look at them after it returns. */
 
@@ -1445,6 +1450,8 @@ ofproto_send_packet(struct ofproto *p, const struct flow *flow,
     struct ofpbuf *odp_actions;
 
     action_xlate_ctx_init(&ctx, p, flow, packet);
+    /* Always xlate packets originated in this function. */
+    ctx.check_admissible = false;
     odp_actions = xlate_actions(&ctx, actions, n_actions);
 
     /* XXX Should we translate the dpif_execute() errno value into an OpenFlow
@@ -3109,6 +3116,7 @@ action_xlate_ctx_init(struct action_xlate_ctx *ctx,
     ctx->flow = *flow;
     ctx->packet = packet;
     ctx->resubmit_hook = NULL;
+    ctx->check_admissible = true;
 }
 
 static struct ofpbuf *
@@ -3123,7 +3131,16 @@ xlate_actions(struct action_xlate_ctx *ctx,
     ctx->nf_output_iface = NF_OUT_DROP;
     ctx->recurse = 0;
     ctx->last_pop_priority = -1;
-    do_xlate_actions(in, n_in, ctx);
+
+    if (!ctx->check_admissible
+        || (ctx->ofproto->ofhooks->admissible_cb
+            && ctx->ofproto->ofhooks->admissible_cb(&ctx->flow, ctx->packet,
+                                                    ctx->ofproto->aux))) {
+        do_xlate_actions(in, n_in, ctx);
+    } else {
+        ctx->may_set_up_flow = false;
+    }
+
     remove_pop_action(ctx);
 
     /* Check with in-band control to see if we're allowed to set up this
@@ -4367,6 +4384,12 @@ handle_miss_upcall(struct ofproto *p, struct dpif_upcall *upcall)
     /* Set header pointers in 'flow'. */
     flow_extract(upcall->packet, flow.tun_id, flow.in_port, &flow);
 
+    if (p->ofhooks->admissible_cb
+        && !p->ofhooks->admissible_cb(&flow, upcall->packet, p->aux)) {
+        ofpbuf_delete(upcall->packet);
+        return;
+    }
+
     /* Check with in-band control to see if this packet should be sent
      * to the local port regardless of the flow table. */
     if (in_band_msg_in_hook(p->in_band, &flow, upcall->packet)) {
@@ -5123,5 +5146,6 @@ default_normal_ofhook_cb(const struct flow *flow, const struct ofpbuf *packet,
 static const struct ofhooks default_ofhooks = {
     default_normal_ofhook_cb,
     NULL,
+    NULL,
     NULL
 };
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 8e4e2a6..cedd4e1 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -149,6 +149,8 @@ struct ofhooks {
     bool (*normal_cb)(const struct flow *, const struct ofpbuf *packet,
                       struct ofpbuf *odp_actions, tag_type *,
                       uint16_t *nf_output_iface, void *aux);
+    bool (*admissible_cb)(const struct flow *flow, const struct ofpbuf *packet,
+                          void *aux);
     void (*account_flow_cb)(const struct flow *, tag_type tags,
                             const struct nlattr *odp_actions,
                             size_t actions_len,
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f786108..d35eadd 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -75,6 +75,8 @@ VLOG_DEFINE_THIS_MODULE(bridge);
 
 COVERAGE_DEFINE(bridge_flush);
 COVERAGE_DEFINE(bridge_process_flow);
+COVERAGE_DEFINE(bridge_process_cfm);
+COVERAGE_DEFINE(bridge_process_lacp);
 COVERAGE_DEFINE(bridge_reconfigure);
 
 enum lacp_status {
@@ -3000,26 +3002,38 @@ bridge_normal_ofhook_cb(const struct flow *flow, const struct ofpbuf *packet,
                         struct ofpbuf *actions, tag_type *tags,
                         uint16_t *nf_output_iface, void *br_)
 {
-    struct iface *iface;
     struct bridge *br = br_;
 
     COVERAGE_INC(bridge_process_flow);
+    return process_flow(br, flow, packet, actions, tags, nf_output_iface);
+}
+
+static bool
+bridge_admissible_ofhook_cb(const struct flow *flow,
+                            const struct ofpbuf *packet, void *br_)
+{
+    struct iface *iface;
+    struct bridge *br = br_;
 
     iface = iface_from_dp_ifidx(br, flow->in_port);
 
     if (cfm_should_process_flow(flow)) {
-        if (packet && iface->cfm) {
+
+        if (iface && packet && iface->cfm) {
+            COVERAGE_INC(bridge_process_cfm);
             cfm_process_heartbeat(iface->cfm, packet);
         }
         return false;
     } else if (flow->dl_type == htons(ETH_TYPE_LACP)) {
-        if (packet) {
+
+        if (iface && packet) {
+            COVERAGE_INC(bridge_process_lacp);
             lacp_process_packet(packet, iface);
         }
         return false;
     }
 
-    return process_flow(br, flow, packet, actions, tags, nf_output_iface);
+    return true;
 }
 
 static void
@@ -3089,6 +3103,7 @@ bridge_account_checkpoint_ofhook_cb(void *br_)
 
 static struct ofhooks bridge_ofhooks = {
     bridge_normal_ofhook_cb,
+    bridge_admissible_ofhook_cb,
     bridge_account_flow_ofhook_cb,
     bridge_account_checkpoint_ofhook_cb,
 };
-- 
1.7.4





More information about the dev mailing list