[ovs-dev] [PATCH] bridge: Immediately drop interfaces that can't be opened.

Justin Pettit jpettit at nicira.com
Thu Apr 29 22:48:33 UTC 2010


Looks good to me.

--Justin


On Apr 29, 2010, at 3:21 PM, Jesse Gross wrote:

> Previously we would keep interfaces around that couldn't be opened
> because they might be internal interfaces that are created later.
> However, this leads to a race condition if the interface appears
> after we try to create it and fails since some operations may
> succeed.  Instead, give up on the interface immediately if it can't
> be opened and isn't internal (which we control and so won't have
> this issue).
> ---
> vswitchd/bridge.c |   29 +++++++++++++++++++----------
> 1 files changed, 19 insertions(+), 10 deletions(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 6254251..c883cf5 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1474,7 +1474,13 @@ bridge_reconfigure_one(const struct ovsrec_open_vswitch *ovs_cfg,
>         if (!port) {
>             port = port_create(br, node->name);
>         }
> +
>         port_reconfigure(port, node->data);
> +        if (!port->n_ifaces) {
> +            VLOG_WARN("bridge %s: port %s has no interfaces, dropping",
> +                      br->name, port->name);
> +            port_destroy(port);
> +        }
>     }
>     shash_destroy(&old_ports);
>     shash_destroy(&new_ports);
> @@ -3604,6 +3610,19 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
>     iface->netdev = NULL;
>     iface->cfg = if_cfg;
> 
> +    /* Attempt to create the network interface in case it doesn't exist yet. */
> +    if (!iface_is_internal(port->bridge, iface->name)) {
> +        error = set_up_iface(if_cfg, iface, true);
> +        if (error) {
> +            VLOG_WARN("could not create iface %s: %s", iface->name,
> +                      strerror(error));
> +
> +            free(iface->name);
> +            free(iface);
> +            return NULL;
> +        }
> +    }
> +
>     if (port->n_ifaces >= port->allocated_ifaces) {
>         port->ifaces = x2nrealloc(port->ifaces, &port->allocated_ifaces,
>                                   sizeof *port->ifaces);
> @@ -3613,16 +3632,6 @@ iface_create(struct port *port, const struct ovsrec_interface *if_cfg)
>         port->bridge->has_bonded_ports = true;
>     }
> 
> -    /* Attempt to create the network interface in case it
> -     * doesn't exist yet. */
> -    if (!iface_is_internal(port->bridge, iface->name)) {
> -        error = set_up_iface(if_cfg, iface, true);
> -        if (error) {
> -            VLOG_WARN("could not create iface %s: %s", iface->name,
> -                    strerror(error));
> -        }
> -    }
> -
>     VLOG_DBG("attached network device %s to port %s", iface->name, port->name);
> 
>     bridge_flush(port->bridge);
> -- 
> 1.7.0.4
> 
> 
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev_openvswitch.org





More information about the dev mailing list