Thanks for review. Please find v2 in the following e-mail.

-Yi-Hung

On Wed, Mar 8, 2017 at 9:56 AM, Joe Stringer <[email protected]> wrote:

> On 7 March 2017 at 09:39, Yi-Hung Wei <[email protected]> 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 <[email protected]>
> > ---
>
> 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
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to