On Mon, Jul 23, 2012 at 05:08:39PM +0900, Isaku Yamahata wrote: > This patch adds instruction OF11 apply-actions/goto-table supports. > > Okay, I re-write the patch series by introducing > OFPACT_{CLEAR_ACTIONS, WRITE_ACTIONS, GOTO_TABLE}. > I did only compile-tested for now. If this approach is okay, I'll completes > the patch series and post it.
So, after having reviewed it, my major comment is that it appears that the initial patches introduce a bunch of stuff that could cause aborts at runtime due to NOT_REACHED(), and then the later patches fix those up one by one. I don't really like that kind of approach, because it means that the intermediate commits are buggy. I'd suggest one of two approaches: - Fold together all the commits from the point where the NOT_REACHED()s are introduced to the point where they are fixed. - Instead of breaking up patches by area (e.g. decoding, encoding, formatting, parsing), break them up by instruction (e.g. one patch for Clear-Actions, one for Goto-Table, and so on). Normally I'd insist on the latter but this stuff appears pretty straightforward so I don't think it's a big deal. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev