[ovs-dev] [xlate v1 17/18] tunnel: Hide 'struct tnl_port' internally.
Ethan Jackson
ethan at nicira.com
Tue Jun 25 01:59:31 UTC 2013
This simplifies the tunnel module's interface and prevents us from
having to sync 'struct tnl_port' once ofproto-dpif and
ofproto-dpif-xlate are disentangled.
Signed-off-by: Ethan Jackson <ethan at nicira.com>
---
ofproto/ofproto-dpif-xlate.c | 4 +--
ofproto/ofproto-dpif.c | 26 +++++++--------
ofproto/ofproto-dpif.h | 2 +-
ofproto/tunnel.c | 73 +++++++++++++++++++++++++-----------------
ofproto/tunnel.h | 11 +++----
5 files changed, 64 insertions(+), 52 deletions(-)
diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
index df3e4b6..b9b899f 100644
--- a/ofproto/ofproto-dpif-xlate.c
+++ b/ofproto/ofproto-dpif-xlate.c
@@ -899,13 +899,13 @@ compose_output_action__(struct xlate_ctx *ctx, ofp_port_t ofp_port,
flow->nw_tos |= dscp;
}
- if (ofport->tnl_port) {
+ if (ofport->is_tunnel) {
/* Save tunnel metadata so that changes made due to
* the Logical (tunnel) Port are not visible for any further
* matches, while explicit set actions on tunnel metadata are.
*/
struct flow_tnl flow_tnl = flow->tunnel;
- odp_port = tnl_port_send(ofport->tnl_port, flow, &ctx->xout->wc);
+ odp_port = tnl_port_send(ofport, flow, &ctx->xout->wc);
if (odp_port == ODPP_NONE) {
xlate_report(ctx, "Tunneling decided against output");
goto out; /* restore flow_nw_tos */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 3594cda..e643d67 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -567,7 +567,7 @@ type_run(const char *type)
char namebuf[NETDEV_VPORT_NAME_BUFSIZE];
const char *dp_port;
- if (!iter->tnl_port) {
+ if (!iter->is_tunnel) {
continue;
}
@@ -593,8 +593,8 @@ type_run(const char *type)
}
iter->odp_port = node ? u32_to_odp(node->data) : ODPP_NONE;
- if (tnl_port_reconfigure(iter, iter->up.netdev, iter->odp_port,
- &iter->tnl_port)) {
+ if (tnl_port_reconfigure(iter, iter->up.netdev,
+ iter->odp_port)) {
backer->need_revalidate = REV_RECONFIGURE;
}
}
@@ -1452,7 +1452,7 @@ port_construct(struct ofport *port_)
port->may_enable = true;
port->stp_port = NULL;
port->stp_state = STP_DISABLED;
- port->tnl_port = NULL;
+ port->is_tunnel = false;
port->peer = NULL;
hmap_init(&port->priorities);
port->realdev_ofp_port = 0;
@@ -1481,7 +1481,8 @@ port_construct(struct ofport *port_)
port->odp_port = dpif_port.port_no;
if (netdev_get_tunnel_config(netdev)) {
- port->tnl_port = tnl_port_add(port, port->up.netdev, port->odp_port);
+ tnl_port_add(port, port->up.netdev, port->odp_port);
+ port->is_tunnel = true;
} else {
/* Sanity-check that a mapping doesn't already exist. This
* shouldn't happen for non-tunnel ports. */
@@ -1522,7 +1523,7 @@ port_destruct(struct ofport *port_)
* happens when the ofproto is being destroyed, since the caller
* assumes that removal of attached ports will happen as part of
* destruction. */
- if (!port->tnl_port) {
+ if (!port->is_tunnel) {
dpif_port_del(ofproto->backer->dpif, port->odp_port);
}
}
@@ -1532,11 +1533,11 @@ port_destruct(struct ofport *port_)
port->peer = NULL;
}
- if (port->odp_port != ODPP_NONE && !port->tnl_port) {
+ if (port->odp_port != ODPP_NONE && !port->is_tunnel) {
hmap_remove(&ofproto->backer->odp_to_ofport_map, &port->odp_port_node);
}
- tnl_port_del(port->tnl_port);
+ tnl_port_del(port);
sset_find_and_delete(&ofproto->ports, devname);
sset_find_and_delete(&ofproto->ghost_ports, devname);
bundle_remove(port_);
@@ -1563,9 +1564,8 @@ port_modified(struct ofport *port_)
cfm_set_netdev(port->cfm, port->up.netdev);
}
- if (port->tnl_port && tnl_port_reconfigure(port, port->up.netdev,
- port->odp_port,
- &port->tnl_port)) {
+ if (port->is_tunnel && tnl_port_reconfigure(port, port->up.netdev,
+ port->odp_port)) {
ofproto_dpif_cast(port->up.ofproto)->backer->need_revalidate =
REV_RECONFIGURE;
}
@@ -2829,7 +2829,7 @@ port_del(struct ofproto *ofproto_, ofp_port_t ofp_port)
sset_find_and_delete(&ofproto->ghost_ports,
netdev_get_name(ofport->up.netdev));
ofproto->backer->need_revalidate = REV_RECONFIGURE;
- if (!ofport->tnl_port) {
+ if (!ofport->is_tunnel) {
error = dpif_port_del(ofproto->backer->dpif, ofport->odp_port);
if (!error) {
/* The caller is going to close ofport->up.netdev. If this is a
@@ -4600,7 +4600,7 @@ facet_push_stats(struct facet *facet, bool may_learn)
facet->prev_used = facet->used;
in_port = get_ofp_port(ofproto, facet->flow.in_port.ofp_port);
- if (in_port && in_port->tnl_port) {
+ if (in_port && in_port->is_tunnel) {
netdev_vport_inc_rx(in_port->up.netdev, &stats);
}
diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h
index 421b77d..396af9b 100644
--- a/ofproto/ofproto-dpif.h
+++ b/ofproto/ofproto-dpif.h
@@ -126,8 +126,8 @@ struct ofport_dpif {
struct bfd *bfd; /* BFD, if any. */
tag_type tag; /* Tag associated with this port. */
bool may_enable; /* May be enabled in bonds. */
+ bool is_tunnel; /* This port is a tunnel. */
long long int carrier_seq; /* Carrier status changes. */
- struct tnl_port *tnl_port; /* Tunnel handle, or null. */
struct ofport_dpif *peer; /* Peer if patch port. */
/* Spanning tree. */
diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 4db1cd3..8fb1e01 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -43,6 +43,7 @@ struct tnl_match {
};
struct tnl_port {
+ struct hmap_node ofport_node;
struct hmap_node match_node;
const struct ofport_dpif *ofport;
@@ -53,25 +54,22 @@ struct tnl_port {
};
static struct hmap tnl_match_map = HMAP_INITIALIZER(&tnl_match_map);
-
-/* Returned to callers when their ofport will never be used to receive or send
- * tunnel traffic. Alternatively, we could ask the caller to delete their
- * ofport, but this would be unclean in the reconfguration case. For the first
- * time, an ofproto provider would have to call ofproto_port_del() on itself.*/
-static struct tnl_port void_tnl_port;
+static struct hmap ofport_map = HMAP_INITIALIZER(&ofport_map);
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5);
static struct vlog_rate_limit dbg_rl = VLOG_RATE_LIMIT_INIT(60, 60);
static struct tnl_port *tnl_find(struct tnl_match *);
static struct tnl_port *tnl_find_exact(struct tnl_match *);
+static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *);
+
static uint32_t tnl_hash(struct tnl_match *);
static void tnl_match_fmt(const struct tnl_match *, struct ds *);
static char *tnl_port_fmt(const struct tnl_port *);
static void tnl_port_mod_log(const struct tnl_port *, const char *action);
static const char *tnl_port_get_name(const struct tnl_port *);
-static struct tnl_port *
+static bool
tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev,
odp_port_t odp_port, bool warn)
{
@@ -107,59 +105,59 @@ tnl_port_add__(const struct ofport_dpif *ofport, const struct netdev *netdev,
ds_destroy(&ds);
free(tnl_port);
}
- return &void_tnl_port;
+ return false;
}
+ hmap_insert(&ofport_map, &tnl_port->ofport_node, hash_pointer(ofport, 0));
hmap_insert(&tnl_match_map, &tnl_port->match_node,
tnl_hash(&tnl_port->match));
tnl_port_mod_log(tnl_port, "adding");
- return tnl_port;
+ return true;
}
/* Adds 'ofport' to the module with datapath port number 'odp_port'. 'ofport's
* must be added before they can be used by the module. 'ofport' must be a
* tunnel. */
-struct tnl_port *
+void
tnl_port_add(const struct ofport_dpif *ofport, const struct netdev *netdev,
odp_port_t odp_port)
{
- return tnl_port_add__(ofport, netdev, odp_port, true);
+ tnl_port_add__(ofport, netdev, odp_port, true);
}
-/* Checks if the tnl_port pointed to by 'tnl_portp' needs reconfiguration due
- * to changes in its netdev_tunnel_config. If it does, updates 'tnl_portp' to
- * point to a new tnl_port and returns true. Otherwise, returns false.
- * 'ofport' and 'odp_port' should be the same as would be passed to
+/* Checks if the tunnel represented by 'ofport' reconfiguration due to changes
+ * in its netdev_tunnel_config. If it does, returns true. Otherwise, returns
+ * false. 'ofport' and 'odp_port' should be the same as would be passed to
* tnl_port_add(). */
bool
tnl_port_reconfigure(const struct ofport_dpif *ofport,
- const struct netdev *netdev, odp_port_t odp_port,
- struct tnl_port **tnl_portp)
+ const struct netdev *netdev, odp_port_t odp_port)
{
- struct tnl_port *tnl_port = *tnl_portp;
+ struct tnl_port *tnl_port = tnl_find_ofport(ofport);
- if (tnl_port == &void_tnl_port) {
- *tnl_portp = tnl_port_add__(ofport, netdev, odp_port, false);
- return *tnl_portp != &void_tnl_port;
- } else if (tnl_port->ofport != ofport
- || tnl_port->netdev != netdev
+ if (!tnl_port) {
+ return tnl_port_add__(ofport, netdev, odp_port, false);
+ } else if (tnl_port->netdev != netdev
|| tnl_port->match.odp_port != odp_port
|| tnl_port->netdev_seq != netdev_change_seq(netdev)) {
VLOG_DBG("reconfiguring %s", tnl_port_get_name(tnl_port));
- tnl_port_del(tnl_port);
- *tnl_portp = tnl_port_add(ofport, netdev, odp_port);
+ tnl_port_del(ofport);
+ tnl_port_add(ofport, netdev, odp_port);
return true;
}
return false;
}
-/* Removes 'tnl_port' from the module. */
+/* Removes 'ofport' from the module. */
void
-tnl_port_del(struct tnl_port *tnl_port)
+tnl_port_del(const struct ofport_dpif *ofport)
{
- if (tnl_port && tnl_port != &void_tnl_port) {
+ struct tnl_port *tnl_port = ofport ? tnl_find_ofport(ofport) : NULL;
+
+ if (tnl_port) {
tnl_port_mod_log(tnl_port, "removing");
hmap_remove(&tnl_match_map, &tnl_port->match_node);
+ hmap_remove(&ofport_map, &tnl_port->ofport_node);
netdev_close(tnl_port->netdev);
free(tnl_port);
}
@@ -219,13 +217,14 @@ tnl_port_receive(const struct flow *flow)
* port that the output should happen on. May return ODPP_NONE if the output
* shouldn't occur. */
odp_port_t
-tnl_port_send(const struct tnl_port *tnl_port, struct flow *flow,
+tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow,
struct flow_wildcards *wc)
{
+ struct tnl_port *tnl_port = tnl_find_ofport(ofport);
const struct netdev_tunnel_config *cfg;
char *pre_flow_str = NULL;
- if (tnl_port == &void_tnl_port) {
+ if (!tnl_port) {
return ODPP_NONE;
}
@@ -298,6 +297,20 @@ tnl_hash(struct tnl_match *match)
}
static struct tnl_port *
+tnl_find_ofport(const struct ofport_dpif *ofport)
+{
+ struct tnl_port *tnl_port;
+
+ HMAP_FOR_EACH_IN_BUCKET (tnl_port, ofport_node, hash_pointer(ofport, 0),
+ &ofport_map) {
+ if (tnl_port->ofport == ofport) {
+ return tnl_port;
+ }
+ }
+ return NULL;
+}
+
+static struct tnl_port *
tnl_find_exact(struct tnl_match *match)
{
struct tnl_port *tnl_port;
diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h
index 19035c3..f175f1a 100644
--- a/ofproto/tunnel.h
+++ b/ofproto/tunnel.h
@@ -30,17 +30,16 @@
struct ofport_dpif;
struct netdev;
-struct tnl_port;
bool tnl_port_reconfigure(const struct ofport_dpif *, const struct netdev *,
- odp_port_t, struct tnl_port **);
+ odp_port_t);
-struct tnl_port *tnl_port_add(const struct ofport_dpif *, const struct netdev *,
- odp_port_t);
-void tnl_port_del(struct tnl_port *);
+void tnl_port_add(const struct ofport_dpif *, const struct netdev *,
+ odp_port_t odp_port);
+void tnl_port_del(const struct ofport_dpif *);
const struct ofport_dpif *tnl_port_receive(const struct flow *);
-odp_port_t tnl_port_send(const struct tnl_port *, struct flow *,
+odp_port_t tnl_port_send(const struct ofport_dpif *, struct flow *,
struct flow_wildcards *wc);
/* Returns true if 'flow' should be submitted to tnl_port_receive(). */
--
1.7.9.5
More information about the dev
mailing list