> On Oct 9, 2015, at 9:15 PM, Ben Pfaff <[email protected]> wrote:
>
> @@ -30,8 +30,10 @@ struct action_context {
> /* Input. */
> struct lexer *lexer; /* Lexer for pulling more tokens. */
> const struct shash *symtab; /* Symbol table. */
> - uint8_t next_table_id; /* OpenFlow table for 'next' to resubmit. */
> - uint8_t output_table_id; /* OpenFlow table for 'output' to resubmit.
> */
> + uint8_t first_table; /* First OpenFlow table. */
> + uint8_t n_tables; /* Number of OpenFlow tables. */
I think these two descriptions could be a bit misleading, since they define the
range of tables that a "next" action can jump to, but it sounds like it's the
full range of supported OpenFlow tables.
> + uint8_t cur_table; /* 0 <= cur_table < n_tables. */
This member name seems a little confusing, since all the other "_table" members
refer to an OpenFlow port, but this one is an offset. What about something
like "cur_table_offset"? Another option would be to relabel them things like
"*_of_table" (or "*_phy_table") and "*_log_table".
> +static void
> +parse_next_action(struct action_context *ctx)
> +{
> + if (!ctx->n_tables) {
> + action_error(ctx, "\"next\" action not allowed here.");
> + } else if (lexer_match(ctx->lexer, LEX_T_LPAREN)) {
> + int table;
Similar to above, this is actually a logical table or an offset to an OpenFlow
table. It might be nice to clarify that for later readers.
This is a nice addition, but I think some of the older phrasing related to
logical and physical tables was a bit clearer than exposing that they're
offsets.
Acked-by: Justin Pettit <[email protected]>
--Justin
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
