- dev - passt

[PATCH v3 3/6] flow: Include flow details with higher priority log messages
by David Gibson 19 Jun '26

19 Jun '26

Currently flow_log() and related functions / macros have a 'details' parameter which indicates whether to add extra messages with details of the flow's addresses. This is still a bit awkward to invoke, and only used in a few places. Change the logic, to automatically include the details if and only if the log priority is greater than LOG_DEBUG. Rationale: If at debug log level, there are already a bunch of debug messages tracking the flow life cycle, which include those details (we make sure to retain those). It's usually pretty easy to cross reference a specific flow debug message with the flow's history including the details. If at higher log level, and we generate a flow-connected error or warning we don't have those life cycle messages. So, just giving the flow index doesn't really tell you anything about which flow tripped the error. Adding the address details make the error message significantly more useful. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 2 +- flow.h | 22 +++++++++++----------- tcp.c | 2 +- udp.c | 5 ++--- 4 files changed, 15 insertions(+), 16 deletions(-) diff --git a/flow.c b/flow.c index 8f8435e6..b2963396 100644 --- a/flow.c +++ b/flow.c @@ -554,7 +554,7 @@ norule: /* This shouldn't happen, because if there's no rule for it we should * have no listening socket that would let us get here */ - flow_log(flow, LOG_DEBUG, false, true, "Missing forward rule"); + flow_dbg(flow, "Missing forward rule"); nofwd: flow_err(flow, "No rules to forward %s %s [%s]:%hu -> [%s]:%hu", diff --git a/flow.h b/flow.h index 2b78067a..61dde6d9 100644 --- a/flow.h +++ b/flow.h @@ -284,19 +284,19 @@ void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, enum flow_state state, const char *fmt, ...) __attribute__((format(printf, 6, 7))); -#define flow_log_(f_, pri_, perror_, details_, ...) \ - flow_log__((f_), (pri_), (perror_), (details_), (f_)->state, \ - __VA_ARGS__) +#define flow_log_(f_, pri_, perror_, ...) \ + flow_log__((f_), (pri_), (perror_), (pri_) > LOG_DEBUG, \ + (f_)->state, __VA_ARGS__) -#define flow_log(flow_, pri_, perror_, details_, ...) \ - flow_log_(&(flow_)->f, (pri_), (perror_), (details_), __VA_ARGS__) +#define flow_log(flow_, pri_, perror_, ...) \ + flow_log_(&(flow_)->f, (pri_), (perror_), __VA_ARGS__) #define flow_dbg(flow_, ...) \ - flow_log((flow_), LOG_DEBUG, false, false, __VA_ARGS__) + flow_log((flow_), LOG_DEBUG, false, __VA_ARGS__) #define flow_warn(flow_, ...) \ - flow_log((flow_), LOG_WARNING, false, false, __VA_ARGS__) + flow_log((flow_), LOG_WARNING, false, __VA_ARGS__) #define flow_err(flow_, ...) \ - flow_log((flow_), LOG_ERR, false, false, __VA_ARGS__) + flow_log((flow_), LOG_ERR, false, __VA_ARGS__) #define flow_trace(flow_, ...) \ do { \ if (log_trace) \ @@ -304,11 +304,11 @@ void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, } while (0) #define flow_dbg_perror(flow_, ...) \ - flow_log((flow_), LOG_DEBUG, true, false, __VA_ARGS__) + flow_log((flow_), LOG_DEBUG, true, __VA_ARGS__) #define flow_warn_perror(flow_, ...) \ - flow_log((flow_), LOG_WARNING, true, false, __VA_ARGS__) + flow_log((flow_), LOG_WARNING, true, __VA_ARGS__) #define flow_perror(flow_, ...) \ - flow_log((flow_), LOG_ERR, true, false, __VA_ARGS__) + flow_log((flow_), LOG_ERR, true, __VA_ARGS__) #define flow_dbg_ratelimit(flow_, now_, ...) \ logmsg_ratelimit(flow_dbg, debug, (now_), (flow_), __VA_ARGS__) diff --git a/tcp.c b/tcp.c index 3538eec2..1c902ac2 100644 --- a/tcp.c +++ b/tcp.c @@ -1404,7 +1404,7 @@ void tcp_linger0_(const struct flow_common *f, int s) */ if (setsockopt(s, SOL_SOCKET, SO_LINGER, &linger0, sizeof(linger0)) < 0) { - flow_log_(f, LOG_DEBUG, true, false, + flow_log_(f, LOG_DEBUG, true, "SO_LINGER failed, may not send RST to peer"); } } diff --git a/udp.c b/udp.c index c95bb200..2d9e2f04 100644 --- a/udp.c +++ b/udp.c @@ -943,8 +943,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, if (events & EPOLLERR) { if (udp_sock_errs(c, ref.fd, ref.flowside, PIF_NONE, 0) < 0) { - flow_log(uflow, LOG_ERR, false, true, - "Unrecoverable error on flow socket"); + flow_err(uflow, "Unrecoverable error on flow socket"); goto fail; } } @@ -975,7 +974,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, udp_buf_sock_to_tap(c, s, n, tosidx); } } else { - flow_log(uflow, LOG_ERR, false, true, + flow_err(uflow, "No support for forwarding UDP from %s to %s", pif_name(pif_at_sidx(ref.flowside)), pif_name(topif)); -- 2.54.0

