On Thu, Sep 13, 2012 at 08:00:20AM -0700, Ben Pfaff wrote: > On Thu, Sep 13, 2012 at 02:47:09PM +0900, Simon Horman wrote: > > On Tue, Sep 11, 2012 at 09:55:17PM -0700, Ben Pfaff wrote: > > > On Mon, Sep 10, 2012 at 10:47:52AM -0700, Ben Pfaff wrote: > > > > On Mon, Sep 10, 2012 at 09:59:28AM +0900, Simon Horman wrote: > > > > > On Sat, Sep 08, 2012 at 12:15:53PM +0900, Simon Horman wrote: > > > > > > On Fri, Sep 07, 2012 at 10:33:00AM -0700, Ben Pfaff wrote: > > > > > > > On Fri, Aug 31, 2012 at 05:42:48PM +0900, Simon Horman wrote: > > > > > > > > On Thu, Aug 30, 2012 at 10:02:51AM +0900, Simon Horman wrote: > > > > > > > > > On Wed, Aug 29, 2012 at 10:07:55AM -0700, Ben Pfaff wrote: > > > > > > > > > > On Tue, Aug 21, 2012 at 01:55:36PM +0900, Simon Horman > > > > > > > > > > wrote: > > > > > > > > > > > Signed-off-by: Simon Horman <[email protected]> > > > > > > > > > > > > > > > > > > > > I'm not really happy with this approach. It seems fairly > > > > > > > > > > awkward. I > > > > > > > > > > would much rather define some "struct ofputil_*" that has a > > > > > > > > > > superset of > > > > > > > > > > the 1.[012] table stats and define converter functions. > > > > > > > > > > > > > > > > > > Sure, I'll see what I can do. > > > > > > > > > I do agree that the approach I have taken is somewhat awkward. > > > > > > > > > > > > > > > > is this closer to what you would like? > > > > > > > > > > > > > > Much closer. I think that if you move the conversion code from > > > > > > > ofproto.c to single-purpose functions in lib/ofp-util.c then it's > > > > > > > what > > > > > > > I had in mind. > > > > > > > > > > > > Sure, I'll make that change and see how it looks. > > > > > > > > > > > > > (I did mean to actually invent a new struct for that, > > > > > > > but it seems like the OF1.2 version is good enough for now.) > > > > > > > > > > > > (me too) > > > > > > > > > > Hi Ben, > > > > > > > > > > The patch below attempts to implement your suggestion. > > > > > > > > > > I have created ofputil function to both convert to Open Flow version > > > > > specific table stats structures, and create Open Flow version specific > > > > > table stats reply messages. This much seems clean. Going beyond that > > > > > seemed to get a bit messy. > > > > > > > > Thanks. It's moving in the right direction. I'm working on massaging > > > > it a little more toward further abstraction. (If you have time for > > > > anything else in the series, please don't let that hold you up.) > > > > > > Here's the version I'm proposing. Please review. > > > > > > --8<--------------------------cut here-------------------------->8-- > > > > > > From: Simon Horman <[email protected]> > > > Date: Tue, 11 Sep 2012 21:47:27 -0700 > > > Subject: [PATCH] ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table > > > Statistics Request Messages > > > > > > Co-authored-by: Ben Pfaff <[email protected]> > > > Signed-off-by: Simon Horman <[email protected]> > > > Signed-off-by: Ben Pfaff <[email protected]> > > > > Reviewed-by: Simon Horman <[email protected]> > > Thanks. > > The chain of authorship was confusing, so I ended up with: > > commit 307975da7111cc223a9e2b859a0b12532fa4f2c8 > Author: Simon Horman <[email protected]> > Date: Tue Sep 11 21:47:27 2012 -0700 > > ofp-util: Allow decoding of Open Flow 1.1 & 1.2 Table Statistics Request > Mes > > Signed-off-by: Simon Horman <[email protected]> > [[email protected] then made substantial changes that were then:] > Reviewed-by: Simon Horman <[email protected]> > Signed-off-by: Ben Pfaff <[email protected]> > > and applied to master in that form.
Thanks, that works for me. I'm very happy to get this one merged. _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
