aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBen Pfaff <blp@nicira.com>2013-10-22 16:57:46 -0700
committerBen Pfaff <blp@nicira.com>2013-10-22 21:12:06 -0700
commitd4fa4e792e55c60142f86a82822132ac584559ce (patch)
tree4c327c3ccf4823580b0c6e89db026cf2b541c663
parent22ee3544769ae9e9d35e8efa92b4b50468badf09 (diff)
ofp-util: Use correct cookie value in "packet_in"s when no flow involved.
OpenFlow 1.3 uses all-1-bits in a packet_in to indicate that the packet_in was not generated by a flow, but Open vSwitch incorrectly used 0. This fixes the problem. For consistency, this commit also changes NXT_PACKET_IN to use all-1-bits for this case, event though NXT_PACKET_IN was previously defined to use zero. This doesn't appear to make a difference for the NVP controller; if it causes a problem for some other controller then I will revert that part of the change. Found by inspection. Signed-off-by: Ben Pfaff <blp@nicira.com>
-rw-r--r--include/openflow/nicira-ext.h4
-rw-r--r--lib/ofp-print.c2
-rw-r--r--lib/ofp-util.c1
-rw-r--r--lib/ofp-util.h2
-rw-r--r--ofproto/ofproto-dpif-upcall.c2
-rw-r--r--ofproto/ofproto-dpif-xlate.c4
-rw-r--r--tests/ofproto-dpif.at2
7 files changed, 10 insertions, 7 deletions
diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index ca272fdf..3362a491 100644
--- a/include/openflow/nicira-ext.h
+++ b/include/openflow/nicira-ext.h
@@ -199,8 +199,8 @@ OFP_ASSERT(sizeof(struct nx_set_packet_in_format) == 4);
* might support fields (new registers, new protocols, etc.) that the
* controller does not. The controller must prepared to tolerate these.
*
- * The 'cookie' and 'table_id' fields have no meaning when 'reason' is
- * OFPR_NO_MATCH. In this case they should be set to 0. */
+ * The 'cookie' field has no meaning when 'reason' is OFPR_NO_MATCH. In this
+ * case it should be UINT64_MAX. */
struct nx_packet_in {
ovs_be32 buffer_id; /* ID assigned by datapath. */
ovs_be16 total_len; /* Full length of frame. */
diff --git a/lib/ofp-print.c b/lib/ofp-print.c
index 9a84645f..6ff66908 100644
--- a/lib/ofp-print.c
+++ b/lib/ofp-print.c
@@ -105,7 +105,7 @@ ofp_print_packet_in(struct ds *string, const struct ofp_header *oh,
ds_put_format(string, " table_id=%"PRIu8, pin.table_id);
}
- if (pin.cookie) {
+ if (pin.cookie != OVS_BE64_MAX) {
ds_put_format(string, " cookie=0x%"PRIx64, ntohll(pin.cookie));
}
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index e295c209..8c200ce9 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -2822,6 +2822,7 @@ ofputil_decode_packet_in(struct ofputil_packet_in *pin,
struct ofpbuf b;
memset(pin, 0, sizeof *pin);
+ pin->cookie = OVS_BE64_MAX;
ofpbuf_use_const(&b, oh, ntohs(oh->length));
raw = ofpraw_pull_assert(&b);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index edf7ad2e..1afdce00 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -400,7 +400,7 @@ struct ofputil_packet_in {
/* Information about the OpenFlow flow that triggered the packet-in.
*
* A packet-in triggered by a flow table miss has no associated flow. In
- * that case, 'cookie' is 0. */
+ * that case, 'cookie' is UINT64_MAX. */
uint8_t table_id; /* OpenFlow table ID. */
ovs_be64 cookie; /* Flow's cookie. */
};
diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index cc10ed66..491f11e5 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -846,7 +846,7 @@ handle_upcalls(struct udpif *udpif, struct list *upcalls)
pin->up.packet_len = packet->size;
pin->up.reason = OFPR_NO_MATCH;
pin->up.table_id = 0;
- pin->up.cookie = 0;
+ pin->up.cookie = OVS_BE64_MAX;
flow_get_metadata(&miss->flow, &pin->up.fmd);
pin->send_len = 0; /* Not used for flow table misses. */
ofproto_dpif_send_packet_in(miss->ofproto, pin);
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 8308dd33..c2812c89 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1848,7 +1848,9 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
pin->up.packet = ofpbuf_steal_data(packet);
pin->up.reason = reason;
pin->up.table_id = ctx->table_id;
- pin->up.cookie = ctx->rule ? rule_dpif_get_flow_cookie(ctx->rule) : 0;
+ pin->up.cookie = (ctx->rule
+ ? rule_dpif_get_flow_cookie(ctx->rule)
+ : OVS_BE64_MAX);
flow_get_metadata(&ctx->xin->flow, &pin->up.fmd);
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index bcf5e09e..c569463d 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -168,7 +168,7 @@ AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir --pidfile
AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=2,frag=no)' -generate], [0], [stdout])
OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
AT_CHECK([cat ofctl_monitor.log], [0], [dnl
-NXT_PACKET_IN (xid=0x0): table_id=1 total_len=42 in_port=1 (via invalid_ttl) data_len=42 (unbuffered)
+NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=1 (via invalid_ttl) data_len=42 (unbuffered)
icmp,metadata=0,in_port=0,vlan_tci=0x0000,dl_src=50:54:00:00:00:05,dl_dst=50:54:00:00:00:07,nw_src=192.168.0.1,nw_dst=192.168.0.2,nw_tos=0,nw_ecn=0,nw_ttl=1,icmp_type=0,icmp_code=0
])
OVS_VSWITCHD_STOP