1 0

[PATCH v3 2/6] flow: Indent flow details messages
by David Gibson 19 Jun '26

19 Jun '26

flow_log__() has the option to append an extra log message with address details of the flow, to give more context to errors or warnings. Add an indent to this part of the message, to emphasise that it's additional information related to a nearby message, rather than a message in its own right. Suggested-by: Stefano Brivio <sbrivio(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flow.c b/flow.c index c6741251..8f8435e6 100644 --- a/flow.c +++ b/flow.c @@ -315,7 +315,7 @@ void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, if (state >= FLOW_STATE_TGT) { flow_log__(f, pri, false, false, state, -"%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", +" %s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", pif_name(f->pif[INISIDE]), inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), ini->eport, @@ -328,7 +328,7 @@ void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, tgt->eport); } else if (state >= FLOW_STATE_INI) { flow_log__(f, pri, false, false, state, - "%s [%s]:%hu -> [%s]:%hu => ?", + " %s [%s]:%hu -> [%s]:%hu => ?", pif_name(f->pif[INISIDE]), inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), ini->eport, -- 2.54.0

1 0

[PATCH v3 1/6] flow: Regularise flow specific logging helpers
by David Gibson 19 Jun '26

19 Jun '26

flow.h has a collection of logging helpers that automatically include information about a specific flow. Which variants are present are a bit ad-hoc, based on what we happened to want use (e.g. there are no LOG_WARNING level versions, at present). There's also a rather awkward and only occasionally used flow_log_details_() helper to print additional log messages with more details of the flow (basically its addresses). It's particularly awkward to try to combine that with ratelimiting. Re-organise this to be based around a flow_log__() internal helper, which has bool parameters to include strerror() / perror information and/or the extra details. Add wrapper macros for all combinations of perror, ratelimiting and DEBUG/WARNING/ERR priorities. Be a little more consistent about parameter order between the various functions / macros / wrappers while we're at it. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 94 ++++++++++++++++++++++++++++++---------------------------- flow.h | 71 +++++++++++++++++++++++--------------------- tcp.c | 4 +-- udp.c | 6 ++-- 4 files changed, 91 insertions(+), 84 deletions(-) diff --git a/flow.c b/flow.c index 7a5c81c2..c6741251 100644 --- a/flow.c +++ b/flow.c @@ -269,17 +269,25 @@ int flowside_connect(const struct ctx *c, int s, return connect(s, &sa.sa, socklen_inany(&sa)); } -/** flow_log_ - Log flow-related message +/** flow_log__ - Log flow-related message, internal helper * @f: flow the message is related to - * @newline: Append newline at the end of the message, if missing * @pri: Log priority + * @perror: Append strerror(errno) output + * @details: Add lines with flow addresses + * @state: State to display details for (used during state changes) * @fmt: Format string * @...: printf-arguments + * + * NOTE: This function is, technically speaking, recursive. However it recurses + * at most one layer, and as a tail call. clang-tidy doesn't like it, hence the + * suppression below. */ -void flow_log_(const struct flow_common *f, bool newline, int pri, - const char *fmt, ...) +/* NOLINTNEXTLINE(misc-no-recursion) */ +void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, + enum flow_state state, const char *fmt, ...) { const char *type_or_state; + int errno_ = errno; char msg[BUFSIZ]; va_list args; @@ -293,45 +301,41 @@ void flow_log_(const struct flow_common *f, bool newline, int pri, else type_or_state = FLOW_TYPE(f); - logmsg(newline, false, pri, + logmsg(!perror, false, pri, "Flow %u (%s): %s", flow_idx(f), type_or_state, msg); -} -/** flow_log_details_() - Log the details of a flow - * @f: flow to log - * @pri: Log priority - * @state: State to log details according to - * - * Logs the details of the flow: endpoints, interfaces, type etc. - */ -void flow_log_details_(const struct flow_common *f, int pri, - enum flow_state state) -{ - char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; - char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; - const struct flowside *ini = &f->side[INISIDE]; - const struct flowside *tgt = &f->side[TGTSIDE]; - - if (state >= FLOW_STATE_TGT) - 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)), - ini->eport, - inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), - ini->oport, - pif_name(f->pif[TGTSIDE]), - inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)), - tgt->oport, - inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), - tgt->eport); - else if (state >= FLOW_STATE_INI) - flow_log_(f, true, pri, "%s [%s]:%hu -> [%s]:%hu => ?", - pif_name(f->pif[INISIDE]), - inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), - ini->eport, - inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), - ini->oport); + if (perror) + logmsg(true, true, pri, ": %s", strerror_(errno_)); + + if (details) { + char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; + char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; + const struct flowside *ini = &f->side[INISIDE]; + const struct flowside *tgt = &f->side[TGTSIDE]; + + if (state >= FLOW_STATE_TGT) { + flow_log__(f, pri, false, false, state, +"%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", + pif_name(f->pif[INISIDE]), + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), + ini->eport, + inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), + ini->oport, + pif_name(f->pif[TGTSIDE]), + inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)), + tgt->oport, + inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), + tgt->eport); + } else if (state >= FLOW_STATE_INI) { + flow_log__(f, pri, false, false, state, + "%s [%s]:%hu -> [%s]:%hu => ?", + pif_name(f->pif[INISIDE]), + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), + ini->eport, + inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), + ini->oport); + } + } } /** @@ -347,10 +351,9 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) assert(oldstate < FLOW_NUM_STATES); f->state = state; - flow_log_(f, true, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate], + flow_log__(f, LOG_DEBUG, false, true, MAX(state, oldstate), + "%s -> %s", flow_state_str[oldstate], FLOW_STATE(f)); - - flow_log_details_(f, LOG_DEBUG, MAX(state, oldstate)); } /** @@ -551,8 +554,7 @@ norule: /* This shouldn't happen, because if there's no rule for it we should * have no listening socket that would let us get here */ - flow_dbg(flow, "Missing forward rule"); - flow_log_details_(f, LOG_DEBUG, f->state); + flow_log(flow, LOG_DEBUG, false, true, "Missing forward rule"); nofwd: flow_err(flow, "No rules to forward %s %s [%s]:%hu -> [%s]:%hu", diff --git a/flow.h b/flow.h index 2623b3db..2b78067a 100644 --- a/flow.h +++ b/flow.h @@ -280,42 +280,47 @@ 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, 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__) - -#define flow_trace(f, ...) \ +void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, + enum flow_state state, const char *fmt, ...) + __attribute__((format(printf, 6, 7))); + +#define flow_log_(f_, pri_, perror_, details_, ...) \ + flow_log__((f_), (pri_), (perror_), (details_), (f_)->state, \ + __VA_ARGS__) + +#define flow_log(flow_, pri_, perror_, details_, ...) \ + flow_log_(&(flow_)->f, (pri_), (perror_), (details_), __VA_ARGS__) + +#define flow_dbg(flow_, ...) \ + flow_log((flow_), LOG_DEBUG, false, false, __VA_ARGS__) +#define flow_warn(flow_, ...) \ + flow_log((flow_), LOG_WARNING, false, false, __VA_ARGS__) +#define flow_err(flow_, ...) \ + flow_log((flow_), LOG_ERR, false, false, __VA_ARGS__) +#define flow_trace(flow_, ...) \ do { \ if (log_trace) \ - 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_)); \ + flow_dbg((flow_), __VA_ARGS__); \ } 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) \ - flow_log_details_(&((f_)->f), (pri), (f_)->f.state) -#define flow_dbg_details(f_) flow_log_details((f_), LOG_DEBUG) -#define flow_err_details(f_) flow_log_details((f_), LOG_ERR) - -#define flow_dbg_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_dbg, debug, now, f, __VA_ARGS__) -#define flow_err_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_err, err, now, f, __VA_ARGS__) -#define flow_perror_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_perror, err, now, f, __VA_ARGS__) +#define flow_dbg_perror(flow_, ...) \ + flow_log((flow_), LOG_DEBUG, true, false, __VA_ARGS__) +#define flow_warn_perror(flow_, ...) \ + flow_log((flow_), LOG_WARNING, true, false, __VA_ARGS__) +#define flow_perror(flow_, ...) \ + flow_log((flow_), LOG_ERR, true, false, __VA_ARGS__) + +#define flow_dbg_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_dbg, debug, (now_), (flow_), __VA_ARGS__) +#define flow_dbg_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_dbg_perror, debug, (now_), (flow_), __VA_ARGS__) +#define flow_warn_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_warn, warn, (now_), (flow_), __VA_ARGS__) +#define flow_warn_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_warn_perror, warn, (now_), (flow_), __VA_ARGS__) +#define flow_err_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_err, err, (now_), (flow_), __VA_ARGS__) +#define flow_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_perror, err, (now_), (flow_), __VA_ARGS__) #endif /* FLOW_H */ diff --git a/tcp.c b/tcp.c index 097ab72e..3538eec2 100644 --- a/tcp.c +++ b/tcp.c @@ -1404,8 +1404,8 @@ void tcp_linger0_(const struct flow_common *f, int s) */ if (setsockopt(s, SOL_SOCKET, SO_LINGER, &linger0, sizeof(linger0)) < 0) { - flow_log_perror_(f, LOG_DEBUG, - "SO_LINGER failed, may not send RST to peer"); + flow_log_(f, LOG_DEBUG, true, false, + "SO_LINGER failed, may not send RST to peer"); } } diff --git a/udp.c b/udp.c index 375befa7..c95bb200 100644 --- a/udp.c +++ b/udp.c @@ -943,7 +943,8 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, if (events & EPOLLERR) { if (udp_sock_errs(c, ref.fd, ref.flowside, PIF_NONE, 0) < 0) { - flow_err(uflow, "Unrecoverable error on flow socket"); + flow_log(uflow, LOG_ERR, false, true, + "Unrecoverable error on flow socket"); goto fail; } } @@ -974,7 +975,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, udp_buf_sock_to_tap(c, s, n, tosidx); } } else { - flow_err(uflow, + flow_log(uflow, LOG_ERR, false, true, "No support for forwarding UDP from %s to %s", pif_name(pif_at_sidx(ref.flowside)), pif_name(topif)); @@ -984,7 +985,6 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, return; fail: - flow_err_details(uflow); udp_flow_close(c, uflow); } -- 2.54.0

