[ovs-dev] [PATCH] ofp-parse: Fix parsing of "resubmit" action.

Ben Pfaff blp at nicira.com
Fri Jul 20 03:30:13 UTC 2012


On Thu, Jul 19, 2012 at 11:19:13AM -0700, Ben Pfaff wrote:
> On Thu, Jul 19, 2012 at 10:28:03AM -0700, Justin Pettit wrote:
> > 
> > On Jul 19, 2012, at 10:04 AM, Ben Pfaff wrote:
> > 
> > > On Thu, Jul 19, 2012 at 12:45:19AM -0700, Justin Pettit wrote:
> > >> The new abstraction introduced in commit f25d0cf (Introduce ofpacts, an
> > >> abstraction of OpenFlow actions.) provides a mechanism for storing the
> > >> original action type for when converting from the internal representation
> > >> to a public one may be ambiguous.  The "resubmit" action must
> > >> distinguish between OFPUTIL_NXAST_RESUBMIT_TABLE and
> > >> OFPUTIL_NXAST_RESUBMIT, so the parsing function needs to properly fill
> > >> out the "compat" member of the ofpact structure.
> > >> 
> > >> Bug #8899
> > >> Reported-by: Luca Giraudo <lgiraudo at nicira.com>
> > >> Signed-off-by: Justin Pettit <jpettit at nicira.com>
> > > 
> > > Good catch.
> > > 
> > > Hmm.  I hadn't considered this issue when I wrote the ofpacts code.  If
> > > I understand what's going on, it's essentially that ofpacts that differ
> > > bitwise can yield the same representation as a string (or as OpenFlow
> > > 1.0 actions).  That's going to continue to come up, even if we paper
> > > over it in this one case.
> > > 
> > > So, I'd suggest that, instead of comparing ofpacts bytewise, we compare
> > > some other representation.  We could, for example, format the old rule
> > > and the new rule as strings, then if they're the same just don't print
> > > the difference (it's false).  This would avoid printing false
> > > differences too.
> > 
> > Yes, I think this is a better approach and more future-proof as new actions get added.
> > 
> > > Do you have a test case we could add to the unit tests?
> > 
> > No, I don't have one written up beyond what Luca described in the bug report.
> 
> OK, I wrote one.  I'm glad I did since the patch I posted didn't
> work.  Here's a fixed version for final review.

I want to make sure you noticed that I'm waiting for a review of this
version.



More information about the dev mailing list