[ovs-dev] [PATCH 1/2] nx-match: Use vl_mff_map to parse match field.

Joe Stringer joe at ovn.org
Wed Mar 8 17:56:06 UTC 2017


On 7 March 2017 at 09:39, Yi-Hung Wei <yihung.wei at gmail.com> wrote:
> vl_mff_map is introduced in patch (04f48a68c428 ofp-actions: Fix variable
> length meta-flow OXMs) to account variable length mf_field, and it is used

Usually we format this in the kernel patch style, ie 'commit
04f48a68c428 ("ofp-actions: Fix variable length meta-flow OXMs")'.

> to decode variable length mf_field in ofp_action. In this patch, vl_mff_map
> is further used to decode the variable length match field as well.
>
> Signed-off-by: Yi-Hung Wei <yihung.wei at gmail.com>
> ---

There's a few functions, for example ofputil_decode_packet_in(), which
this patch touches but it doesn't update the comment above the
function in the C file to specify whether the vl_mff_map is optional
or required, or explain the repercussions of specifying/not specifying
the option. Please ensure the comments are up to date for each of the
functions that have comments which have changed arguments due to this
patch.

One more minor comment below.

<snip>

> diff --git a/lib/nx-match.h b/lib/nx-match.h
> index 5dca24a01a49..411c67a288a5 100644
> --- a/lib/nx-match.h
> +++ b/lib/nx-match.h
> @@ -52,17 +52,18 @@ char *mf_parse_subfield(struct mf_subfield *, const char *s)
>  enum ofperr nx_pull_match(struct ofpbuf *, unsigned int match_len,
>                            struct match *,
>                            ovs_be64 *cookie, ovs_be64 *cookie_mask,
> -                          const struct tun_table *);
> +                          const struct tun_table *, const struct vl_mff_map *);
>  enum ofperr nx_pull_match_loose(struct ofpbuf *, unsigned int match_len,
>                                  struct match *, ovs_be64 *cookie,
>                                  ovs_be64 *cookie_mask,
> -                                const struct tun_table *);
> +                                const struct tun_table *,
> +                                const struct vl_mff_map *);
>  enum ofperr oxm_pull_match(struct ofpbuf *, const struct tun_table *,
> -                           struct match *);
> +                           const struct vl_mff_map*, struct match *);

The style guide calls for pointer arguments to be declared with a
space between the name and the asterisk. (Also the next declaration).
I sent a patch to add a check for this to checkpatch.py:
https://mail.openvswitch.org/pipermail/ovs-dev/2017-March/329510.html


More information about the dev mailing list