Skip to content

Commit

Permalink
Create specific types for ofp and odp port
Browse files Browse the repository at this point in the history
Until now, datapath ports and openflow ports were both represented by
unsigned integers of various sizes. With implicit conversions, etc., it is
easy to mix them up and use one where the other is expected.  This commit
creates two typedefs, ofp_port_t and odp_port_t.  Both of these two types
are marked by "__attribute__((bitwise))" so that sparse can be used to
detect any misuse.

Signed-off-by: Alex Wang <[email protected]>
Signed-off-by: Ben Pfaff <[email protected]>
  • Loading branch information
yew011 authored and blp committed Jun 20, 2013
1 parent e6cc0ba commit 4e022ec
Show file tree
Hide file tree
Showing 60 changed files with 779 additions and 634 deletions.
38 changes: 16 additions & 22 deletions include/openflow/openflow-1.0.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,22 @@
* 0xff00...0xfff7 "reserved" but not assigned a meaning by OpenFlow 1.0
* 0xfff8...0xffff "reserved" OFPP_* ports with assigned meanings
*/
enum ofp_port {
/* Ranges. */
OFPP_MAX = 0xff00, /* Maximum number of physical switch ports. */
OFPP_FIRST_RESV = 0xfff8, /* First assigned reserved port number. */
OFPP_LAST_RESV = 0xffff, /* Last assigned reserved port number. */

/* Reserved output "ports". */
OFPP_IN_PORT = 0xfff8, /* Send the packet out the input port. This
virtual port must be explicitly used
in order to send back out of the input
port. */
OFPP_TABLE = 0xfff9, /* Perform actions in flow table.
NB: This can only be the destination
port for packet-out messages. */
OFPP_NORMAL = 0xfffa, /* Process with normal L2/L3 switching. */
OFPP_FLOOD = 0xfffb, /* All physical ports except input port and
those disabled by STP. */
OFPP_ALL = 0xfffc, /* All physical ports except input port. */
OFPP_CONTROLLER = 0xfffd, /* Send to controller. */
OFPP_LOCAL = 0xfffe, /* Local openflow "port". */
OFPP_NONE = 0xffff /* Not associated with a physical port. */
};

/* Ranges. */
#define OFPP_MAX OFP_PORT_C(0xff00) /* Max # of switch ports. */
#define OFPP_FIRST_RESV OFP_PORT_C(0xfff8) /* First assigned reserved port. */
#define OFPP_LAST_RESV OFP_PORT_C(0xffff) /* Last assigned reserved port. */

/* Reserved output "ports". */
#define OFPP_IN_PORT OFP_PORT_C(0xfff8) /* Where the packet came in. */
#define OFPP_TABLE OFP_PORT_C(0xfff9) /* Perform actions in flow table. */
#define OFPP_NORMAL OFP_PORT_C(0xfffa) /* Process with normal L2/L3. */
#define OFPP_FLOOD OFP_PORT_C(0xfffb) /* All ports except input port and
* ports disabled by STP. */
#define OFPP_ALL OFP_PORT_C(0xfffc) /* All ports except input port. */
#define OFPP_CONTROLLER OFP_PORT_C(0xfffd) /* Send to controller. */
#define OFPP_LOCAL OFP_PORT_C(0xfffe) /* Local openflow "port". */
#define OFPP_NONE OFP_PORT_C(0xffff) /* Not associated with any port. */

/* OpenFlow 1.0 specific capabilities supported by the datapath (struct
* ofp_switch_features, member capabilities). */
Expand Down
4 changes: 2 additions & 2 deletions include/openflow/openflow-1.1.h
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
* an OpenFlow 1.0 reserved port number to or from, respectively, the
* corresponding OpenFlow 1.1 reserved port number.
*/
#define OFPP11_MAX 0xffffff00
#define OFPP11_OFFSET (OFPP11_MAX - OFPP_MAX)
#define OFPP11_MAX OFP11_PORT_C(0xffffff00)
#define OFPP11_OFFSET 0xffff0000 /* OFPP11_MAX - OFPP_MAX */

/* Reserved wildcard port used only for flow mod (delete) and flow stats
* requests. Selects all flows regardless of output port
Expand Down
14 changes: 13 additions & 1 deletion include/openvswitch/types.h
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2011 Nicira, Inc.
* Copyright (c) 2010, 2011, 2013 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -60,4 +60,16 @@ typedef struct {
ovs_be32 hi, lo;
} ovs_32aligned_be64;

/* ofp_port_t represents the port number of a OpenFlow switch.
* odp_port_t represents the port number on the datapath.
* ofp11_port_t represents the OpenFlow-1.1 port number. */
typedef uint16_t OVS_BITWISE ofp_port_t;
typedef uint32_t OVS_BITWISE odp_port_t;
typedef uint32_t OVS_BITWISE ofp11_port_t;

