On Mon, Mar 04, 2013 at 10:56:18PM -0800, Justin Pettit wrote:
> For VLAN splinters, an "initial_tci" value was introduced that is passed
> around during flow processing to be used later for action translation.
> This commit switches to passing around a struct so that additional
> values beyond TCI can be used. A future commit will use this.
>
> Signed-off-by: Justin Pettit <[email protected]>
"git am" says:
Applying: ofproto-dpif: Make initial packet value handling generic.
/home/blp/nicira/ovs/.git/rebase-apply/patch:140: trailing whitespace.
action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
warning: 1 line adds whitespace errors.
The "*" in '*initial_vals->vlan_tci' is wrong I think:
> + * Optionally, if nonnull, sets '*initial_vals->vlan_tci' to the VLAN TCI
> + * with which the packet was really received, that is, the actual VLAN
> + * TCI extracted by odp_flow_key_to_flow(). (This differs from the
> + * value returned in flow->vlan_tci only for packets received on VLAN
> + * splinters.)
> *
> * Similarly, this function also includes some logic to help with tunnels.
> It
> * may modify 'flow' as necessary to make the tunneling implementation
The use of 'one_subfacet' here worries me. 'one_subfacet' can
theoretically not be in use at any given time. It would be better to
find a subfacet via the facet's list of subfacets:
> + action_xlate_ctx_init(&ctx, ofproto, &facet->flow,
> + &facet->one_subfacet.initial_vals, rule, 0, NULL);
> ctx.resubmit_stats = stats;
> xlate_actions_for_side_effects(&ctx, rule->up.ofpacts,
> rule->up.ofpacts_len);
Otherwise this looks good, thank you.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
