flow: Separate "flow_t" from "struct odp_flow_key".

The "struct odp_flow_key" used in the kernel datapath is conceptually
separate from the "flow_t" used in userspace, but until now we have
used the latter as a typedef for the former for convenience.  This commit
separates them.  This makes it possible in upcoming commits to change
them independently.

This is cross-ported from the "wdp" branch, which has had it for months.

diff --git a/lib/classifier.c b/lib/classifier.c
index 8d711a1..9a8d494 100644 (file)
--- a/lib/classifier.c
+++ b/lib/classifier.c
@@ -58,7 +58,6 @@ void
 cls_rule_from_flow(const flow_t *flow, uint32_t wildcards,
                    unsigned int priority, struct cls_rule *rule)
 {
-    assert(!flow->reserved[0] && !flow->reserved[1] && !flow->reserved[2]);
     rule->flow = *flow;
     flow_wildcards_init(&rule->wc, wildcards);
     rule->priority = priority;

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 11ad370..2fb1339 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -94,7 +94,7 @@ struct dp_netdev_port {
 /* A flow in dp_netdev's 'flow_table'. */
 struct dp_netdev_flow {
     struct hmap_node node;      /* Element in dp_netdev's 'flow_table'. */
-    flow_t key;
+    struct flow key;
 
     /* Statistics. */
     struct timespec used;       /* Last used time. */
@@ -135,7 +135,7 @@ static int do_del_port(struct dp_netdev *, uint16_t port_no);
 static int dp_netdev_output_control(struct dp_netdev *, const struct ofpbuf *,
                                     int queue_no, int port_no, uint32_t arg);
 static int dp_netdev_execute_actions(struct dp_netdev *,
-                                     struct ofpbuf *, const flow_t *,
+                                     struct ofpbuf *, struct flow *,
                                      const union odp_action *, int n);
 
 static struct dpif_netdev *
@@ -588,11 +588,10 @@ dpif_netdev_port_poll_wait(const struct dpif *dpif_)
 }
 
 static struct dp_netdev_flow *
-dp_netdev_lookup_flow(const struct dp_netdev *dp, const flow_t *key)
+dp_netdev_lookup_flow(const struct dp_netdev *dp, const struct flow *key)
 {
     struct dp_netdev_flow *flow;
 
-    assert(!key->reserved[0] && !key->reserved[1] && !key->reserved[2]);
     HMAP_FOR_EACH_WITH_HASH (flow, node, flow_hash(key, 0), &dp->flow_table) {
         if (flow_equal(&flow->key, key)) {
             return flow;
@@ -601,12 +600,12 @@ dp_netdev_lookup_flow(const struct dp_netdev *dp, const flow_t *key)
     return NULL;
 }
 
+/* The caller must fill in odp_flow->key itself. */
 static void
 answer_flow_query(struct dp_netdev_flow *flow, uint32_t query_flags,
                   struct odp_flow *odp_flow)
 {
     if (flow) {
-        odp_flow->key = flow->key;
         odp_flow->stats.n_packets = flow->packet_count;
         odp_flow->stats.n_bytes = flow->byte_count;
         odp_flow->stats.used_sec = flow->used.tv_sec;
@@ -638,7 +637,10 @@ dpif_netdev_flow_get(const struct dpif *dpif, struct odp_flow flows[], int n)
 
     for (i = 0; i < n; i++) {
         struct odp_flow *odp_flow = &flows[i];
-        answer_flow_query(dp_netdev_lookup_flow(dp, &odp_flow->key),
+        struct flow key;
+
+        odp_flow_key_to_flow(&odp_flow->key, &key);
+        answer_flow_query(dp_netdev_lookup_flow(dp, &key),
                           odp_flow->flags, odp_flow);
     }
     return 0;
@@ -732,8 +734,7 @@ add_flow(struct dpif *dpif, struct odp_flow *odp_flow)
     int error;
 
     flow = xzalloc(sizeof *flow);
-    flow->key = odp_flow->key;
-    memset(flow->key.reserved, 0, sizeof flow->key.reserved);
+    odp_flow_key_to_flow(&odp_flow->key, &flow->key);
 
     error = set_flow_actions(flow, odp_flow);
     if (error) {
@@ -760,8 +761,10 @@ dpif_netdev_flow_put(struct dpif *dpif, struct odp_flow_put *put)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
+    struct flow key;
 
-    flow = dp_netdev_lookup_flow(dp, &put->flow.key);
+    odp_flow_key_to_flow(&put->flow.key, &key);
+    flow = dp_netdev_lookup_flow(dp, &key);
     if (!flow) {
         if (put->flags & ODPPF_CREATE) {
             if (hmap_count(&dp->flow_table) < MAX_FLOWS) {
@@ -791,8 +794,10 @@ dpif_netdev_flow_del(struct dpif *dpif, struct odp_flow *odp_flow)
 {
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct dp_netdev_flow *flow;
+    struct flow key;
 
-    flow = dp_netdev_lookup_flow(dp, &odp_flow->key);
+    odp_flow_key_to_flow(&odp_flow->key, &key);
+    flow = dp_netdev_lookup_flow(dp, &key);
     if (flow) {
         answer_flow_query(flow, 0, odp_flow);
         dp_netdev_free_flow(dp, flow);
@@ -814,7 +819,10 @@ dpif_netdev_flow_list(const struct dpif *dpif, struct odp_flow flows[], int n)
         if (i >= n) {
             break;
         }
-        answer_flow_query(flow, 0, &flows[i++]);
+
+        odp_flow_key_from_flow(&flows[i].key, &flow->key);
+        answer_flow_query(flow, 0, &flows[i]);
+        i++;
     }
     return hmap_count(&dp->flow_table);
 }
@@ -827,7 +835,7 @@ dpif_netdev_execute(struct dpif *dpif,
     struct dp_netdev *dp = get_dp_netdev(dpif);
     struct ofpbuf copy;
     bool mutates;
-    flow_t flow;
+    flow_t key;
     int error;
 
     if (packet->size < ETH_HEADER_LEN || packet->size > UINT16_MAX) {
@@ -852,8 +860,8 @@ dpif_netdev_execute(struct dpif *dpif,
          * if we don't. */
         copy = *packet;
     }
-    flow_extract(&copy, 0, -1, &flow);
-    error = dp_netdev_execute_actions(dp, &copy, &flow, actions, n_actions);
+    flow_extract(&copy, 0, -1, &key);
+    error = dp_netdev_execute_actions(dp, &copy, &key, actions, n_actions);
     if (mutates) {
         ofpbuf_uninit(&copy);
     }
@@ -922,7 +930,7 @@ dpif_netdev_recv_wait(struct dpif *dpif)
 }
 \f
 static void
-dp_netdev_flow_used(struct dp_netdev_flow *flow, const flow_t *key,
+dp_netdev_flow_used(struct dp_netdev_flow *flow, struct flow *key,
                     const struct ofpbuf *packet)
 {
     time_timespec(&flow->used);
@@ -939,7 +947,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port,
                      struct ofpbuf *packet)
 {
     struct dp_netdev_flow *flow;
-    flow_t key;
+    struct flow key;
 
     if (packet->size < ETH_HEADER_LEN) {
         return;
@@ -1075,13 +1083,13 @@ dp_netdev_set_dl_dst(struct ofpbuf *packet, const uint8_t dl_addr[ETH_ADDR_LEN])
 }
 
 static bool
-is_ip(const struct ofpbuf *packet, const flow_t *key)
+is_ip(const struct ofpbuf *packet, const struct flow *key)
 {
     return key->dl_type == htons(ETH_TYPE_IP) && packet->l4;
 }
 
 static void
-dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key,
+dp_netdev_set_nw_addr(struct ofpbuf *packet, struct flow *key,
                       const struct odp_action_nw_addr *a)
 {
     if (is_ip(packet, key)) {
@@ -1107,7 +1115,7 @@ dp_netdev_set_nw_addr(struct ofpbuf *packet, const flow_t *key,
 }
 
 static void
-dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key,
+dp_netdev_set_nw_tos(struct ofpbuf *packet, struct flow *key,
                      const struct odp_action_nw_tos *a)
 {
     if (is_ip(packet, key)) {
@@ -1124,7 +1132,7 @@ dp_netdev_set_nw_tos(struct ofpbuf *packet, const flow_t *key,
 }
 
 static void
-dp_netdev_set_tp_port(struct ofpbuf *packet, const flow_t *key,
+dp_netdev_set_tp_port(struct ofpbuf *packet, struct flow *key,
                       const struct odp_action_tp_port *a)
 {
        if (is_ip(packet, key)) {
@@ -1186,7 +1194,7 @@ dp_netdev_output_control(struct dp_netdev *dp, const struct ofpbuf *packet,
  * screwy or truncated header fields or one whose inner and outer Ethernet
  * address differ. */
 static bool
-dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct odp_flow_key *key)
+dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct flow *key)
 {
     struct arp_eth_header *arp;
     struct eth_header *eth;
@@ -1212,7 +1220,7 @@ dp_netdev_is_spoofed_arp(struct ofpbuf *packet, const struct odp_flow_key *key)
 
 static int
 dp_netdev_execute_actions(struct dp_netdev *dp,
-                          struct ofpbuf *packet, const flow_t *key,
+                          struct ofpbuf *packet, struct flow *key,
                           const union odp_action *actions, int n_actions)
 {
     int i;
@@ -1232,7 +1240,7 @@ dp_netdev_execute_actions(struct dp_netdev *dp,
         case ODPAT_SET_VLAN_VID:
             dp_netdev_modify_vlan_tci(packet, ntohs(a->vlan_vid.vlan_vid),
                                       VLAN_VID_MASK);
-            break;
+             break;
 
         case ODPAT_SET_VLAN_PCP:
             dp_netdev_modify_vlan_tci(packet,

diff --git a/lib/dpif.c b/lib/dpif.c
index b3e82af..a7706e4 100644 (file)
--- a/lib/dpif.c
+++ b/lib/dpif.c
@@ -1106,7 +1106,8 @@ should_log_flow_message(int error)
 
 static void
 log_flow_message(const struct dpif *dpif, int error, const char *operation,
-                 const flow_t *flow, const struct odp_flow_stats *stats,
+                 const struct odp_flow_key *flow,
+                 const struct odp_flow_stats *stats,
                  const union odp_action *actions, size_t n_actions)
 {
     struct ds ds = DS_EMPTY_INITIALIZER;
@@ -1118,7 +1119,7 @@ log_flow_message(const struct dpif *dpif, int error, const char *operation,
     if (error) {
         ds_put_format(&ds, "(%s) ", strerror(error));
     }
-    flow_format(&ds, flow);
+    format_odp_flow_key(&ds, flow);
     if (stats) {
         ds_put_cstr(&ds, ", ");
         format_odp_flow_stats(&ds, stats);

diff --git a/lib/flow.c b/lib/flow.c
index 462df08..f2f7727 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -304,8 +304,6 @@ flow_from_match(const struct ofp_match *match, bool tun_id_from_cookie,
     memcpy(flow->dl_dst, match->dl_dst, ETH_ADDR_LEN);
     flow->nw_tos = match->nw_tos;
     flow->nw_proto = match->nw_proto;
-    memset(flow->reserved, 0, sizeof flow->reserved);
-
     if (flow_wildcards) {
         *flow_wildcards = wildcards;
     }

diff --git a/lib/flow.h b/lib/flow.h
index 603c4ac..7b5f144 100644 (file)
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -31,7 +31,30 @@ struct ds;
 struct ofp_match;
 struct ofpbuf;
 
-typedef struct odp_flow_key flow_t;
+typedef struct flow flow_t;
+struct flow {
+    uint32_t tun_id;            /* Encapsulating tunnel ID. */
+    uint32_t nw_src;            /* IP source address. */
+    uint32_t nw_dst;            /* IP destination address. */
+    uint16_t in_port;           /* Input switch port. */
+    uint16_t dl_vlan;           /* Input VLAN. */
+    uint16_t dl_type;           /* Ethernet frame type. */
+    uint16_t tp_src;            /* TCP/UDP source port. */
+    uint16_t tp_dst;            /* TCP/UDP destination port. */
+    uint8_t dl_src[6];          /* Ethernet source address. */
+    uint8_t dl_dst[6];          /* Ethernet destination address. */
+    uint8_t nw_proto;           /* IP protocol or low 8 bits of ARP opcode. */
+    uint8_t dl_vlan_pcp;        /* Input VLAN priority. */
+    uint8_t nw_tos;             /* IP ToS (DSCP field, 6 bits). */
+};
+
+/* Assert that there are FLOW_SIG_SIZE bytes of significant data in "struct
+ * flow", followed by FLOW_PAD_SIZE bytes of padding. */
+#define FLOW_SIG_SIZE 37
+#define FLOW_PAD_SIZE 3
+BUILD_ASSERT_DECL(offsetof(struct flow, nw_tos) == FLOW_SIG_SIZE - 1);
+BUILD_ASSERT_DECL(sizeof(((struct flow *)0)->nw_tos) == 1);
+BUILD_ASSERT_DECL(sizeof(struct flow) == FLOW_SIG_SIZE + FLOW_PAD_SIZE);
 
 int flow_extract(struct ofpbuf *, uint32_t tun_id, uint16_t in_port, flow_t *);
 void flow_extract_stats(const flow_t *flow, struct ofpbuf *packet,
@@ -50,7 +73,7 @@ static inline size_t flow_hash(const flow_t *, uint32_t basis);
 static inline int
 flow_compare(const flow_t *a, const flow_t *b)
 {
-    return memcmp(a, b, sizeof *a);
+    return memcmp(a, b, FLOW_SIG_SIZE);
 }
 
 static inline bool
@@ -62,9 +85,7 @@ flow_equal(const flow_t *a, const flow_t *b)
 static inline size_t
 flow_hash(const flow_t *flow, uint32_t basis)
 {
-    BUILD_ASSERT_DECL(!(sizeof *flow % sizeof(uint32_t)));
-    return hash_words((const uint32_t *) flow,
-                      sizeof *flow / sizeof(uint32_t), basis);
+    return hash_bytes(flow, FLOW_SIG_SIZE, basis);
 }
 
 /* Information on wildcards for a flow, as a supplement to flow_t. */

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 8278c22..49d9d34 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -39,6 +39,19 @@ odp_actions_add(struct odp_actions *actions, uint16_t type)
     return a;
 }
 
+void
+format_odp_flow_key(struct ds *ds, const struct odp_flow_key *key)
+{
+    ds_put_format(ds, "in_port%04x:vlan%d:pcp%d mac"ETH_ADDR_FMT
+                  "->"ETH_ADDR_FMT" type%04x proto%"PRId8" tos%"PRIu8
+                  " ip"IP_FMT"->"IP_FMT" port%d->%d",
+                  key->in_port, ntohs(key->dl_vlan), key->dl_vlan_pcp,
+                  ETH_ADDR_ARGS(key->dl_src), ETH_ADDR_ARGS(key->dl_dst),
+                  ntohs(key->dl_type), key->nw_proto, key->nw_tos,
+                  IP_ARGS(&key->nw_src), IP_ARGS(&key->nw_dst),
+                  ntohs(key->tp_src), ntohs(key->tp_dst));
+}
+
 void
 format_odp_action(struct ds *ds, const union odp_action *a)
 {
@@ -134,10 +147,46 @@ format_odp_flow_stats(struct ds *ds, const struct odp_flow_stats *s)
 void
 format_odp_flow(struct ds *ds, const struct odp_flow *f)
 {
-    flow_format(ds, &f->key);
+    format_odp_flow_key(ds, &f->key);
     ds_put_cstr(ds, ", ");
     format_odp_flow_stats(ds, &f->stats);
     ds_put_cstr(ds, ", actions:");
     format_odp_actions(ds, f->actions, f->n_actions);
 }
+\f
+void
+odp_flow_key_from_flow(struct odp_flow_key *key, const struct flow *flow)
+{
+    key->tun_id = flow->tun_id;
+    key->nw_src = flow->nw_src;
+    key->nw_dst = flow->nw_dst;
+    key->in_port = flow->in_port;
+    key->dl_vlan = flow->dl_vlan;
+    key->dl_type = flow->dl_type;
+    key->tp_src = flow->tp_src;
+    key->tp_dst = flow->tp_dst;
+    memcpy(key->dl_src, flow->dl_src, ETH_ADDR_LEN);
+    memcpy(key->dl_dst, flow->dl_dst, ETH_ADDR_LEN);
+    key->nw_proto = flow->nw_proto;
+    key->dl_vlan_pcp = flow->dl_vlan_pcp;
+    key->nw_tos = flow->nw_tos;
+    memset(key->reserved, 0, sizeof key->reserved);
+}
 
+void
+odp_flow_key_to_flow(const struct odp_flow_key *key, struct flow *flow)
+{
+    flow->tun_id = key->tun_id;
+    flow->nw_src = key->nw_src;
+    flow->nw_dst = key->nw_dst;
+    flow->in_port = key->in_port;
+    flow->dl_vlan = key->dl_vlan;
+    flow->dl_type = key->dl_type;
+    flow->tp_src = key->tp_src;
+    flow->tp_dst = key->tp_dst;
+    memcpy(flow->dl_src, key->dl_src, ETH_ADDR_LEN);
+    memcpy(flow->dl_dst, key->dl_dst, ETH_ADDR_LEN);
+    flow->nw_proto = key->nw_proto;
+    flow->dl_vlan_pcp = key->dl_vlan_pcp;
+    flow->nw_tos = key->nw_tos;
+}

diff --git a/lib/odp-util.h b/lib/odp-util.h
index 420bde5..9cbf7dd 100644 (file)
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -20,11 +20,14 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <stdint.h>
+#include <string.h>
+#include "hash.h"
 #include "openflow/openflow.h"
 #include "openvswitch/datapath-protocol.h"
 #include "util.h"
 
 struct ds;
+struct flow;
 
 /* The kernel datapaths limits actions to those that fit in a single page of
  * memory, so there is no point in allocating more than that.  */
@@ -78,10 +81,28 @@ odp_port_to_ofp_port(uint16_t odp_port)
     }
 }
 
+void format_odp_flow_key(struct ds *, const struct odp_flow_key *);
 void format_odp_action(struct ds *, const union odp_action *);
 void format_odp_actions(struct ds *, const union odp_action *actions,
                         size_t n_actions);
 void format_odp_flow_stats(struct ds *, const struct odp_flow_stats *);
 void format_odp_flow(struct ds *, const struct odp_flow *);
 
+void odp_flow_key_from_flow(struct odp_flow_key *, const struct flow *);
+void odp_flow_key_to_flow(const struct odp_flow_key *, struct flow *);
+
+static inline bool
+odp_flow_key_equal(const struct odp_flow_key *a, const struct odp_flow_key *b)
+{
+    return !memcmp(a, b, sizeof *a);
+}
+
+static inline size_t
+odp_flow_key_hash(const struct odp_flow_key *flow, uint32_t basis)
+{
+    BUILD_ASSERT_DECL(!(sizeof *flow % sizeof(uint32_t)));
+    return hash_words((const uint32_t *) flow,
+                      sizeof *flow / sizeof(uint32_t), basis);
+}
+
 #endif /* odp-util.h */

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 5e8a1d8..5ac5016 100644 (file)
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -2120,7 +2120,7 @@ do_put_flow(struct ofproto *ofproto, struct rule *rule, int flags,
             struct odp_flow_put *put)
 {
     memset(&put->flow.stats, 0, sizeof put->flow.stats);
-    put->flow.key = rule->cr.flow;
+    odp_flow_key_from_flow(&put->flow.key, &rule->cr.flow);
     put->flow.actions = rule->odp_actions;
     put->flow.n_actions = rule->n_odp_actions;
     put->flow.flags = 0;
@@ -2224,7 +2224,7 @@ rule_uninstall(struct ofproto *p, struct rule *rule)
     if (rule->installed) {
         struct odp_flow odp_flow;
 
-        odp_flow.key = rule->cr.flow;
+        odp_flow_key_from_flow(&odp_flow.key, &rule->cr.flow);
         odp_flow.actions = NULL;
         odp_flow.n_actions = 0;
         odp_flow.flags = 0;
@@ -3199,12 +3199,12 @@ query_stats(struct ofproto *p, struct rule *rule,
     if (rule->cr.wc.wildcards) {
         size_t i = 0;
         LIST_FOR_EACH (subrule, list, &rule->list) {
-            odp_flows[i++].key = subrule->cr.flow;
+            odp_flow_key_from_flow(&odp_flows[i++].key, &subrule->cr.flow);
             packet_count += subrule->packet_count;
             byte_count += subrule->byte_count;
         }
     } else {
-        odp_flows[0].key = rule->cr.flow;
+        odp_flow_key_from_flow(&odp_flows[0].key, &rule->cr.flow);
     }
 
     /* Fetch up-to-date statistics from the datapath and add them in. */
@@ -4282,9 +4282,12 @@ ofproto_update_used(struct ofproto *p)
     for (i = 0; i < n_flows; i++) {
         struct odp_flow *f = &flows[i];
         struct rule *rule;
+        flow_t flow;
+
+        odp_flow_key_to_flow(&f->key, &flow);
 
         rule = rule_from_cls_rule(
-            classifier_find_rule_exactly(&p->cls, &f->key, 0, UINT16_MAX));
+            classifier_find_rule_exactly(&p->cls, &flow, 0, UINT16_MAX));
 
         if (rule && rule->installed) {
             update_time(p, rule, &f->stats);
@@ -4405,7 +4408,7 @@ rule_active_timeout(struct ofproto *ofproto, struct rule *rule)
          * ofproto_update_used() zeroed TCP flags. */
         memset(&odp_flow, 0, sizeof odp_flow);
         if (rule->installed) {
-            odp_flow.key = rule->cr.flow;
+            odp_flow_key_from_flow(&odp_flow.key, &rule->cr.flow);
             odp_flow.flags = ODPFF_ZERO_TCP_FLAGS;
             dpif_flow_get(ofproto->dpif, &odp_flow);
 

diff --git a/tests/test-classifier.c b/tests/test-classifier.c
index 4227c18..e471582 100644 (file)
--- a/tests/test-classifier.c
+++ b/tests/test-classifier.c
@@ -380,7 +380,6 @@ compare_classifiers(struct classifier *cls, struct tcls *tcls)
                ETH_ADDR_LEN);
         flow.nw_proto = nw_proto_values[get_value(&x, N_NW_PROTO_VALUES)];
         flow.nw_tos = nw_tos_values[get_value(&x, N_NW_TOS_VALUES)];
-        memset(flow.reserved, 0, sizeof flow.reserved);
 
         for (include = 1; include <= 3; include++) {
             cr0 = lookup_with_include_bits(cls, &flow, include);