learning-switch: Break packet-in processing into two steps.

diff --git a/lib/learning-switch.c b/lib/learning-switch.c
index c837fd0..dc2073d 100644 (file)
--- a/lib/learning-switch.c
+++ b/lib/learning-switch.c
@@ -392,62 +392,77 @@ process_switch_features(struct lswitch *sw, struct rconn *rconn, void *osf_)
     }
 }
 
-static void
-process_packet_in(struct lswitch *sw, struct rconn *rconn, void *opi_)
+static uint16_t
+lswitch_choose_destination(struct lswitch *sw, const flow_t *flow)
 {
-    struct ofp_packet_in *opi = opi_;
-    uint16_t in_port = ntohs(opi->in_port);
-    uint16_t out_port = OFPP_FLOOD;
-
-    size_t pkt_ofs, pkt_len;
-    struct ofpbuf pkt;
-    flow_t flow;
-
-    /* Extract flow data from 'opi' into 'flow'. */
-    pkt_ofs = offsetof(struct ofp_packet_in, data);
-    pkt_len = ntohs(opi->header.length) - pkt_ofs;
-    pkt.data = opi->data;
-    pkt.size = pkt_len;
-    flow_extract(&pkt, 0, in_port, &flow);
+    uint16_t out_port;
 
-    if (may_learn(sw, in_port) && sw->ml) {
-        if (mac_learning_learn(sw->ml, flow.dl_src, 0, in_port,
+    /* Learn the source MAC. */
+    if (may_learn(sw, flow->in_port) && sw->ml) {
+        if (mac_learning_learn(sw->ml, flow->dl_src, 0, flow->in_port,
                                GRAT_ARP_LOCK_NONE)) {
             VLOG_DBG_RL(&rl, "%016llx: learned that "ETH_ADDR_FMT" is on "
                         "port %"PRIu16, sw->datapath_id,
-                        ETH_ADDR_ARGS(flow.dl_src), in_port);
+                        ETH_ADDR_ARGS(flow->dl_src), flow->in_port);
         }
     }
 
     /* Drop frames for reserved multicast addresses. */
-    if (eth_addr_is_reserved(flow.dl_dst)) {
-        goto drop_it;
+    if (eth_addr_is_reserved(flow->dl_dst)) {
+        return OFPP_NONE;
     }
 
-    if (!may_recv(sw, in_port, false)) {
+    if (!may_recv(sw, flow->in_port, false)) {
         /* STP prevents receiving anything on this port. */
-        goto drop_it;
+        return OFPP_NONE;
     }
 
+    out_port = OFPP_FLOOD;
     if (sw->ml) {
-        int learned_port = mac_learning_lookup(sw->ml, flow.dl_dst, 0, NULL);
+        int learned_port = mac_learning_lookup(sw->ml, flow->dl_dst, 0, NULL);
         if (learned_port >= 0 && may_send(sw, learned_port)) {
             out_port = learned_port;
+            if (out_port == flow->in_port) {
+                /* Don't send a packet back out its input port. */
+                return OFPP_NONE;
+            }
         }
     }
 
-    if (in_port == out_port) {
-        /* Don't send out packets on their input ports. */
-        goto drop_it;
-    } else if (sw->max_idle >= 0 && (!sw->ml || out_port != OFPP_FLOOD)) {
+    /* Check if we need to use "NORMAL" action. */
+    if (sw->action_normal && out_port != OFPP_FLOOD) {
+        return OFPP_NORMAL;
+    }
+
+    return out_port;
+}
+
+static void
+process_packet_in(struct lswitch *sw, struct rconn *rconn, void *opi_)
+{
+    struct ofp_packet_in *opi = opi_;
+    uint16_t in_port = ntohs(opi->in_port);
+    uint16_t out_port;
+
+    size_t pkt_ofs, pkt_len;
+    struct ofpbuf pkt;
+    flow_t flow;
+
+    /* Extract flow data from 'opi' into 'flow'. */
+    pkt_ofs = offsetof(struct ofp_packet_in, data);
+    pkt_len = ntohs(opi->header.length) - pkt_ofs;
+    pkt.data = opi->data;
+    pkt.size = pkt_len;
+    flow_extract(&pkt, 0, in_port, &flow);
+
+    /* Choose output port. */
+    out_port = lswitch_choose_destination(sw, &flow);
+
+    /* Send the packet, and possibly the whole flow, to the output port. */
+    if (sw->max_idle >= 0 && (!sw->ml || out_port != OFPP_FLOOD)) {
         struct ofpbuf *buffer;
         struct ofp_flow_mod *ofm;
 
-        /* Check if we need to use "NORMAL" action. */
-        if (sw->action_normal && out_port != OFPP_FLOOD) {
-            out_port = OFPP_NORMAL;
-        }
-
         /* The output port is known, or we always flood everything, so add a
          * new flow. */
         buffer = make_add_simple_flow(&flow, ntohl(opi->buffer_id),
@@ -457,41 +472,24 @@ process_packet_in(struct lswitch *sw, struct rconn *rconn, void *opi_)
         queue_tx(sw, rconn, buffer);
 
         /* If the switch didn't buffer the packet, we need to send a copy. */
-        if (ntohl(opi->buffer_id) == UINT32_MAX) {
+        if (ntohl(opi->buffer_id) == UINT32_MAX && out_port != OFPP_NONE) {
             queue_tx(sw, rconn,
                      make_unbuffered_packet_out(&pkt, in_port, out_port));
         }
     } else {
-        struct ofpbuf *b;
-
-        /* Check if we need to use "NORMAL" action. */
-        if (sw->action_normal && out_port != OFPP_FLOOD) {
-            out_port = OFPP_NORMAL;
-        }
-
         /* We don't know that MAC, or we don't set up flows.  Send along the
          * packet without setting up a flow. */
         if (ntohl(opi->buffer_id) == UINT32_MAX) {
-            b = make_unbuffered_packet_out(&pkt, in_port, out_port);
+            if (out_port != OFPP_NONE) {
+                queue_tx(sw, rconn,
+                         make_unbuffered_packet_out(&pkt, in_port, out_port));
+            }
         } else {
-            b = make_buffered_packet_out(ntohl(opi->buffer_id),
-                                         in_port, out_port);
+            queue_tx(sw, rconn,
+                     make_buffered_packet_out(ntohl(opi->buffer_id),
+                                              in_port, out_port));
         }
-        queue_tx(sw, rconn, b);
-    }
-    return;
-
-drop_it:
-    if (sw->max_idle >= 0) {
-        /* Set up a flow to drop packets. */
-        queue_tx(sw, rconn, make_add_flow(&flow, ntohl(opi->buffer_id),
-                                          sw->max_idle, 0));
-    } else {
-        /* Just drop the packet, since we don't set up flows at all.
-         * XXX we should send a packet_out with no actions if buffer_id !=
-         * UINT32_MAX, to avoid clogging the kernel buffers. */
     }
-    return;
 }
 
 static void

diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index 16a3eee..cedeb67 100644 (file)
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -186,14 +186,19 @@ make_add_simple_flow(const flow_t *flow,
                      uint32_t buffer_id, uint16_t out_port,
                      uint16_t idle_timeout)
 {
-    struct ofp_action_output *oao;
-    struct ofpbuf *buffer = make_add_flow(flow, buffer_id, idle_timeout,
-                                          sizeof *oao);
-    oao = ofpbuf_put_zeros(buffer, sizeof *oao);
-    oao->type = htons(OFPAT_OUTPUT);
-    oao->len = htons(sizeof *oao);
-    oao->port = htons(out_port);
-    return buffer;
+    if (out_port != OFPP_NONE) {
+        struct ofp_action_output *oao;
+        struct ofpbuf *buffer;
+
+        buffer = make_add_flow(flow, buffer_id, idle_timeout, sizeof *oao);
+        oao = ofpbuf_put_zeros(buffer, sizeof *oao);
+        oao->type = htons(OFPAT_OUTPUT);
+        oao->len = htons(sizeof *oao);
+        oao->port = htons(out_port);
+        return buffer;
+    } else {
+        return make_add_flow(flow, buffer_id, idle_timeout, 0);
+    }
 }
 
 struct ofpbuf *
@@ -259,12 +264,16 @@ struct ofpbuf *
 make_buffered_packet_out(uint32_t buffer_id,
                          uint16_t in_port, uint16_t out_port)
 {
-    struct ofp_action_output action;
-    action.type = htons(OFPAT_OUTPUT);
-    action.len = htons(sizeof action);
-    action.port = htons(out_port);
-    return make_packet_out(NULL, buffer_id, in_port,
-                           (struct ofp_action_header *) &action, 1);
+    if (out_port != OFPP_NONE) {
+        struct ofp_action_output action;
+        action.type = htons(OFPAT_OUTPUT);
+        action.len = htons(sizeof action);
+        action.port = htons(out_port);
+        return make_packet_out(NULL, buffer_id, in_port,
+                               (struct ofp_action_header *) &action, 1);
+    } else {
+        return make_packet_out(NULL, buffer_id, in_port, NULL, 0);
+    }
 }
 
 /* Creates and returns an OFPT_ECHO_REQUEST message with an empty payload. */