<div dir="ltr">Anyone reviewing it? Thanks!<div><br></div><div style>Best,</div><div style>Jing</div></div><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, May 31, 2013 at 5:46 PM, Jing Ai <span dir="ltr"><<a href="mailto:jinga@google.com" target="_blank">jinga@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>--- Resent the patch since the last one was messed up in format</div><div>--- Added and changed AUTHOR information</div>
<div><br></div>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")<div>
<br></div><div>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</div>
<div><br></div><div>Best,</div><div>Jing</div><div><br></div><div>Signed-off-by: Jing Ai <<a href="mailto:jinga@google.com" target="_blank">jinga@google.com</a>><br></div><div>---</div><div><div> AUTHORS | 3 ++-</div>
<div> lib/ofp-actions.c | 5 +++++</div><div> lib/ofp-actions.h | 1 +</div><div> lib/ofp-util.c | 8 +++++---</div><div> tests/<a href="http://ofp-actions.at" target="_blank">ofp-actions.at</a> | 4 ++++</div>
<div> utilities/ovs-ofctl.c | 17 ++++++++++++++---</div>
<div> 6 files changed, 31 insertions(+), 7 deletions(-)</div><div><br></div><div>diff --git a/AUTHORS b/AUTHORS</div><div>index d15173e..a2b0c4d 100644</div><div>--- a/AUTHORS</div><div>+++ b/AUTHORS</div><div>@@ -45,6 +45,7 @@ Jarno Rajahalme <a href="mailto:jarno.rajahalme@nsn.com" target="_blank">jarno.rajahalme@nsn.com</a></div>
<div> Jean Tourrilhes <a href="mailto:jt@hpl.hp.com" target="_blank">jt@hpl.hp.com</a></div><div> Jeremy Stribling <a href="mailto:strib@nicira.com" target="_blank">strib@nicira.com</a></div><div> Jesse Gross <a href="mailto:jesse@nicira.com" target="_blank">jesse@nicira.com</a></div>
<div>+Jing Ai <a href="mailto:jinga@google.com" target="_blank">jinga@google.com</a></div><div> Joe Perches <a href="mailto:joe@perches.com" target="_blank">joe@perches.com</a></div><div> Joe Stringer <a href="mailto:joe@wand.net.nz" target="_blank">joe@wand.net.nz</a></div>
<div> Jun Nakajima <a href="mailto:jun.nakajima@intel.com" target="_blank">jun.nakajima@intel.com</a></div><div>@@ -153,7 +154,7 @@ Jari Sundell <a href="mailto:sundell.software@gmail.com" target="_blank">sundell.software@gmail.com</a></div>
<div> Jed Daniels <a href="mailto:openvswitch@jeddaniels.com" target="_blank">openvswitch@jeddaniels.com</a></div><div> Jeff Merrick <a href="mailto:jmerrick@vmware.com" target="_blank">jmerrick@vmware.com</a></div>
<div> Jeongkeun Lee <a href="mailto:jklee@hp.com" target="_blank">jklee@hp.com</a></div>
<div>-Jing Ai <a href="mailto:ai_jing2000@hotmail.com" target="_blank">ai_jing2000@hotmail.com</a></div><div>+Jing Ai <a href="mailto:jinga@google.com" target="_blank">jinga@google.com</a></div>
<div> Joan Cirer <a href="mailto:joan@ev0.net" target="_blank">joan@ev0.net</a></div>
<div> John Galgay <a href="mailto:john@galgay.net" target="_blank">john@galgay.net</a></div><div> Kevin Mancuso <a href="mailto:kevin.mancuso@rackspace.com" target="_blank">kevin.mancuso@rackspace.com</a></div>
<div>diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c</div>
<div>index 026a376..30f3f65 100644</div><div>--- a/lib/ofp-actions.c</div><div>+++ b/lib/ofp-actions.c</div><div>@@ -1052,6 +1052,7 @@ ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,</div><div> enum ofperr</div>
<div>
ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,</div><div> unsigned int instructions_len,</div><div>+ uint8_t table_id,</div><div> struct ofpbuf *ofpacts)</div>
<div> {</div><div> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);</div><div>@@ -1119,6 +1120,10 @@ ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,</div><div> </div><div> oigt = instruction_get_OFPIT11_GOTO_TABLE(</div>
<div> insts[OVSINST_OFPIT11_GOTO_TABLE]);</div><div>+ if (table_id >= oigt->table_id) {</div><div>+ error = OFPERR_OFPBRC_BAD_TABLE_ID;</div><div>+ goto exit;</div><div>+ }</div>
<div> ogt = ofpact_put_GOTO_TABLE(ofpacts);</div><div> ogt->table_id = oigt->table_id;</div><div> }</div><div>diff --git a/lib/ofp-actions.h b/lib/ofp-actions.h</div><div>index ffceb05..9a74bcc 100644</div>
<div>--- a/lib/ofp-actions.h</div><div>+++ b/lib/ofp-actions.h</div><div>@@ -490,6 +490,7 @@ enum ofperr ofpacts_pull_openflow11_actions(struct ofpbuf *openflow,</div><div> struct ofpbuf *ofpacts);</div>
<div> enum ofperr ofpacts_pull_openflow11_instructions(struct ofpbuf *openflow,</div><div> unsigned int instructions_len,</div><div>+ uint8_t table_id,</div>
<div> struct ofpbuf *ofpacts);</div><div> enum ofperr ofpacts_check(const struct ofpact[], size_t ofpacts_len,</div><div> const struct flow *, int max_ports);</div>
<div>diff --git a/lib/ofp-util.c b/lib/ofp-util.c</div><div>index 3c8d5a2..42021a8 100644</div><div>--- a/lib/ofp-util.c</div><div>+++ b/lib/ofp-util.c</div><div>@@ -1502,7 +1502,8 @@ ofputil_decode_flow_mod(struct ofputil_flow_mod *fm,</div>
<div> return error;</div><div> }</div><div> </div><div>- error = ofpacts_pull_openflow11_instructions(&b, b.size, ofpacts);</div><div>+ error = ofpacts_pull_openflow11_instructions(&b, b.size, ofm->table_id,</div>
<div>+ ofpacts);</div><div> if (error) {</div><div> return error;</div><div> }</div><div>@@ -1652,7 +1653,7 @@ ofputil_encode_flow_mod(const struct ofputil_flow_mod *fm,</div>
<div> case OFPUTIL_P_OF13_OXM: {</div><div> struct ofp11_flow_mod *ofm;</div><div> </div><div>- msg = ofpraw_alloc(OFPRAW_OFPT11_FLOW_MOD, </div><div>+ msg = ofpraw_alloc(OFPRAW_OFPT11_FLOW_MOD,</div>
<div> ofputil_protocol_to_ofp_version(protocol),</div><div> NXM_TYPICAL_LEN + fm->ofpacts_len);</div><div> ofm = ofpbuf_put_zeros(msg, sizeof *ofm);</div><div>
@@ -2014,7 +2015,8 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,</div><div> }</div><div> </div><div> if (ofpacts_pull_openflow11_instructions(msg, length - sizeof *ofs -</div><div>- padded_match_len, ofpacts)) {</div>
<div>+ padded_match_len,</div><div>+ ofs->table_id, ofpacts)) {</div><div> VLOG_WARN_RL(&bad_ofmsg_rl, "OFPST_FLOW reply bad instructions");</div>
<div> return EINVAL;</div><div> }</div><div>diff --git a/tests/<a href="http://ofp-actions.at" target="_blank">ofp-actions.at</a> b/tests/<a href="http://ofp-actions.at" target="_blank">ofp-actions.at</a></div>
<div>index 2ecbdb5..b455bb9 100644</div>
<div>--- a/tests/<a href="http://ofp-actions.at" target="_blank">ofp-actions.at</a></div><div>+++ b/tests/<a href="http://ofp-actions.at" target="_blank">ofp-actions.at</a></div><div>@@ -356,6 +356,10 @@ dnl Goto-Table 1 instruction non-zero padding</div>
<div> # 7: 01 -> 00</div><div> 0001 0008 01 000001</div><div> </div><div>+dnl Goto-Table 1 instruction go back to the previous table.</div><div>+# bad OF1.1 instructions: OFPBRC_BAD_TABLE_ID</div><div>+2,0001 0008 01 000000</div>
<div>+</div><div> dnl Goto-Table 1</div><div> # actions=goto_table:1</div><div> 0001 0008 01 000000</div><div>diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c</div><div>index 24b9434..009768a 100644</div><div>--- a/utilities/ovs-ofctl.c</div>
<div>+++ b/utilities/ovs-ofctl.c</div><div>@@ -2607,18 +2607,29 @@ ofctl_parse_ofp11_instructions(int argc OVS_UNUSED, char *argv[] OVS_UNUSED)</div><div> enum ofperr error;</div><div> size_t size;</div><div>
struct ds s;</div><div>+ const char *instructions;</div><div>+ const char *table_id;</div><div>+</div><div>+ /* Parse table_id separated with the follow-up instructions by ",", if</div>
<div>+ * any. */</div><div>+ instructions = ds_cstr(&in);</div><div>+ table_id = NULL;</div><div>+ if (strstr(instructions, ",")) {</div><div>+ table_id = strsep(&instructions, ",");</div>
<div>+ }</div><div> </div><div> /* Parse hex bytes. */</div><div> ofpbuf_init(&of11_in, 0);</div><div>- if (ofpbuf_put_hex(&of11_in, ds_cstr(&in), NULL)[0] != '\0') {</div>
<div>+ if (ofpbuf_put_hex(&of11_in, instructions, NULL)[0] != '\0') {</div><div> ovs_fatal(0, "Trailing garbage in hex data");</div><div> }</div><div> </div><div> /* Convert to ofpacts. */</div>
<div> ofpbuf_init(&ofpacts, 0);</div><div> size = of11_in.size;</div><div>- error = ofpacts_pull_openflow11_instructions(&of11_in, of11_in.size,</div><div>- &ofpacts);</div>
<div>+ error = ofpacts_pull_openflow11_instructions(</div><div>+ &of11_in, of11_in.size, table_id ? (uint8_t)atoi(table_id) : 0,</div><div>+ &ofpacts);</div><div> if (error) {</div>
<div> printf("bad OF1.1 instructions: %s\n\n", ofperr_get_name(error));</div><div> ofpbuf_uninit(&ofpacts);</div></div><div>--</div><div>1.8.2.1</div><div><br></div></div>
</blockquote></div><br></div>