From 42943cdef10197befc9acc8c6fdb632a210c055f Mon Sep 17 00:00:00 2001 From: Ethan Jackson Date: Tue, 18 Jun 2013 15:24:33 -0700 Subject: [PATCH] tunnel: Hide 'struct tnl_port' internally. 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 Acked-by: Ben Pfaff --- 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 78617a292..0e7b9a076 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -904,13 +904,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 95371724d..f8a191215 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -571,7 +571,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; } @@ -597,8 +597,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; } } @@ -1457,7 +1457,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; @@ -1486,7 +1486,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. */ @@ -1527,7 +1528,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); } } @@ -1537,11 +1538,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_); @@ -1568,9 +1569,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; } @@ -3060,7 +3060,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 @@ -4840,7 +4840,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 493ee4cb4..070429737 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -133,8 +133,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 0f95527be..4b7f3046c 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; } @@ -300,6 +299,20 @@ tnl_hash(struct tnl_match *match) return hash_words((uint32_t *) match, sizeof *match / sizeof(uint32_t), 0); } +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) { diff --git a/ofproto/tunnel.h b/ofproto/tunnel.h index 19035c3ef..f175f1a15 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(). */ -- 2.43.0