From: Ben Pfaff Date: Tue, 22 Oct 2013 23:57:46 +0000 (-0700) Subject: ofp-util: Use correct cookie value in "packet_in"s when no flow involved. X-Git-Url: http://git.onelab.eu/?p=sliver-openvswitch.git;a=commitdiff_plain;h=d4fa4e792e55c60142f86a82822132ac584559ce 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 --- diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h index ca272fdf3..3362a4919 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 9a84645fe..6ff669082 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 e295c2090..8c200ce9f 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 edf7ad2e1..1afdce003 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 cc10ed667..491f11e5c 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 8308dd33a..c2812c89d 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 bcf5e09e1..c569463d1 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