From e16b024e1b68b2b1c616cb5faa8f5f640e74861a Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 12 May 2011 17:24:34 -0700 Subject: [PATCH] Fix bugs lingering from merge mistakes. I should have caught these when I did the merge from "master" earlier today, but I forgot to run the testsuite. --- lib/ofp-util.c | 39 ++++++++++++++++++++------------------- tests/ofproto.at | 1 - tests/ovs-ofctl.at | 9 +++++---- 3 files changed, 25 insertions(+), 24 deletions(-) diff --git a/lib/ofp-util.c b/lib/ofp-util.c index 3487cbac..2e12eb54 100644 --- a/lib/ofp-util.c +++ b/lib/ofp-util.c @@ -339,27 +339,16 @@ ofputil_lookup_openflow_message(const struct ofputil_msg_category *cat, const struct ofputil_msg_type **typep) { const struct ofputil_msg_type *type; - bool found; - found = false; for (type = cat->types; type < &cat->types[cat->n_types]; type++) { if (type->value == value) { - if (ofputil_length_ok(cat, type, size)) { - *typep = type; - return 0; + if (!ofputil_length_ok(cat, type, size)) { + return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); } - - /* We found a matching command type but it had the wrong length. - * Probably this is just an error. However, a screwup means that - * NXT_SET_FLOW_FORMAT and NXT_FLOW_MOD_TABLE_ID have the same - * value. They do have different lengths, so we can distinguish - * them that way. */ - found = true; + *typep = type; + return 0; } } - if (found) { - return ofp_mkerr(OFPET_BAD_REQUEST, OFPBRC_BAD_LEN); - } VLOG_WARN_RL(&bad_ofmsg_rl, "received %s of unknown type %"PRIu32, cat->name, value); @@ -386,10 +375,6 @@ ofputil_decode_vendor(const struct ofp_header *oh, NXT_SET_FLOW_FORMAT, "NXT_SET_FLOW_FORMAT", sizeof(struct nxt_set_flow_format), 0 }, - { OFPUTIL_NXT_FLOW_MOD_TABLE_ID, - NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID", - sizeof(struct nxt_flow_mod_table_id), 0 }, - { OFPUTIL_NXT_FLOW_MOD, NXT_FLOW_MOD, "NXT_FLOW_MOD", sizeof(struct nx_flow_mod), 8 }, @@ -423,6 +408,21 @@ ofputil_decode_vendor(const struct ofp_header *oh, } nh = (const struct nicira_header *) oh; + + if (nh->subtype == htonl(NXT_FLOW_MOD_TABLE_ID) + && oh->length == htons(sizeof(struct nxt_flow_mod_table_id))) { + /* NXT_SET_FLOW_FORMAT and NXT_FLOW_MOD_TABLE_ID accidentally have the + * same value but different lengths. ofputil_lookup_openflow_message() + * doesn't support this case, so special case it here. */ + static const struct ofputil_msg_type nxt_flow_mod_table_id = + { OFPUTIL_NXT_FLOW_MOD_TABLE_ID, + NXT_FLOW_MOD_TABLE_ID, "NXT_FLOW_MOD_TABLE_ID", + sizeof(struct nxt_flow_mod_table_id), 0 }; + + *typep = &nxt_flow_mod_table_id; + return 0; + } + return ofputil_lookup_openflow_message(&nxt_category, ntohl(nh->subtype), ntohs(oh->length), typep); } @@ -1004,6 +1004,7 @@ ofputil_encode_flow_mod(const struct flow_mod *fm, msg = ofpbuf_new(sizeof *ofm + actions_len); ofm = put_openflow(sizeof *ofm, OFPT_FLOW_MOD, msg); ofputil_cls_rule_to_match(&fm->cr, &ofm->match); + ofm->cookie = fm->cookie; ofm->command = htons(fm->command); ofm->idle_timeout = htons(fm->idle_timeout); ofm->hard_timeout = htons(fm->hard_timeout); diff --git a/tests/ofproto.at b/tests/ofproto.at index 713cc769..bb43149f 100644 --- a/tests/ofproto.at +++ b/tests/ofproto.at @@ -51,7 +51,6 @@ AT_CHECK([ovs-ofctl add-flow br0 in_port=0,actions=1]) AT_CHECK([ovs-ofctl dump-flows br0 | STRIP_XIDS | STRIP_DURATION | sort], [0], [dnl cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, in_port=0 actions=output:1 cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, in_port=1 actions=output:0 - cookie=0x0, duration=?s, table_id=0, n_packets=0, n_bytes=0, in_port=65534 actions=output:1 NXST_FLOW reply: ]) AT_CHECK([ovs-ofctl del-flows br0]) diff --git a/tests/ovs-ofctl.at b/tests/ovs-ofctl.at index a8cdbe9a..3a8f0b6d 100644 --- a/tests/ovs-ofctl.at +++ b/tests/ovs-ofctl.at @@ -32,8 +32,9 @@ NXT_SET_FLOW_FORMAT: format=nxm NXT_FLOW_MOD: ADD tun_id=0x1234 cookie:0x5678 actions=FLOOD NXT_FLOW_MOD: ADD actions=set_tunnel:0x1234,set_tunnel64:0x9876,set_tunnel64:0x123456789 NXT_FLOW_MOD: ADD actions=multipath(eth_src,50,hrw,12,0,NXM_NX_REG0[0..3]),multipath(symmetric_l4,1024,iter_hash,5000,5050,NXM_NX_REG0[0..12]) -NXT_FLOW_MOD: ADD actions=drop -NXT_FLOW_MOD: ADD tun_id=0x1234000056780000/0xffff0000ffff0000 actions=drop +NXT_FLOW_MOD_TABLE_ID: enable +NXT_FLOW_MOD: ADD table_id:1 actions=drop +NXT_FLOW_MOD: ADD table_id:255 tun_id=0x1234000056780000/0xffff0000ffff0000 actions=drop ]]) AT_CHECK([sed 's/.*|//' stderr], [0], [dnl normalization changed ofp_match, details: @@ -43,8 +44,8 @@ normalization changed ofp_match, details: pre: wildcards= 0x3820ff in_port= 0 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 post: wildcards= 0x3fffff in_port= 0 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 normalization changed ofp_match, details: - pre: wildcards= 0x3820ff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 -post: wildcards= 0x3fffff in_port=65534 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 + pre: wildcards= 0x3820ff in_port= 0 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 +post: wildcards= 0x3fffff in_port= 0 dl_src=00:00:00:00:00:00 dl_dst=00:00:00:00:00:00 dl_vlan= 0 dl_vlan_pcp= 0 dl_type= 0 nw_tos= 0 nw_proto= 0 nw_src= 0 nw_dst= 0 tp_src= 0 tp_dst= 0 ]) AT_CLEANUP -- 2.30.2