/* Macro functions that cast int types to ofp/odp/ofp11 types. */
#define OFP_PORT_C(X) ((OVS_FORCE ofp_port_t) (X))
#define ODP_PORT_C(X) ((OVS_FORCE odp_port_t) (X))
#define OFP11_PORT_C(X) ((OVS_FORCE ofp11_port_t) (X))

#endif /* openvswitch/types.h */
23 changes: 12 additions & 11 deletions lib/bundle.c
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,14 @@

VLOG_DEFINE_THIS_MODULE(bundle);

static uint16_t
static ofp_port_t
execute_ab(const struct ofpact_bundle *bundle,
bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
bool (*slave_enabled)(ofp_port_t ofp_port, void *aux), void *aux)
{
size_t i;

for (i = 0; i < bundle->n_slaves; i++) {
uint16_t slave = bundle->slaves[i];
ofp_port_t slave = bundle->slaves[i];
if (slave_enabled(slave, aux)) {
return slave;
}
Expand All @@ -51,10 +51,10 @@ execute_ab(const struct ofpact_bundle *bundle,
return OFPP_NONE;
}

static uint16_t
static ofp_port_t
execute_hrw(const struct ofpact_bundle *bundle,
const struct flow *flow, struct flow_wildcards *wc,
bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
bool (*slave_enabled)(ofp_port_t ofp_port, void *aux), void *aux)
{
uint32_t flow_hash, best_hash;
int best, i;
Expand Down Expand Up @@ -85,10 +85,11 @@ execute_hrw(const struct ofpact_bundle *bundle,
* calculate the result. Uses 'slave_enabled' to determine if the slave
* designated by 'ofp_port' is up. Returns the chosen slave, or
* OFPP_NONE if none of the slaves are acceptable. */
uint16_t
ofp_port_t
bundle_execute(const struct ofpact_bundle *bundle,
const struct flow *flow, struct flow_wildcards *wc,
bool (*slave_enabled)(uint16_t ofp_port, void *aux), void *aux)
bool (*slave_enabled)(ofp_port_t ofp_port, void *aux),
void *aux)
{
switch (bundle->algorithm) {
case NX_BD_ALG_HRW:
Expand Down Expand Up @@ -186,7 +187,7 @@ bundle_from_openflow(const struct nx_action_bundle *nab,
}

enum ofperr
bundle_check(const struct ofpact_bundle *bundle, int max_ports,
bundle_check(const struct ofpact_bundle *bundle, ofp_port_t max_ports,
const struct flow *flow)
{
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
Expand All @@ -200,7 +201,7 @@ bundle_check(const struct ofpact_bundle *bundle, int max_ports,
}

for (i = 0; i < bundle->n_slaves; i++) {
uint16_t ofp_port = bundle->slaves[i];
ofp_port_t ofp_port = bundle->slaves[i];
enum ofperr error;

error = ofputil_check_output_port(ofp_port, max_ports);
Expand Down Expand Up @@ -246,7 +247,7 @@ bundle_to_nxast(const struct ofpact_bundle *bundle, struct ofpbuf *openflow)

slaves = ofpbuf_put_zeros(openflow, slaves_len);
for (i = 0; i < bundle->n_slaves; i++) {
slaves[i] = htons(bundle->slaves[i]);
slaves[i] = htons(ofp_to_u16(bundle->slaves[i]));
}
}

Expand All @@ -271,7 +272,7 @@ bundle_parse__(const char *s, char **save_ptr,
bundle = ofpact_put_BUNDLE(ofpacts);

for (;;) {
uint16_t slave_port;
ofp_port_t slave_port;
char *slave;

slave = strtok_r(NULL, ", []", save_ptr);
Expand Down
6 changes: 3 additions & 3 deletions lib/bundle.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,13 +35,13 @@ struct ofpbuf;
*
* See include/openflow/nicira-ext.h for NXAST_BUNDLE specification. */

uint16_t bundle_execute(const struct ofpact_bundle *, const struct flow *,
ofp_port_t bundle_execute(const struct ofpact_bundle *, const struct flow *,
struct flow_wildcards *wc,
bool (*slave_enabled)(uint16_t ofp_port, void *aux),
bool (*slave_enabled)(ofp_port_t ofp_port, void *aux),
void *aux);
enum ofperr bundle_from_openflow(const struct nx_action_bundle *,
struct ofpbuf *ofpact);
enum ofperr bundle_check(const struct ofpact_bundle *, int max_ports,
enum ofperr bundle_check(const struct ofpact_bundle *, ofp_port_t max_ports,
const struct flow *);
void bundle_to_nxast(const struct ofpact_bundle *, struct ofpbuf *of10);
void bundle_parse(const char *, struct ofpbuf *ofpacts);
Expand Down
60 changes: 32 additions & 28 deletions lib/dpif-linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ static int dpif_linux_init(void);
static void open_dpif(const struct dpif_linux_dp *, struct dpif **);
static bool dpif_linux_nln_parse(struct ofpbuf *, void *);
static void dpif_linux_port_changed(const void *vport, void *dpif);
static uint32_t dpif_linux_port_get_pid(const struct dpif *, uint32_t port_no);
static uint32_t dpif_linux_port_get_pid(const struct dpif *,
odp_port_t port_no);

static void dpif_linux_vport_to_ofpbuf(const struct dpif_linux_vport *,
struct ofpbuf *);
Expand Down Expand Up @@ -261,7 +262,7 @@ open_dpif(const struct dpif_linux_dp *dp, struct dpif **dpifp)
static void
destroy_channels(struct dpif_linux *dpif)
{
int i;
unsigned int i;

if (dpif->epoll_fd < 0) {
return;
Expand All @@ -280,7 +281,7 @@ destroy_channels(struct dpif_linux *dpif)
dpif_linux_vport_init(&vport_request);
vport_request.cmd = OVS_VPORT_CMD_SET;
vport_request.dp_ifindex = dpif->dp_ifindex;
vport_request.port_no = i;
vport_request.port_no = u32_to_odp(i);
vport_request.upcall_pid = &upcall_pid;
dpif_linux_vport_transact(&vport_request, NULL, NULL);

Expand All @@ -300,19 +301,20 @@ destroy_channels(struct dpif_linux *dpif)
}

static int
add_channel(struct dpif_linux *dpif, uint32_t port_no, struct nl_sock *sock)
add_channel(struct dpif_linux *dpif, odp_port_t port_no, struct nl_sock *sock)
{
struct epoll_event event;
uint32_t port_idx = odp_to_u32(port_no);

if (dpif->epoll_fd < 0) {
return 0;
}

/* We assume that the datapath densely chooses port numbers, which
* can therefore be used as an index into an array of channels. */
if (port_no >= dpif->uc_array_size) {
int new_size = port_no + 1;
int i;
if (port_idx >= dpif->uc_array_size) {
uint32_t new_size = port_idx + 1;
uint32_t i;

if (new_size > MAX_PORTS) {
VLOG_WARN_RL(&error_rl, "%s: datapath port %"PRIu32" too big",
Expand All @@ -333,29 +335,30 @@ add_channel(struct dpif_linux *dpif, uint32_t port_no, struct nl_sock *sock)

memset(&event, 0, sizeof event);
event.events = EPOLLIN;
event.data.u32 = port_no;
event.data.u32 = port_idx;
if (epoll_ctl(dpif->epoll_fd, EPOLL_CTL_ADD, nl_sock_fd(sock),
&event) < 0) {
return errno;
}

nl_sock_destroy(dpif->channels[port_no].sock);
dpif->channels[port_no].sock = sock;
dpif->channels[port_no].last_poll = LLONG_MIN;
nl_sock_destroy(dpif->channels[port_idx].sock);
dpif->channels[port_idx].sock = sock;
dpif->channels[port_idx].last_poll = LLONG_MIN;

return 0;
}

static void
del_channel(struct dpif_linux *dpif, uint32_t port_no)
del_channel(struct dpif_linux *dpif, odp_port_t port_no)
{
struct dpif_channel *ch;
uint32_t port_idx = odp_to_u32(port_no);

if (dpif->epoll_fd < 0 || port_no >= dpif->uc_array_size) {
if (dpif->epoll_fd < 0 || port_idx >= dpif->uc_array_size) {
return;
}

ch = &dpif->channels[port_no];
ch = &dpif->channels[port_idx];
if (!ch->sock) {
return;
}
Expand Down Expand Up @@ -482,7 +485,7 @@ netdev_to_ovs_vport_type(const struct netdev *netdev)

static int
dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
uint32_t *port_nop)
odp_port_t *port_nop)
{
struct dpif_linux *dpif = dpif_linux_cast(dpif_);
const struct netdev_tunnel_config *tnl_cfg;
Expand Down Expand Up @@ -541,7 +544,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
VLOG_DBG("%s: assigning port %"PRIu32" to netlink pid %"PRIu32,
dpif_name(dpif_), reply.port_no, upcall_pid);
} else {
if (error == EBUSY && *port_nop != UINT32_MAX) {
if (error == EBUSY && *port_nop != ODPP_NONE) {
VLOG_INFO("%s: requested port %"PRIu32" is in use",
dpif_name(dpif_), *port_nop);
}
Expand Down Expand Up @@ -573,7 +576,7 @@ dpif_linux_port_add(struct dpif *dpif_, struct netdev *netdev,
}

static int
dpif_linux_port_del(struct dpif *dpif_, uint32_t port_no)
dpif_linux_port_del(struct dpif *dpif_, odp_port_t port_no)
{
struct dpif_linux *dpif = dpif_linux_cast(dpif_);
struct dpif_linux_vport vport;
Expand All @@ -591,7 +594,7 @@ dpif_linux_port_del(struct dpif *dpif_, uint32_t port_no)
}

static int
dpif_linux_port_query__(const struct dpif *dpif, uint32_t port_no,
dpif_linux_port_query__(const struct dpif *dpif, odp_port_t port_no,
const char *port_name, struct dpif_port *dpif_port)
{
struct dpif_linux_vport request;
Expand Down Expand Up @@ -622,7 +625,7 @@ dpif_linux_port_query__(const struct dpif *dpif, uint32_t port_no,
}

static int
dpif_linux_port_query_by_number(const struct dpif *dpif, uint32_t port_no,
dpif_linux_port_query_by_number(const struct dpif *dpif, odp_port_t port_no,
struct dpif_port *dpif_port)
{
return dpif_linux_port_query__(dpif, port_no, NULL, dpif_port);
Expand All @@ -635,23 +638,24 @@ dpif_linux_port_query_by_name(const struct dpif *dpif, const char *devname,
return dpif_linux_port_query__(dpif, 0, devname, dpif_port);
}

static int
static odp_port_t
dpif_linux_get_max_ports(const struct dpif *dpif OVS_UNUSED)
{
return MAX_PORTS;
return u32_to_odp(MAX_PORTS);
}

static uint32_t
dpif_linux_port_get_pid(const struct dpif *dpif_, uint32_t port_no)
dpif_linux_port_get_pid(const struct dpif *dpif_, odp_port_t port_no)
{
struct dpif_linux *dpif = dpif_linux_cast(dpif_);
uint32_t port_idx = odp_to_u32(port_no);

if (dpif->epoll_fd < 0) {
return 0;
} else {
/* The UINT32_MAX "reserved" port number uses the "ovs-system"'s
/* The ODPP_NONE "reserved" port number uses the "ovs-system"'s
* channel, since it is not heavily loaded. */
int idx = (port_no >= dpif->uc_array_size) ? 0 : port_no;
uint32_t idx = port_idx >= dpif->uc_array_size ? 0 : port_idx;
return nl_sock_pid(dpif->channels[idx].sock);
}
}
Expand Down Expand Up @@ -1562,7 +1566,7 @@ dpif_linux_vport_from_ofpbuf(struct dpif_linux_vport *vport,

vport->cmd = genl->cmd;
vport->dp_ifindex = ovs_header->dp_ifindex;
vport->port_no = nl_attr_get_u32(a[OVS_VPORT_ATTR_PORT_NO]);
vport->port_no = nl_attr_get_odp_port(a[OVS_VPORT_ATTR_PORT_NO]);
vport->type = nl_attr_get_u32(a[OVS_VPORT_ATTR_TYPE]);
vport->name = nl_attr_get_string(a[OVS_VPORT_ATTR_NAME]);
if (a[OVS_VPORT_ATTR_UPCALL_PID]) {
Expand Down Expand Up @@ -1592,8 +1596,8 @@ dpif_linux_vport_to_ofpbuf(const struct dpif_linux_vport *vport,
ovs_header = ofpbuf_put_uninit(buf, sizeof *ovs_header);
ovs_header->dp_ifindex = vport->dp_ifindex;

if (vport->port_no != UINT32_MAX) {
nl_msg_put_u32(buf, OVS_VPORT_ATTR_PORT_NO, vport->port_no);
if (vport->port_no != ODPP_NONE) {
nl_msg_put_odp_port(buf, OVS_VPORT_ATTR_PORT_NO, vport->port_no);
}

if (vport->type != OVS_VPORT_TYPE_UNSPEC) {
Expand Down Expand Up @@ -1624,7 +1628,7 @@ void
dpif_linux_vport_init(struct dpif_linux_vport *vport)
{
memset(vport, 0, sizeof *vport);
vport->port_no = UINT32_MAX;
vport->port_no = ODPP_NONE;
}

/* Executes 'request' in the kernel datapath. If the command fails, returns a
Expand Down
4 changes: 3 additions & 1 deletion lib/dpif-linux.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
#include <stdint.h>
#include <linux/openvswitch.h>

#include "flow.h"

struct ofpbuf;

struct dpif_linux_vport {
Expand All @@ -30,7 +32,7 @@ struct dpif_linux_vport {

/* ovs_vport header. */
int dp_ifindex;
uint32_t port_no; /* UINT32_MAX if unknown. */
odp_port_t port_no; /* ODPP_NONE if unknown. */
enum ovs_vport_type type;

/* Attributes.
Expand Down
Loading

0 comments on commit 4e022ec

Please sign in to comment.