ofproto-dpif: Refactor checking for in-band special case.

The comments on in_band_rule_check() were more or less wrong (the return
value was no longer used to determine whether a flow could be set up).
This commit fixes the comments and refactors the interface to make better
sense in the current context.

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Ethan Jackson <ethan@nicira.com>

diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 0746bab..03d4661 100644 (file)
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011, 2012 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2012, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -222,31 +222,16 @@ refresh_local(struct in_band *ib)
     return true;
 }
 
-/* Returns true if the rule that would match 'flow' with 'actions' is
- * allowed to be set up in the datapath. */
+/* Returns true if packets in 'flow' should be directed to the local port.
+ * (This keeps the flow table from preventing DHCP replies from being seen by
+ * the local port.) */
 bool
-in_band_rule_check(const struct flow *flow, odp_port_t local_odp_port,
-                   const struct nlattr *actions, size_t actions_len)
+in_band_must_output_to_local_port(const struct flow *flow)
 {
-    /* Don't allow flows that would prevent DHCP replies from being seen
-     * by the local port. */
-    if (flow->dl_type == htons(ETH_TYPE_IP)
+    return (flow->dl_type == htons(ETH_TYPE_IP)
             && flow->nw_proto == IPPROTO_UDP
             && flow->tp_src == htons(DHCP_SERVER_PORT)
-            && flow->tp_dst == htons(DHCP_CLIENT_PORT)) {
-        const struct nlattr *a;
-        unsigned int left;
-
-        NL_ATTR_FOR_EACH_UNSAFE (a, left, actions, actions_len) {
-            if (nl_attr_type(a) == OVS_ACTION_ATTR_OUTPUT
-                && nl_attr_get_odp_port(a) == local_odp_port) {
-                return true;
-            }
-        }
-        return false;
-    }
-
-    return true;
+            && flow->tp_dst == htons(DHCP_CLIENT_PORT));
 }
 
 static void

diff --git a/ofproto/in-band.h b/ofproto/in-band.h
index 5449312..ad16dc2 100644 (file)
--- a/ofproto/in-band.h
+++ b/ofproto/in-band.h
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2008, 2009, 2010, 2011 Nicira, Inc.
+ * Copyright (c) 2008, 2009, 2010, 2011, 2013 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -40,7 +40,6 @@ void in_band_set_remotes(struct in_band *,
 bool in_band_run(struct in_band *);
 void in_band_wait(struct in_band *);
 
-bool in_band_rule_check(const struct flow *, odp_port_t local_odp_port,
-                        const struct nlattr *odp_actions, size_t actions_len);
+bool in_band_must_output_to_local_port(const struct flow *);
 
 #endif /* in-band.h */

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 852afdb..8d72876 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1839,6 +1839,22 @@ xlate_out_copy(struct xlate_out *dst, const struct xlate_out *src)
                src->odp_actions.size);
 }
 \f
+static bool
+actions_output_to_local_port(const struct xlate_ctx *ctx)
+{
+    odp_port_t local_odp_port = ofp_port_to_odp_port(ctx->ofproto, OFPP_LOCAL);
+    const struct nlattr *a;
+    unsigned int left;
+
+    NL_ATTR_FOR_EACH_UNSAFE (a, left, ctx->xout->odp_actions.data,
+                             ctx->xout->odp_actions.size) {
+        if (nl_attr_type(a) == OVS_ACTION_ATTR_OUTPUT
+            && nl_attr_get_odp_port(a) == local_odp_port) {
+            return true;
+        }
+    }
+    return false;
+}
 
 /* Translates the 'ofpacts_len' bytes of "struct ofpacts" starting at 'ofpacts'
  * into datapath actions in 'odp_actions', using 'ctx'. */
@@ -1972,7 +1988,6 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
     } else {
         static struct vlog_rate_limit trace_rl = VLOG_RATE_LIMIT_INIT(1, 1);
         size_t sample_actions_len;
-        odp_port_t local_odp_port;
 
         if (flow->in_port.ofp_port
             != vsp_realdev_to_vlandev(ctx.ofproto, flow->in_port.ofp_port,
@@ -2009,11 +2024,9 @@ xlate_actions(struct xlate_in *xin, struct xlate_out *xout)
             }
         }
 
-        local_odp_port = ofp_port_to_odp_port(ctx.ofproto, OFPP_LOCAL);
         if (connmgr_has_in_band(ctx.ofproto->up.connmgr)
-            && !in_band_rule_check(flow, local_odp_port,
-                                   ctx.xout->odp_actions.data,
-                                   ctx.xout->odp_actions.size)) {
+            && in_band_must_output_to_local_port(flow)
+            && !actions_output_to_local_port(&ctx)) {
             compose_output_action(&ctx, OFPP_LOCAL);
         }
         if (ctx.ofproto->has_mirrors) {