From b5e16b07237eb6c3daeeb4faeb7d115b396b0111 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Thu, 4 Sep 2008 09:47:05 -0700 Subject: [PATCH] Respin "Make vconns keep track of their names and include them in log messages." This time, leave out the segfaults. --- include/vconn-provider.h | 129 +++++++++++++++++++++++++++++++++++++++ include/vconn.h | 89 +-------------------------- lib/vconn-netlink.c | 5 +- lib/vconn-ssl.c | 9 ++- lib/vconn-stream.c | 5 +- lib/vconn-tcp.c | 1 + lib/vconn-unix.c | 1 + lib/vconn.c | 28 ++++++--- 8 files changed, 163 insertions(+), 104 deletions(-) create mode 100644 include/vconn-provider.h diff --git a/include/vconn-provider.h b/include/vconn-provider.h new file mode 100644 index 000000000..57778e7bb --- /dev/null +++ b/include/vconn-provider.h @@ -0,0 +1,129 @@ +/* Copyright (c) 2008 The Board of Trustees of The Leland Stanford + * Junior University + * + * We are making the OpenFlow specification and associated documentation + * (Software) available for public use and benefit with the expectation + * that others will use, modify and enhance the Software and contribute + * those enhancements back to the community. However, since we would + * like to make the Software available for broadest use, with as few + * restrictions as possible permission is hereby granted, free of + * charge, to any person obtaining a copy of this Software to deal in + * the Software under the copyrights without restriction, including + * without limitation the rights to use, copy, modify, merge, publish, + * distribute, sublicense, and/or sell copies of the Software, and to + * permit persons to whom the Software is furnished to do so, subject to + * the following conditions: + * + * The above copyright notice and this permission notice shall be + * included in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + * + * The name and trademarks of copyright holder(s) may NOT be used in + * advertising or publicity pertaining to the Software or any + * derivatives without specific, written prior permission. + */ + +#ifndef VCONN_PROVIDER_H +#define VCONN_PROVIDER_H 1 + +/* Provider interface, which provide a virtual connection to an OpenFlow + * device. */ + +#include "vconn.h" + +/* Virtual connection to an OpenFlow device. */ +struct vconn { + struct vconn_class *class; + int connect_status; + uint32_t ip; + char *name; +}; + +void vconn_init(struct vconn *, struct vconn_class *, int connect_status, + uint32_t ip, const char *name); + +struct vconn_class { + /* Prefix for connection names, e.g. "nl", "tcp". */ + const char *name; + + /* Attempts to connect to an OpenFlow device. 'name' is the full + * connection name provided by the user, e.g. "nl:0", "tcp:1.2.3.4". This + * name is useful for error messages but must not be modified. + * + * 'suffix' is a copy of 'name' following the colon and may be modified. + * + * Returns 0 if successful, otherwise a positive errno value. If + * successful, stores a pointer to the new connection in '*vconnp'. + * + * The open function must not block waiting for a connection to complete. + * If the connection cannot be completed immediately, it should return + * EAGAIN (not EINPROGRESS, as returned by the connect system call) and + * continue the connection in the background. */ + int (*open)(const char *name, char *suffix, struct vconn **vconnp); + + /* Closes 'vconn' and frees associated memory. */ + void (*close)(struct vconn *vconn); + + /* Tries to complete the connection on 'vconn', which must be an active + * vconn. If 'vconn''s connection is complete, returns 0 if the connection + * was successful or a positive errno value if it failed. If the + * connection is still in progress, returns EAGAIN. + * + * The connect function must not block waiting for the connection to + * complete; instead, it should return EAGAIN immediately. */ + int (*connect)(struct vconn *vconn); + + /* Tries to accept a new connection on 'vconn', which must be a passive + * vconn. If successful, stores the new connection in '*new_vconnp' and + * returns 0. Otherwise, returns a positive errno value. + * + * The accept function must not block waiting for a connection. If no + * connection is ready to be accepted, it should return EAGAIN. + * + * Nonnull iff this is a passive vconn (one that accepts connections and + * does not transfer data). */ + int (*accept)(struct vconn *vconn, struct vconn **new_vconnp); + + /* Tries to receive an OpenFlow message from 'vconn', which must be an + * active vconn. If successful, stores the received message into '*msgp' + * and returns 0. The caller is responsible for destroying the message + * with buffer_delete(). On failure, returns a positive errno value and + * stores a null pointer into '*msgp'. + * + * If the connection has been closed in the normal fashion, returns EOF. + * + * The recv function must not block waiting for a packet to arrive. If no + * packets have been received, it should return EAGAIN. + * + * Nonnull iff this is an active vconn (one that transfers data and does + * not accept connections). */ + int (*recv)(struct vconn *vconn, struct buffer **msgp); + + /* Tries to queue 'msg' for transmission on 'vconn', which must be an + * active vconn. If successful, returns 0, in which case ownership of + * 'msg' is transferred to the vconn. Success does not guarantee that + * 'msg' has been or ever will be delivered to the peer, only that it has + * been queued for transmission. + * + * Returns a positive errno value on failure, in which case the caller + * retains ownership of 'msg'. + * + * The send function must not block. If 'msg' cannot be immediately + * accepted for transmission, it should return EAGAIN. + * + * Nonnull iff this is an active vconn (one that transfers data and does + * not accept connections). */ + int (*send)(struct vconn *vconn, struct buffer *msg); + + void (*wait)(struct vconn *vconn, enum vconn_wait_type); +}; + +#endif /* vconn-provider.h */ diff --git a/include/vconn.h b/include/vconn.h index 22a1da835..ec98859df 100644 --- a/include/vconn.h +++ b/include/vconn.h @@ -42,15 +42,10 @@ struct buffer; struct flow; struct pollfd; struct ofp_header; +struct vconn; -/* Client interface. */ - -/* Virtual connection to an OpenFlow device. */ -struct vconn { - struct vconn_class *class; - int connect_status; - uint32_t ip; -}; +/* Client interface to vconns, which provide a virtual connection to an + * OpenFlow device. */ void vconn_usage(bool active, bool passive); int vconn_open(const char *name, struct vconn **); @@ -94,84 +89,6 @@ struct buffer *make_unbuffered_packet_out(const struct buffer *packet, uint16_t in_port, uint16_t out_port); struct buffer *make_echo_request(void); struct buffer *make_echo_reply(const struct ofp_header *rq); - -/* Provider interface. */ - -struct vconn_class { - /* Prefix for connection names, e.g. "nl", "tcp". */ - const char *name; - - /* Attempts to connect to an OpenFlow device. 'name' is the full - * connection name provided by the user, e.g. "nl:0", "tcp:1.2.3.4". This - * name is useful for error messages but must not be modified. - * - * 'suffix' is a copy of 'name' following the colon and may be modified. - * - * Returns 0 if successful, otherwise a positive errno value. If - * successful, stores a pointer to the new connection in '*vconnp'. - * - * The open function must not block waiting for a connection to complete. - * If the connection cannot be completed immediately, it should return - * EAGAIN (not EINPROGRESS, as returned by the connect system call) and - * continue the connection in the background. */ - int (*open)(const char *name, char *suffix, struct vconn **vconnp); - - /* Closes 'vconn' and frees associated memory. */ - void (*close)(struct vconn *vconn); - - /* Tries to complete the connection on 'vconn', which must be an active - * vconn. If 'vconn''s connection is complete, returns 0 if the connection - * was successful or a positive errno value if it failed. If the - * connection is still in progress, returns EAGAIN. - * - * The connect function must not block waiting for the connection to - * complete; instead, it should return EAGAIN immediately. */ - int (*connect)(struct vconn *vconn); - - /* Tries to accept a new connection on 'vconn', which must be a passive - * vconn. If successful, stores the new connection in '*new_vconnp' and - * returns 0. Otherwise, returns a positive errno value. - * - * The accept function must not block waiting for a connection. If no - * connection is ready to be accepted, it should return EAGAIN. - * - * Nonnull iff this is a passive vconn (one that accepts connections and - * does not transfer data). */ - int (*accept)(struct vconn *vconn, struct vconn **new_vconnp); - - /* Tries to receive an OpenFlow message from 'vconn', which must be an - * active vconn. If successful, stores the received message into '*msgp' - * and returns 0. The caller is responsible for destroying the message - * with buffer_delete(). On failure, returns a positive errno value and - * stores a null pointer into '*msgp'. - * - * If the connection has been closed in the normal fashion, returns EOF. - * - * The recv function must not block waiting for a packet to arrive. If no - * packets have been received, it should return EAGAIN. - * - * Nonnull iff this is an active vconn (one that transfers data and does - * not accept connections). */ - int (*recv)(struct vconn *vconn, struct buffer **msgp); - - /* Tries to queue 'msg' for transmission on 'vconn', which must be an - * active vconn. If successful, returns 0, in which case ownership of - * 'msg' is transferred to the vconn. Success does not guarantee that - * 'msg' has been or ever will be delivered to the peer, only that it has - * been queued for transmission. - * - * Returns a positive errno value on failure, in which case the caller - * retains ownership of 'msg'. - * - * The send function must not block. If 'msg' cannot be immediately - * accepted for transmission, it should return EAGAIN. - * - * Nonnull iff this is an active vconn (one that transfers data and does - * not accept connections). */ - int (*send)(struct vconn *vconn, struct buffer *msg); - - void (*wait)(struct vconn *vconn, enum vconn_wait_type); -}; extern struct vconn_class tcp_vconn_class; extern struct vconn_class ptcp_vconn_class; diff --git a/lib/vconn-netlink.c b/lib/vconn-netlink.c index 403f3fc18..4f609bb33 100644 --- a/lib/vconn-netlink.c +++ b/lib/vconn-netlink.c @@ -51,6 +51,7 @@ #include "socket-util.h" #include "util.h" #include "openflow.h" +#include "vconn-provider.h" #include "vlog.h" #define THIS_MODULE VLM_VCONN_NETLINK @@ -83,9 +84,7 @@ netlink_open(const char *name, char *suffix, struct vconn **vconnp) } netlink = xmalloc(sizeof *netlink); - netlink->vconn.class = &netlink_vconn_class; - netlink->vconn.connect_status = 0; - netlink->vconn.ip = 0; + vconn_init(&netlink->vconn, &netlink_vconn_class, 0, 0, name); retval = dpif_open(dp_idx, subscribe, &netlink->dp); if (retval) { free(netlink); diff --git a/lib/vconn-ssl.c b/lib/vconn-ssl.c index 41fd1d1d6..d553834f0 100644 --- a/lib/vconn-ssl.c +++ b/lib/vconn-ssl.c @@ -52,6 +52,7 @@ #include "ofp-print.h" #include "socket-util.h" #include "vconn.h" +#include "vconn-provider.h" #include "vlog.h" #define THIS_MODULE VLM_vconn_ssl @@ -226,9 +227,8 @@ new_ssl_vconn(const char *name, int fd, enum session_type type, /* Create and return the ssl_vconn. */ sslv = xmalloc(sizeof *sslv); - sslv->vconn.class = &ssl_vconn_class; - sslv->vconn.connect_status = EAGAIN; - sslv->vconn.ip = sin->sin_addr.s_addr; + vconn_init(&sslv->vconn, &ssl_vconn_class, EAGAIN, sin->sin_addr.s_addr, + name); sslv->state = state; sslv->type = type; sslv->fd = fd; @@ -731,8 +731,7 @@ pssl_open(const char *name, char *suffix, struct vconn **vconnp) } pssl = xmalloc(sizeof *pssl); - pssl->vconn.class = &pssl_vconn_class; - pssl->vconn.connect_status = 0; + vconn_init(&pssl->vconn, &pssl_vconn_class, 0, 0, name); pssl->fd = fd; *vconnp = &pssl->vconn; return 0; diff --git a/lib/vconn-stream.c b/lib/vconn-stream.c index beb239c3c..c419d530c 100644 --- a/lib/vconn-stream.c +++ b/lib/vconn-stream.c @@ -46,6 +46,7 @@ #include "poll-loop.h" #include "socket-util.h" #include "vconn.h" +#include "vconn-provider.h" #include "vlog.h" #define THIS_MODULE VLM_vconn_stream @@ -72,9 +73,7 @@ new_stream_vconn(const char *name, int fd, int connect_status, struct stream_vconn *s; s = xmalloc(sizeof *s); - s->vconn.class = &stream_vconn_class; - s->vconn.connect_status = connect_status; - s->vconn.ip = ip; + vconn_init(&s->vconn, &stream_vconn_class, connect_status, ip, name); s->fd = fd; s->txbuf = NULL; s->tx_waiter = NULL; diff --git a/lib/vconn-tcp.c b/lib/vconn-tcp.c index be4cd9574..7c314cc07 100644 --- a/lib/vconn-tcp.c +++ b/lib/vconn-tcp.c @@ -45,6 +45,7 @@ #include "socket-util.h" #include "util.h" #include "openflow.h" +#include "vconn-provider.h" #include "vconn-stream.h" #include "vlog.h" diff --git a/lib/vconn-unix.c b/lib/vconn-unix.c index f8b6cbc20..356109dad 100644 --- a/lib/vconn-unix.c +++ b/lib/vconn-unix.c @@ -50,6 +50,7 @@ #include "ofp-print.h" #include "packets.h" #include "poll-loop.h" +#include "vconn-provider.h" #include "vconn-stream.h" #include "vlog.h" diff --git a/lib/vconn.c b/lib/vconn.c index bac38d91f..3e06dc344 100644 --- a/lib/vconn.c +++ b/lib/vconn.c @@ -32,7 +32,7 @@ */ #include -#include "vconn.h" +#include "vconn-provider.h" #include #include #include @@ -174,6 +174,7 @@ vconn_open(const char *name, struct vconn **vconnp) if (!retval) { assert(vconn->connect_status != EAGAIN || vconn->class->connect); + vconn->name = xstrdup(name); *vconnp = vconn; } return retval; @@ -210,7 +211,9 @@ void vconn_close(struct vconn *vconn) { if (vconn != NULL) { + char *name = vconn->name; (vconn->class->close)(vconn); + free(name); } } @@ -287,15 +290,15 @@ vconn_recv(struct vconn *vconn, struct buffer **msgp) if (VLOG_IS_DBG_ENABLED()) { char *s = ofp_to_string((*msgp)->data, (*msgp)->size, 1); - VLOG_DBG_RL(&rl, "received: %s", s); + VLOG_DBG_RL(&rl, "%s: received: %s", vconn->name, s); free(s); } oh = buffer_at_assert(*msgp, 0, sizeof *oh); if (oh->version != OFP_VERSION) { - VLOG_ERR_RL(&rl, "received OpenFlow version %02"PRIx8" " + VLOG_ERR_RL(&rl, "%s: received OpenFlow version %02"PRIx8" " "!= expected %02x", - oh->version, OFP_VERSION); + vconn->name, oh->version, OFP_VERSION); buffer_delete(*msgp); *msgp = NULL; return EPROTO; @@ -332,7 +335,7 @@ vconn_send(struct vconn *vconn, struct buffer *msg) char *s = ofp_to_string(msg->data, msg->size, 1); retval = (vconn->class->send)(vconn, msg); if (retval != EAGAIN) { - VLOG_DBG_RL(&rl, "sent (%s): %s", strerror(retval), s); + VLOG_DBG_RL(&rl, "%s: sent (%s): %s", vconn->name, strerror(retval), s); } free(s); } @@ -397,8 +400,8 @@ vconn_transact(struct vconn *vconn, struct buffer *request, return 0; } - VLOG_DBG_RL(&rl, "received reply with xid %08"PRIx32" != expected " - "%08"PRIx32, recv_xid, send_xid); + VLOG_DBG_RL(&rl, "%s: received reply with xid %08"PRIx32" != expected " + "%08"PRIx32, vconn->name, recv_xid, send_xid); buffer_delete(reply); } } @@ -602,3 +605,14 @@ make_echo_reply(const struct ofp_header *rq) reply->type = OFPT_ECHO_REPLY; return out; } + +void +vconn_init(struct vconn *vconn, struct vconn_class *class, int connect_status, + uint32_t ip, const char *name) +{ + vconn->class = class; + vconn->connect_status = connect_status; + vconn->ip = ip; + vconn->name = xstrdup(name); +} + -- 2.43.0