[ovs-dev] [tests+nxm-ofctl 40/42] ofp-print: Implement printing for OFPUTIL_NXT_FLOW_MOD.
Ben Pfaff
blp at nicira.com
Tue Dec 7 21:22:33 UTC 2010
On Mon, Dec 06, 2010 at 06:50:41PM -0800, Justin Pettit wrote:
> On Nov 23, 2010, at 2:44 PM, Ben Pfaff wrote:
>
> > + error = ofputil_decode_flow_mod(&fm, oh, NXFF_OPENFLOW10);
>
> In the function ofputil_decode_flow_mod(), I think it would be useful
> to mention in the comment that the third element is only used when the
> message is not OFPUTIL_NXT_FLOW_MOD. It's a little confusing
> otherwise.
Thanks, I updated the comment to:
/* Converts an OFPT_FLOW_MOD or NXT_FLOW_MOD message 'oh' into an abstract
* flow_mod in 'fm'. Returns 0 if successful, otherwise an OpenFlow error
* code.
*
* For OFPT_FLOW_MOD messages, 'flow_format' should be the current flow format
* at the time when the message was received. Otherwise 'flow_format' is
* ignored.
*
* Does not validate the flow_mod actions. */
> > default:
> > - ds_put_format(string, "cmd:%d", ntohs(ofm->command));
> > + ds_put_format(s, "cmd%d", fm.command);
>
> Did you mean to remove the colon?
I don't remember. For now I put it back.
> > + if (fm.cr.priority != 32768 && verbosity >= 3) {
>
> I think it would be clearer to use OFP_DEFAULT_PRIORITY instead of 32768.
Done, thanks.
> > static void
> > +ofp_print_error(struct ds *string, int error)
> > +{
> > ...
> > + ds_put_format(string, " ***decode error type%d(%s) code%d(%s)***",
> > + type, lookup_error_type(type),
> > + code, lookup_error_code(type, code));
>
> I'm noticing a trend that you are likely dropping the colons on purpose...
I put them back.
> > static void
> > +ofp_print_nxt_tun_id_from_cookie(struct ds *string,
> > + const struct nxt_tun_id_cookie *ntic)
> > +{
> > + ds_put_format(string, " set=%"PRIu32, ntic->set);
> > +}
>
> The element "ntic->set" is only 8 bits, but you seem to be printing it
> as if it were 32 bits.
Oops. Thanks, fixed.
More information about the dev
mailing list