Until now, asking ovs-vswitchd to shut down gracefully, e.g. with
"ovs-appctl exit", would cause it to first remove all the ports from
kernel-based datapaths. This has the unfortunate side effect that IP
addresses on any removed "internal" ports are lost, even if the ports are
added again when ovs-vswitchd is restarted. This is long-standing
behavior, but it only became important when the OVS control scripts were
changed to try to do graceful shutdown first instead of using a signal.
This commit changes graceful shutdown so that it leaves ports in the
datapath, fixing the problem.
Fixes: 9b5422a98f8 (ovs-lib: Try to call exit before killing.)
Signed-off-by: Ben Pfaff <b...@ovn.org>
---
ofproto/ofproto-dpif.c | 4 ++--
ofproto/ofproto-provider.h | 28 ++++++++++++++++++++++++++--
ofproto/ofproto.c | 12 ++++++------
ofproto/ofproto.h | 4 ++--
vswitchd/bridge.c | 14 +++++++-------
5 files changed, 43 insertions(+), 19 deletions(-)
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 89e06aa..2b75c76 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1802,7 +1802,7 @@ port_construct(struct ofport *port_)
}
static void
-port_destruct(struct ofport *port_)
+port_destruct(struct ofport *port_, bool del)
{
struct ofport_dpif *port = ofport_dpif_cast(port_);
struct ofproto_dpif *ofproto = ofproto_dpif_cast(port->up.ofproto);
@@ -1817,7 +1817,7 @@ port_destruct(struct ofport *port_)
dp_port_name = netdev_vport_get_dpif_port(port->up.netdev, namebuf,
sizeof namebuf);
- if (dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
+ if (del && dpif_port_exists(ofproto->backer->dpif, dp_port_name)) {
/* The underlying device is still there, so delete it. This
* happens when the ofproto is being destroyed, since the caller
* assumes that removal of attached ports will happen as part of
diff --git a/ofproto/ofproto-provider.h b/ofproto/ofproto-provider.h
index b6aac0a..004335b 100644
--- a/ofproto/ofproto-provider.h
+++ b/ofproto/ofproto-provider.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -899,12 +899,36 @@ struct ofproto_class {
* initialization, and construct and destruct ofports to reflect all of
* the changes.
*
+ * - On graceful shutdown, the base ofproto code will destruct all
+ * the ports.
+ *
* ->port_construct() returns 0 if successful, otherwise a positive errno
* value.
+ *
+ *
+ * ->port_destruct()
+ * =================
+ *
+ * ->port_destruct() takes a 'del' parameter. If the provider implements
+ * the datapath itself (e.g. dpif-netdev, it can ignore 'del'. On the
+ * other hand, if the provider is an interface to an external datapath
+ * (e.g. to a kernel module that implement the datapath) then 'del' should
+ * influence destruction behavior in the following way:
+ *
+ * - If 'del' is true, it should remove the port from the underlying
+ * implementation. This is the common case.
+ *
+ * - If 'del' is false, it should leave the port in the underlying
+ * implementation. This is used when Open vSwitch is performing a
+ * graceful shutdown and ensures that, across Open vSwitch restarts,
+ * the underlying ports are not removed and recreated. That makes an
+ * important difference for, e.g., "internal" ports that have
+ * configured IP addresses; without this distinction, the IP address
+ * and other configured state for the port is lost.
*/
struct ofport *(*port_alloc)(void);
int (*port_construct)(struct ofport *ofport);
- void (*port_destruct)(struct ofport *ofport);
+ void (*port_destruct)(struct ofport *ofport, bool del);
void (*port_dealloc)(struct ofport *ofport);
/* Called after 'ofport->netdev' is replaced by a new netdev object. If
diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index bba30ae..6d21155 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -217,7 +217,7 @@ static void learned_cookies_flush(struct ofproto *, struct
ovs_list *dead_cookie
/* ofport. */
static void ofport_destroy__(struct ofport *) OVS_EXCLUDED(ofproto_mutex);
-static void ofport_destroy(struct ofport *);
+static void ofport_destroy(struct ofport *, bool del);
static int update_port(struct ofproto *, const char *devname);
static int init_ports(struct ofproto *);
@@ -1580,7 +1580,7 @@ ofproto_destroy_defer__(struct ofproto *ofproto)
}
void
-ofproto_destroy(struct ofproto *p)
+ofproto_destroy(struct ofproto *p, bool del)
OVS_EXCLUDED(ofproto_mutex)
{
struct ofport *ofport, *next_ofport;
@@ -1599,7 +1599,7 @@ ofproto_destroy(struct ofproto *p)
ofproto_flush__(p);
HMAP_FOR_EACH_SAFE (ofport, next_ofport, hmap_node, &p->ports) {
- ofport_destroy(ofport);
+ ofport_destroy(ofport, del);
}
HMAP_FOR_EACH_SAFE (usage, next_usage, hmap_node, &p->ofport_usage) {
@@ -2399,7 +2399,7 @@ ofport_remove(struct ofport *ofport)
{
connmgr_send_port_status(ofport->ofproto->connmgr, NULL, &ofport->pp,
OFPPR_DELETE);
- ofport_destroy(ofport);
+ ofport_destroy(ofport, true);
}
/* If 'ofproto' contains an ofport named 'name', removes it from 'ofproto' and
@@ -2485,11 +2485,11 @@ ofport_destroy__(struct ofport *port)
}
static void
-ofport_destroy(struct ofport *port)
+ofport_destroy(struct ofport *port, bool del)
{
if (port) {
dealloc_ofp_port(port->ofproto, port->ofp_port);
- port->ofproto->ofproto_class->port_destruct(port);
+ port->ofproto->ofproto_class->port_destruct(port, del);
ofport_destroy__(port);
}
}
diff --git a/ofproto/ofproto.h b/ofproto/ofproto.h
index 7504027..ebeb590 100644
--- a/ofproto/ofproto.h
+++ b/ofproto/ofproto.h
@@ -1,5 +1,5 @@
/*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -239,7 +239,7 @@ void ofproto_type_wait(const char *datapath_type);
int ofproto_create(const char *datapath, const char *datapath_type,
struct ofproto **ofprotop);
-void ofproto_destroy(struct ofproto *);
+void ofproto_destroy(struct ofproto *, bool del);
int ofproto_delete(const char *name, const char *type);
int ofproto_run(struct ofproto *);
diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index f8324a2..3f3054e 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -227,7 +227,7 @@ static bool ifaces_changed = false;
static void add_del_bridges(const struct ovsrec_open_vswitch *);
static void bridge_run__(void);
static void bridge_create(const struct ovsrec_bridge *);
-static void bridge_destroy(struct bridge *);
+static void bridge_destroy(struct bridge *, bool del);
static struct bridge *bridge_lookup(const char *name);
static unixctl_cb_func bridge_unixctl_dump_flows;
static unixctl_cb_func bridge_unixctl_reconnect;
@@ -500,7 +500,7 @@ bridge_exit(void)
if_notifier_destroy(ifnotifier);
HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
- bridge_destroy(br);
+ bridge_destroy(br, false);
}
ovsdb_idl_destroy(idl);
}
@@ -635,7 +635,7 @@ bridge_reconfigure(const struct ovsrec_open_vswitch
*ovs_cfg)
VLOG_ERR("failed to create bridge %s: %s", br->name,
ovs_strerror(error));
shash_destroy(&br->wanted_ports);
- bridge_destroy(br);
+ bridge_destroy(br, true);
} else {
/* Trigger storing datapath version. */
seq_change(connectivity_seq_get());
@@ -1714,7 +1714,7 @@ add_del_bridges(const struct ovsrec_open_vswitch *cfg)
br->cfg = shash_find_data(&new_br, br->name);
if (!br->cfg || strcmp(br->type, ofproto_normalize_type(
br->cfg->datapath_type))) {
- bridge_destroy(br);
+ bridge_destroy(br, true);
}
}
@@ -2910,7 +2910,7 @@ bridge_run(void)
(long int) getpid());
HMAP_FOR_EACH_SAFE (br, next_br, node, &all_bridges) {
- bridge_destroy(br);
+ bridge_destroy(br, false);
}
/* Since we will not be running system_stats_run() in this process
* with the current situation of multiple ovs-vswitchd daemons,
@@ -3195,7 +3195,7 @@ bridge_create(const struct ovsrec_bridge *br_cfg)
}
static void
-bridge_destroy(struct bridge *br)
+bridge_destroy(struct bridge *br, bool del)
{
if (br) {
struct mirror *mirror, *next_mirror;
@@ -3209,7 +3209,7 @@ bridge_destroy(struct bridge *br)
}
hmap_remove(&all_bridges, &br->node);
- ofproto_destroy(br->ofproto);
+ ofproto_destroy(br->ofproto, del);
hmap_destroy(&br->ifaces);
hmap_destroy(&br->ports);
hmap_destroy(&br->iface_by_name);
--
2.1.3
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
