Anyone reviewing it? Thanks!
Best,
Jing
On Fri, May 31, 2013 at 5:46 PM, Jing Ai <ji...@google.com> wrote:
> --- Resent the patch since the last one was messed up in format
> --- Added and changed AUTHOR information
>
> Found a bug that OVS does not return any error message when the
> goto_table_id is smaller than (or equals to) then current table where the
> flow is installed. As a result, I install a flow as follows (using
> "ovs-ofctl dump-flows br0")
>
> table_id=4, duration=40s, priority=32768, n_packets=0, n_bytes=0,
> tcp,in_port=1,dl_src=00:06:07:08:09:0a,dl_dst=00:01:02:03:04:05,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,tp_src=1234,tp_dst=80,actions=goto_table:3
>
> Best,
> Jing
>
> Signed-off-by: Jing Ai <ji...@google.com>
> ---
> AUTHORS | 3 ++-
> lib/ofp-actions.c | 5 +++++
> lib/ofp-actions.h | 1 +
> lib/ofp-util.c | 8 +++++---
> tests/ofp-actions.at | 4 ++++
> utilities/ovs-ofctl.c | 17 ++++++++++++++---
> 6 files changed, 31 insertions(+), 7 deletions(-)
>
> diff --git a/AUTHORS b/AUTHORS
> index d15173e..a2b0c4d 100644
> --- a/AUTHORS
> +++ b/AUTHORS
> @@ -45,6 +45,7 @@ Jarno Rajahalme jarno.rajaha...@nsn.com
> Jean Tourrilhes j...@hpl.hp.com
> Jeremy Stribling st...@nicira.com
> Jesse Gross je...@nicira.com
> +Jing Ai ji...@google.com
> Joe Perches j...@perches.com
> Joe Stringer j...@wand.net.nz
> Jun Nakajima jun.nakaj...@intel.com
> @@ -153,7 +154,7 @@ Jari Sundell sundell.softw...@gmail.com
> Jed Daniels openvswi...@jeddaniels.com
> Jeff Merrick jmerr...@vmware.com
> Jeongkeun Lee jk...@hp.com
> -Jing Ai ai_jing2...@hotmail.com
> +Jing Ai ji...@google.com
> Joan Cirer j...@ev0.net
> John Galgay j...@galgay.net
> Kevin Mancuso kevin.manc...@rackspace.com
> diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
> index 026a376..30f3f65 100644
> --- a/lib/ofp-actions.c
> +++ b/lib/ofp-actions.c
> @@ -1052,6 +1052,7 @@ ofpacts_pull_openflow11_actions(struct ofpbuf
> *openflow,
> enum ofperr
> ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
> unsigned int instructions_len,
> + uint8_t table_id,
> struct ofpbuf *ofpacts)
> {
> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
> @@ -1119,6 +1120,10 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf
> *openflow,
>
> oigt = instruction_get_OFPIT11_GOTO_TABLE(
> insts[OVSINST_OFPIT11_GOTO_TABLE]);
> + if (table_id >= oigt->table_id) {
> + error = OFPERR_OFPBRC_BAD_TABLE_ID;
> + goto exit;
> + }
> ogt = ofpact_put_GOTO_TABLE(ofpacts);
> ogt->table_id = oigt->table_id;
> }
> diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h
> index ffceb05..9a74bcc 100644
> --- a/lib/ofp-actions.h
> +++ b/lib/ofp-actions.h
> @@ -490,6 +490,7 @@ enum ofperr ofpacts_pull_openflow11_actions(struct
> ofpbuf *openflow,
> struct ofpbuf *ofpacts);
> enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,
> unsigned int
> instructions_len,
> + uint8_t table_id,
> struct ofpbuf *ofpacts);
> enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,
> const struct flow *, int max_ports);
> diff --git a/lib/ofp-util.c b/lib/ofp-util.c
> index 3c8d5a2..42021a8 100644
> --- a/lib/ofp-util.c
> +++ b/lib/ofp-util.c
> @@ -1502,7 +1502,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,
> return error;
> }
>
> - error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);
> + error = ofpacts_pull_openflow11_instructions(&b, b.size,
> ofm->table_id,
> + ofpacts);
> if (error) {
> return error;
> }
> @@ -1652,7 +1653,7 @@ ofputil_encode_flow_mod(const struct
> ofputil_flow_mod *fm,
> case OFPUTIL_P_OF13_OXM: {
> struct ofp11_flow_mod *ofm;
>
> - msg = ofpraw_alloc(OFPRAW_OFPT11_FLOW_MOD,
> + msg = ofpraw_alloc(OFPRAW_OFPT11_FLOW_MOD,
> ofputil_protocol_to_ofp_version(protocol),
> NXM_TYPICAL_LEN + fm->ofpacts_len);
> ofm = ofpbuf_put_zeros(msg, sizeof *ofm);
> @@ -2014,7 +2015,8 @@ ofputil_decode_flow_stats_reply(struct
> ofputil_flow_stats *fs,
> }
>
> if (ofpacts_pull_openflow11_instructions(msg, length - sizeof
> *ofs -
> - padded_match_len,
> ofpacts)) {
> + padded_match_len,
> + ofs->table_id, ofpacts))
> {
> VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad
> instructions");
> return EINVAL;
> }
> diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at
> index 2ecbdb5..b455bb9 100644
> --- a/tests/ofp-actions.at
> +++ b/tests/ofp-actions.at
> @@ -356,6 +356,10 @@ dnl Goto-Table 1 instruction non-zero padding
> # 7: 01 -> 00
> 0001 0008 01 000001
>
> +dnl Goto-Table 1 instruction go back to the previous table.
> +# bad OF1.1 instructions: OFPBRC_BAD_TABLE_ID
> +2,0001 0008 01 000000
> +
> dnl Goto-Table 1
> # actions=goto_table:1
> 0001 0008 01 000000
> diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
> index 24b9434..009768a 100644
> --- a/utilities/ovs-ofctl.c
> +++ b/utilities/ovs-ofctl.c
> @@ -2607,18 +2607,29 @@ ofctl_parse_ofp11_instructions(int argc
> OVS_UNUSED, char *argv[] OVS_UNUSED)
> enum ofperr error;
> size_t size;
> struct ds s;
> + const char *instructions;
> + const char *table_id;
> +
> + /* Parse table_id separated with the follow-up instructions by
> ",", if
> + * any. */
> + instructions = ds_cstr(&in);
> + table_id = NULL;
> + if (strstr(instructions, ",")) {
> + table_id = strsep(&instructions, ",");
> + }
>
> /* Parse hex bytes. */
> ofpbuf_init(&of11_in, 0);
> - if (ofpbuf_put_hex(&of11_in, ds_cstr(&in), NULL)[0] != '\0') {
> + if (ofpbuf_put_hex(&of11_in, instructions, NULL)[0] != '\0') {
> ovs_fatal(0, "Trailing garbage in hex data");
> }
>
> /* Convert to ofpacts. */
> ofpbuf_init(&ofpacts, 0);
> size = of11_in.size;
> - error = ofpacts_pull_openflow11_instructions(&of11_in,
> of11_in.size,
> - &ofpacts);
> + error = ofpacts_pull_openflow11_instructions(
> + &of11_in, of11_in.size, table_id ? (uint8_t)atoi(table_id) :
> 0,
> + &ofpacts);
> if (error) {
> printf("bad OF1.1 instructions: %s\n\n",
> ofperr_get_name(error));
> ofpbuf_uninit(&ofpacts);
> --
> 1.8.2.1
>
>
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
