[ovs-dev] [PATCH 1/2] nx-match: Use vl_mff_map to parse match field.
Yi-Hung Wei
yihung.wei at gmail.com
Wed Mar 8 19:21:49 UTC 2017
Thanks for review. Please find v2 in the following e-mail.
-Yi-Hung
On Wed, Mar 8, 2017 at 9:56 AM, Joe Stringer <joe at ovn.org> wrote:
> 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