From 640c7c945f3c748240609dc572858fc188db1c06 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Tue, 7 Dec 2010 14:30:07 -0800 Subject: [PATCH] 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. --- lib/ofp-parse.c | 3 ++- tests/ofproto.at | 23 +++++++++++++++++++++-- utilities/ovs-ofctl.c | 3 ++- 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c index 72c115ba..1ccbcd02 100644 --- 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 b994950d..9777e161 100644 --- 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 0f1c67e6..8da6e1a0 100644 --- 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); -- 2.30.2