[ovs-dev] [bug 4566 2/2] ovs-ofctl: Implement documented semantics of --flow-format for flow_mods.
Justin Pettit
jpettit at nicira.com
Mon Feb 28 23:19:08 UTC 2011
Looks good.
--Justin
On Feb 22, 2011, at 1:44 PM, Ben Pfaff wrote:
> Also adds a test and moves some code around in tests/ to make sure that
> OFPROTO_START and OFPROTO_STOP are available in tests/ovs-ofctl.at.
>
> Reported-by: Michael Mao <mmao at nicira.com>
> Bug #4566.
> ---
> tests/automake.mk | 1 +
> tests/ofproto-macros.at | 16 ++++++++++++++++
> tests/ofproto.at | 17 -----------------
> tests/ovs-ofctl.at | 22 ++++++++++++++++++++++
> tests/testsuite.at | 3 ++-
> utilities/ovs-ofctl.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
> 6 files changed, 83 insertions(+), 20 deletions(-)
> create mode 100644 tests/ofproto-macros.at
>
> diff --git a/tests/automake.mk b/tests/automake.mk
> index 09ec23f..e2d3388 100644
> --- a/tests/automake.mk
> +++ b/tests/automake.mk
> @@ -25,6 +25,7 @@ TESTSUITE_AT = \
> tests/timeval.at \
> tests/lockfile.at \
> tests/reconnect.at \
> + tests/ofproto-macros.at \
> tests/ofproto.at \
> tests/ovsdb.at \
> tests/ovsdb-log.at \
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> new file mode 100644
> index 0000000..06f9b65
> --- /dev/null
> +++ b/tests/ofproto-macros.at
> @@ -0,0 +1,16 @@
> +m4_define([STRIP_XIDS], [[sed 's/ (xid=0x[0-9a-fA-F]*)//']])
> +m4_define([STRIP_DURATION], [[sed 's/\bduration=[0-9.]*s/duration=?s/']])
> +
> +m4_define([OFPROTO_START],
> + [OVS_RUNDIR=$PWD; export OVS_RUNDIR
> + OVS_LOGDIR=$PWD; export OVS_LOGDIR
> + trap 'kill `cat ovs-openflowd.pid`' 0
> + AT_CAPTURE_FILE([ovs-openflowd.log])
> + AT_CHECK(
> + [ovs-openflowd --detach --pidfile --enable-dummy --log-file dummy at br0 none --datapath-id=fedcba9876543210 $1],
> + [0], [ignore], [ignore])
> +])
> +
> +m4_define([OFPROTO_STOP],
> + [AT_CHECK([ovs-appctl -t ovs-openflowd exit])
> + trap '' 0])
> diff --git a/tests/ofproto.at b/tests/ofproto.at
> index a7dda06..f266f34 100644
> --- a/tests/ofproto.at
> +++ b/tests/ofproto.at
> @@ -1,22 +1,5 @@
> AT_BANNER([ofproto])
>
> -m4_define([STRIP_XIDS], [[sed 's/ (xid=0x[0-9a-fA-F]*)//']])
> -m4_define([STRIP_DURATION], [[sed 's/\bduration=[0-9.]*s/duration=?s/']])
> -
> -m4_define([OFPROTO_START],
> - [OVS_RUNDIR=$PWD; export OVS_RUNDIR
> - OVS_LOGDIR=$PWD; export OVS_LOGDIR
> - trap 'kill `cat ovs-openflowd.pid`' 0
> - AT_CAPTURE_FILE([ovs-openflowd.log])
> - AT_CHECK(
> - [ovs-openflowd --detach --pidfile --enable-dummy --log-file dummy at br0 none --datapath-id=fedcba9876543210 $1],
> - [0], [ignore], [ignore])
> -])
> -
> -m4_define([OFPROTO_STOP],
> - [AT_CHECK([ovs-appctl -t ovs-openflowd exit])
> - trap '' 0])
> -
> AT_SETUP([ofproto - echo request])
> OFPROTO_START
> AT_CHECK([ovs-ofctl -vANY:ANY:WARN probe br0])
> diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at
> index 111ed88..dfdebd3 100644
> --- a/tests/ovs-ofctl.at
> +++ b/tests/ovs-ofctl.at
> @@ -464,3 +464,25 @@ nx_pull_match() returned error 44010101
> nx_pull_match() returned error 44010101
> ])
> AT_CLEANUP
> +
> +dnl Check that "-F openflow10" rejects a flow_mod with a tun_id, since
> +dnl OpenFlow 1.0 doesn't support tunnels.
> +AT_SETUP([ovs-ofctl -F option and tun_id])
> +AT_CHECK([ovs-ofctl -F openflow10 add-flow dummy tun_id=123,actions=drop],
> + [1], [], [ovs-ofctl: flow cannot be expressed in flow format openflow10 (flow format tun_id_from_cookie or better is required)
> +])
> +AT_CLEANUP
> +
> +dnl Check that "-F nxm" really forces add-flow to use the NXM flow format.
> +dnl (If it doesn't, then either the tun_id won't show up at all, or it will
> +dnl additionally show up as the top 32 bits of the cookie.) This checks
> +dnl for regression against bug #4566.
> +AT_SETUP([ovs-ofctl -F option with flow_mods])
> +OFPROTO_START
> +AT_CHECK([ovs-ofctl -F nxm add-flow br0 tun_id=0x12345678,actions=drop])
> +AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION], [0], [dnl
> +NXST_FLOW reply:
> + cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, tun_id=0x12345678 actions=drop
> +])
> +OFPROTO_STOP
> +AT_CLEANUP
> diff --git a/tests/testsuite.at b/tests/testsuite.at
> index d41f504..5625f8b 100644
> --- a/tests/testsuite.at
> +++ b/tests/testsuite.at
> @@ -1,6 +1,6 @@
> AT_INIT
>
> -AT_COPYRIGHT([Copyright (c) 2009, 2010 Nicira Networks.
> +AT_COPYRIGHT([Copyright (c) 2009, 2010, 2011 Nicira Networks.
>
> Licensed under the Apache License, Version 2.0 (the "License");
> you may not use this file except in compliance with the License.
> @@ -36,6 +36,7 @@ m4_define([OVS_WAIT_UNTIL], [OVS_WAIT([if $1; then exit 0; fi], [$2])])
> m4_define([OVS_WAIT_WHILE], [OVS_WAIT([if $1; then :; else exit 0; fi], [$2])])
>
> m4_include([tests/ovsdb-macros.at])
> +m4_include([tests/ofproto-macros.at])
>
> m4_include([tests/library.at])
> m4_include([tests/classifier.at])
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 68f2eda..d6bacd6 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -622,6 +622,43 @@ do_queue_stats(int argc, char *argv[])
> dump_stats_transaction(argv[1], request);
> }
>
> +/* Sets up the flow format for a vconn that will be used to modify the flow
> + * table. Returns the flow format used, after possibly adding an OpenFlow
> + * request to 'requests'.
> + *
> + * If 'preferred_flow_format' is -1, returns NXFF_OPENFLOW10 without modifying
> + * 'requests', since NXFF_OPENFLOW10 is the default flow format for any
> + * OpenFlow connection.
> + *
> + * If 'preferred_flow_format' is a specific format, adds a request to set that
> + * format to 'requests' and returns the format. */
> +static enum nx_flow_format
> +set_initial_format_for_flow_mod(struct list *requests)
> +{
> + if (preferred_flow_format < 0) {
> + return NXFF_OPENFLOW10;
> + } else {
> + struct ofpbuf *sff;
> +
> + sff = ofputil_make_set_flow_format(preferred_flow_format);
> + list_push_back(requests, &sff->list_node);
> + return preferred_flow_format;
> + }
> +}
> +
> +/* Checks that 'flow_format' is acceptable as a flow format after a flow_mod
> + * operation, given the global 'preferred_flow_format'. */
> +static void
> +check_final_format_for_flow_mod(enum nx_flow_format flow_format)
> +{
> + if (preferred_flow_format >= 0 && flow_format != preferred_flow_format) {
> + ovs_fatal(0, "flow cannot be expressed in flow format %s "
> + "(flow format %s or better is required)",
> + ofputil_flow_format_to_string(preferred_flow_format),
> + ofputil_flow_format_to_string(flow_format));
> + }
> +}
> +
> static void
> do_flow_mod__(int argc OVS_UNUSED, char *argv[], uint16_t command)
> {
> @@ -630,9 +667,11 @@ do_flow_mod__(int argc OVS_UNUSED, char *argv[], uint16_t command)
> struct vconn *vconn;
>
> list_init(&requests);
> - flow_format = NXFF_OPENFLOW10;
> + flow_format = set_initial_format_for_flow_mod(&requests);
> +
> parse_ofp_flow_mod_str(&requests, &flow_format, argc > 2 ? argv[2] : "",
> command);
> + check_final_format_for_flow_mod(flow_format);
>
> open_vconn(argv[1], &vconn);
> transact_multiple_noreply(vconn, &requests);
> @@ -659,10 +698,11 @@ do_add_flows(int argc OVS_UNUSED, char *argv[])
> }
>
> list_init(&requests);
> - flow_format = NXFF_OPENFLOW10;
> + flow_format = set_initial_format_for_flow_mod(&requests);
>
> open_vconn(argv[1], &vconn);
> while (parse_ofp_add_flow_file(&requests, &flow_format, file)) {
> + check_final_format_for_flow_mod(flow_format);
> transact_multiple_noreply(vconn, &requests);
> }
> vconn_close(vconn);
> --
> 1.7.1
>
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
More information about the dev
mailing list