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 f043f8a599d0b13fe2dc3208eb56462739e0d4d0..6b36ef600ac0e711fbdf8ac666fb532783e024a9 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 9ebf35520e4d2e709ea4973123bbe000bd0d963f..08f3abf548194fb204d39d7ba22ca14ca0061a62 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 ad599348b196b79b0ff1e57f743a07b91c96bd53..75087c943515de5747744db787b923ce7a297999 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 9388f20ab16eaebb4c85641881ce06d15a0674cb..5f4b8cb530a0372b70e44e3af653ff18454e6ccc 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 4084e55d626835a8c96bf158dd37a67fad217fd5..caf5188e55cdf0b6a1accf15594ba1c6a307890d 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 b6c5344ba86da7357142bfb7c6655d34d51a91fb..358096e731aee24744ff49d5113b2574df07bde4 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 c07f40bfebacb1afdf5ae22480a6bf5f6d4fa46d..97a94eb006844328475b678a955a48416e1daaf0 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 aa8b6f0f47398ce98445a4d8ed7b147ebbf7a9a3..f2d9b8245156b14b9141f5f1b62c45db317f6b3b 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 b990c1338c9e52383e02301ffa20ea8b9205f473..8a355677d607b1ffe22fea374b8d483fd63ae9cd 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 dd80766f7c155249b36ca519cb8399bb32ef78da..5ed31a9e1bbde326c2b20fa621e11c6fd7a0c02d 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",