[ovs-dev] [RFC flow tunnels 7/8] lib: Switch to flow based tunneling.

Jarno Rajahalme jarno.rajahalme at nsn.com
Fri Jan 11 16:56:54 UTC 2013


On Jan 10, 2013, at 1:43 , ext Ethan Jackson wrote:

> @@ -1531,12 +1550,19 @@ port_construct(struct ofport *port_)
> 
>     port->odp_port = dpif_port.port_no;
> 
> -    /* Sanity-check that a mapping doesn't already exist.  This
> -     * shouldn't happen. */
> -    if (odp_port_to_ofp_port(ofproto, port->odp_port) != OFPP_NONE) {
> -        VLOG_ERR("port %s already has an OpenFlow port number\n",
> -                 dpif_port.name);
> -        return EBUSY;
> +    if (!netdev_get_tunnel_config(port->up.netdev)) {
> +        /* Sanity-check that a mapping doesn't already exist.  This
> +         * shouldn't happen for non-tunnel ports. */
> +        if (odp_port_to_ofp_port(ofproto, port->odp_port) != OFPP_NONE) {
> +            VLOG_ERR("port %s already has an OpenFlow port number\n",
> +                     dpif_port.name);
> +            return EBUSY;
> +        }
> +    }
> +
> +    error = tnl_port_add(&port->up, port->odp_port);
> +    if (error) {
> +        return error;
>     }
> 
>     hmap_insert(&ofproto->backer->odp_to_ofport_map, &port->odp_port_node,

The last line makes the odp_to_ofport_map a "multimap", when multiple tunnels
with the same ODP port number are inserted there.  After that, the results for the
map use will depend on which ofproto makes the call using the map, as for
example odp_port_to_ofp_port() and get_odp_port() would fail depending on
which of the ofproto_dpif's happens to have its ofport_dpif first in the hash bucket.

It would be more consistent if calls like get_odp_port() never work for any tunnel ports,
rather than have them fail randomly when multiple ofprotos are running simultaneously.

It seems to me that this map is mostly used for VLAN splinters, bonds and bundles which
should probably not deal with tunnels anyway.

Indeed, it seems that skipping the hmap_insert() and the corresponding hmap_remove()
for tunnel ports results in running code, i.e., all of the current tests pass.

On the same note, maybe the tunnel maps should live inside the backer like the
odp_to_ofport_map? Now, when the tnl_match_map and ofport_map are in global
scope, they will be shared across all dpif's, even across different types, which may
have overlapping ODP port numbers.
 
  Jarno




More information about the dev mailing list