[ovs-dev] [PATCH 4/4] vconn-unix: Unlink Unix sockets for vconns at close and free memory.
Ben Pfaff
blp at nicira.com
Mon Sep 21 21:53:57 UTC 2009
The make_unix_socket() function that Unix vconns use to create their
bindings calls fatal_signal_add_file_to_unlink() to make sure that the
binding socket gets unlinked from the file system if the process is killed
by a fatal signal. However, this doesn't happen until the process is
actually killed, even if the vconn that owns the socket is actually closed.
This wasn't a problem when the vconn-unix code was written, because all
of the unix vconns were created at process start time and never destroyed
during the normal process runtime. However, these days the vswitch can
create and destroy unix vconns at runtime depending on the contents of its
configuration file, so it's better to clean up the file system and free
the memory required to keep track of these sockets.
This commit makes unix vconns and pvconns delete their files and free
the memory used to track them when the (p)vconns are closed.
This is only a very minor leak most of the time.
Bug #1817.
---
lib/vconn-stream.c | 49 +++++++++++++++++++++++++++++++++++++++++++++----
lib/vconn-stream.h | 3 ++-
lib/vconn-tcp.c | 4 ++--
lib/vconn-unix.c | 14 ++++++++------
4 files changed, 57 insertions(+), 13 deletions(-)
diff --git a/lib/vconn-stream.c b/lib/vconn-stream.c
index e2991dd..0551c9e 100644
--- a/lib/vconn-stream.c
+++ b/lib/vconn-stream.c
@@ -23,6 +23,7 @@
#include <string.h>
#include <sys/types.h>
#include <unistd.h>
+#include "fatal-signal.h"
#include "leak-checker.h"
#include "ofpbuf.h"
#include "openflow/openflow.h"
@@ -44,6 +45,7 @@ struct stream_vconn
struct ofpbuf *rxbuf;
struct ofpbuf *txbuf;
struct poll_waiter *tx_waiter;
+ char *unlink_path;
};
static struct vconn_class stream_vconn_class;
@@ -51,10 +53,20 @@ static struct vconn_class stream_vconn_class;
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(10, 25);
static void stream_clear_txbuf(struct stream_vconn *);
+static void maybe_unlink_and_free(char *path);
+/* Creates a new vconn named 'name' that will send and receive data on 'fd' and
+ * stores a pointer to the vconn in '*vconnp'. Initial connection status
+ * 'connect_status' is interpreted as described for vconn_init().
+ *
+ * When '*vconnp' is closed, then 'unlink_path' (if nonnull) will be passed to
+ * fatal_signal_unlink_file_now() and then freed with free().
+ *
+ * Returns 0 if successful, otherwise a positive errno value. (The current
+ * implementation never fails.) */
int
new_stream_vconn(const char *name, int fd, int connect_status,
- struct vconn **vconnp)
+ char *unlink_path, struct vconn **vconnp)
{
struct stream_vconn *s;
@@ -64,6 +76,7 @@ new_stream_vconn(const char *name, int fd, int connect_status,
s->txbuf = NULL;
s->tx_waiter = NULL;
s->rxbuf = NULL;
+ s->unlink_path = unlink_path;
*vconnp = &s->vconn;
return 0;
}
@@ -83,6 +96,7 @@ stream_close(struct vconn *vconn)
stream_clear_txbuf(s);
ofpbuf_delete(s->rxbuf);
close(s->fd);
+ maybe_unlink_and_free(s->unlink_path);
free(s);
}
@@ -252,6 +266,7 @@ struct pstream_pvconn
int fd;
int (*accept_cb)(int fd, const struct sockaddr *, size_t sa_len,
struct vconn **);
+ char *unlink_path;
};
static struct pvconn_class pstream_pvconn_class;
@@ -263,16 +278,31 @@ pstream_pvconn_cast(struct pvconn *pvconn)
return CONTAINER_OF(pvconn, struct pstream_pvconn, pvconn);
}
+/* Creates a new pvconn named 'name' that will accept new socket connections on
+ * 'fd' and stores a pointer to the vconn in '*pvconnp'.
+ *
+ * When a connection has been accepted, 'accept_cb' will be called with the new
+ * socket fd 'fd' and the remote address of the connection 'sa' and 'sa_len'.
+ * accept_cb must return 0 if the connection is successful, in which case it
+ * must initialize '*vconnp' to the new vconn, or a positive errno value on
+ * error. In either case accept_cb takes ownership of the 'fd' passed in.
+ *
+ * When '*pvconnp' is closed, then 'unlink_path' (if nonnull) will be passed to
+ * fatal_signal_unlink_file_now() and freed with free().
+ *
+ * Returns 0 if successful, otherwise a positive errno value. (The current
+ * implementation never fails.) */
int
new_pstream_pvconn(const char *name, int fd,
- int (*accept_cb)(int fd, const struct sockaddr *,
- size_t sa_len, struct vconn **),
- struct pvconn **pvconnp)
+ int (*accept_cb)(int fd, const struct sockaddr *sa,
+ size_t sa_len, struct vconn **vconnp),
+ char *unlink_path, struct pvconn **pvconnp)
{
struct pstream_pvconn *ps = xmalloc(sizeof *ps);
pvconn_init(&ps->pvconn, &pstream_pvconn_class, name);
ps->fd = fd;
ps->accept_cb = accept_cb;
+ ps->unlink_path = unlink_path;
*pvconnp = &ps->pvconn;
return 0;
}
@@ -282,6 +312,7 @@ pstream_close(struct pvconn *pvconn)
{
struct pstream_pvconn *ps = pstream_pvconn_cast(pvconn);
close(ps->fd);
+ maybe_unlink_and_free(ps->unlink_path);
free(ps);
}
@@ -327,3 +358,13 @@ static struct pvconn_class pstream_pvconn_class = {
pstream_accept,
pstream_wait
};
+
+/* Helper functions. */
+static void
+maybe_unlink_and_free(char *path)
+{
+ if (path) {
+ fatal_signal_unlink_file_now(path);
+ free(path);
+ }
+}
diff --git a/lib/vconn-stream.h b/lib/vconn-stream.h
index fd3d8bd..91904ff 100644
--- a/lib/vconn-stream.h
+++ b/lib/vconn-stream.h
@@ -26,10 +26,11 @@ struct pvconn;
struct sockaddr;
int new_stream_vconn(const char *name, int fd, int connect_status,
- struct vconn **vconnp);
+ char *unlink_path, struct vconn **vconnp);
int new_pstream_pvconn(const char *name, int fd,
int (*accept_cb)(int fd, const struct sockaddr *,
size_t sa_len, struct vconn **),
+ char *unlink_path,
struct pvconn **pvconnp);
#endif /* vconn-stream.h */
diff --git a/lib/vconn-tcp.c b/lib/vconn-tcp.c
index 44f49f1..b4e5234 100644
--- a/lib/vconn-tcp.c
+++ b/lib/vconn-tcp.c
@@ -58,7 +58,7 @@ new_tcp_vconn(const char *name, int fd, int connect_status,
return errno;
}
- retval = new_stream_vconn(name, fd, connect_status, vconnp);
+ retval = new_stream_vconn(name, fd, connect_status, NULL, vconnp);
if (!retval) {
struct vconn *vconn = *vconnp;
vconn_set_remote_ip(vconn, remote->sin_addr.s_addr);
@@ -108,7 +108,7 @@ ptcp_open(const char *name UNUSED, char *suffix, struct pvconn **pvconnp)
if (fd < 0) {
return -fd;
} else {
- return new_pstream_pvconn("ptcp", fd, ptcp_accept, pvconnp);
+ return new_pstream_pvconn("ptcp", fd, ptcp_accept, NULL, pvconnp);
}
}
diff --git a/lib/vconn-unix.c b/lib/vconn-unix.c
index e2b6f7a..f24c846 100644
--- a/lib/vconn-unix.c
+++ b/lib/vconn-unix.c
@@ -47,20 +47,21 @@ static int
unix_open(const char *name, char *suffix, struct vconn **vconnp)
{
const char *connect_path = suffix;
- char bind_path[128];
+ char *bind_path;
int fd;
- sprintf(bind_path, "/tmp/vconn-unix.%ld.%d",
- (long int) getpid(), n_unix_sockets++);
+ bind_path = xasprintf("/tmp/vconn-unix.%ld.%d",
+ (long int) getpid(), n_unix_sockets++);
fd = make_unix_socket(SOCK_STREAM, true, false, bind_path, connect_path);
if (fd < 0) {
VLOG_ERR("%s: connection to %s failed: %s",
bind_path, connect_path, strerror(-fd));
+ free(bind_path);
return -fd;
}
return new_stream_vconn(name, fd, check_connection_completion(fd),
- vconnp);
+ bind_path, vconnp);
}
struct vconn_class unix_vconn_class = {
@@ -102,7 +103,8 @@ punix_open(const char *name UNUSED, char *suffix, struct pvconn **pvconnp)
return error;
}
- return new_pstream_pvconn("punix", fd, punix_accept, pvconnp);
+ return new_pstream_pvconn("punix", fd, punix_accept,
+ xstrdup(suffix), pvconnp);
}
static int
@@ -118,7 +120,7 @@ punix_accept(int fd, const struct sockaddr *sa, size_t sa_len,
} else {
strcpy(name, "unix");
}
- return new_stream_vconn(name, fd, 0, vconnp);
+ return new_stream_vconn(name, fd, 0, NULL, vconnp);
}
struct pvconn_class punix_pvconn_class = {
--
1.6.3.3
More information about the dev
mailing list