On 20 March 2018 at 13:46, Ben Pfaff <[email protected]> wrote:

> struct match has had a 'tun_md' member for a long time, but struct
> minimatch has never had one.  This doesn't matter for the purposes for
> which minimatch is currently used, but it means that a minimatch is not
> completely substitutable for a match and therefore blocks some new uses.
> This patch adds the member.
>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
>  include/openvswitch/match.h        |  1 +
>  include/openvswitch/tun-metadata.h |  5 +++++
>  lib/match.c                        |  7 ++++++-
>  lib/tun-metadata.c                 | 17 +++++++++++++++++
>  4 files changed, 29 insertions(+), 1 deletion(-)
>
> diff --git a/include/openvswitch/match.h b/include/openvswitch/match.h
> index 61a67de2c413..e5cf1a0d7c14 100644
> --- a/include/openvswitch/match.h
> +++ b/include/openvswitch/match.h
> @@ -242,6 +242,7 @@ struct minimatch {
>          };
>          struct miniflow *flows[2];
>      };
> +    struct tun_metadata_allocation *tun_md;
>  };
>
>  void minimatch_init(struct minimatch *, const struct match *);
> diff --git a/include/openvswitch/tun-metadata.h
> b/include/openvswitch/tun-metadata.h
> index 935c5c495027..dc0312ecb724 100644
> --- a/include/openvswitch/tun-metadata.h
> +++ b/include/openvswitch/tun-metadata.h
> @@ -101,6 +101,11 @@ struct tun_metadata_allocation {
>      bool valid;                 /* Set to true after any allocation
> occurs. */
>  };
>
> +struct tun_metadata_allocation *tun_metadata_allocation_clone(
> +    const struct tun_metadata_allocation *);
> +void tun_metadata_allocation_copy(struct tun_metadata_allocation *,
> +                                  const struct tun_metadata_allocation *);
> +
>  #ifdef __cplusplus
>  }
>  #endif
> diff --git a/lib/match.c b/lib/match.c
> index 97a5282997b0..3eab0fd5dec9 100644
> --- a/lib/match.c
> +++ b/lib/match.c
> @@ -1663,6 +1663,8 @@ minimatch_init(struct minimatch *dst, const struct
> match *src)
>      miniflow_alloc(dst->flows, 2, &tmp);
>      miniflow_init(dst->flow, &src->flow);
>      minimask_init(dst->mask, &src->wc);
> +
> +    dst->tun_md = tun_metadata_allocation_clone(&src->tun_md);
>  }
>
>  /* Initializes 'dst' as a copy of 'src'.  The caller must eventually free
> 'dst'
> @@ -1677,6 +1679,7 @@ minimatch_clone(struct minimatch *dst, const struct
> minimatch *src)
>             miniflow_get_values(src->flow), data_size);
>      memcpy(miniflow_values(&dst->mask->masks),
>             miniflow_get_values(&src->mask->masks), data_size);
> +    dst->tun_md = tun_metadata_allocation_clone(src->tun_md);
>  }
>
>  /* Initializes 'dst' with the data in 'src', destroying 'src'.  The
> caller must
> @@ -1686,6 +1689,7 @@ minimatch_move(struct minimatch *dst, struct
> minimatch *src)
>  {
>      dst->flow = src->flow;
>      dst->mask = src->mask;
> +    dst->tun_md = src->tun_md;
>  }
>
>  /* Frees any memory owned by 'match'.  Does not free the storage in which
> @@ -1694,6 +1698,7 @@ void
>  minimatch_destroy(struct minimatch *match)
>  {
>      free(match->flow);
> +    free(match->tun_md);
>  }
>
>  /* Initializes 'dst' as a copy of 'src'. */
> @@ -1702,7 +1707,7 @@ minimatch_expand(const struct minimatch *src, struct
> match *dst)
>  {
>      miniflow_expand(src->flow, &dst->flow);
>      minimask_expand(src->mask, &dst->wc);
> -    memset(&dst->tun_md, 0, sizeof dst->tun_md);
> +    tun_metadata_allocation_copy(&dst->tun_md, src->tun_md);
>  }
>
>  /* Returns true if 'a' and 'b' match the same packets, false otherwise.
> */
> diff --git a/lib/tun-metadata.c b/lib/tun-metadata.c
> index c0d9180e020e..f8a0e19524e9 100644
> --- a/lib/tun-metadata.c
> +++ b/lib/tun-metadata.c
> @@ -924,3 +924,20 @@ tun_metadata_match_format(struct ds *s, const struct
> match *match)
>          ds_put_char(s, ',');
>      }
>  }
> +
> +struct tun_metadata_allocation *
> +tun_metadata_allocation_clone(const struct tun_metadata_allocation *src)
> +{
> +    return src && src->valid ? xmemdup(src, sizeof *src) : NULL;
> +}
> +
> +void
> +tun_metadata_allocation_copy(struct tun_metadata_allocation *dst,
> +                             const struct tun_metadata_allocation *src)
> +{
> +    if (src && src->valid) {
> +        *dst = *src;
> +    } else {
> +        memset(dst, 0, sizeof *dst);
> +    }
> +}
> --
> 2.16.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

LGTM. I wonder if this and the related patches are worth considering for a
backport.

Thanks!

Reviewed-by: Armando Migliaccio <[email protected]>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to