ovs-ofctl: Fix del-flows command parsing bugs.

"ovs-ofctl del-flows br0" segfaulted because do_flow_mod__() assumed that
it always had a "flow" argument, which is not true for the del-flows
command.

Beyond that, parse_ofp_flow_mod_str() rejected "ovs-ofctl del-flows
br0" because no actions were supplied, even though supplying actions
doesn't make sense for deleting flows.

This commit fixes both problems and adds a simple test that would have
caught both problems.

Bug #4112.

diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 72c115ba6bd88349f28e1853eab2b640483c93e2..1ccbcd02cff5b1d8079adb840ac5f57d21feea8b 100644 (file)
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -648,13 +648,14 @@ void
 parse_ofp_flow_mod_str(struct list *packets, enum nx_flow_format *cur_format,
                        char *string, uint16_t command)
 {
+    bool is_del = command == OFPFC_DELETE || command == OFPFC_DELETE_STRICT;
     enum nx_flow_format min_format, next_format;
     struct ofpbuf actions;
     struct ofpbuf *ofm;
     struct flow_mod fm;
 
     ofpbuf_init(&actions, 64);
-    parse_ofp_str(&fm, NULL, &actions, string);
+    parse_ofp_str(&fm, NULL, is_del ? NULL : &actions, string);
     fm.command = command;
 
     min_format = ofputil_min_flow_format(&fm.cr, true, fm.cookie);

diff --git a/tests/ofproto.at b/tests/ofproto.at
index b994950d40696f305231cc6d2887f42bb08532a9..9777e1613ad1625e33bd9d1b77ee7f9cb75203a0 100644 (file)
--- a/tests/ofproto.at
+++ b/tests/ofproto.at
@@ -1,5 +1,8 @@
 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
@@ -23,7 +26,7 @@ AT_CLEANUP
 AT_SETUP([ofproto - feature request, config request])
 OFPROTO_START
 AT_CHECK([ovs-ofctl -vANY:ANY:WARN show br0], [0], [stdout])
-AT_CHECK([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0], [dnl
+AT_CHECK([STRIP_XIDS stdout], [0], [dnl
 OFPT_FEATURES_REPLY: ver:0x1, dpid:fedcba9876543210
 n_tables:2, n_buffers:256
 features: capabilities:0x87, actions:0xfff
@@ -45,7 +48,7 @@ do
     command=$[1] config=$[2] state=$[3]
     AT_CHECK([ovs-ofctl -vANY:ANY:WARN mod-port br0 br0 $command])
     AT_CHECK([ovs-ofctl -vANY:ANY:WARN show br0], [0], [stdout])
-    AT_CHECK_UNQUOTED([[sed 's/ (xid=0x[0-9a-fA-F]*)//' stdout]], [0], [dnl
+    AT_CHECK_UNQUOTED([STRIP_XIDS stdout], [0], [dnl
 OFPT_FEATURES_REPLY: ver:0x1, dpid:fedcba9876543210
 n_tables:2, n_buffers:256
 features: capabilities:0x87, actions:0xfff
@@ -55,3 +58,19 @@ OFPT_GET_CONFIG_REPLY: miss_send_len=0
 done
 OFPROTO_STOP
 AT_CLEANUP
+
+AT_SETUP([ofproto - basic flow_mod commands])
+OFPROTO_START
+AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS], [0], [NXST_FLOW reply:
+])
+AT_CHECK([ovs-ofctl add-flow br0 in_port=1,actions=0])
+AT_CHECK([ovs-ofctl add-flow br0 in_port=0,actions=1])
+AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION], [0], [dnl
+NXST_FLOW reply: cookie=0x0, duration=?s, table_id=0, priority=32768, n_packets=0, n_bytes=0, in_port=1 actions=output:0
+ cookie=0x0, duration=?s, table_id=0, priority=32768, n_packets=0, n_bytes=0, in_port=65534 actions=output:1
+])
+AT_CHECK([ovs-ofctl del-flows br0])
+AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS], [0], [NXST_FLOW reply:
+])
+OFPROTO_STOP
+AT_CLEANUP

diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 0f1c67e69ca8fa2a9efd5a16215fcf98f37084a8..8da6e1a033cec188665140773576f9af8968e637 100644 (file)
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -631,7 +631,8 @@ do_flow_mod__(int argc OVS_UNUSED, char *argv[], uint16_t command)
 
     list_init(&requests);
     flow_format = NXFF_OPENFLOW10;
-    parse_ofp_flow_mod_str(&requests, &flow_format, argv[2], command);
+    parse_ofp_flow_mod_str(&requests, &flow_format, argc > 2 ? argv[2] : "",
+                           command);
 
     open_vconn(argv[1], &vconn);
     transact_multiple_noreply(vconn, &requests);