ofproto, ofp-util: Begin disentangling packet-in wire format and handling.

struct ofputil_packet_in mixes data included in OpenFlow packet_in messages
with data that used internally by ofproto and connmgr to queue and route
packet_ins.  This commit begins disentangling these purposes by adding a
new struct ofproto_packet_in that wraps struct ofputil_packet_in.  Adding
this new level of indirection causes a lot of code churn, so this commit
mainly takes care of that to make the remaining changes easier to read.

This commit does move the list node used for queuing packet_ins into the
new wrapper structure.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/ofp-util.h b/lib/ofp-util.h
index 7355559..2de6e76 100644 (file)
--- a/lib/ofp-util.h
+++ b/lib/ofp-util.h
@@ -376,8 +376,6 @@ struct ofpbuf *ofputil_encode_flow_removed(const struct ofputil_flow_removed *,
 
 /* Abstract packet-in message. */
 struct ofputil_packet_in {
-    struct list list_node; /* For queueing packet_ins. */
-
     const void *packet;
     size_t packet_len;
 

diff --git a/ofproto/connmgr.c b/ofproto/connmgr.c
index 8bb96f0..2c32525 100644 (file)
--- a/ofproto/connmgr.c
+++ b/ofproto/connmgr.c
@@ -1435,7 +1435,7 @@ ofconn_send(const struct ofconn *ofconn, struct ofpbuf *msg,
 \f
 /* Sending asynchronous messages. */
 
-static void schedule_packet_in(struct ofconn *, struct ofputil_packet_in);
+static void schedule_packet_in(struct ofconn *, struct ofproto_packet_in);
 
 /* Sends an OFPT_PORT_STATUS message with 'opp' and 'reason' to appropriate
  * controllers managed by 'mgr'. */
@@ -1488,13 +1488,13 @@ connmgr_send_flow_removed(struct connmgr *mgr,
  * The caller doesn't need to fill in pin->buffer_id or pin->total_len. */
 void
 connmgr_send_packet_in(struct connmgr *mgr,
-                       const struct ofputil_packet_in *pin)
+                       const struct ofproto_packet_in *pin)
 {
     struct ofconn *ofconn;
 
     LIST_FOR_EACH (ofconn, node, &mgr->all_conns) {
-        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->reason)
-            && ofconn->controller_id == pin->controller_id) {
+        if (ofconn_receives_async_msg(ofconn, OAM_PACKET_IN, pin->up.reason)
+            && ofconn->controller_id == pin->up.controller_id) {
             schedule_packet_in(ofconn, *pin);
         }
     }
@@ -1513,15 +1513,15 @@ do_send_packet_in(struct ofpbuf *ofp_packet_in, void *ofconn_)
 /* Takes 'pin', composes an OpenFlow packet-in message from it, and passes it
  * to 'ofconn''s packet scheduler for sending. */
 static void
-schedule_packet_in(struct ofconn *ofconn, struct ofputil_packet_in pin)
+schedule_packet_in(struct ofconn *ofconn, struct ofproto_packet_in pin)
 {
     struct connmgr *mgr = ofconn->connmgr;
     uint16_t controller_max_len;
 
-    pin.total_len = pin.packet_len;
+    pin.up.total_len = pin.up.packet_len;
 
-    if (pin.reason == OFPR_ACTION) {
-        controller_max_len = pin.send_len;  /* max_len */
+    if (pin.up.reason == OFPR_ACTION) {
+        controller_max_len = pin.up.send_len;  /* max_len */
     } else {
         controller_max_len = ofconn->miss_send_len;
     }
@@ -1530,31 +1530,33 @@ schedule_packet_in(struct ofconn *ofconn, struct ofputil_packet_in pin)
      * For OpenFlow 1.2+, OFPCML_NO_BUFFER (== UINT16_MAX) specifies
      * unbuffered.  This behaviour doesn't violate prior versions, too. */
     if (controller_max_len == UINT16_MAX) {
-        pin.buffer_id = UINT32_MAX;
+        pin.up.buffer_id = UINT32_MAX;
     } else if (mgr->fail_open && fail_open_is_active(mgr->fail_open)) {
-        pin.buffer_id = pktbuf_get_null();
+        pin.up.buffer_id = pktbuf_get_null();
     } else if (!ofconn->pktbuf) {
-        pin.buffer_id = UINT32_MAX;
+        pin.up.buffer_id = UINT32_MAX;
     } else {
-        pin.buffer_id = pktbuf_save(ofconn->pktbuf, pin.packet, pin.packet_len,
-                                    pin.fmd.in_port);
+        pin.up.buffer_id = pktbuf_save(ofconn->pktbuf,
+                                       pin.up.packet, pin.up.packet_len,
+                                       pin.up.fmd.in_port);
     }
 
     /* Figure out how much of the packet to send.
      * If not buffered, send the entire packet.  Otherwise, depending on
      * the reason of packet-in, send what requested by the controller. */
-    if (pin.buffer_id == UINT32_MAX) {
-        pin.send_len = pin.packet_len;
+    if (pin.up.buffer_id == UINT32_MAX) {
+        pin.up.send_len = pin.up.packet_len;
     } else {
-        pin.send_len = MIN(pin.packet_len, controller_max_len);
+        pin.up.send_len = MIN(pin.up.packet_len, controller_max_len);
     }
 
     /* Make OFPT_PACKET_IN and hand over to packet scheduler.  It might
      * immediately call into do_send_packet_in() or it might buffer it for a
      * while (until a later call to pinsched_run()). */
-    pinsched_send(ofconn->schedulers[pin.reason == OFPR_NO_MATCH ? 0 : 1],
-                  pin.fmd.in_port,
-                  ofputil_encode_packet_in(&pin, ofconn_get_protocol(ofconn),
+    pinsched_send(ofconn->schedulers[pin.up.reason == OFPR_NO_MATCH ? 0 : 1],
+                  pin.up.fmd.in_port,
+                  ofputil_encode_packet_in(&pin.up,
+                                           ofconn_get_protocol(ofconn),
                                            ofconn->packet_in_format),
                   do_send_packet_in, ofconn);
 }

diff --git a/ofproto/connmgr.h b/ofproto/connmgr.h
index 505a757..4846da9 100644 (file)
--- a/ofproto/connmgr.h
+++ b/ofproto/connmgr.h
@@ -22,6 +22,7 @@
 #include "list.h"
 #include "match.h"
 #include "ofp-errors.h"
+#include "ofp-util.h"
 #include "ofproto.h"
 #include "openflow/nicira-ext.h"
 #include "openvswitch/types.h"
@@ -29,9 +30,6 @@
 struct nlattr;
 struct ofconn;
 struct ofopgroup;
-struct ofputil_flow_removed;
-struct ofputil_packet_in;
-struct ofputil_phy_port;
 struct rule;
 struct simap;
 struct sset;
@@ -64,6 +62,12 @@ enum ofconn_async_msg_type {
     OAM_N_TYPES
 };
 
+/* A packet_in, with extra members to assist in queuing and routing it. */
+struct ofproto_packet_in {
+    struct ofputil_packet_in up;
+    struct list list_node;      /* For queuing. */
+};
+
 /* Basics. */
 struct connmgr *connmgr_create(struct ofproto *ofproto,
                                const char *dpif_name, const char *local_name);
@@ -141,7 +145,7 @@ void connmgr_send_port_status(struct connmgr *,
 void connmgr_send_flow_removed(struct connmgr *,
                                const struct ofputil_flow_removed *);
 void connmgr_send_packet_in(struct connmgr *,
-                            const struct ofputil_packet_in *);
+                            const struct ofproto_packet_in *);
 
 /* Fail-open settings. */
 enum ofproto_fail_mode connmgr_get_fail_mode(const struct connmgr *);

diff --git a/ofproto/fail-open.c b/ofproto/fail-open.c
index 4900c05..40859a4 100644 (file)
--- a/ofproto/fail-open.c
+++ b/ofproto/fail-open.c
@@ -116,7 +116,7 @@ fail_open_is_active(const struct fail_open *fo)
 static void
 send_bogus_packet_ins(struct fail_open *fo)
 {
-    struct ofputil_packet_in pin;
+    struct ofproto_packet_in pin;
     uint8_t mac[ETH_ADDR_LEN];
     struct ofpbuf b;
 
@@ -125,11 +125,11 @@ send_bogus_packet_ins(struct fail_open *fo)
     compose_rarp(&b, mac);
 
     memset(&pin, 0, sizeof pin);
-    pin.packet = b.data;
-    pin.packet_len = b.size;
-    pin.reason = OFPR_NO_MATCH;
-    pin.send_len = b.size;
-    pin.fmd.in_port = OFPP_LOCAL;
+    pin.up.packet = b.data;
+    pin.up.packet_len = b.size;
+    pin.up.reason = OFPR_NO_MATCH;
+    pin.up.send_len = b.size;
+    pin.up.fmd.in_port = OFPP_LOCAL;
     connmgr_send_packet_in(fo->connmgr, &pin);
 
     ofpbuf_uninit(&b);

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index 954e92f..41d8ee4 100644 (file)
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -19,6 +19,7 @@
 #include <stdbool.h>
 #include <inttypes.h>
 
+#include "connmgr.h"
 #include "coverage.h"
 #include "dynamic-string.h"
 #include "dpif.h"
@@ -838,17 +839,17 @@ handle_upcalls(struct udpif *udpif, struct list *upcalls)
         LIST_FOR_EACH (upcall, list_node, upcalls) {
             struct flow_miss *miss = upcall->flow_miss;
             struct ofpbuf *packet = upcall->dpif_upcall.packet;
-            struct ofputil_packet_in *pin;
+            struct ofproto_packet_in *pin;
 
             pin = xmalloc(sizeof *pin);
-            pin->packet = xmemdup(packet->data, packet->size);
-            pin->packet_len = packet->size;
-            pin->reason = OFPR_NO_MATCH;
-            pin->controller_id = 0;
-            pin->table_id = 0;
-            pin->cookie = 0;
-            pin->send_len = 0; /* Not used for flow table misses. */
-            flow_get_metadata(&miss->flow, &pin->fmd);
+            pin->up.packet = xmemdup(packet->data, packet->size);
+            pin->up.packet_len = packet->size;
+            pin->up.reason = OFPR_NO_MATCH;
+            pin->up.controller_id = 0;
+            pin->up.table_id = 0;
+            pin->up.cookie = 0;
+            pin->up.send_len = 0; /* Not used for flow table misses. */
+            flow_get_metadata(&miss->flow, &pin->up.fmd);
             ofproto_dpif_send_packet_in(miss->ofproto, pin);
         }
     }

diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index 7371750..a7026ba 100644 (file)
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -1820,7 +1820,7 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
                           enum ofp_packet_in_reason reason,
                           uint16_t controller_id)
 {
-    struct ofputil_packet_in *pin;
+    struct ofproto_packet_in *pin;
     struct ofpbuf *packet;
     struct flow key;
 
@@ -1844,15 +1844,15 @@ execute_controller_action(struct xlate_ctx *ctx, int len,
                         ctx->xout->odp_actions.size, NULL, NULL);
 
     pin = xmalloc(sizeof *pin);
-    pin->packet_len = packet->size;
-    pin->packet = ofpbuf_steal_data(packet);
-    pin->reason = reason;
-    pin->controller_id = controller_id;
-    pin->table_id = ctx->table_id;
-    pin->cookie = ctx->rule ? rule_dpif_get_flow_cookie(ctx->rule) : 0;
-
-    pin->send_len = len;
-    flow_get_metadata(&ctx->xin->flow, &pin->fmd);
+    pin->up.packet_len = packet->size;
+    pin->up.packet = ofpbuf_steal_data(packet);
+    pin->up.reason = reason;
+    pin->up.controller_id = controller_id;
+    pin->up.table_id = ctx->table_id;
+    pin->up.cookie = ctx->rule ? rule_dpif_get_flow_cookie(ctx->rule) : 0;
+
+    pin->up.send_len = len;
+    flow_get_metadata(&ctx->xin->flow, &pin->up.fmd);
 
     ofproto_dpif_send_packet_in(ctx->xbridge->ofproto, pin);
     ofpbuf_delete(packet);

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 90ce715..f53384d 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -545,11 +545,11 @@ ofproto_dpif_flow_mod(struct ofproto_dpif *ofproto,
  * Takes ownership of 'pin' and pin->packet. */
 void
 ofproto_dpif_send_packet_in(struct ofproto_dpif *ofproto,
-                            struct ofputil_packet_in *pin)
+                            struct ofproto_packet_in *pin)
 {
     if (!guarded_list_push_back(&ofproto->pins, &pin->list_node, 1024)) {
         COVERAGE_INC(packet_in_overflow);
-        free(CONST_CAST(void *, pin->packet));
+        free(CONST_CAST(void *, pin->up.packet));
         free(pin);
     }
 }
@@ -1359,7 +1359,7 @@ destruct(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
     struct rule_dpif *rule, *next_rule;
-    struct ofputil_packet_in *pin, *next_pin;
+    struct ofproto_packet_in *pin, *next_pin;
     struct facet *facet, *next_facet;
     struct cls_cursor cursor;
     struct oftable *table;
@@ -1397,7 +1397,7 @@ destruct(struct ofproto *ofproto_)
     guarded_list_pop_all(&ofproto->pins, &pins);
     LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) {
         list_remove(&pin->list_node);
-        free(CONST_CAST(void *, pin->packet));
+        free(CONST_CAST(void *, pin->up.packet));
         free(pin);
     }
     guarded_list_destroy(&ofproto->pins);
@@ -1428,7 +1428,7 @@ static int
 run_fast(struct ofproto *ofproto_)
 {
     struct ofproto_dpif *ofproto = ofproto_dpif_cast(ofproto_);
-    struct ofputil_packet_in *pin, *next_pin;
+    struct ofproto_packet_in *pin, *next_pin;
     struct list pins;
 
     /* Do not perform any periodic activity required by 'ofproto' while
@@ -1441,7 +1441,7 @@ run_fast(struct ofproto *ofproto_)
     LIST_FOR_EACH_SAFE (pin, next_pin, list_node, &pins) {
         connmgr_send_packet_in(ofproto->up.connmgr, pin);
         list_remove(&pin->list_node);
-        free(CONST_CAST(void *, pin->packet));
+        free(CONST_CAST(void *, pin->up.packet));
         free(pin);
     }
 

diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 51b1979..0f96031 100644 (file)
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -28,6 +28,7 @@
 union user_action_cookie;
 struct dpif_flow_stats;
 struct ofproto_dpif;
+struct ofproto_packet_in;
 struct ofport_dpif;
 struct dpif_backer;
 struct OVS_LOCKABLE rule_dpif;
@@ -97,7 +98,7 @@ int ofproto_dpif_execute_actions(struct ofproto_dpif *, const struct flow *,
                                  struct rule_dpif *, const struct ofpact *,
                                  size_t ofpacts_len, struct ofpbuf *);
 void ofproto_dpif_send_packet_in(struct ofproto_dpif *,
-                                 struct ofputil_packet_in *pin);
+                                 struct ofproto_packet_in *);
 int ofproto_dpif_send_packet(const struct ofport_dpif *, struct ofpbuf *);
 void ofproto_dpif_flow_mod(struct ofproto_dpif *, struct ofputil_flow_mod *);