[ovs-dev] [tests+nxm-ofctl 41/42] ovs-ofctl: Add NXM support.
Justin Pettit
jpettit at nicira.com
Tue Dec 7 08:42:54 UTC 2010
On Nov 23, 2010, at 2:44 PM, Ben Pfaff wrote:
> +parse_ofp_str(struct flow_mod *fm, uint8_t *table_idx,
> + struct ofpbuf *actions, char *string)
> {
> ...
> + fm->cookie = htonl(0);
Not that the effect will be different, but I believe that should be a htonll(0).
> @@ -629,40 +640,41 @@ parse_ofp_str(struct parsed_flow *pf, struct ofpbuf *actions, char *string)
>
> /* Parses 'string' as an OFPT_FLOW_MOD with command 'command' (one of OFPFC_*)
> * and returns an ofpbuf that contains it. */
> +void
> +parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
> + char *string, uint16_t command)
> {
This function is no longer strictly limited to OFPT_FLOW_MOD, right?
> /* Parses an OFPT_FLOW_MOD with subtype OFPFC_ADD from 'stream' and returns an
> + * ofpbuf that contains it. Returns false if end-of-file is reached before
> + * reading a flow, otherwise true. */
> +bool
> +parse_ofp_add_flow_file(struct list *packets, enum nx_flow_format *cur,
> + FILE *stream)
> {
Same here.
> +/* Returns an OpenFlow message that can be used to set the flow format to
> + * 'flow_format'. */
> +struct ofpbuf *
> +ofputil_make_set_flow_format(enum nx_flow_format flow_format)
> +{
> ...
> + sff = make_nxmsg(sizeof *sff, NXT_SET_FLOW_FORMAT, &msg);
> + sff->format = flow_format;
Shouldn't that have a htonl() on it?
> --- a/utilities/ovs-ofctl.8.in
> +++ b/utilities/ovs-ofctl.8.in
> @@ -620,6 +620,37 @@ described in \fBFlow Syntax\fR, above.
> ...
> +.IP "\fBtun_id_from_cookie\fR"
> +This Nicira extension to OpenFlow adds minimal and limited support for
> +\fBtun_id\fR, but it does not support any other Nicira flow
> +extensions.
I hope we can retire this mode before too long. What do you think of specifically discouraging people from using it?
> +static bool
> +try_set_flow_format(struct vconn *vconn, enum nx_flow_format flow_format)
> +{
> + struct ofpbuf *sff, *reply;
> +
> + sff = ofputil_make_set_flow_format(NXFF_NXM);
Shouldn't that be "flow_format" instead of "NXFF_NXM"?
> static void
> +do_dump_flows__(int argc, char *argv[], bool aggregate)
> {
> ...
> + open_vconn(argv[1], &vconn);
> + flow_format = negotiate_highest_flow_format(vconn, &fsr.match);
> + request = ofputil_encode_flow_stats_request(&fsr, flow_format);
> dump_stats_transaction(argv[1], request);
> }
Should you be closing that vconn you opened?
Thanks!
--Justin
More information about the dev
mailing list