On Wed, Feb 13, 2013 at 03:31:42PM -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 <jpet...@nicira.com>
It seems reasonable.
"sparse" complains. I think the new 'vlan_tci' member of initial_vals
should be an ovs_be16.
The comment on 'vlan_tci' here:
> +/* Initial values of fields of the packet that may be changed during
> + * flow processing and needed later. */
> +struct initial_vals {
> + /* This value is normally the same as ->facet->flow.vlan_tci. Only VLAN
> + * splinters can cause it to differ. This value should be removed when
> + * the VLAN splinters feature is no longer needed. */
> + uint16_t vlan_tci;
> +};
has the annoying property that it sort of dances around what the value
really is. How about this for the comment instead:
/* This is the value of vlan_tci in the packet as actually received from
* dpif. This is the same as the facet's flow.vlan_tci unless the packet
* was received via a VLAN splinter. In that case, this value is 0
* (because the packet as actually received from the dpif had no 802.1Q
* tag) but the facet's flow.vlan_tci is set to the VLAN that the splinter
* represents.
*
* This member should be removed when the VLAN splinters feature is no
* longer needed. */
I think that's about as clear as I can make it.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
