From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 0664D5A026D for ; Thu, 23 Nov 2023 07:58:50 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700722729; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dqF+hmD1UJ4EebJXHwga5ZqNEWjIxxa+YdV6zxL353Q=; b=JlxTZk0HButFirryb1sd/d7+2yiNhO0r7ZSBnmJO0lsFtFBYXQnnWuoWnYYr4w6BeLPE5f om0ypQ9AIZbc+ZSSTkrvD4JTcpuNsFdJ3J3e8Bw+9IIe5aeNsvwDdm0Zogs8rCHoLsTpmq cSiVVPZv1xmu5lHvtb4h4DLw4yWRgsA= Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-552-b-UBqg2GPzStAI6Q4C3VQQ-1; Thu, 23 Nov 2023 01:58:48 -0500 X-MC-Unique: b-UBqg2GPzStAI6Q4C3VQQ-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 22C093C00205; Thu, 23 Nov 2023 06:58:48 +0000 (UTC) Received: from elisabeth (unknown [10.39.208.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 3A0BA492BFA; Thu, 23 Nov 2023 06:58:46 +0000 (UTC) Date: Thu, 23 Nov 2023 07:58:45 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 6/6] flow, tcp: Add logging helpers for connection related messages Message-ID: <20231123075845.6dfd16fa@elisabeth> In-Reply-To: <20231123023629.2024938-7-david@gibson.dropbear.id.au> References: <20231123023629.2024938-1-david@gibson.dropbear.id.au> <20231123023629.2024938-7-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: T6CNBZXZ4IJG5IJZ3EYEK2TR36DZEAEK X-Message-ID-Hash: T6CNBZXZ4IJG5IJZ3EYEK2TR36DZEAEK X-MailFrom: sbrivio@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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? > + */ > +void flow_log_(const struct flow_common *f, int pri, const char *fmt, ...) > +{ > + char msg[BUFSIZ]; > + va_list args; > + > + va_start(args, fmt); > + (void)vsnprintf(msg, sizeof(msg), fmt, args); > + va_end(args); > + > + logmsg(pri, "Flow %u (%s): %s", flow_idx(f), FLOW_TYPE(f), msg); > +} > diff --git a/flow.h b/flow.h > index 9f49195..b6da516 100644 > --- a/flow.h > +++ b/flow.h > @@ -43,4 +43,18 @@ union flow; > > void flow_table_compact(struct ctx *c, union flow *hole); > > +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__) > + > +#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, ...) \ > + do { \ > + if (log_trace) \ > + flow_dbg((f), __VA_ARGS__); \ > + } while (0) > + > #endif /* FLOW_H */ -- Stefano