[ovs-dev] [PATCH 06/11] ofproto-dpif: Deferred setting of packet layer pointers.

Jarno Rajahalme jarno.rajahalme at nsn.com
Mon Feb 11 14:46:22 UTC 2013


Signed-off-by: Jarno Rajahalme <jarno.rajahalme at nsn.com>
---
 lib/packets.h          |   10 +++++-
 ofproto/in-band.c      |   29 ++++++++++--------
 ofproto/ofproto-dpif.c |   80 ++++++++++++++++++++++++++++++------------------
 3 files changed, 76 insertions(+), 43 deletions(-)

diff --git a/lib/packets.h b/lib/packets.h
index 0f97fe6..428d702 100644
--- a/lib/packets.h
+++ b/lib/packets.h
@@ -27,8 +27,8 @@
 #include "openvswitch/types.h"
 #include "random.h"
 #include "util.h"
+#include "ofpbuf.h"
 
-struct ofpbuf;
 struct ds;
 
 bool dpid_from_string(const char *s, uint64_t *dpidp);
@@ -586,4 +586,12 @@ void packet_set_udp_port(struct ofpbuf *, ovs_be16 src, ovs_be16 dst);
 uint8_t packet_get_tcp_flags(const struct ofpbuf *, const struct flow *);
 void packet_format_tcp_flags(struct ds *, uint8_t);
 
+static inline void packet_check_pointers(struct ofpbuf *packet) {
+    /* Need to set packet pointers l2_5/l3/l4/l7 ? */
+    if (packet->l3 == NULL) {
+        struct flow flow;
+        flow_extract_l2_onwards(packet, &flow);
+    }
+}
+
 #endif /* packets.h */
