On Tue, Mar 11, 2014 at 04:56:20PM -0700, Andy Zhou wrote:
> Add basic recirculation infrastructure and user space
> data path support for it. The following bond mega flow patch will
> make use of this infrastructure.
>
> Signed-off-by: Andy Zhou <az...@nicira.com>
>
> ---
> v1->v2: Rewritten based on having post recirculation rules stored
> in table 254.
[snip]
> diff --git a/include/linux/openvswitch.h b/include/linux/openvswitch.h
> index d1ff5ec..af951f5 100644
> --- a/include/linux/openvswitch.h
> +++ b/include/linux/openvswitch.h
[snip]
> @@ -530,6 +531,36 @@ struct ovs_action_push_vlan {
> __be16 vlan_tci; /* 802.1Q TCI (VLAN ID and priority). */
> };
>
> +/* Recirculation ID needs to be unique per back-end.
> + * It can be any value except zero. Use the following
> + * defines to restrict the range further.
> + */
> +#define RECIRC_ID_BASE 300
> +#define RECIRC_ID_SIZE 65535
> +
> +/* Data path hash algorithm for computing Datapath hash.
> + *
> + * The Algorithm type only specifies the fields in a flow
> + * will be used as part of the hash. Each datapath is free
> + * to use its own hash algorithm. The hash value will be
> + * opaque to the user space daemon.
> + */
> +enum ovs_recirc_hash_alg {
> + OVS_RECIRC_HASH_ALG_NONE,
> + OVS_RECIRC_HASH_ALG_L4,
> +};
> +/*
> + * struct ovs_action_recirc - %OVS_ACTION_ATTR_RECIRC action argument.
> + * @recirc_id: The Recirculation label, Zero is invalid.
> + * @hash_alg: Algorithm used to compute hash prior to recirculation.
> + * @hash_bias: bias used for computing hash. used to compute hash prior to
> recirculation.
> + */
> +struct ovs_action_recirc {
> + uint8_t hash_alg; /* One of ovs_dp_hash_alg */
I believe that there is a 3 bytes hole here and that as the
contents of the hole is undefined it may cause comparisons
of struct ovs_action_recirc to fail.
In particular, I believe this may cause
ofpbuf_equal(&xout_actions, actions) to fail
in revalidate_ukey() sometimes.
I have observed this while developing my MPLS recirculation code
which is based on this patchset.
My suggestion is to add a zero field:
uint8_t zero[3];
And to initialise it to all zeros whenever a struct ovs_action_recirc is
populated. I believe at this time that means in the modification to
compose_output_action__() made in the 5th patch of this series.
> + __be32 hash_bias;
> + __be32 recirc_id; /* Recirculation label. */
> +};
> +
> /**
> * enum ovs_action_attr - Action types.
> *
[snip]
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
