[ovs-dev] [PATCH 4/5] ofp-actions: encode OF12 output/set-field actions

Ben Pfaff blp at nicira.com
Thu Sep 13 05:10:09 UTC 2012


On Thu, Sep 13, 2012 at 01:37:08PM +0900, Simon Horman wrote:
> On Wed, Sep 12, 2012 at 09:09:49PM -0700, Ben Pfaff wrote:
> > On Thu, Sep 13, 2012 at 10:44:00AM +0900, Simon Horman wrote:
> > > On Wed, Sep 12, 2012 at 12:06:50PM -0700, Ben Pfaff wrote:
> > > > On Wed, Sep 12, 2012 at 05:44:31PM +0900, Simon Horman wrote:
> > > > > From: Isaku Yamahata <yamahata at valinux.co.jp>
> > > > > 
> > > > > Signed-off-by: Isaku Yamahata <yamahata at valinux.co.jp>
> > > > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > > > 
> > > > I think that, if I'm reading this right, then attempting to convert
> > > > many actions to OF1.2 will cause an assertion failure.  We need to do
> > > > something more graceful than that.
> > > 
> > > Is the problem the use of NOT_REACHED() in ofpact_to_openflow12()
> > > and ofpact_to_openflow12_common()?
> > > 
> > > If so, I wonder if the code could be re-factored to allow an error to be
> > > returned.
> > 
> > I don't think it's useful to report an error in response to a request to
> > dump the flow table.
> > 
> > > I also notice that ofpacts_put_openflow12_actions() isn't actually used,
> > > rather ofpacts_put_openflow11_actions is used for Open Flow 1.2, but that
> > > should be easy enough to fix.
> > 
> > OF1.2 actions are a superset of OF1.1 actions, so I'm not sure there's
> > any value in distinguishing for the purpose of outputting actions.
> 
> Ok, in that case I think I will keep the tests portion of this patch
> and drop the rest of it.

That sounds reasonable.  Thanks!



More information about the dev mailing list