On Mon, Jun 18, 2012 at 01:28:19PM +0900, Simon Horman wrote:
> On Thu, Jun 14, 2012 at 11:35:20PM -0700, Ben Pfaff wrote:
> > OpenFlow actions have always been somewhat awkward to handle.
> > Moreover, over time we've started creating actions that require more
> > complicated parsing.  When we maintain those actions internally in
> > their wire format, we end up parsing them multiple times, whenever
> > we have to look at the set of actions.
> > 
> > When we add support for OpenFlow 1.1 or later protocols, the situation
> > will get worse, because these newer protocols support many of the same
> > actions but with different representations.  It becomes unrealistic to
> > handle each protocol in its wire format.
> > 
> > This commit adopts a new strategy, by converting OpenFlow actions into
> > an internal form from the wire format when they are read, and converting
> > them back to the wire format when flows are dumped.  I believe that this
> > will be more maintainable over time.
> > 
> > Signed-off-by: Ben Pfaff <[email protected]>
> 
> > @@ -5235,7 +5226,7 @@ xlate_autopath(struct action_xlate_ctx *ctx,
> >              ofp_port = slave->up.ofp_port;
> >          }
> >      }
> > -    autopath_execute(naa, &ctx->flow, ofp_port);
> > +    nxm_reg_load(&ap->dst, ofp_port, &ctx->flow);
> >  }
> 
> GCC tells me that there are now cases where
> ofp_port is not set.

Thanks, it's right about that.

I folded in the following incremental, which brings the code closer to
what's on master:

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index bddb107..f2ea36f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -5212,10 +5212,9 @@ static void
 xlate_autopath(struct action_xlate_ctx *ctx,
                const struct ofpact_autopath *ap)
 {
-    struct ofport_dpif *port;
-    uint32_t ofp_port;
+    uint16_t ofp_port = ap->port;
+    struct ofport_dpif *port = get_ofp_port(ctx->ofproto, ofp_port);
 
-    port = get_ofp_port(ctx->ofproto, ap->port);
     if (!port || !port->bundle) {
         ofp_port = OFPP_NONE;
     } else if (port->bundle->bond) {
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to