1 0

[PATCH v2 0/2] dhcpv6: Add --dhcpv6-opt for custom DHCPv6 options
by Anshu Kumari 19 Jun '26

19 Jun '26

This series adds a --dhcpv6-opt CODE,VALUE command-line option to inject custom options into DHCPv6 replies, complementing the existing --dhcp-opt support for DHCPv4. The primary use case is UEFI HTTP Boot, which requires Vendor Class (option 16) with the correct enterprise-number + length-prefixed wire encoding per RFC 8415 Section 21.16, and Boot File URL (option 59). Value formats are determined automatically from the option code via a type table. Supported types include plain strings, IPv6 addresses (single and list), 8/16/32-bit integers, vendor class (ENTERPRISE:DATA), and length-prefixed string lists. Anshu Kumari (2): dhcpv6: Add --dhcpv6-opt with option type table and value parser dhcpv6: Inject custom options into DHCPv6 replies conf.c | 60 ++++++++++++- dhcpv6.c | 253 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ dhcpv6.h | 2 + passt.1 | 31 +++++++ passt.h | 12 +++ 5 files changed, 357 insertions(+), 1 deletion(-) -- 2.54.0

2 4

[PATCH 0/4] RFC: Improvements to flow specific logging
by David Gibson 17 Jun '26

