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 <et...@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
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
