On Mon, Mar 12, 2018 at 06:11:48PM -0700, Justin Pettit wrote: > > > On Feb 16, 2018, at 2:54 PM, Ben Pfaff <[email protected]> wrote: > > > > diff --git a/lib/ofp-protocol.c b/lib/ofp-protocol.c > > index 175318380f3b..32cf1c3fca40 100644 > > --- a/lib/ofp-protocol.c > > +++ b/lib/ofp-protocol.c > > ... > > +/* Returns an NXT_SET_FLOW_FORMAT message that can be used to set the flow > > + * format to 'nxff'. */ > > +struct ofpbuf * > > +ofputil_encode_nx_set_flow_format(enum ofputil_protocol protocol) > > +{ > > + struct ofpbuf *msg = ofpraw_alloc(OFPRAW_NXT_SET_FLOW_FORMAT, > > + OFP10_VERSION, 0); > > + ovs_be32 *nxff = ofpbuf_put_uninit(msg, sizeof *nxff); > > + if (protocol == OFPUTIL_P_OF10_STD) { > > + *nxff = htonl(NXFF_OPENFLOW10); > > + } else if (protocol == OFPUTIL_P_OF10_NXM) { > > + *nxff = htonl(NXFF_NXM); > > + } else { > > + OVS_NOT_REACHED(); > > + } > > + > > + return msg; > > +} > > The comment describing the function is a little strange, since it references > 'nxff' which is now just a local variable in the function. > > > +/* Decodes the NXT_SET_MOD_TABLE_ID message at 'oh'. Returns the message's > > + * argument, that is, whether the flow_mod_table_id feature should be > > + * enabled. */ > > +bool > > +ofputil_decode_nx_flow_mod_table_id(const struct ofp_header *oh) > > +{ > > + struct ofpbuf b = ofpbuf_const_initializer(oh, ntohs(oh->length)); > > + ovs_assert(ofpraw_pull_assert(&b) == OFPRAW_NXT_FLOW_MOD_TABLE_ID); > > + uint8_t *enable = ofpbuf_pull(&b, 8); > > + return *enable != 0; > > Should that function description be referencing "NXT_FLOW_MOD_TABLE_ID" > instead?
Thanks for looking this over carefully. I fixed both comments. > Acked-by: Justin Pettit <[email protected]> I'll apply this to master in a bit. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
