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
[email protected]
http://openvswitch.org/mailman/listinfo/dev
