[ovs-dev] [ofp-print 07/18] ovs-ofctl: Fix del-flows command parsing bugs.
Justin Pettit
jpettit at nicira.com
Thu Dec 9 01:30:11 UTC 2010
Looks good.
--Justin
On Dec 8, 2010, at 4:26 PM, Ben Pfaff wrote:
> "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 72c115b..1ccbcd0 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 b994950..9777e16 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 0f1c67e..8da6e1a 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);
> --
> 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