[ovs-dev] [PATCH 02/11] ofp-msgs: Open Flow 1.1 and 1.2 Port Status Messages

Simon Horman horms at verge.net.au
Thu Oct 11 00:14:51 UTC 2012


On Wed, Oct 10, 2012 at 10:33:00AM -0700, Ben Pfaff wrote:
> On Sat, Oct 06, 2012 at 07:39:49PM +0900, Simon Horman wrote:
> > On Thu, Oct 04, 2012 at 10:18:26AM -0700, Ben Pfaff wrote:
> > > On Thu, Sep 20, 2012 at 01:10:41PM +0900, Simon Horman wrote:
> > > > This allows for encoding and decoding Open Flow 1.1 and 1.2 Port Stats
> > > > Request and Reply message
> > > > 
> > > > Signed-off-by: Simon Horman <horms at verge.net.au>
> > > 
> > > I see a few careless errors here.  print_port_stat() should have its
> > > return type on a separate line.  ofputil_parse_key_value() shouldn't
> > > be changed at all.  ofputlil_dump_ports() has a typo in its name.  So
> > > does ofptutil_decode_one_port_reply().  So does
> > > ofptutil_decode_port_stats_request().
> > > 
> > > I see some incorrect uses of assert, that could fire in production if
> > > an unsupported OF1.1 port number is used.  They would also drop the
> > > port numbers from the output if NDEBUG was defined.
> > > 
> > > I'm not really fond of the use of ofp11_port_stats as the common
> > > format.  I think it would be better to define a host-byte-order
> > > structure.  It could have the same fields as ofp_port_stats, or it
> > > could have a uint16_t port number and a "struct netdev_stats".  (The
> > > latter would be convenient for ofproto.)
> > 
> > Hi Ben,
> > 
> > I believe that the revised patch below addresses the concerns that
> > you raised above. I have taken the uint16_t + struct netdev_stats
> > approach that you suggested.
> 
> I made some changes and applied to master as follows.

Thanks. I'll use this as the basis for a reworked
queue stats patch.



More information about the dev mailing list