[ovs-dev] [tests+nxm-ofctl 40/42] ofp-print: Implement printing for OFPUTIL_NXT_FLOW_MOD.

Justin Pettit jpettit at nicira.com
Tue Dec 7 02:50:41 UTC 2010


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.

>     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?

> +    if (fm.cr.priority != 32768 && verbosity >= 3) {

I think it would be clearer to use OFP_DEFAULT_PRIORITY instead of 32768.

> 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...

> 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.

Otherwise, it looks good.

--Justin






More information about the dev mailing list