From 0fbc9f11c027cce9c89de12525de852e6b2ae332 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 22 Feb 2011 13:43:14 -0800 Subject: [PATCH] ovs-ofctl: Implement documented semantics of --flow-format for flow_mods. 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 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 09ec23f2..e2d3388a 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 00000000..06f9b65f --- /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@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 a7dda06f..f266f34b 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@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 111ed881..dfdebd3e 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 d41f504b..5625f8b0 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 9f5a0a52..a96b77a9 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); -- 2.30.2