17 Jun '26

While working on podman bug 23739, I found some deficiences in how and when we report various socket errors related to specific flows. Here are some preliminary patches to improve that. I think patches 1..3 are pretty straightforward and safe. 4/4 I think is a good idea in principle - promoting the priority of a number of messages now that we have ratelimiting. However, plumbing 'now' all the places we need it resuls in a *lot* of churn, so we might want to postpone or split it up. Still, current drafted posted for consideration. David Gibson (4): flow: Regularise flow specific logging helpers flow: Include flow details with higher priority log messages flow: Safer errno handling in flowside_connect() callers flow, treewide: Promote priority of selected flow-linked messages flow.c | 107 +++++++------- flow.h | 72 +++++----- icmp.c | 27 ++-- icmp.h | 3 +- migrate.c | 27 ++-- migrate.h | 8 +- passt.c | 8 +- tcp.c | 381 ++++++++++++++++++++++++++++--------------------- tcp.h | 5 +- tcp_buf.c | 78 +++++----- tcp_buf.h | 8 +- tcp_conn.h | 8 +- tcp_internal.h | 27 ++-- tcp_splice.c | 59 +++++--- tcp_splice.h | 3 +- tcp_vu.c | 42 +++--- tcp_vu.h | 6 +- udp.c | 48 ++++--- udp_flow.c | 22 +-- 19 files changed, 535 insertions(+), 404 deletions(-) -- 2.54.0

2 11

[PATCH v2 4/6] flow, udp: Fix errno handling in udp_flow_sock()
by David Gibson 17 Jun '26

17 Jun '26

flowside_connect() behaves much like connect(2) itself, returning -1 on error with errno set to the error code. One of the callers, in udp_flow_sock(), uses the errno code with flow_dbg_perror() *after* it's called epoll_del() and close() either of which could clobber errno. Fix that by moving the debug message earlier: there's no reason it has to wait until the epoll_del() and close(). While we're at it, adjust the comment on flowside_connect() to clarify that it sets errno (rather than, say, returning an error code). Suggested-by: Stefano Brivio <sbrivio(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 2 +- udp_flow.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/flow.c b/flow.c index 596a4722..20938034 100644 --- a/flow.c +++ b/flow.c @@ -258,7 +258,7 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, * * Connect @s to the endpoint address and port from @tgt. * - * Return: 0 on success, negative on error + * Return: 0 on success, negative on error (setting errno) */ int flowside_connect(const struct ctx *c, int s, uint8_t pif, const struct flowside *tgt) diff --git a/udp_flow.c b/udp_flow.c index 35417bc4..31e2f8a7 100644 --- a/udp_flow.c +++ b/udp_flow.c @@ -90,11 +90,11 @@ static int udp_flow_sock(const struct ctx *c, if (flowside_connect(c, s, pif, side) < 0) { rc = -errno; + flow_dbg_perror(uflow, "Couldn't connect flow socket"); epoll_del(flow_epollfd(&uflow->f), s); close(s); - flow_dbg_perror(uflow, "Couldn't connect flow socket"); return rc; } uflow->s[sidei] = s; -- 2.54.0

1 0

[PATCH v2 2/6] flow: Indent flow details messages
by David Gibson 17 Jun '26

17 Jun '26

flow_log__() has the option to append an extra log message with address details of the flow, to give more context to errors or warnings. Add an indent to this part of the message, to emphasise that it's additional information related to a nearby message, rather than a message in its own right. Suggested-by: Stefano Brivio <sbrivio(a)redhat.com> Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flow.c b/flow.c index e71f3de7..2f1530b5 100644 --- a/flow.c +++ b/flow.c @@ -310,7 +310,7 @@ void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, if (state >= FLOW_STATE_TGT) { flow_log__(f, pri, false, false, state, -"%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", +" %s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", pif_name(f->pif[INISIDE]), inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), ini->eport, @@ -323,7 +323,7 @@ void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, tgt->eport); } else if (state >= FLOW_STATE_INI) { flow_log__(f, pri, false, false, state, - "%s [%s]:%hu -> [%s]:%hu => ?", + " %s [%s]:%hu -> [%s]:%hu => ?", pif_name(f->pif[INISIDE]), inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), ini->eport, -- 2.54.0

1 0

[PATCH 1/4] flow: Regularise flow specific logging helpers
by David Gibson 17 Jun '26

17 Jun '26

flow.h has a collection of logging helpers that automatically include information about a specific flow. Which variants are present are a bit ad-hoc, based on what we happened to want use (e.g. there are no LOG_WARNING level versions, at present). There's also a rather awkward and only occasionally used flow_log_details_() helper to print additional log messages with more details of the flow (basically its addresses). It's particularly awkward to try to combine that with ratelimiting. Re-organise this to be based around a flow_log__() internal helper, which has bool parameters to include strerror() / perror information and/or the extra details. Add wrapper macros for all combinations of perror, ratelimiting and DEBUG/WARNING/ERR priorities. Be a little more consistent about parameter order between the various functions / macros / wrappers while we're at it. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 89 ++++++++++++++++++++++++++++------------------------------ flow.h | 70 +++++++++++++++++++++++---------------------- tcp.c | 4 +-- udp.c | 6 ++-- 4 files changed, 85 insertions(+), 84 deletions(-) diff --git a/flow.c b/flow.c index 565ed2b2..6cf3905a 100644 --- a/flow.c +++ b/flow.c @@ -270,17 +270,20 @@ int flowside_connect(const struct ctx *c, int s, return connect(s, &sa.sa, socklen_inany(&sa)); } -/** flow_log_ - Log flow-related message +/** flow_log__ - Log flow-related message, internal helper * @f: flow the message is related to - * @newline: Append newline at the end of the message, if missing * @pri: Log priority + * @perror: Append strerror(errno) output + * @details: Add lines with flow addresses + * @state: State to display details for (used during state changes) * @fmt: Format string * @...: printf-arguments */ -void flow_log_(const struct flow_common *f, bool newline, int pri, - const char *fmt, ...) +void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, + enum flow_state state, const char *fmt, ...) { const char *type_or_state; + int errno_ = errno; char msg[BUFSIZ]; va_list args; @@ -294,45 +297,41 @@ void flow_log_(const struct flow_common *f, bool newline, int pri, else type_or_state = FLOW_TYPE(f); - logmsg(newline, false, pri, + logmsg(!perror, false, pri, "Flow %u (%s): %s", flow_idx(f), type_or_state, msg); -} -/** flow_log_details_() - Log the details of a flow - * @f: flow to log - * @pri: Log priority - * @state: State to log details according to - * - * Logs the details of the flow: endpoints, interfaces, type etc. - */ -void flow_log_details_(const struct flow_common *f, int pri, - enum flow_state state) -{ - char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; - char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; - const struct flowside *ini = &f->side[INISIDE]; - const struct flowside *tgt = &f->side[TGTSIDE]; - - if (state >= FLOW_STATE_TGT) - 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)), - ini->eport, - inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), - ini->oport, - pif_name(f->pif[TGTSIDE]), - inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)), - tgt->oport, - inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), - tgt->eport); - else if (state >= FLOW_STATE_INI) - flow_log_(f, true, pri, "%s [%s]:%hu -> [%s]:%hu => ?", - pif_name(f->pif[INISIDE]), - inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), - ini->eport, - inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), - ini->oport); + if (perror) + logmsg(true, true, pri, ": %s", strerror_(errno_)); + + if (details) { + char estr0[INANY_ADDRSTRLEN], fstr0[INANY_ADDRSTRLEN]; + char estr1[INANY_ADDRSTRLEN], fstr1[INANY_ADDRSTRLEN]; + const struct flowside *ini = &f->side[INISIDE]; + const struct flowside *tgt = &f->side[TGTSIDE]; + + if (state >= FLOW_STATE_TGT) { + flow_log__(f, pri, false, false, state, +"%s [%s]:%hu -> [%s]:%hu => %s [%s]:%hu -> [%s]:%hu", + pif_name(f->pif[INISIDE]), + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), + ini->eport, + inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), + ini->oport, + pif_name(f->pif[TGTSIDE]), + inany_ntop(&tgt->oaddr, fstr1, sizeof(fstr1)), + tgt->oport, + inany_ntop(&tgt->eaddr, estr1, sizeof(estr1)), + tgt->eport); + } else if (state >= FLOW_STATE_INI) { + flow_log__(f, pri, false, false, state, + "%s [%s]:%hu -> [%s]:%hu => ?", + pif_name(f->pif[INISIDE]), + inany_ntop(&ini->eaddr, estr0, sizeof(estr0)), + ini->eport, + inany_ntop(&ini->oaddr, fstr0, sizeof(fstr0)), + ini->oport); + } + } } /** @@ -348,10 +347,9 @@ static void flow_set_state(struct flow_common *f, enum flow_state state) assert(oldstate < FLOW_NUM_STATES); f->state = state; - flow_log_(f, true, LOG_DEBUG, "%s -> %s", flow_state_str[oldstate], + flow_log__(f, LOG_DEBUG, false, true, MAX(state, oldstate), + "%s -> %s", flow_state_str[oldstate], FLOW_STATE(f)); - - flow_log_details_(f, LOG_DEBUG, MAX(state, oldstate)); } /** @@ -552,8 +550,7 @@ norule: /* This shouldn't happen, because if there's no rule for it we should * have no listening socket that would let us get here */ - flow_dbg(flow, "Missing forward rule"); - flow_log_details_(f, LOG_DEBUG, f->state); + flow_log(flow, LOG_DEBUG, false, true, "Missing forward rule"); nofwd: flow_err(flow, "No rules to forward %s %s [%s]:%hu -> [%s]:%hu", diff --git a/flow.h b/flow.h index 6c6a9260..d168a35a 100644 --- a/flow.h +++ b/flow.h @@ -280,42 +280,46 @@ 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, 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__) - -#define flow_trace(f, ...) \ +void flow_log__(const struct flow_common *f, int pri, bool perror, bool details, + enum flow_state state, const char *fmt, ...); + +#define flow_log_(f_, pri_, perror_, details_, ...) \ + flow_log__((f_), (pri_), (perror_), (details_), (f_)->state, \ + __VA_ARGS__) + +#define flow_log(flow_, pri_, perror_, details_, ...) \ + flow_log_(&(flow_)->f, (pri_), (perror_), (details_), __VA_ARGS__) + +#define flow_dbg(flow_, ...) \ + flow_log((flow_), LOG_DEBUG, false, false, __VA_ARGS__) +#define flow_warn(flow_, ...) \ + flow_log((flow_), LOG_WARNING, false, false, __VA_ARGS__) +#define flow_err(flow_, ...) \ + flow_log((flow_), LOG_ERR, false, false, __VA_ARGS__) +#define flow_trace(flow_, ...) \ do { \ if (log_trace) \ - 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_)); \ + flow_dbg((flow_), __VA_ARGS__); \ } 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) \ - flow_log_details_(&((f_)->f), (pri), (f_)->f.state) -#define flow_dbg_details(f_) flow_log_details((f_), LOG_DEBUG) -#define flow_err_details(f_) flow_log_details((f_), LOG_ERR) - -#define flow_dbg_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_dbg, debug, now, f, __VA_ARGS__) -#define flow_err_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_err, err, now, f, __VA_ARGS__) -#define flow_perror_ratelimit(f, now, ...) \ - logmsg_ratelimit(flow_perror, err, now, f, __VA_ARGS__) +#define flow_dbg_perror(flow_, ...) \ + flow_log((flow_), LOG_DEBUG, true, false, __VA_ARGS__) +#define flow_warn_perror(flow_, ...) \ + flow_log((flow_), LOG_WARNING, true, false, __VA_ARGS__) +#define flow_perror(flow_, ...) \ + flow_log((flow_), LOG_ERR, true, false, __VA_ARGS__) + +#define flow_dbg_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_dbg, debug, (now_), (flow_), __VA_ARGS__) +#define flow_dbg_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_dbg_perror, debug, (now_), (flow_), __VA_ARGS__) +#define flow_warn_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_warn, warn, (now_), (flow_), __VA_ARGS__) +#define flow_warn_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_warn_perror, warn, (now_), (flow_), __VA_ARGS__) +#define flow_err_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_err, err, (now_), (flow_), __VA_ARGS__) +#define flow_perror_ratelimit(flow_, now_, ...) \ + logmsg_ratelimit(flow_perror, err, (now_), (flow_), __VA_ARGS__) #endif /* FLOW_H */ diff --git a/tcp.c b/tcp.c index c4000754..6fba865f 100644 --- a/tcp.c +++ b/tcp.c @@ -1404,8 +1404,8 @@ void tcp_linger0_(const struct flow_common *f, int s) */ if (setsockopt(s, SOL_SOCKET, SO_LINGER, &linger0, sizeof(linger0)) < 0) { - flow_log_perror_(f, LOG_DEBUG, - "SO_LINGER failed, may not send RST to peer"); + flow_log_(f, LOG_DEBUG, true, false, + "SO_LINGER failed, may not send RST to peer"); } } diff --git a/udp.c b/udp.c index c28d6ee2..f29ca3da 100644 --- a/udp.c +++ b/udp.c @@ -943,7 +943,8 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, if (events & EPOLLERR) { if (udp_sock_errs(c, ref.fd, ref.flowside, PIF_NONE, 0) < 0) { - flow_err(uflow, "Unrecoverable error on flow socket"); + flow_log(uflow, LOG_ERR, false, true, + "Unrecoverable error on flow socket"); goto fail; } } @@ -974,7 +975,7 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, udp_buf_sock_to_tap(c, s, n, tosidx); } } else { - flow_err(uflow, + flow_log(uflow, LOG_ERR, false, true, "No support for forwarding UDP from %s to %s", pif_name(pif_at_sidx(ref.flowside)), pif_name(topif)); @@ -984,7 +985,6 @@ void udp_sock_handler(const struct ctx *c, union epoll_ref ref, return; fail: - flow_err_details(uflow); udp_flow_close(c, uflow); } -- 2.54.0

2 2

[PATCH v2 0/4] Assorted minor socket creation cleanups
by David Gibson 17 Jun '26

17 Jun '26

As discussed on our recent call, I was looking again at bug 167. I discovered it's still fairly fiddly to address this, but while investigating spotted a number of cleanups to make in the vicinity. I think they make sense even without fixing bug 167 (yet), so here they are. Tested on top of my recent cppcheck series, but I don't think they conflict. v2: * Fixed some cppcheck warnings I missed initially David Gibson (4): flow: Correct misleading signature of flowside_sock_l4() Makefile: Remove unused DUAL_STACK_SOCKETS define fwd, pif: Remove duplicated logic between tcp_listen() and udp_listen() pif, util: Move listen(2) call from sock_l4_() to pif_listen() Makefile | 5 ----- flow.c | 11 +++++------ flow.h | 2 +- fwd.c | 8 +------- pif.c | 41 +++++++++++++++++++++++++++++++++++------ pif.h | 2 +- tcp.c | 38 -------------------------------------- tcp.h | 2 -- udp.c | 38 -------------------------------------- udp.h | 2 -- util.c | 7 ------- 11 files changed, 43 insertions(+), 113 deletions(-) -- 2.54.0

2 5

[PATCH 0/5] Fix cppcheck-2.21.0 warnings
by David Gibson 17 Jun '26

17 Jun '26

Another cppcheck version, another batch of false positives :(. These appears to be cppcheck regressions, I've filed two bugs on their tracker. Still, here are workarounds in the passt code. At least I managed to make a few genuine if minor improvements to the code along the way. David Gibson (5): cppcheck: Remove unused CPPCHECK_6936 cppcheck: Add workaround for cppcheck bug 14847 tcp: Merge common sequence logic from tcp_{buf,vu}_data_from_sock() tcp: Avoid SEQ_*() comparisons against 0 tcp: MAX_WINDOW should be unsigned Makefile | 3 +-- tcp.c | 50 +++++++++++++++++++++++++++++++++++--------------- tcp_buf.c | 28 ++++------------------------ tcp_buf.h | 3 ++- tcp_internal.h | 4 ++-- tcp_vu.c | 27 ++++----------------------- tcp_vu.h | 3 ++- util.c | 6 ++++-- 8 files changed, 54 insertions(+), 70 deletions(-) -- 2.54.0

3 6