diff --git a/ofproto/in-band.c b/ofproto/in-band.c
index 3b98005..ae43e12 100644
--- a/ofproto/in-band.c
+++ b/ofproto/in-band.c
@@ -233,20 +233,23 @@ in_band_msg_in_hook(struct in_band *in_band, const struct flow *flow,
     if (flow->dl_type == htons(ETH_TYPE_IP)
             && flow->nw_proto == IPPROTO_UDP
             && flow->tp_src == htons(DHCP_SERVER_PORT)
-            && flow->tp_dst == htons(DHCP_CLIENT_PORT)
-            && packet->l7) {
-        struct dhcp_header *dhcp;
-
-        dhcp = ofpbuf_at(packet, (char *)packet->l7 - (char *)packet->data,
-                         sizeof *dhcp);
-        if (!dhcp) {
-            return false;
-        }
+            && flow->tp_dst == htons(DHCP_CLIENT_PORT)) {
+        /* Deferred setting of packet layer pointers? */
+        packet_check_pointers(CONST_CAST(struct ofpbuf *, packet));
+        if (packet->l7) {
+            struct dhcp_header *dhcp;
+
+            dhcp = ofpbuf_at(packet, (char *)packet->l7 - (char *)packet->data,
+                             sizeof *dhcp);
+            if (!dhcp) {
+                return false;
+            }
 
-        refresh_local(in_band);
-        if (!eth_addr_is_zero(in_band->local_mac)
-            && eth_addr_equals(dhcp->chaddr, in_band->local_mac)) {
-            return true;
+            refresh_local(in_band);
+            if (!eth_addr_is_zero(in_band->local_mac)
+                && eth_addr_equals(dhcp->chaddr, in_band->local_mac)) {
+                return true;
+            }
         }
     }
 
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 594f3f3..d0459e5 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3321,17 +3321,23 @@ process_special(struct ofproto_dpif *ofproto, const struct flow *flow,
 
     if (ofport->cfm && cfm_should_process_flow(ofport->cfm, flow)) {
         if (packet) {
+            /* Deferred setting of packet layer pointers? */
+            packet_check_pointers(CONST_CAST(struct ofpbuf *, packet));
             cfm_process_heartbeat(ofport->cfm, packet);
         }
         return SLOW_CFM;
     } else if (ofport->bundle && ofport->bundle->lacp
                && flow->dl_type == htons(ETH_TYPE_LACP)) {
         if (packet) {
+            /* Deferred setting of packet layer pointers? */
+            packet_check_pointers(CONST_CAST(struct ofpbuf *, packet));
             lacp_process_packet(ofport->bundle->lacp, ofport, packet);
         }
         return SLOW_LACP;
     } else if (ofproto->stp && stp_should_process_flow(flow)) {
         if (packet) {
+            /* Deferred setting of packet layer pointers? */
+            packet_check_pointers(CONST_CAST(struct ofpbuf *, packet));
             stp_process_packet(ofport, packet);
         }
         return SLOW_STP;
@@ -3521,6 +3527,8 @@ handle_flow_miss_with_facet(struct flow_miss *miss, struct facet *facet,
 
         packet_stats_extract(packet, now, &stats);
         if (facet->has_fin_timeout || ofproto->netflow) {
+            packet_check_pointers(packet); /* Deferred setting of packet layer
+                                            * pointers? */
             stats.tcp_flags = packet_get_tcp_flags(packet, &facet->flow);
         }
         subfacet_update_stats(subfacet, &stats);
@@ -3642,7 +3650,7 @@ drop_key_clear(struct dpif_backer *backer)
     }
 }
 
-/* Given a datpath, packet, and flow metadata ('backer', 'packet', and 'key'
+/* Given a datapath, packet, and flow metadata ('backer', 'packet', and 'key'
  * respectively), populates 'flow' with the result of odp_flow_key_to_flow().
  * Optionally, if nonnull, populates 'fitnessp' with the fitness of 'flow' as
  * returned by odp_flow_key_to_flow().  Also, optionally populates 'ofproto'
@@ -3792,7 +3800,6 @@ do_handle_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls,
         struct flow_miss *existing_miss;
         struct ofproto_dpif *ofproto;
         uint32_t odp_in_port;
-        struct flow flow;
         uint32_t hash;
         int error;
 
@@ -3810,38 +3817,50 @@ do_handle_upcalls(struct dpif_backer *backer, struct dpif_upcall *upcalls,
         }
 
         error = ofproto_receive(backer, upcall->packet, upcall->key,
-                                upcall->key_len, &flow, &miss->key_fitness,
+                                upcall->key_len, &miss->flow, &miss->key_fitness,
                                 &ofproto, &odp_in_port, &miss->initial_tci);
-        if (error == ENODEV) {
-            struct drop_key *drop_key;
-
-            /* Received packet on port for which we couldn't associate
-             * an ofproto.  This can happen if a port is removed while
-             * traffic is being received.  Print a rate-limited message
-             * in case it happens frequently.  Install a drop flow so
-             * that future packets of the flow are inexpensively dropped
-             * in the kernel. */
-            VLOG_INFO_RL(&rl, "received packet on unassociated port %"PRIu32,
-                         flow.in_port);
-
-            drop_key = drop_key_lookup(backer, upcall->key, upcall->key_len);
-            if (!drop_key) {
-                drop_key = xmalloc(sizeof *drop_key);
-                drop_key->key = xmemdup(upcall->key, upcall->key_len);
-                drop_key->key_len = upcall->key_len;
-
-                hmap_insert(&backer->drop_keys, &drop_key->hmap_node,
-                            hash_bytes(drop_key->key, drop_key->key_len, 0));
-                dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
-                              drop_key->key, drop_key->key_len, NULL, 0, NULL);
+        if (error) {
+            if (error == ENODEV) {
+                struct drop_key *drop_key;
+
+                /* Received packet on port for which we couldn't associate
+                 * an ofproto.  This can happen if a port is removed while
+                 * traffic is being received.  Print a rate-limited message
+                 * in case it happens frequently.  Install a drop flow so
+                 * that future packets of the flow are inexpensively dropped
+                 * in the kernel. */
+                VLOG_INFO_RL(&rl, "received packet on unassociated port %"PRIu32,
+                             miss->flow.in_port);
+
+                drop_key = drop_key_lookup(backer, upcall->key, upcall->key_len);
+                if (!drop_key) {
+                    drop_key = xmalloc(sizeof *drop_key);
+                    drop_key->key = xmemdup(upcall->key, upcall->key_len);
+                    drop_key->key_len = upcall->key_len;
+
+                    hmap_insert(&backer->drop_keys, &drop_key->hmap_node,
+                                hash_bytes(drop_key->key, drop_key->key_len, 0));
+                    dpif_flow_put(backer->dpif, DPIF_FP_CREATE | DPIF_FP_MODIFY,
+                                  drop_key->key, drop_key->key_len, NULL, 0, NULL);
+                }
             }
             continue;
         }
-        if (error) {
-            continue;
+
+        if (miss->key_fitness == ODP_FIT_PERFECT) {
+            /* We have a perfect key fitness, keep the flow.
+             * This is safe because MISS_UPCALLs never have the
+             * packet modified by any actions before being passed to us. */
+
+            /* Init l2 layer pointer and leave the rest for later */
+            upcall->packet->l2 = upcall->packet->data;
+        } else {
+            /* The packet may have been modified since the key extraction,
+             * or the kernel provided key may otherwise be insufficient.
+             * Do full flow key extraction, but keep the metadata.
+             */
+            flow_extract_l2_onwards(upcall->packet, &miss->flow);
         }
-        flow_extract(upcall->packet, flow.skb_priority, flow.skb_mark,
-                     &flow.tunnel, flow.in_port, &miss->flow);
 
         /* Add other packets to a to-do list. */
         hash = flow_hash(&miss->flow, 0);
@@ -5935,6 +5954,9 @@ execute_controller_action(struct action_xlate_ctx *ctx, int len,
         return;
     }
 
+    /* Deferred setting of packet layer pointers? */
+    packet_check_pointers(CONST_CAST(struct ofpbuf *, ctx->packet));
+
     packet = ofpbuf_clone(ctx->packet);
 
     if (packet->l2 && packet->l3) {
-- 
1.7.10.4




More information about the dev mailing list