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

Ben Pfaff blp at nicira.com
Thu Oct 4 17:18:26 UTC 2012


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.)

Thanks,

Ben.



More information about the dev mailing list