Thanks for fix this up. It looks good.
I will apply it in a few minutes, with the fix of following style warning:
WARNING: braces {} are not necessary for single statement blocks
#53: FILE: datapath/flow_netlink.c:931:
+ if (match->key->tun_key.ipv4_dst) {
+ SW_FLOW_KEY_MEMSET_FIELD(match, tun_key, 0xff, true);
+ }
Any branches other than master need this fix?
On Wed, Jul 16, 2014 at 3:36 PM, Daniele Di Proietto
<[email protected]> wrote:
> If the userspace wants to match on a flow with some tunnel attributesset to 0,
> it simply omits them in the netlink attributes stream.
> Since our wildcarding logic (when megaflows are disabled) is based on the
> attributes in the netlink stream, we set our mask incorrectly.
>
> This commit adds a check to detect if the userspace wants to match on a
> tunnel,
> in which case we simply unwildcard the whole tun_key
>
> Reported-by: Andy Zhou <[email protected]>
> Signed-off-by: Daniele Di Proietto <[email protected]>
> ---
> v3:
> introducing SW_FLOW_KEY_MEMSET_FIELD() macro
> v2:
> using memset() instead of setting individual members, as suggested by Andy
> ---
> datapath/flow_netlink.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index 5f975a1..2d7b464 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -106,6 +106,20 @@ static void update_range__(struct sw_flow_match *match,
> SW_FLOW_KEY_MEMCPY_OFFSET(match, offsetof(struct sw_flow_key, field),
> \
> value_p, len, is_mask)
>
> +#define SW_FLOW_KEY_MEMSET_FIELD(match, field, value, is_mask) \
> + do { \
> + update_range__(match, offsetof(struct sw_flow_key, field), \
> + sizeof((match)->key->field), is_mask); \
> + if (is_mask) { \
> + if ((match)->mask) \
> + memset((u8 *)&(match)->mask->key.field,
> value,\
> + sizeof((match)->mask->key.field)); \
> + } else { \
> + memset((u8 *)&(match)->key->field, value, \
> + sizeof((match)->key->field)); \
> + } \
> + } while (0)
> +
> static bool match_validate(const struct sw_flow_match *match,
> u64 key_attrs, u64 mask_attrs)
> {
> @@ -912,6 +926,12 @@ int ovs_nla_get_match(struct sw_flow_match *match,
>
> mask_set_nlattr(newmask, 0xff);
>
> + /* The userspace does not send tunnel attributes that are 0,
> + * but we should not wildcard them nonetheless. */
> + if (match->key->tun_key.ipv4_dst) {
> + SW_FLOW_KEY_MEMSET_FIELD(match, tun_key, 0xff, true);
> + }
> +
> mask = newmask;
> }
>
> --
> 2.0.0
>
> _______________________________________________
> dev mailing list
> [email protected]
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev
