[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