Skip to content

Commit

Permalink
Use ODP ports in dpif layer and below.
Browse files Browse the repository at this point in the history
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 <[email protected]>
  • Loading branch information
Justin Pettit committed Nov 2, 2012
1 parent 9b56fe1 commit ddbfda8
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 28 deletions.
2 changes: 1 addition & 1 deletion lib/dpif-linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions lib/dpif-netdev.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion lib/flow.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions lib/flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand All @@ -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. */
Expand All @@ -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);

Expand Down
27 changes: 14 additions & 13 deletions lib/odp-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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. */
Expand Down
4 changes: 3 additions & 1 deletion lib/odp-util.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
12 changes: 8 additions & 4 deletions ofproto/ofproto-dpif.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
1 change: 0 additions & 1 deletion tests/test-bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
1 change: 0 additions & 1 deletion tests/test-multipath.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/test-odp.c
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit ddbfda8

Please sign in to comment.