Use ODP ports in dpif layer and below.

The current code has a simple mapping between datapath and OpenFlow port
numbers (the port numbers were the same other than OFPP_LOCAL which maps
to datapath port 0).  Since the translation was know at compile time,
this allowed different layers to easily translate between the two, so
the translation often occurred late.

A future commit will break this simple mapping, so this commit draws a
line between where datapath and OpenFlow port numbers are used.  The
ofproto-dpif layer will be responsible for the translations.  Callers
above will use OpenFlow port numbers.  Providers below will use
datapath port numbers.

Signed-off-by: Justin Pettit <jpettit@nicira.com>

diff --git a/lib/dpif-linux.c b/lib/dpif-linux.c
index f043f8a..6b36ef6 100644 (file)
--- a/lib/dpif-linux.c
+++ b/lib/dpif-linux.c
@@ -1402,7 +1402,7 @@ dpif_linux_vport_send(int dp_ifindex, uint32_t port_no,
     flow_extract(&packet, 0, NULL, 0, &flow);
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-    odp_flow_key_from_flow(&key, &flow);
+    odp_flow_key_from_flow(&key, &flow, OVSP_NONE);
 
     ofpbuf_use_stack(&actions, &action, sizeof action);
     nl_msg_put_u32(&actions, OVS_ACTION_ATTR_OUTPUT, port_no);

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 9ebf355..08f3abf 100644 (file)
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -866,7 +866,7 @@ dpif_netdev_flow_dump_next(const struct dpif *dpif, void *state_,
         struct ofpbuf buf;
 
         ofpbuf_use_stack(&buf, &state->keybuf, sizeof state->keybuf);
-        odp_flow_key_from_flow(&buf, &flow->key);
+        odp_flow_key_from_flow(&buf, &flow->key, flow->key.in_port);
 
         *key = buf.data;
         *key_len = buf.size;
@@ -1014,7 +1014,7 @@ dp_netdev_port_input(struct dp_netdev *dp, struct dp_netdev_port *port,
     if (packet->size < ETH_HEADER_LEN) {
         return;
     }
-    flow_extract(packet, 0, NULL, odp_port_to_ofp_port(port->port_no), &key);
+    flow_extract(packet, 0, NULL, port->port_no, &key);
     flow = dp_netdev_lookup_flow(dp, &key);
     if (flow) {
         dp_netdev_flow_used(flow, packet);
@@ -1104,7 +1104,7 @@ dp_netdev_output_userspace(struct dp_netdev *dp, const struct ofpbuf *packet,
 
     buf = &u->buf;
     ofpbuf_init(buf, ODPUTIL_FLOW_KEY_BYTES + 2 + packet->size);
-    odp_flow_key_from_flow(buf, flow);
+    odp_flow_key_from_flow(buf, flow, flow->in_port);
     key_len = buf->size;
     ofpbuf_pull(buf, key_len);
     ofpbuf_reserve(buf, 2);

diff --git a/lib/flow.c b/lib/flow.c
index ad59934..75087c9 100644 (file)
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -509,7 +509,6 @@ void
 flow_wildcards_init_exact(struct flow_wildcards *wc)
 {
     memset(&wc->masks, 0xff, sizeof wc->masks);
-    memset(wc->masks.zeros, 0, sizeof wc->masks.zeros);
 }
 
 /* Returns true if 'wc' matches every packet, false if 'wc' fixes any bits or

diff --git a/lib/flow.h b/lib/flow.h
index 9388f20..5f4b8cb 100644 (file)
--- a/lib/flow.h
+++ b/lib/flow.h
@@ -65,6 +65,14 @@ struct flow_tnl {
     uint8_t ip_ttl;
 };
 
+/*
+* A flow in the network.
+*
+* The meaning of 'in_port' is context-dependent.  In most cases, it is a
+* 16-bit OpenFlow 1.0 port number.  In the software datapath interface (dpif)
+* layer and its implementations (e.g. dpif-linux, dpif-netdev), it is instead
+* a 32-bit datapath port number.
+*/
 struct flow {
     struct flow_tnl tunnel;     /* Encapsulating tunnel parameters. */
     ovs_be64 metadata;          /* OpenFlow Metadata. */
@@ -76,7 +84,9 @@ struct flow {
     ovs_be32 nw_src;            /* IPv4 source address. */
     ovs_be32 nw_dst;            /* IPv4 destination address. */
     ovs_be32 ipv6_label;        /* IPv6 flow label. */
-    uint16_t in_port;           /* OpenFlow port number of input port. */
+    uint32_t in_port;           /* Input port. OpenFlow port number
+                                   unless in DPIF code, in which case it
+                                   is the datapath port number. */
     ovs_be16 vlan_tci;          /* If 802.1Q, TCI | VLAN_CFI; otherwise 0. */
     ovs_be16 dl_type;           /* Ethernet frame type. */
     ovs_be16 tp_src;            /* TCP/UDP source port. */
@@ -89,7 +99,6 @@ struct flow {
     uint8_t arp_tha[6];         /* ARP/ND target hardware address. */
     uint8_t nw_ttl;             /* IP TTL/Hop Limit. */
     uint8_t nw_frag;            /* FLOW_FRAG_* flags. */
-    uint8_t zeros[2];           /* Must be zero. */
 };
 BUILD_ASSERT_DECL(sizeof(struct flow) % 4 == 0);
 

diff --git a/lib/odp-util.c b/lib/odp-util.c
index 4084e55..caf5188 100644 (file)
--- a/lib/odp-util.c
+++ b/lib/odp-util.c
@@ -1284,11 +1284,16 @@ ovs_to_odp_frag(uint8_t nw_frag)
 }
 
 /* Appends a representation of 'flow' as OVS_KEY_ATTR_* attributes to 'buf'.
+ * 'flow->in_port' is ignored (since it is likely to be an OpenFlow port
+ * number rather than a datapath port number).  Instead, if 'odp_in_port'
+ * is anything other than OVSP_NONE, it is included in 'buf' as the input
+ * port.
  *
  * 'buf' must have at least ODPUTIL_FLOW_KEY_BYTES bytes of space, or be
  * capable of being expanded to allow for that much space. */
 void
-odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow)
+odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow,
+                       uint32_t odp_in_port)
 {
     struct ovs_key_ethernet *eth_key;
     size_t encap;
@@ -1301,9 +1306,8 @@ odp_flow_key_from_flow(struct ofpbuf *buf, const struct flow *flow)
         nl_msg_put_be64(buf, OVS_KEY_ATTR_TUN_ID, flow->tunnel.tun_id);
     }
 
-    if (flow->in_port != OFPP_NONE && flow->in_port != OFPP_CONTROLLER) {
-        nl_msg_put_u32(buf, OVS_KEY_ATTR_IN_PORT,
-                       ofp_port_to_odp_port(flow->in_port));
+    if (odp_in_port != OVSP_NONE) {
+        nl_msg_put_u32(buf, OVS_KEY_ATTR_IN_PORT, odp_in_port);
     }
 
     eth_key = nl_msg_put_unspec_uninit(buf, OVS_KEY_ATTR_ETHERNET,
@@ -1757,6 +1761,10 @@ parse_8021q_onward(const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1],
  * structure in 'flow'.  Returns an ODP_FIT_* value that indicates how well
  * 'key' fits our expectations for what a flow key should contain.
  *
+ * The 'in_port' will be the datapath's understanding of the port.  The
+ * caller will need to translate with odp_port_to_ofp_port() if the
+ * OpenFlow port is needed.
+ *
  * This function doesn't take the packet itself as an argument because none of
  * the currently understood OVS_KEY_ATTR_* attributes require it.  Currently,
  * it is always possible to infer which additional attribute(s) should appear
@@ -1768,7 +1776,6 @@ enum odp_key_fitness
 odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
                      struct flow *flow)
 {
-    static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
     const struct nlattr *attrs[OVS_KEY_ATTR_MAX + 1];
     uint64_t expected_attrs;
     uint64_t present_attrs;
@@ -1795,16 +1802,10 @@ odp_flow_key_to_flow(const struct nlattr *key, size_t key_len,
     }
 
     if (present_attrs & (UINT64_C(1) << OVS_KEY_ATTR_IN_PORT)) {
-        uint32_t in_port = nl_attr_get_u32(attrs[OVS_KEY_ATTR_IN_PORT]);
-        if (in_port >= UINT16_MAX || in_port >= OFPP_MAX) {
-            VLOG_ERR_RL(&rl, "in_port %"PRIu32" out of supported range",
-                        in_port);
-            return ODP_FIT_ERROR;
-        }
-        flow->in_port = odp_port_to_ofp_port(in_port);
+        flow->in_port = nl_attr_get_u32(attrs[OVS_KEY_ATTR_IN_PORT]);
         expected_attrs |= UINT64_C(1) << OVS_KEY_ATTR_IN_PORT;
     } else {
-        flow->in_port = OFPP_NONE;
+        flow->in_port = OVSP_NONE;
     }
 
     /* Ethernet header. */

diff --git a/lib/odp-util.h b/lib/odp-util.h
index b6c5344..358096e 100644 (file)
--- a/lib/odp-util.h
+++ b/lib/odp-util.h
@@ -41,6 +41,7 @@ ofp_port_to_odp_port(uint16_t ofp_port)
     case OFPP_LOCAL:
         return OVSP_LOCAL;
     case OFPP_NONE:
+    case OFPP_CONTROLLER:
         return OVSP_NONE;
     default:
         return ofp_port;
@@ -109,7 +110,8 @@ void odp_flow_key_format(const struct nlattr *, size_t, struct ds *);
 int odp_flow_key_from_string(const char *s, const struct simap *port_names,
                              struct ofpbuf *);
 
-void odp_flow_key_from_flow(struct ofpbuf *, const struct flow *);
+void odp_flow_key_from_flow(struct ofpbuf *, const struct flow *,
+                            uint32_t odp_in_port);
 
 uint32_t odp_flow_key_hash(const struct nlattr *, size_t);
 

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index c07f40b..97a94eb 100644 (file)
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -3045,6 +3045,7 @@ ofproto_dpif_extract_flow_key(const struct ofproto_dpif *ofproto,
     enum odp_key_fitness fitness;
 
     fitness = odp_flow_key_to_flow(key, key_len, flow);
+    flow->in_port = odp_port_to_ofp_port(flow->in_port);
     if (fitness == ODP_FIT_ERROR) {
         return fitness;
     }
@@ -3677,7 +3678,7 @@ execute_odp_actions(struct ofproto_dpif *ofproto, const struct flow *flow,
     int error;
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-    odp_flow_key_from_flow(&key, flow);
+    odp_flow_key_from_flow(&key, flow, ofp_port_to_odp_port(flow->in_port));
 
     error = dpif_execute(ofproto->dpif, key.data, key.size,
                          odp_actions, actions_len, packet);
@@ -4326,6 +4327,7 @@ subfacet_find(struct ofproto_dpif *ofproto,
     struct flow flow;
 
     fitness = odp_flow_key_to_flow(key, key_len, &flow);
+    flow.in_port = odp_port_to_ofp_port(flow.in_port);
     if (fitness == ODP_FIT_ERROR) {
         return NULL;
     }
@@ -4374,8 +4376,10 @@ subfacet_get_key(struct subfacet *subfacet, struct odputil_keybuf *keybuf,
                  struct ofpbuf *key)
 {
     if (!subfacet->key) {
+        struct flow *flow = &subfacet->facet->flow;
+
         ofpbuf_use_stack(key, keybuf, sizeof *keybuf);
-        odp_flow_key_from_flow(key, &subfacet->facet->flow);
+        odp_flow_key_from_flow(key, flow, ofp_port_to_odp_port(flow->in_port));
     } else {
         ofpbuf_use_const(key, subfacet->key, subfacet->key_len);
     }
@@ -4773,7 +4777,7 @@ send_packet(const struct ofport_dpif *ofport, struct ofpbuf *packet)
     }
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-    odp_flow_key_from_flow(&key, &flow);
+    odp_flow_key_from_flow(&key, &flow, ofp_port_to_odp_port(flow.in_port));
 
     ofpbuf_init(&odp_actions, 32);
     compose_sflow_action(ofproto, &odp_actions, &flow, odp_port);
@@ -6477,7 +6481,7 @@ packet_out(struct ofproto *ofproto_, struct ofpbuf *packet,
     struct ofpbuf odp_actions;
 
     ofpbuf_use_stack(&key, &keybuf, sizeof keybuf);
-    odp_flow_key_from_flow(&key, flow);
+    odp_flow_key_from_flow(&key, flow, ofp_port_to_odp_port(flow->in_port));
 
     dpif_flow_stats_extract(flow, packet, time_msec(), &stats);
 

diff --git a/tests/test-bundle.c b/tests/test-bundle.c
index aa8b6f0..f2d9b82 100644 (file)
--- a/tests/test-bundle.c
+++ b/tests/test-bundle.c
@@ -136,7 +136,6 @@ main(int argc, char *argv[])
     flows = xmalloc(N_FLOWS * sizeof *flows);
     for (i = 0; i < N_FLOWS; i++) {
         random_bytes(&flows[i], sizeof flows[i]);
-        memset(flows[i].zeros, 0, sizeof flows[i].zeros);
         flows[i].regs[0] = OFPP_NONE;
     }
 

diff --git a/tests/test-multipath.c b/tests/test-multipath.c
index b990c13..8a35567 100644 (file)
--- a/tests/test-multipath.c
+++ b/tests/test-multipath.c
@@ -60,7 +60,6 @@ main(int argc, char *argv[])
             struct flow flow;
 
             random_bytes(&flow, sizeof flow);
-            memset(flow.zeros, 0, sizeof flow.zeros);
 
             mp.max_link = n - 1;
             multipath_execute(&mp, &flow);

diff --git a/tests/test-odp.c b/tests/test-odp.c
index dd80766..5ed31a9 100644 (file)
--- a/tests/test-odp.c
+++ b/tests/test-odp.c
@@ -70,7 +70,7 @@ parse_keys(void)
         /* Convert cls_rule back to odp_key. */
         ofpbuf_uninit(&odp_key);
         ofpbuf_init(&odp_key, 0);
-        odp_flow_key_from_flow(&odp_key, &flow);
+        odp_flow_key_from_flow(&odp_key, &flow, flow.in_port);
 
         if (odp_key.size > ODPUTIL_FLOW_KEY_BYTES) {
             printf ("too long: %zu > %d\n",