On Thu, Jul 19, 2012 at 11:28:47PM -0700, Ben Pfaff wrote: > OpenFlow headers are not as uniform as they could be, with size, alignment, > and numbering changes from one version to another and across varieties > (e.g. ordinary messages vs. "stats" messages). Until now the Open vSwitch > internal APIs haven't done a good job of abstracting those differences in > header formats. This commit changes that; from this commit forward very > little code actually needs to understand the header format or numbering. > Instead, it can just encode or decode, or pull or put, the header using > a more abstract API using the ofpraw_, ofptype_, and other APIs in the > new ofp-msgs module. > > Signed-off-by: Ben Pfaff <b...@nicira.com>
I have run into a lot of this code while porting my Open Flow 1.2 patches, which I estimate I am now about half way though. I don't have any objections to anything I see here. As discussed elsewhere[1] I think that lib/ofp-msgs.c needs to be slightly enhanced in order to handle OpenFlow 1.2, so assuming the changes you indicated you will roll into this patch are rolled in. Reviewed-by: Simon Horman <ho...@verge.net.au> [1] elsewhere: http://openvswitch.org/pipermail/dev/2012-July/019442.html "Re: [PATCH 05/24] ofp-util: Allow decoder to use OFPT11_STATS_{REQUEST, REPLY} for OpenFlow 1.2" > --- > v3: New commit. This isn't complete yet: in particular the program for > extracting the numbers from the ofp-msgs.h header file isn't written, > although there's a skeleton. That means that nothing has been tested or > carefully looked over, either. > > v3->v4: Almost ready. The ofp-msgs.c functions need some comments > and careful review. > > v4->v5: Ready for review. > > v5->v6: Rebased against master (more work than you might guess). > Fixed bugs in extract-ofp-msgs reported by Simon Horman. > --- > build-aux/extract-ofp-msgs | 351 +++++++++ > include/openflow/nicira-ext.h | 117 +--- > include/openflow/openflow-1.0.h | 74 +-- > include/openflow/openflow-1.1.h | 83 +-- > include/openflow/openflow-1.2.h | 23 +- > include/openflow/openflow-common.h | 75 +-- > lib/.gitignore | 1 + > lib/automake.mk | 9 + > lib/learning-switch.c | 128 ++-- > lib/ofp-errors.c | 41 +- > lib/ofp-errors.h | 4 +- > lib/ofp-msgs.c | 962 +++++++++++++++++++++++++ > lib/ofp-msgs.h | 433 ++++++++++++ > lib/ofp-print.c | 321 +++++---- > lib/ofp-util.c | 1372 > ++++++------------------------------ > lib/ofp-util.h | 134 +---- > lib/rconn.c | 72 ++- > lib/stream.c | 4 +- > lib/vconn.c | 68 +- > ofproto/connmgr.c | 24 +- > ofproto/ofproto.c | 228 +++---- > tests/ofp-print.at | 2 +- > tests/ofproto-dpif.at | 1 + > tests/ofproto-macros.at | 2 +- > tests/test-vconn.c | 68 +- > utilities/ovs-ofctl.c | 176 +++--- > 26 files changed, 2669 insertions(+), 2104 deletions(-) I know that breaking up is hard to do, but something shy of 5000 lines with a bonus 1000 lines of generated code is a big patch to review. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
