[ovs-dev] [PATCH 1/4] ofproto-dpif: Make initial packet value handling generic.

Ben Pfaff blp at nicira.com
Thu Feb 14 19:07:25 UTC 2013


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 <jpettit at 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.



More information about the dev mailing list