[ovs-dev] [packet-in 5/8] ofp-util: Use correct cookie value in "packet_in"s when no flow involved.

Ben Pfaff blp at nicira.com
Wed Oct 23 05:05:17 UTC 2013


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 at nicira.com>
---
 include/openflow/nicira-ext.h |    4 ++--
 lib/ofp-print.c               |    2 +-
 lib/ofp-util.c                |    1 +
 lib/ofp-util.h                |    2 +-
 ofproto/ofproto-dpif-upcall.c |    2 +-
 ofproto/ofproto-dpif-xlate.c  |    4 +++-
 tests/ofproto-dpif.at         |    2 +-
 7 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/openflow/nicira-ext.h b/include/openflow/nicira-ext.h
index ca272fd..3362a49 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 9a84645..6ff6690 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 e295c20..8c200ce 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 edf7ad2..1afdce0 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 cc10ed6..491f11e 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 8308dd3..c2812c8 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 bcf5e09..c569463 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
-- 
1.7.10.4




More information about the dev mailing list