Re: [PATCH 6/6] flow, tcp: Add logging helpers for connection related messages

On Thu, Nov 23, 2023 at 07:58:45AM +0100, Stefano Brivio wrote:

On Thu, 23 Nov 2023 13:36:29 +1100 David Gibson wrote:

...

Most of the messages logged by the TCP code (be they errors, debug or trace messages) are related to a specific connection / flow. We're fairly consistent about prefixing these with the type of connection and the connection / flow index. However there are a few places where we put the index later in the message or omit it entirely. The template with the prefix is also a little bulky to carry around for every message, particularly for spliced connections.

To help keep this consistent, introduce some helpers to log messages linked to a specific flow. It takes the flow as a parameter and adds a uniform prefix to each message. This makes things slightly neater now, but more importantly will help keep formatting consistent as we add more things to the flow table.

Signed-off-by: David Gibson --- flow.c | 21 ++++++++++++++ flow.h | 14 +++++++++ tcp.c | 81 ++++++++++++++++++++++++---------------------------- tcp_splice.c | 61 +++++++++++++++++---------------------- 4 files changed, 99 insertions(+), 78 deletions(-)

diff --git a/flow.c b/flow.c index 0fff119..cb2cf62 100644 --- a/flow.c +++ b/flow.c @@ -64,3 +64,24 @@ void flow_table_compact(struct ctx *c, union flow *hole)

memset(from, 0, sizeof(*from)); } + +/** flow_log_ - Log flow-related message + * @f: flow the message is related to + * @pri: Log priority + * @fmt: Format string + * @...: printf-arguments + * + * @fmt must include an initial "%s" to expand to the prefix we generate + * (typically added by the flow_log() macro).

I don't understand how it does that -- flow_log() seems to just pass __VA_ARGS__ through?

Oops, that's a leftover from an earlier version. I formatting the prefix first, then adding it to the message, rather than the message first and adding it to the prefix. I was attempting to avoid a fixed size buffer for the message, but it just turned out weird and troublesome. Plus vlogmsg() has a fixed sized buffer anyway, so why bother. Anyway, I've removed the stale comment. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson