Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

libknet: rework knet_link_enable into knet_link_set_enable

make it thread safe and fix all return codes (ret vs errno)

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
  • Loading branch information...
commit b2302b3f225ee0d6afdfd5001afbad47bae412db 1 parent 9250826
@fabbione authored
View
6 kronosnetd/vty_cli_cmds.c
@@ -897,7 +897,7 @@ static int knet_cmd_no_link(struct knet_vty *vty)
knet_link_get_status(knet_iface->cfg_ring.knet_h, vty->host_id, vty->link_id, &status);
if (status.enabled) {
- if (knet_link_enable(knet_iface->cfg_ring.knet_h, vty->host_id, vty->link_id, 0)) {
+ if (knet_link_set_enable(knet_iface->cfg_ring.knet_h, vty->host_id, vty->link_id, 0)) {
knet_vty_write(vty, "Error: unable to update switching cache%s", telnet_newline);
return -1;
}
@@ -955,7 +955,7 @@ static int knet_cmd_link(struct knet_vty *vty)
knet_link_set_timeout(knet_iface->cfg_ring.knet_h, vty->host_id, vty->link_id, 1000, 5000, 2048);
- knet_link_enable(knet_iface->cfg_ring.knet_h, vty->host_id, vty->link_id, 1);
+ knet_link_set_enable(knet_iface->cfg_ring.knet_h, vty->host_id, vty->link_id, 1);
}
vty->node = NODE_LINK;
@@ -1390,7 +1390,7 @@ static int knet_cmd_no_interface(struct knet_vty *vty)
knet_host_get_host_list(knet_iface->cfg_ring.knet_h, host_ids, &host_ids_entries);
for (j = 0; j < host_ids_entries; j++) {
for (i = 0; i < KNET_MAX_LINK; i++) {
- knet_link_enable(knet_iface->cfg_ring.knet_h, host_ids[j], i, 0);
+ knet_link_set_enable(knet_iface->cfg_ring.knet_h, host_ids[j], i, 0);
}
}
View
8 libknet/handle.c
@@ -38,6 +38,13 @@ static int _init_locks(knet_handle_t knet_h)
knet_h->lock_init_done = 1;
+ savederrno = pthread_rwlock_init(&knet_h->listener_rwlock, NULL);
+ if (savederrno) {
+ log_err(knet_h, KNET_SUB_HANDLE, "Unable to initialize listener rwlock: %s",
+ strerror(savederrno));
+ goto exit_fail;
+ }
+
savederrno = pthread_rwlock_init(&knet_h->host_rwlock, NULL);
if (savederrno) {
log_err(knet_h, KNET_SUB_HANDLE, "Unable to initialize host rwlock: %s",
@@ -70,6 +77,7 @@ static void _destroy_locks(knet_handle_t knet_h)
{
knet_h->lock_init_done = 0;
pthread_rwlock_destroy(&knet_h->list_rwlock);
+ pthread_rwlock_destroy(&knet_h->listener_rwlock);
pthread_rwlock_destroy(&knet_h->host_rwlock);
pthread_mutex_destroy(&knet_h->host_mutex);
pthread_cond_destroy(&knet_h->host_cond);
View
1  libknet/internals.h
@@ -98,6 +98,7 @@ struct knet_handle {
pthread_t dst_link_handler_thread;
int lock_init_done;
pthread_rwlock_t list_rwlock;
+ pthread_rwlock_t listener_rwlock;
pthread_rwlock_t host_rwlock;
pthread_mutex_t host_mutex;
pthread_cond_t host_cond;
View
20 libknet/libknet.h
@@ -488,9 +488,25 @@ int knet_link_get_config(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
struct sockaddr_storage *src_addr,
struct sockaddr_storage *dst_addr);
+/*
+ * knet_link_set_enable
+ *
+ * knet_h - pointer to knet_handle_t
+ *
+ * host_id - see above
+ *
+ * link_id - see above
+ *
+ * enabled - 0 disable the link, 1 enable the link
+ *
+ * knet_link_set_enable returns:
+ *
+ * 0 on success
+ * -1 on error and errno is set.
+ */
-int knet_link_enable(knet_handle_t knet_h, uint16_t node_id, uint8_t link_id,
- int enabled);
+int knet_link_set_enable(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id,
+ int enabled);
int knet_link_set_timeout(knet_handle_t knet_h, uint16_t node_id, uint8_t link_id, time_t interval, time_t timeout, unsigned int precision);
int knet_link_get_timeout(knet_handle_t knet_h, uint16_t node_id, uint8_t link_id, time_t *interval, time_t *timeout, unsigned int *precision);
View
222 libknet/link.c
@@ -23,90 +23,38 @@
#include "host.h"
int _link_updown(knet_handle_t knet_h, uint16_t node_id,
- struct knet_link *lnk, int enabled, int connected)
+ struct knet_link *link, int enabled, int connected)
{
- unsigned int old_enabled = lnk->status.enabled;
- unsigned int old_connected = lnk->status.connected;
+ int savederrno = 0, err = 0;
+ unsigned int old_enabled = link->status.enabled;
+ unsigned int old_connected = link->status.connected;
- if ((lnk->status.enabled == enabled) && (lnk->status.connected == connected))
+ if ((link->status.enabled == enabled) &&
+ (link->status.connected == connected))
return 0;
- lnk->status.enabled = enabled;
- lnk->status.connected = connected;
+ link->status.enabled = enabled;
+ link->status.connected = connected;
- if (_dst_cache_update(knet_h, node_id)) {
+ err = _dst_cache_update(knet_h, node_id);
+ if (err) {
+ savederrno = errno;
log_debug(knet_h, KNET_SUB_LINK,
"Unable to update link status (host: %s link: %s enabled: %u connected: %u)",
knet_h->host_index[node_id]->name,
- lnk->status.dst_ipaddr,
- lnk->status.enabled,
- lnk->status.connected);
- lnk->status.enabled = old_enabled;
- lnk->status.connected = old_connected;
- return -1;
- }
-
- if ((lnk->status.dynconnected) && (!lnk->status.connected))
- lnk->status.dynconnected = 0;
-
- return 0;
-}
-
-int knet_link_enable(knet_handle_t knet_h, uint16_t node_id, uint8_t link_id, int enabled)
-{
- int err;
- struct knet_link *lnk;
-
- if (!knet_h->host_index[node_id])
+ link->status.dst_ipaddr,
+ link->status.enabled,
+ link->status.connected);
+ link->status.enabled = old_enabled;
+ link->status.connected = old_connected;
+ errno = savederrno;
return -1;
-
- lnk = &knet_h->host_index[node_id]->link[link_id];
-
- if (lnk->status.enabled == enabled)
- return 0;
-
- if (enabled) {
- if (_listener_add(knet_h, lnk) < 0) {
- log_err(knet_h, KNET_SUB_LINK, "Unable to setup listener for this link");
- return -1;
- }
- log_debug(knet_h, KNET_SUB_LINK, "host: %s link: %s is enabled",
- knet_h->host_index[node_id]->name, lnk->status.dst_ipaddr);
- }
-
- if (!enabled) {
- struct knet_hinfo_data knet_hinfo_data;
-
- knet_hinfo_data.khd_type = KNET_HOST_INFO_LINK_UP_DOWN;
- knet_hinfo_data.khd_bcast = 0;
- knet_hinfo_data.khd_dst_node_id = htons(node_id);
- knet_hinfo_data.khd_dype.link_up_down.khdt_link_id = lnk->link_id;
- knet_hinfo_data.khd_dype.link_up_down.khdt_link_status = 0;
-
- _send_host_info(knet_h, &knet_hinfo_data, sizeof(struct knet_hinfo_data));
}
- err = _link_updown(knet_h, node_id, lnk, enabled, lnk->status.connected);
-
- if ((enabled) && (!err))
- return 0;
-
- if (err)
- return -1;
-
- err = _listener_remove(knet_h, lnk);
+ if ((link->status.dynconnected) &&
+ (!link->status.connected))
+ link->status.dynconnected = 0;
- if ((err) && (err != EBUSY)) {
- log_err(knet_h, KNET_SUB_LINK, "Unable to remove listener for this link");
- if (_link_updown(knet_h, node_id, lnk, 1, lnk->status.connected))
- lnk->status.enabled = 1;
- log_debug(knet_h, KNET_SUB_LINK, "host: %s link: %s is NOT disabled",
- knet_h->host_index[node_id]->name, lnk->status.dst_ipaddr);
- return -1;
- }
- log_debug(knet_h, KNET_SUB_LINK, "host: %s link: %s is disabled",
- knet_h->host_index[node_id]->name, lnk->status.dst_ipaddr);
- lnk->host_info_up_sent = 0;
return 0;
}
@@ -195,6 +143,19 @@ int knet_link_get_timeout(knet_handle_t knet_h, uint16_t node_id, uint8_t link_i
return 0;
}
+int knet_link_get_status(knet_handle_t knet_h,
+ uint16_t node_id,
+ uint8_t link_id,
+ struct knet_link_status *status)
+{
+ if (!knet_h->host_index[node_id])
+ return -1;
+
+ memcpy(status, &knet_h->host_index[node_id]->link[link_id].status, sizeof(struct knet_link_status));
+
+ return 0;
+}
+
int knet_link_set_config(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id,
struct sockaddr_storage *src_addr,
struct sockaddr_storage *dst_addr)
@@ -374,15 +335,118 @@ int knet_link_get_config(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
return err;
}
-int knet_link_get_status(knet_handle_t knet_h,
- uint16_t node_id,
- uint8_t link_id,
- struct knet_link_status *status)
+int knet_link_set_enable(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id,
+ int enabled)
{
- if (!knet_h->host_index[node_id])
+ int savederrno = 0, err = 0;
+ struct knet_host *host;
+ struct knet_link *link;
+
+ if (!knet_h) {
+ errno = EINVAL;
return -1;
+ }
- memcpy(status, &knet_h->host_index[node_id]->link[link_id].status, sizeof(struct knet_link_status));
+ if (link_id >= KNET_MAX_LINK) {
+ errno = EINVAL;
+ return -1;
+ }
- return 0;
+ /*
+ * this read lock might appear as an API violation, but be
+ * very careful because we cannot use a write lock (yet).
+ * the _send_host_info requires threads to be operational.
+ * a write lock here would deadlock.
+ * a read lock is sufficient as all functions invoked by
+ * this code are already thread safe.
+ */
+ savederrno = pthread_rwlock_rdlock(&knet_h->list_rwlock);
+ if (savederrno) {
+ log_err(knet_h, KNET_SUB_LINK, "Unable to get read lock: %s",
+ strerror(savederrno));
+ errno = savederrno;
+ return -1;
+ }
+
+ host = knet_h->host_index[host_id];
+ if (!host) {
+ err = -1;
+ savederrno = EINVAL;
+ log_err(knet_h, KNET_SUB_LINK, "Unable to find host %u: %s",
+ host_id, strerror(savederrno));
+ goto exit_unlock;
+ }
+
+ link = &host->link[link_id];
+
+ if (!link->configured) {
+ err = -1;
+ savederrno = EINVAL;
+ log_err(knet_h, KNET_SUB_LINK, "host %u link %u is not configured: %s",
+ host_id, link_id, strerror(savederrno));
+ goto exit_unlock;
+ }
+
+ if (link->status.enabled == enabled) {
+ err = 0;
+ goto exit_unlock;
+ }
+
+ if (enabled) {
+ if (_listener_add(knet_h, link) < 0) {
+ savederrno = errno;
+ err = -1;
+ log_err(knet_h, KNET_SUB_LINK, "Unable to setup listener for this link");
+ goto exit_unlock;
+ }
+ log_debug(knet_h, KNET_SUB_LINK, "host: %u link: %u is enabled",
+ host_id, link_id);
+ }
+
+ if (!enabled) {
+ struct knet_hinfo_data knet_hinfo_data;
+
+ knet_hinfo_data.khd_type = KNET_HOST_INFO_LINK_UP_DOWN;
+ knet_hinfo_data.khd_bcast = 0;
+ knet_hinfo_data.khd_dst_node_id = htons(host_id);
+ knet_hinfo_data.khd_dype.link_up_down.khdt_link_id = link_id;
+ knet_hinfo_data.khd_dype.link_up_down.khdt_link_status = 0;
+ _send_host_info(knet_h, &knet_hinfo_data, sizeof(struct knet_hinfo_data));
+ }
+
+ err = _link_updown(knet_h, host_id, link, enabled, link->status.connected);
+ savederrno = errno;
+
+ if ((!err) && (enabled)) {
+ err = 0;
+ goto exit_unlock;
+ }
+
+ if (err) {
+ err = -1;
+ goto exit_unlock;
+ }
+
+ err = _listener_remove(knet_h, link);
+ savederrno = errno;
+
+ if ((err) && (savederrno != EBUSY)) {
+ log_err(knet_h, KNET_SUB_LINK, "Unable to remove listener for this link");
+ if (_link_updown(knet_h, host_id, link, 1, link->status.connected)) {
+ /* force link status the hard way */
+ link->status.enabled = 1;
+ }
+ log_debug(knet_h, KNET_SUB_LINK, "host: %s link: %s is NOT disabled",
+ knet_h->host_index[host_id]->name, link->status.dst_ipaddr);
+ err = -1;
+ goto exit_unlock;
+ }
+ log_debug(knet_h, KNET_SUB_LINK, "host: %s link: %s is disabled",
+ knet_h->host_index[host_id]->name, link->status.dst_ipaddr);
+ link->host_info_up_sent = 0;
+
+exit_unlock:
+ pthread_rwlock_unlock(&knet_h->list_rwlock);
+ errno = savederrno;
+ return err;
}
View
128 libknet/listener.c
@@ -23,24 +23,27 @@
#define KNET_RING_RCVBUFF 8388608
-int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
+int _listener_add(knet_handle_t knet_h, struct knet_link *link)
{
- int value;
+ int value, count = 0;
struct epoll_event ev;
- int save_errno = 0;
- struct knet_listener *listener;
-
- if (pthread_rwlock_wrlock(&knet_h->list_rwlock) != 0) {
- save_errno = errno;
- log_err(knet_h, KNET_SUB_LISTENER, "listener_add: Unable to get write lock");
- errno = save_errno;
+ int savederrno = 0, err = 0;
+ struct knet_listener *listener = NULL;
+
+ savederrno = pthread_rwlock_wrlock(&knet_h->listener_rwlock);
+ if (savederrno) {
+ log_err(knet_h, KNET_SUB_LISTENER, "Unable to get write lock: %s",
+ strerror(savederrno));
+ errno = savederrno;
return -1;
}
listener = knet_h->listener_head;
while (listener) {
- if (!memcmp(&lnk->src_addr, &listener->address, sizeof(struct sockaddr_storage))) {
+ count++;
+ log_debug(knet_h, KNET_SUB_LISTENER, "checking listener: %d", count);
+ if (!memcmp(&link->src_addr, &listener->address, sizeof(struct sockaddr_storage))) {
log_debug(knet_h, KNET_SUB_LISTENER, "found active listener");
break;
}
@@ -50,19 +53,23 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
if (!listener) {
listener = malloc(sizeof(struct knet_listener));
if (!listener) {
- save_errno = errno;
- log_debug(knet_h, KNET_SUB_LISTENER, "out of memory to allocate listener");
- goto exit_fail1;
+ savederrno = errno;
+ err = -1;
+ log_err(knet_h, KNET_SUB_LISTENER, "out of memory to allocate listener: %s",
+ strerror(savederrno));
+ goto exit_unlock;
}
memset(listener, 0, sizeof(struct knet_listener));
- memcpy(&listener->address, &lnk->src_addr, sizeof(struct sockaddr_storage));
+ memcpy(&listener->address, &link->src_addr, sizeof(struct sockaddr_storage));
listener->sock = socket(listener->address.ss_family, SOCK_DGRAM, 0);
if (listener->sock < 0) {
- save_errno = errno;
- log_err(knet_h, KNET_SUB_LISTENER, "Unable to create listener socket");
- goto exit_fail2;
+ savederrno = errno;
+ err = -1;
+ log_err(knet_h, KNET_SUB_LISTENER, "Unable to create listener socket: %s",
+ strerror(savederrno));
+ goto exit_unlock;
}
value = KNET_RING_RCVBUFF;
@@ -74,17 +81,20 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
&value, sizeof(value));
}
- if (_fdset_cloexec(listener->sock) != 0) {
- save_errno = errno;
- log_err(knet_h, KNET_SUB_LISTENER, "Unable to set listener socket opts");
- goto exit_fail3;
+ if (_fdset_cloexec(listener->sock)) {
+ savederrno = errno;
+ err = -1;
+ log_err(knet_h, KNET_SUB_LISTENER, "Unable to set listener socket opts: %s",
+ strerror(savederrno));
+ goto exit_unlock;
}
- if (bind(listener->sock, (struct sockaddr *) &listener->address,
- sizeof(struct sockaddr_storage)) != 0) {
- save_errno = errno;
- log_err(knet_h, KNET_SUB_LISTENER, "Unable to bind listener socket");
- goto exit_fail3;
+ if (bind(listener->sock, (struct sockaddr *)&listener->address, sizeof(struct sockaddr_storage))) {
+ savederrno = errno;
+ err = -1;
+ log_err(knet_h, KNET_SUB_LISTENER, "Unable to bind listener socket: %s",
+ strerror(savederrno));
+ goto exit_unlock;
}
memset(&ev, 0, sizeof(struct epoll_event));
@@ -92,73 +102,74 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
ev.events = EPOLLIN;
ev.data.fd = listener->sock;
- if (epoll_ctl(knet_h->recv_from_links_epollfd, EPOLL_CTL_ADD, listener->sock, &ev) != 0) {
- save_errno = errno;
- log_err(knet_h, KNET_SUB_LISTENER, "Unable to add listener to epoll pool");
- goto exit_fail3;
+ if (epoll_ctl(knet_h->recv_from_links_epollfd, EPOLL_CTL_ADD, listener->sock, &ev)) {
+ savederrno = errno;
+ err = -1;
+ log_err(knet_h, KNET_SUB_LISTENER, "Unable to add listener to epoll pool: %s",
+ strerror(savederrno));
+ goto exit_unlock;
}
/* pushing new host to the front */
listener->next = knet_h->listener_head;
knet_h->listener_head = listener;
}
- lnk->listener_sock = listener->sock;
-
- pthread_rwlock_unlock(&knet_h->list_rwlock);
+ link->listener_sock = listener->sock;
- return 0;
-
- exit_fail3:
- close(listener->sock);
-
- exit_fail2:
- if (listener)
+exit_unlock:
+ if ((err) && (listener)) {
+ if (listener->sock >= 0)
+ close(listener->sock);
free(listener);
+ listener = NULL;
+ }
- exit_fail1:
- pthread_rwlock_unlock(&knet_h->list_rwlock);
- errno = save_errno;
- return -1;
+ pthread_rwlock_unlock(&knet_h->listener_rwlock);
+ errno = savederrno;
+ return err;
}
-int _listener_remove(knet_handle_t knet_h, struct knet_link *lnk)
+int _listener_remove(knet_handle_t knet_h, struct knet_link *link)
{
- int link_idx, ret;
+ int err = 0, savederrno = 0;
+ int link_idx;
struct epoll_event ev; /* kernel < 2.6.9 bug (see epoll_ctl man) */
struct knet_host *host;
struct knet_listener *tmp_listener;
struct knet_listener *listener;
int listener_cnt = 0;
- if (pthread_rwlock_wrlock(&knet_h->list_rwlock) != 0) {
- log_err(knet_h, KNET_SUB_LISTENER, "listener_remove: Unable to get write lock");
- return -EINVAL;
+ savederrno = pthread_rwlock_wrlock(&knet_h->listener_rwlock);
+ if (savederrno) {
+ log_err(knet_h, KNET_SUB_LISTENER, "Unable to get write lock: %s",
+ strerror(savederrno));
+ errno = savederrno;
+ return -1;
}
- ret = 0;
-
/* checking if listener is in use */
for (host = knet_h->host_head; host != NULL; host = host->next) {
for (link_idx = 0; link_idx < KNET_MAX_LINK; link_idx++) {
if (host->link[link_idx].status.enabled != 1)
continue;
- if (host->link[link_idx].listener_sock == lnk->listener_sock) {
+ if (host->link[link_idx].listener_sock == link->listener_sock) {
listener_cnt++;
}
}
}
if (listener_cnt) {
- lnk->listener_sock = 0;
+ link->listener_sock = 0;
log_debug(knet_h, KNET_SUB_LISTENER, "listener_remove: listener still in use");
- ret = EBUSY;
+ savederrno = EBUSY;
+ err = -1;
goto exit_unlock;
}
listener = knet_h->listener_head;
while (listener) {
- if (listener->sock == lnk->listener_sock)
+ if (listener->sock == link->listener_sock)
break;
listener = listener->next;
}
@@ -180,9 +191,8 @@ int _listener_remove(knet_handle_t knet_h, struct knet_link *lnk)
free(listener);
exit_unlock:
- pthread_rwlock_unlock(&knet_h->list_rwlock);
+ pthread_rwlock_unlock(&knet_h->listener_rwlock);
- if (ret < 0)
- errno = -ret;
- return ret;
+ errno = savederrno;
+ return err;
}
View
4 libknet/listener.h
@@ -12,7 +12,7 @@
#include "internals.h"
-int _listener_add(knet_handle_t knet_h, struct knet_link *lnk);
-int _listener_remove(knet_handle_t knet_h, struct knet_link *lnk);
+int _listener_add(knet_handle_t knet_h, struct knet_link *link);
+int _listener_remove(knet_handle_t knet_h, struct knet_link *link);
#endif
View
4 libknet/ping_test.c
@@ -156,7 +156,7 @@ static void argv_to_hosts(int argc, char *argv[])
knet_link_set_config(knet_h, node_id, 0, &src_addr, &dst_addr);
knet_link_set_timeout(knet_h, node_id, 0, 1000, 5000, 2048);
- knet_link_enable(knet_h, node_id, 0, 1);
+ knet_link_set_enable(knet_h, node_id, 0, 1);
}
}
@@ -203,7 +203,7 @@ static void sigint_handler(int signum)
printf("Unable to get link data: %s\n",strerror(errno));
if (status.enabled != 1) continue;
- if (knet_link_enable(knet_h, host_ids[i], j, 0))
+ if (knet_link_set_enable(knet_h, host_ids[i], j, 0))
printf("Unable to remove link: %s\n",strerror(errno));
}
if (knet_host_remove(knet_h, host_ids[i]))
Please sign in to comment.
Something went wrong with that request. Please try again.