Our general logging helpers include a number of _perror() variants which,
like perror(3) include the description of the current errno. We didn't
have those for our flow specific logging helpers, though. Fill this gap
with flow_perror() and flow_dbg_perror(), and use them where it's useful.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
flow.c | 12 +++++++-----
flow.h | 18 ++++++++++++++----
icmp.c | 5 ++---
tcp.c | 33 +++++++++++++++------------------
tcp_splice.c | 9 ++++-----
udp_flow.c | 19 +++++++------------
6 files changed, 49 insertions(+), 47 deletions(-)
diff --git a/flow.c b/flow.c
index cc393e06..c68f6bb1 100644
--- a/flow.c
+++ b/flow.c
@@ -289,11 +289,13 @@ int flowside_connect(const struct ctx *c, int s,
/** flow_log_ - Log flow-related message
* @f: flow the message is related to
+ * @newline: Append newline at the end of the message, if missing
* @pri: Log priority
* @fmt: Format string
* @...: printf-arguments
*/
-void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
+void flow_log_(const struct flow_common *f, bool newline, int pri,
+ const char *fmt, ...)
{
const char *type_or_state;
char msg[BUFSIZ];
@@ -309,7 +311,7 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt,
...)
else
type_or_state = FLOW_TYPE(f);
- logmsg(true, false, pri,
+ logmsg(newline, false, pri,
"Flow %u (%s): %s", flow_idx(f), type_or_state, msg);
}
@@ -329,7 +331,7 @@ void flow_log_details_(const struct flow_common *f, int pri,
const struct flowside *tgt = &f->side[TGTSIDE];
if (state >= FLOW_STATE_TGT)
- flow_log_(f, pri,
+ flow_log_(f, true, pri,
"%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu",
pif_name(f->pif[INISIDE]),
inany_ntop(&ini->eaddr, estr0, sizeof(estr0)),
@@ -342,7 +344,7 @@ void flow_log_details_(const struct flow_common *f, int pri,
inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)),
tgt->eport);
else if (state >= FLOW_STATE_INI)
- flow_log_(f, pri, "%s [%s]:%hu -> [%s]:%hu => ?",
+ flow_log_(f, true, pri, "%s [%s]:%hu -> [%s]:%hu => ?",
pif_name(f->pif[INISIDE]),
inany_ntop(&ini->eaddr, estr0, sizeof(estr0)),
ini->eport,
@@ -363,7 +365,7 @@ static void flow_set_state(struct flow_common *f, enum flow_state
state)
ASSERT(oldstate < FLOW_NUM_STATES);
f->state = state;
- flow_log_(f, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate],
+ flow_log_(f, true, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate],
FLOW_STATE(f));
flow_log_details_(f, LOG_DEBUG, MAX(state, oldstate));
diff --git a/flow.h b/flow.h
index 675726eb..dcf7645a 100644
--- a/flow.h
+++ b/flow.h
@@ -258,11 +258,11 @@ int flow_migrate_source(struct ctx *c, const struct migrate_stage
*stage,
int flow_migrate_target(struct ctx *c, const struct migrate_stage *stage,
int fd);
-void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...)
- __attribute__((format(printf, 3, 4)));
-
-#define flow_log(f_, pri, ...) flow_log_(&(f_)->f, (pri), __VA_ARGS__)
+void flow_log_(const struct flow_common *f, bool newline, int pri,
+ const char *fmt, ...)
+ __attribute__((format(printf, 4, 5)));
+#define flow_log(f_, pri, ...) flow_log_(&(f_)->f, true, (pri), __VA_ARGS__)
#define flow_dbg(f, ...) flow_log((f), LOG_DEBUG, __VA_ARGS__)
#define flow_err(f, ...) flow_log((f), LOG_ERR, __VA_ARGS__)
@@ -272,6 +272,16 @@ void flow_log_(const struct flow_common *f, int pri, const char *fmt,
...)
flow_dbg((f), __VA_ARGS__); \
} while (0)
+#define flow_log_perror_(f, pri, ...) \
+ do { \
+ int errno_ = errno; \
+ flow_log_((f), false, (pri), __VA_ARGS__); \
+ logmsg(true, true, (pri), ": %s", strerror_(errno_)); \
+ } while (0)
+
+#define flow_dbg_perror(f_, ...) flow_log_perror_(&(f_)->f, LOG_DEBUG,
__VA_ARGS__)
+#define flow_perror(f_, ...) flow_log_perror_(&(f_)->f, LOG_ERR, __VA_ARGS__)
+
void flow_log_details_(const struct flow_common *f, int pri,
enum flow_state state);
#define flow_log_details(f_, pri) \
diff --git a/icmp.c b/icmp.c
index bcf498d2..7e2b3423 100644
--- a/icmp.c
+++ b/icmp.c
@@ -85,7 +85,7 @@ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref)
n = recvfrom(ref.fd, buf, sizeof(buf), 0, &sr.sa, &sl);
if (n < 0) {
- flow_err(pingf, "recvfrom() error: %s", strerror_(errno));
+ flow_perror(pingf, "recvfrom() error");
return;
}
@@ -300,8 +300,7 @@ int icmp_tap_handler(const struct ctx *c, uint8_t pif, sa_family_t
af,
pif_sockaddr(c, &sa, &sl, PIF_HOST, &tgt->eaddr, 0);
if (sendto(pingf->sock, pkt, l4len, MSG_NOSIGNAL, &sa.sa, sl) < 0) {
- flow_dbg(pingf, "failed to relay request to socket: %s",
- strerror_(errno));
+ flow_dbg_perror(pingf, "failed to relay request to socket");
} else {
flow_dbg(pingf,
"echo request to socket, ID: %"PRIu16", seq: %"PRIu16,
diff --git a/tcp.c b/tcp.c
index 21b6c6c5..f498f5bc 100644
--- a/tcp.c
+++ b/tcp.c
@@ -551,8 +551,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn
*conn)
fd = timerfd_create(CLOCK_MONOTONIC, 0);
if (fd == -1 || fd > FD_REF_MAX) {
- flow_dbg(conn, "failed to get timer: %s",
- strerror_(errno));
+ flow_dbg_perror(conn, "failed to get timer");
if (fd > -1)
close(fd);
conn->timer = -1;
@@ -561,8 +560,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn
*conn)
conn->timer = fd;
if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, conn->timer, &ev)) {
- flow_dbg(conn, "failed to add timer: %s",
- strerror_(errno));
+ flow_dbg_perror(conn, "failed to add timer");
close(conn->timer);
conn->timer = -1;
return;
@@ -587,7 +585,7 @@ static void tcp_timer_ctl(const struct ctx *c, struct tcp_tap_conn
*conn)
(unsigned long long)it.it_value.tv_nsec / 1000 / 1000);
if (timerfd_settime(conn->timer, 0, &it, NULL))
- flow_err(conn, "failed to set timer: %s", strerror_(errno));
+ flow_perror(conn, "failed to set timer");
}
/**
@@ -1386,10 +1384,10 @@ static void tcp_bind_outbound(const struct ctx *c,
if (bind(s, &bind_sa.sa, sl)) {
char sstr[INANY_ADDRSTRLEN];
- flow_dbg(conn,
- "Can't bind TCP outbound socket to %s:%hu: %s",
- inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)),
- tgt->oport, strerror_(errno));
+ flow_dbg_perror(conn,
+ "Can't bind TCP outbound socket to %s:%hu",
+ inany_ntop(&tgt->oaddr, sstr, sizeof(sstr)),
+ tgt->oport);
}
}
@@ -1398,9 +1396,9 @@ static void tcp_bind_outbound(const struct ctx *c,
if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE,
c->ip4.ifname_out,
strlen(c->ip4.ifname_out))) {
- flow_dbg(conn, "Can't bind IPv4 TCP socket to"
- " interface %s: %s", c->ip4.ifname_out,
- strerror_(errno));
+ flow_dbg_perror(conn,
+ "Can't bind IPv4 TCP socket to interface %s",
+ c->ip4.ifname_out);
}
}
} else if (bind_sa.sa_family == AF_INET6) {
@@ -1408,9 +1406,9 @@ static void tcp_bind_outbound(const struct ctx *c,
if (setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE,
c->ip6.ifname_out,
strlen(c->ip6.ifname_out))) {
- flow_dbg(conn, "Can't bind IPv6 TCP socket to"
- " interface %s: %s", c->ip6.ifname_out,
- strerror_(errno));
+ flow_dbg_perror(conn,
+ "Can't bind IPv6 TCP socket to interface %s",
+ c->ip6.ifname_out);
}
}
}
@@ -2193,7 +2191,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
* and we just set the timer to a new point in the future: discard it.
*/
if (timerfd_gettime(conn->timer, &check_armed))
- flow_err(conn, "failed to read timer: %s", strerror_(errno));
+ flow_perror(conn, "failed to read timer");
if (check_armed.it_value.tv_sec || check_armed.it_value.tv_nsec)
return;
@@ -2235,8 +2233,7 @@ void tcp_timer_handler(const struct ctx *c, union epoll_ref ref)
* ~ACK_TO_TAP_DUE or ~ACK_FROM_TAP_DUE.
*/
if (timerfd_settime(conn->timer, 0, &new, &old))
- flow_err(conn, "failed to set timer: %s",
- strerror_(errno));
+ flow_perror(conn, "failed to set timer");
if (old.it_value.tv_sec == ACT_TIMEOUT) {
flow_dbg(conn, "activity timeout");
diff --git a/tcp_splice.c b/tcp_splice.c
index 5d845c92..0d10e3d4 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -164,7 +164,7 @@ static int tcp_splice_epoll_ctl(const struct ctx *c,
if (epoll_ctl(c->epollfd, m, conn->s[0], &ev[0]) ||
epoll_ctl(c->epollfd, m, conn->s[1], &ev[1])) {
int ret = -errno;
- flow_err(conn, "ERROR on epoll_ctl(): %s", strerror_(errno));
+ flow_perror(conn, "ERROR on epoll_ctl()");
return ret;
}
@@ -317,8 +317,8 @@ static int tcp_splice_connect_finish(const struct ctx *c,
if (conn->pipe[sidei][0] < 0) {
if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) {
- flow_err(conn, "cannot create %d->%d pipe: %s",
- sidei, !sidei, strerror_(errno));
+ flow_perror(conn, "cannot create %d->%d pipe",
+ sidei, !sidei);
conn_flag(c, conn, CLOSING);
return -EIO;
}
@@ -482,8 +482,7 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
rc = getsockopt(ref.fd, SOL_SOCKET, SO_ERROR, &err, &sl);
if (rc)
- flow_err(conn, "Error retrieving SO_ERROR: %s",
- strerror_(errno));
+ flow_perror(conn, "Error retrieving SO_ERROR");
else
flow_trace(conn, "Error event on socket: %s",
strerror_(err));
diff --git a/udp_flow.c b/udp_flow.c
index 83c2568a..c6b8630a 100644
--- a/udp_flow.c
+++ b/udp_flow.c
@@ -93,9 +93,8 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow *flow,
*/
uflow->s[INISIDE] = fcntl(s_ini, F_DUPFD_CLOEXEC, 0);
if (uflow->s[INISIDE] < 0) {
- flow_err(uflow,
- "Couldn't duplicate listening socket: %s",
- strerror_(errno));
+ flow_perror(uflow,
+ "Couldn't duplicate listening socket");
goto cancel;
}
}
@@ -113,16 +112,13 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow
*flow,
uflow->s[TGTSIDE] = flowside_sock_l4(c, EPOLL_TYPE_UDP_REPLY,
tgtpif, tgt, fref.data);
if (uflow->s[TGTSIDE] < 0) {
- flow_dbg(uflow,
- "Couldn't open socket for spliced flow: %s",
- strerror_(errno));
+ flow_dbg_perror(uflow,
+ "Couldn't open socket for spliced flow");
goto cancel;
}
if (flowside_connect(c, uflow->s[TGTSIDE], tgtpif, tgt) < 0) {
- flow_dbg(uflow,
- "Couldn't connect flow socket: %s",
- strerror_(errno));
+ flow_dbg_perror(uflow, "Couldn't connect flow socket");
goto cancel;
}
@@ -142,9 +138,8 @@ static flow_sidx_t udp_flow_new(const struct ctx *c, union flow
*flow,
flow_trace(uflow,
"Discarded %d spurious reply datagrams", rc);
} else if (errno != EAGAIN) {
- flow_err(uflow,
- "Unexpected error discarding datagrams: %s",
- strerror_(errno));
+ flow_perror(uflow,
+ "Unexpected error discarding datagrams");
}
}
--
2.48.1