looks good to me.
Acked-by: pritesh <pritesh.koth...@cisco.com>
On Feb 4, 2014, at 1:19 PM, Ben Pfaff <b...@nicira.com> wrote:
> There are 12 possible ways to specify a tunnel (2 * 2 * 3 == 12):
>
> - Specific in_key or flow-based (2 choices).
>
> - Specific ip_dst or flow-based (2 choices).
>
> - Specific ip_src, wildcarded, or flow-based (3 choices).
>
> Until now, only 6 of the 12 possibilities have been supported. We
> have had a couple of requests to add another. This commit adds all the
> possibilities, so that we won't have to add the other 6 one by one.
>
> Signed-off-by: Ben Pfaff <b...@nicira.com>
> Requested-by: Thomas Morin <thomas.mo...@orange.com>
> ---
> ofproto/tunnel.c | 178 +++++++++++++++++++++++++++++++++++++-----------------
> 1 file changed, 122 insertions(+), 56 deletions(-)
>
> diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
> index 09497a3..38b782f 100644
> --- a/ofproto/tunnel.c
> +++ b/ofproto/tunnel.c
> @@ -60,8 +60,46 @@ struct tnl_port {
>
> static struct ovs_rwlock rwlock = OVS_RWLOCK_INITIALIZER;
>
> -static struct hmap tnl_match_map__ = HMAP_INITIALIZER(&tnl_match_map__);
> -static struct hmap *tnl_match_map OVS_GUARDED_BY(rwlock) = &tnl_match_map__;
> +/* Tunnel matches.
> + *
> + * This module maps packets received over tunnel protocols to vports. The
> + * tunnel protocol and, for some protocols, tunnel-specific information
> (e.g.,
> + * for VXLAN, the UDP destination port number) are always use as part of the
> + * mapping. Which other fields are used for the mapping depends on the
> vports
> + * themselves (the parenthesized notations refer to "struct tnl_match"
> fields):
> + *
> + * - in_key: A vport may match a specific tunnel ID (in_key_flow ==
> false)
> + * or arrange for the tunnel ID to be matched as tunnel.tun_id in the
> + * OpenFlow flow (in_key_flow == true).
> + *
> + * - ip_dst: A vport may match a specific destination IP address
> + * (ip_dst_flow == false) or arrange for the destination IP to be
> matched
> + * as tunnel.ip_dst in the OpenFlow flow (ip_dst_flow == true).
> + *
> + * - ip_src: A vport may match a specific IP source address (ip_src_flow
> ==
> + * false, ip_src != 0), wildcard all source addresses (ip_src_flow ==
> + * false, ip_src == 0), or arrange for the IP source address to be
> + * handled in the OpenFlow flow table (ip_src_flow == true).
> + *
> + * Thus, there are 2 * 2 * 3 == 12 possible ways a vport can match against a
> + * tunnel packet. We number the possibilities for each field in increasing
> + * order as listed in each bullet above. We order the 12 overall
> combinations
> + * in lexicographic order considering in_key first, then ip_dst, then
> + * ip_src. */
> +#define N_MATCH_TYPES (2 * 2 * 3)
> +
> +/* The three possibilities (see above) for vport ip_src matches. */
> +enum ip_src_type {
> + IP_SRC_CFG, /* ip_src must equal configured address. */
> + IP_SRC_ANY, /* Any ip_src is acceptable. */
> + IP_SRC_FLOW /* ip_src is handled in flow table. */
> +};
> +
> +/* Each hmap contains "struct tnl_port"s.
> + * The index is a combination of how each of the fields listed under "Tunnel
> + * matches" above matches, see the final paragraph for ordering. */
> +static struct hmap *tnl_match_maps[N_MATCH_TYPES] OVS_GUARDED_BY(rwlock);
> +static struct hmap **tnl_match_map(const struct tnl_match *);
>
> static struct hmap ofport_map__ = HMAP_INITIALIZER(&ofport_map__);
> static struct hmap *ofport_map OVS_GUARDED_BY(rwlock) = &ofport_map__;
> @@ -70,7 +108,7 @@ 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(const struct flow *) OVS_REQ_RDLOCK(rwlock);
> -static struct tnl_port *tnl_find_exact(struct tnl_match *)
> +static struct tnl_port *tnl_find_exact(struct tnl_match *, struct hmap *)
> OVS_REQ_RDLOCK(rwlock);
> static struct tnl_port *tnl_find_ofport(const struct ofport_dpif *)
> OVS_REQ_RDLOCK(rwlock);
> @@ -92,6 +130,7 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
> struct netdev *netdev,
> const struct netdev_tunnel_config *cfg;
> struct tnl_port *existing_port;
> struct tnl_port *tnl_port;
> + struct hmap **map;
>
> cfg = netdev_get_tunnel_config(netdev);
> ovs_assert(cfg);
> @@ -110,7 +149,8 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
> struct netdev *netdev,
> tnl_port->match.in_key_flow = cfg->in_key_flow;
> tnl_port->match.odp_port = odp_port;
>
> - existing_port = tnl_find_exact(&tnl_port->match);
> + map = tnl_match_map(&tnl_port->match);
> + existing_port = tnl_find_exact(&tnl_port->match, *map);
> if (existing_port) {
> if (warn) {
> struct ds ds = DS_EMPTY_INITIALIZER;
> @@ -125,8 +165,12 @@ tnl_port_add__(const struct ofport_dpif *ofport, const
> struct netdev *netdev,
> }
>
> 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));
> +
> + if (!*map) {
> + *map = xmalloc(sizeof **map);
> + hmap_init(*map);
> + }
> + hmap_insert(*map, &tnl_port->match_node, tnl_hash(&tnl_port->match));
> tnl_port_mod_log(tnl_port, "adding");
> return true;
> }
> @@ -182,8 +226,16 @@ tnl_port_del__(const struct ofport_dpif *ofport)
> OVS_REQ_WRLOCK(rwlock)
>
> tnl_port = tnl_find_ofport(ofport);
> if (tnl_port) {
> + struct hmap **map;
> +
> tnl_port_mod_log(tnl_port, "removing");
> - hmap_remove(tnl_match_map, &tnl_port->match_node);
> + map = tnl_match_map(&tnl_port->match);
> + hmap_remove(*map, &tnl_port->match_node);
> + if (hmap_is_empty(*map)) {
> + hmap_destroy(*map);
> + free(*map);
> + *map = NULL;
> + }
> hmap_remove(ofport_map, &tnl_port->ofport_node);
> netdev_close(tnl_port->netdev);
> free(tnl_port);
> @@ -396,14 +448,16 @@ tnl_find_ofport(const struct ofport_dpif *ofport)
> OVS_REQ_RDLOCK(rwlock)
> }
>
> static struct tnl_port *
> -tnl_find_exact(struct tnl_match *match) OVS_REQ_RDLOCK(rwlock)
> +tnl_find_exact(struct tnl_match *match, struct hmap *map)
> + OVS_REQ_RDLOCK(rwlock)
> {
> - struct tnl_port *tnl_port;
> + if (map) {
> + struct tnl_port *tnl_port;
>
> - HMAP_FOR_EACH_WITH_HASH (tnl_port, match_node, tnl_hash(match),
> - tnl_match_map) {
> - if (!memcmp(match, &tnl_port->match, sizeof *match)) {
> - return tnl_port;
> + HMAP_FOR_EACH_WITH_HASH (tnl_port, match_node, tnl_hash(match), map)
> {
> + if (!memcmp(match, &tnl_port->match, sizeof *match)) {
> + return tnl_port;
> + }
> }
> }
> return NULL;
> @@ -414,55 +468,67 @@ tnl_find_exact(struct tnl_match *match)
> OVS_REQ_RDLOCK(rwlock)
> static struct tnl_port *
> tnl_find(const struct flow *flow) OVS_REQ_RDLOCK(rwlock)
> {
> - enum ip_src_type {
> - IP_SRC_CFG, /* ip_src must equal configured address. */
> - IP_SRC_ANY, /* Any ip_src is acceptable. */
> - IP_SRC_FLOW /* ip_src is handled in flow table. */
> - };
> -
> - struct tnl_match_pattern {
> - bool in_key_flow;
> - bool ip_dst_flow;
> - enum ip_src_type ip_src;
> - };
> -
> - static const struct tnl_match_pattern patterns[] = {
> - { false, false, IP_SRC_CFG }, /* remote_ip, local_ip, in_key. */
> - { false, false, IP_SRC_ANY }, /* remote_ip, in_key. */
> - { true, false, IP_SRC_CFG }, /* remote_ip, local_ip. */
> - { true, false, IP_SRC_ANY }, /* remote_ip. */
> - { true, true, IP_SRC_ANY }, /* Flow-based remote. */
> - { true, true, IP_SRC_FLOW }, /* Flow-based everything. */
> - };
> -
> - const struct tnl_match_pattern *p;
> - struct tnl_match match;
> -
> - memset(&match, 0, sizeof match);
> - match.odp_port = flow->in_port.odp_port;
> - match.pkt_mark = flow->pkt_mark;
> -
> - for (p = patterns; p < &patterns[ARRAY_SIZE(patterns)]; p++) {
> - struct tnl_port *tnl_port;
> -
> - match.in_key_flow = p->in_key_flow;
> - match.in_key = p->in_key_flow ? 0 : flow->tunnel.tun_id;
> -
> - match.ip_dst_flow = p->ip_dst_flow;
> - match.ip_dst = p->ip_dst_flow ? 0 : flow->tunnel.ip_src;
> -
> - match.ip_src_flow = p->ip_src == IP_SRC_FLOW;
> - match.ip_src = p->ip_src == IP_SRC_CFG ? flow->tunnel.ip_dst : 0;
> -
> - tnl_port = tnl_find_exact(&match);
> - if (tnl_port) {
> - return tnl_port;
> + enum ip_src_type ip_src;
> + int in_key_flow;
> + int ip_dst_flow;
> + int i;
> +
> + i = 0;
> + for (in_key_flow = 0; in_key_flow < 2; in_key_flow++) {
> + for (ip_dst_flow = 0; ip_dst_flow < 2; ip_dst_flow++) {
> + for (ip_src = 0; ip_src < 3; ip_src++) {
> + struct hmap *map = tnl_match_maps[i];
> +
> + if (map) {
> + struct tnl_port *tnl_port;
> + struct tnl_match match;
> +
> + memset(&match, 0, sizeof match);
> +
> + /* The apparent mix-up of 'ip_dst' and 'ip_src' below is
> + * correct, because "struct tnl_match" is expressed in
> + * terms of packets being sent out, but we are using it
> + * here as a description of how to treat received
> + * packets. */
> + match.in_key = in_key_flow ? 0 : flow->tunnel.tun_id;
> + match.ip_src = (ip_src == IP_SRC_CFG
> + ? flow->tunnel.ip_dst
> + : 0);
> + match.ip_dst = ip_dst_flow ? 0 : flow->tunnel.ip_src;
> + match.odp_port = flow->in_port.odp_port;
> + match.pkt_mark = flow->pkt_mark;
> + match.in_key_flow = in_key_flow;
> + match.ip_dst_flow = ip_dst_flow;
> + match.ip_src_flow = ip_src == IP_SRC_FLOW;
> +
> + tnl_port = tnl_find_exact(&match, map);
> + if (tnl_port) {
> + return tnl_port;
> + }
> + }
> +
> + i++;
> + }
> }
> }
>
> return NULL;
> }
>
> +/* Returns a pointer to the 'tnl_match_maps' element corresponding to 'm''s
> + * matching criteria. */
> +static struct hmap **
> +tnl_match_map(const struct tnl_match *m)
> +{
> + enum ip_src_type ip_src;
> +
> + ip_src = (m->ip_src_flow ? IP_SRC_FLOW
> + : m->ip_src ? IP_SRC_CFG
> + : IP_SRC_ANY);
> +
> + return &tnl_match_maps[6 * m->in_key_flow + 3 * m->ip_dst_flow + ip_src];
> +}
> +
> static void
> tnl_match_fmt(const struct tnl_match *match, struct ds *ds)
> OVS_REQ_RDLOCK(rwlock)
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
