Skip to content
Browse files

cleanup a few bits around logging and interal API

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
  • Loading branch information...
1 parent d8af844 commit 8c5c32176da1a56b3e2031ce1ed0ae26ba72442a @fabbione committed Feb 1, 2013
Showing with 66 additions and 71 deletions.
  1. +0 −2 TODO
  2. +29 −34 libknet/link.c
  3. +2 −2 libknet/link.h
  4. +10 −8 libknet/listener.c
  5. +2 −2 libknet/listener.h
  6. +23 −23 libknet/threads.c
View
2 TODO
@@ -54,8 +54,6 @@ link/host level:
- (rfe) reswitching of packets
libknet:
- - (rfe) cleanup internal (not-exported) api (pointers vs indexes)
- - (rfe) make logging info consistent (name vs id)
- (issue) review logging policy/levels in public api call
example is scanning for active links in a host that would return
a half gazzillion useless log entries
View
63 libknet/link.c
@@ -22,10 +22,11 @@
#include "onwire.h"
#include "host.h"
-int _link_updown(knet_handle_t knet_h, uint16_t node_id,
- struct knet_link *link, int enabled, int connected)
+int _link_updown(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id,
+ int enabled, int connected)
{
int savederrno = 0, err = 0;
+ struct knet_link *link = &knet_h->host_index[host_id]->link[link_id];
unsigned int old_enabled = link->status.enabled;
unsigned int old_connected = link->status.connected;
@@ -36,13 +37,13 @@ int _link_updown(knet_handle_t knet_h, uint16_t node_id,
link->status.enabled = enabled;
link->status.connected = connected;
- err = _dst_cache_update(knet_h, node_id);
+ err = _dst_cache_update(knet_h, host_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,
- link->status.dst_ipaddr,
+ "Unable to update link status for host: %u link: %u enabled: %u connected: %u)",
+ host_id,
+ link_id,
link->status.enabled,
link->status.connected);
link->status.enabled = old_enabled;
@@ -118,13 +119,13 @@ int knet_link_set_config(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
if (err == EAI_SYSTEM) {
savederrno = errno;
log_warn(knet_h, KNET_SUB_LINK,
- "Unable to resolve host: %s link: %u source addr/port: %s",
- host->name, link_id, strerror(savederrno));
+ "Unable to resolve host: %u link: %u source addr/port: %s",
+ host_id, link_id, strerror(savederrno));
} else {
savederrno = EINVAL;
log_warn(knet_h, KNET_SUB_LINK,
- "Unable to resolve host: %s link: %u source addr/port: %s",
- host->name, link_id, gai_strerror(err));
+ "Unable to resolve host: %u link: %u source addr/port: %s",
+ host_id, link_id, gai_strerror(err));
}
err = -1;
goto exit_unlock;
@@ -147,13 +148,13 @@ int knet_link_set_config(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
if (err == EAI_SYSTEM) {
savederrno = errno;
log_warn(knet_h, KNET_SUB_LINK,
- "Unable to resolve host: %s link: %u destination addr/port: %s",
- host->name, link_id, strerror(savederrno));
+ "Unable to resolve host: %u link: %u destination addr/port: %s",
+ host_id, link_id, strerror(savederrno));
} else {
savederrno = EINVAL;
log_warn(knet_h, KNET_SUB_LINK,
- "Unable to resolve host: %s link: %u destination addr/port: %s",
- host->name, link_id, gai_strerror(err));
+ "Unable to resolve host: %u link: %u destination addr/port: %s",
+ host_id, link_id, gai_strerror(err));
}
err = -1;
}
@@ -295,7 +296,7 @@ int knet_link_set_enable(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
}
if (enabled) {
- if (_listener_add(knet_h, link) < 0) {
+ if (_listener_add(knet_h, host_id, link_id) < 0) {
savederrno = errno;
err = -1;
log_err(knet_h, KNET_SUB_LINK, "Unable to setup listener for this link");
@@ -316,7 +317,7 @@ int knet_link_set_enable(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
_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);
+ err = _link_updown(knet_h, host_id, link_id, enabled, link->status.connected);
savederrno = errno;
if ((!err) && (enabled)) {
@@ -329,26 +330,26 @@ int knet_link_set_enable(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id
goto exit_unlock;
}
- err = _listener_remove(knet_h, link);
+ err = _listener_remove(knet_h, host_id, link_id);
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)) {
+ if (_link_updown(knet_h, host_id, link_id, 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);
+ log_debug(knet_h, KNET_SUB_LINK, "host: %u link: %u is NOT disabled",
+ host_id, link_id);
err = -1;
goto exit_unlock;
} else {
err = 0;
savederrno = 0;
}
- log_debug(knet_h, KNET_SUB_LINK, "host: %s link: %s is disabled",
- knet_h->host_index[host_id]->name, link->status.dst_ipaddr);
+ log_debug(knet_h, KNET_SUB_LINK, "host: %u link: %u is disabled",
+ host_id, link_id);
link->host_info_up_sent = 0;
exit_unlock:
@@ -475,9 +476,8 @@ int knet_link_set_timeout(knet_handle_t knet_h, uint16_t host_id, uint8_t link_i
((link->ping_interval * precision) / 8000000);
log_debug(knet_h, KNET_SUB_LINK,
- "host: %s link: %s timeout update - interval: %llu timeout: %llu precision: %d",
- host->name, link->status.dst_ipaddr,
- link->ping_interval, link->pong_timeout, precision);
+ "host: %u link: %u timeout update - interval: %llu timeout: %llu precision: %d",
+ host_id, link_id, link->ping_interval, link->pong_timeout, precision);
exit_unlock:
pthread_rwlock_unlock(&knet_h->list_rwlock);
@@ -611,21 +611,16 @@ int knet_link_set_priority(knet_handle_t knet_h, uint16_t host_id, uint8_t link_
if (_dst_cache_update(knet_h, host_id)) {
savederrno = errno;
log_debug(knet_h, KNET_SUB_LINK,
- "Unable to update link priority (host: %s link: %s priority: %u): %s",
- host->name,
- link->status.dst_ipaddr,
- link->priority,
- strerror(savederrno));
+ "Unable to update link priority (host: %u link: %u priority: %u): %s",
+ host_id, link_id, link->priority, strerror(savederrno));
link->priority = old_priority;
err = -1;
goto exit_unlock;
}
log_debug(knet_h, KNET_SUB_LINK,
- "host: %s link: %s priority set to: %u",
- host->name,
- link->status.dst_ipaddr,
- link->priority);
+ "host: %u link: %u priority set to: %u",
+ host_id, link_id, link->priority);
exit_unlock:
pthread_rwlock_unlock(&knet_h->list_rwlock);
View
4 libknet/link.h
@@ -15,7 +15,7 @@
#define KNET_LINK_STATIC 0 /* link has static ip on both ends */
#define KNET_LINK_DYNIP 1 /* link has dynamic destination ip */
-int _link_updown(knet_handle_t knet_h, uint16_t node_id,
- struct knet_link *lnk, int configured, int connected);
+int _link_updown(knet_handle_t knet_h, uint16_t node_id, uint8_t link_id,
+ int enabled, int connected);
#endif
View
18 libknet/listener.c
@@ -23,11 +23,12 @@
#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, uint16_t host_id, uint8_t link_id)
{
int value, count = 0;
struct epoll_event ev;
int savederrno = 0, err = 0;
+ struct knet_link *link = &knet_h->host_index[host_id]->link[link_id];
struct knet_listener *listener = NULL;
savederrno = pthread_rwlock_wrlock(&knet_h->listener_rwlock);
@@ -43,7 +44,7 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
while (listener) {
count++;
log_debug(knet_h, KNET_SUB_LISTENER, "checking listener: %d", count);
- if (!memcmp(&lnk->src_addr, &listener->address, sizeof(struct sockaddr_storage))) {
+ if (!memcmp(&link->src_addr, &listener->address, sizeof(struct sockaddr_storage))) {
log_debug(knet_h, KNET_SUB_LISTENER, "found active listener");
break;
}
@@ -61,7 +62,7 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
}
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) {
@@ -114,7 +115,7 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
listener->next = knet_h->listener_head;
knet_h->listener_head = listener;
}
- lnk->listener_sock = listener->sock;
+ link->listener_sock = listener->sock;
exit_unlock:
if ((err) && (listener)) {
@@ -129,12 +130,13 @@ int _listener_add(knet_handle_t knet_h, struct knet_link *lnk)
return err;
}
-int _listener_remove(knet_handle_t knet_h, struct knet_link *lnk)
+int _listener_remove(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id)
{
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_link *link = &knet_h->host_index[host_id]->link[link_id];
struct knet_listener *tmp_listener;
struct knet_listener *listener;
int listener_cnt = 0;
@@ -153,14 +155,14 @@ int _listener_remove(knet_handle_t knet_h, struct knet_link *lnk)
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");
savederrno = EBUSY;
err = -1;
@@ -169,7 +171,7 @@ int _listener_remove(knet_handle_t knet_h, struct knet_link *lnk)
listener = knet_h->listener_head;
while (listener) {
- if (listener->sock == lnk->listener_sock)
+ if (listener->sock == link->listener_sock)
break;
listener = listener->next;
}
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, uint16_t host_id, uint8_t link_id);
+int _listener_remove(knet_handle_t knet_h, uint16_t host_id, uint8_t link_id);
#endif
View
46 libknet/threads.c
@@ -242,8 +242,8 @@ static void _handle_recv_from_links(knet_handle_t knet_h, int sockfd)
(knet_h->recv_from_links_buf->kf_link % KNET_MAX_LINK);
if (src_link->dynamic == KNET_LINK_DYNIP) {
if (memcmp(&src_link->dst_addr, &address, sizeof(struct sockaddr_storage)) != 0) {
- log_debug(knet_h, KNET_SUB_LINK_T, "host: %s link: %u appears to have changed ip address",
- src_host->name, src_link->link_id);
+ log_debug(knet_h, KNET_SUB_LINK_T, "host: %u link: %u appears to have changed ip address",
+ src_host->host_id, src_link->link_id);
memcpy(&src_link->dst_addr, &address, sizeof(struct sockaddr_storage));
if (getnameinfo((const struct sockaddr *)&src_link->dst_addr, sizeof(struct sockaddr_storage),
src_link->status.dst_ipaddr, KNET_MAX_HOST_LEN,
@@ -353,9 +353,9 @@ static void _handle_recv_from_links(knet_handle_t knet_h, int sockfd)
if (src_link->status.latency < src_link->pong_timeout) {
if (!src_link->status.connected) {
- log_info(knet_h, KNET_SUB_LINK, "host: %s link: %s is up",
- src_host->name, src_link->status.dst_ipaddr);
- _link_updown(knet_h, src_host->host_id, src_link, src_link->status.enabled, 1);
+ log_info(knet_h, KNET_SUB_LINK, "host: %u link: %u is up",
+ src_host->host_id, src_link->link_id);
+ _link_updown(knet_h, src_host->host_id, src_link->link_id, src_link->status.enabled, 1);
}
}
@@ -391,22 +391,22 @@ static void _handle_recv_from_links(knet_handle_t knet_h, int sockfd)
} else {
src_link->donnotremoteupdate = 0;
}
- log_debug(knet_h, KNET_SUB_LINK, "host message up/down. from host: %s link: %s remote connected: %u",
- src_host->name,
- src_link->status.dst_ipaddr,
+ log_debug(knet_h, KNET_SUB_LINK, "host message up/down. from host: %u link: %u remote connected: %u",
+ src_host->host_id,
+ src_link->link_id,
src_link->remoteconnected);
if (_dst_cache_update(knet_h, src_host->host_id)) {
log_debug(knet_h, KNET_SUB_LINK,
- "Unable to update switch cache (host: %s link: %s remote connected: %u)",
- src_host->name,
- src_link->status.dst_ipaddr,
+ "Unable to update switch cache for host: %u link: %u remote connected: %u)",
+ src_host->host_id,
+ src_link->link_id,
src_link->remoteconnected);
}
break;
case KNET_HOST_INFO_LINK_TABLE:
break;
default:
- log_warn(knet_h, KNET_SUB_LINK, "Receiving unknown host info message from host %s", src_host->name);
+ log_warn(knet_h, KNET_SUB_LINK, "Receiving unknown host info message from host %u", src_host->host_id);
break;
}
break;
@@ -449,7 +449,7 @@ static void _handle_dst_link_updates(knet_handle_t knet_h)
dst_host = knet_h->host_index[dst_host_id];
if (!dst_host) {
- log_debug(knet_h, KNET_SUB_SWITCH_T, "Unable to find host: %d", dst_host_id);
+ log_debug(knet_h, KNET_SUB_SWITCH_T, "Unable to find host: %u", dst_host_id);
goto out_unlock;
}
@@ -490,24 +490,24 @@ static void _handle_dst_link_updates(knet_handle_t knet_h)
}
if (dst_host->link_handler_policy == KNET_LINK_POLICY_PASSIVE) {
- log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %s (passive) best link: %s (%u)",
- dst_host->name, dst_host->link[dst_host->active_links[0]].status.dst_ipaddr,
+ log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %u (passive) best link: %u (pri: %u)",
+ dst_host->host_id, dst_host->link[dst_host->active_links[0]].link_id,
dst_host->link[dst_host->active_links[0]].priority);
} else {
- log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %s has %u active links",
- dst_host->name, dst_host->active_link_entries);
+ log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %u has %u active links",
+ dst_host->host_id, dst_host->active_link_entries);
}
/* no active links, we can clean the circular buffers and indexes */
if ((!dst_host->active_link_entries) || (clear_cbuffer) || (!host_has_remote)) {
if (!host_has_remote) {
- log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %s has no active remote links", dst_host->name);
+ log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %u has no active remote links", dst_host->host_id);
}
if (!dst_host->active_link_entries) {
- log_warn(knet_h, KNET_SUB_SWITCH_T, "host: %s has no active links", dst_host->name);
+ log_warn(knet_h, KNET_SUB_SWITCH_T, "host: %u has no active links", dst_host->host_id);
}
if (clear_cbuffer) {
- log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %s is coming back to life", dst_host->name);
+ log_debug(knet_h, KNET_SUB_SWITCH_T, "host: %u is coming back to life", dst_host->host_id);
}
_clear_cbuffers(dst_host);
}
@@ -585,9 +585,9 @@ static void _handle_check_each(knet_handle_t knet_h, struct knet_host *dst_host,
timespec_diff(pong_last, clock_now, &diff_ping);
if (diff_ping >= (dst_link->pong_timeout * 1000llu)) {
- log_info(knet_h, KNET_SUB_LINK, "host: %s link: %s is down",
- dst_host->name, dst_link->status.dst_ipaddr);
- _link_updown(knet_h, dst_host->host_id, dst_link, dst_link->status.enabled, 0);
+ log_info(knet_h, KNET_SUB_LINK, "host: %u link: %u is down",
+ dst_host->host_id, dst_link->link_id);
+ _link_updown(knet_h, dst_host->host_id, dst_link->link_id, dst_link->status.enabled, 0);
}
}
}

0 comments on commit 8c5c321

Please sign in to comment.
Something went wrong with that request. Please try again.