[ovs-dev] [packet_in 09/13] ofp-util: Don't use ofpbuf in ofputil_packet_in struct.

Ethan Jackson ethan at nicira.com
Tue Jan 3 18:05:52 UTC 2012


Here's another version of the patch which takes into account your comments.
---

This will make the memory ownership clearer when future patches
make more extensive use of ofputil_packet_in.

Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
 lib/ofp-util.c         |    9 ++++-----
 lib/ofp-util.h         |    4 +++-
 ofproto/connmgr.c      |   11 +++++++----
 ofproto/ofproto-dpif.c |    6 ++++--
 4 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 42dab87..0268614 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1549,7 +1549,7 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr,
  * returned ofpbuf.
  *
  * If 'rw_packet' is nonnull, then it must contain the same data as
- * pin->packet.  'rw_packet' is allowed to be the same ofpbuf as pin->packet.
+ * pin->packet.  'rw_packet'->data is allowed to be the same as pin->packet.
  * It is modified in-place into an OFPT_PACKET_IN message according to 'pin',
  * and then ofputil_encode_packet_in() returns 'rw_packet'.  If 'rw_packet' has
  * enough headroom to insert a "struct ofp_packet_in", this is more efficient
@@ -1557,9 +1557,8 @@ ofputil_encode_flow_removed(const struct ofputil_flow_removed *fr,
  * payload. */
 struct ofpbuf *
 ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
-                        struct ofpbuf *rw_packet)
+                         struct ofpbuf *rw_packet)
 {
-    int total_len = pin->packet->size;
     struct ofp_packet_in opi;
 
     if (rw_packet) {
@@ -1568,7 +1567,7 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
         }
     } else {
         rw_packet = ofpbuf_clone_data_with_headroom(
-            pin->packet->data, MIN(pin->send_len, pin->packet->size),
+            pin->packet, MIN(pin->send_len, pin->packet_len),
             offsetof(struct ofp_packet_in, data));
     }
 
@@ -1576,7 +1575,7 @@ ofputil_encode_packet_in(const struct ofputil_packet_in *pin,
     memset(&opi, 0, sizeof opi);
     opi.header.version = OFP_VERSION;
     opi.header.type = OFPT_PACKET_IN;
-    opi.total_len = htons(total_len);
+    opi.total_len = htons(pin->packet_len);
     opi.in_port = htons(pin->in_port);
     opi.reason = pin->reason;
     opi.buffer_id = htonl(pin->buffer_id);
diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 8fa729e..5f7f454 100644
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -214,7 +214,9 @@ struct ofpbuf *ofputil_encode_flow_removed(const struct ofputil_flow_removed *,
 
 /* Abstract packet-in message. */
 struct ofputil_packet_in {
-    struct ofpbuf *packet;
+    const void *packet;
+    size_t packet_len;
+
     uint16_t in_port;
     uint8_t reason;             /* One of OFPR_*. */
 
diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 6caad06..44352d2 100644
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1154,8 +1154,8 @@ connmgr_send_flow_removed(struct connmgr *mgr,
  * necessary according to their individual configurations.
  *
  * 'rw_packet' may be NULL.  Otherwise, 'rw_packet' must contain the same data
- * as pin->packet.  (rw_packet == pin->packet is also valid.)  Ownership of
- * 'rw_packet' is transferred to this function. */
+ * as pin->packet.  Ownership of 'rw_packet' is transferred to this function.
+ */
 void
 connmgr_send_packet_in(struct connmgr *mgr,
                        const struct ofputil_packet_in *pin,
@@ -1210,12 +1210,15 @@ schedule_packet_in(struct ofconn *ofconn, struct ofputil_packet_in pin,
     } else if (!ofconn->pktbuf) {
         pin.buffer_id = UINT32_MAX;
     } else {
-        pin.buffer_id = pktbuf_save(ofconn->pktbuf, pin.packet, flow->in_port);
+        struct ofpbuf packet;
+
+        ofpbuf_use_const(&packet, pin.packet, pin.packet_len);
+        pin.buffer_id = pktbuf_save(ofconn->pktbuf, &packet, flow->in_port);
     }
 
     /* Figure out how much of the packet to send. */
     if (pin.reason == OFPR_NO_MATCH) {
-        pin.send_len = pin.packet->size;
+        pin.send_len = pin.packet_len;
     } else {
         /* Caller should have initialized 'send_len' to 'max_len' specified in
          * struct ofp_action_output. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 7b793b6..fbebada 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -2406,7 +2406,8 @@ send_packet_in_miss(struct ofproto_dpif *ofproto, struct ofpbuf *packet,
 {
     struct ofputil_packet_in pin;
 
-    pin.packet = packet;
+    pin.packet = packet->data;
+    pin.packet_len = packet->size;
     pin.in_port = flow->in_port;
     pin.reason = OFPR_NO_MATCH;
     pin.buffer_id = 0;          /* not yet known */
@@ -2433,7 +2434,8 @@ send_packet_in_action(struct ofproto_dpif *ofproto, struct ofpbuf *packet,
 
     memcpy(&cookie, &userdata, sizeof(cookie));
 
-    pin.packet = packet;
+    pin.packet = packet->data;
+    pin.packet_len = packet->size;
     pin.in_port = flow->in_port;
     pin.reason = OFPR_ACTION;
     pin.buffer_id = 0;          /* not yet known */
-- 
1.7.7.1




More information about the dev mailing list