[ovs-dev] [PATCH 3/6] ofp-packet: Better abstract packet-in format.

[Ben Pfaff]

On Mon, Mar 12, 2018 at 06:22:06PM -0700, Justin Pettit wrote:
> 
> > On Feb 16, 2018, at 2:54 PM, Ben Pfaff <blp at ovn.org> wrote:
> > 
> > diff --git a/lib/ofp-print.c b/lib/ofp-print.c
> > index b13bc380386a..c0bfa92843c6 100644
> > --- a/lib/ofp-print.c
> > +++ b/lib/ofp-print.c
> > @@ -2287,18 +2287,15 @@ ofp_print_nxt_set_flow_format(struct ds *string, const struct ofp_header *oh)
> > 
> > static enum ofperr
> > ofp_print_nxt_set_packet_in_format(struct ds *string,
> > -                                   const struct nx_set_packet_in_format *nspf)
> > +                                   const struct ofp_header *oh)
> > {
> > -    uint32_t format = ntohl(nspf->format);
> > -
> > -    ds_put_cstr(string, " format=");
> > -    if (ofputil_packet_in_format_is_valid(format)) {
> > -        ds_put_cstr(string, ofputil_packet_in_format_to_string(format));
> > -    } else {
> > -        ds_put_format(string, "%"PRIu32, format);
> > +    enum ofputil_packet_in_format format;
> > +    enum ofperr error = ofputil_decode_set_packet_in_format(oh, &format);
> > +    if (!error) {
> > +        ds_put_format(string, " format=%s",
> > +                      ofputil_packet_in_format_to_string(format));
> >     }
> > +    return error;
> > }
> 
> Do you think it's worth printing some sort of error message if it's
> not a known protocol?  The previous version just printed the raw
> number.

We'll get two error messages: first, the log warn message from
ofputil_decode_set_packet_in_format(), second, the ofperr printed by the
caller when ofp_print_nxt_set_packet_in_format() returns an error.  I
don't think it's worth worrying otherwise about this corner case.

> Signed-off-by: Justin Pettit <jpettit at ovn.org>

Thanks, I'll take that as an "ack".