[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