Re: [PATCH] netconsole: Append kernel version to message

From: Breno Leitao <leitao@debian.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	sergey.senozhatsky@gmail.com, pmladek@suse.com, tj@kernel.or,
	Dave Jones <davej@codemonkey.org.uk>,
	"open list:NETWORKING DRIVERS" <netdev@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] netconsole: Append kernel version to message
Date: Tue, 4 Jul 2023 08:47:23 -0700	[thread overview]
Message-ID: <ZKQ/C7z2RMG5a4XN@gmail.com> (raw)
In-Reply-To: <20230703124427.228f7a9e@kernel.org>

On Mon, Jul 03, 2023 at 12:44:27PM -0700, Jakub Kicinski wrote:
> On Mon,  3 Jul 2023 08:41:54 -0700 leitao@debian.org wrote:
> > +	uname = init_utsname()->release;
> > +
> > +	newmsg = kasprintf(GFP_KERNEL, "%s;%s", uname, msg);
> > +	if (!newmsg)
> > +		/* In case of ENOMEM, just ignore this entry */
> > +		return;
> > +	newlen = strlen(uname) + len + 1;
> > +
> > +	send_ext_msg_udp(nt, newmsg, newlen);
> > +
> > +	kfree(newmsg);
> 
> You can avoid the memory allocation by putting this code into
> send_ext_msg_udp(), I reckon? There's already a buffer there.

This is doable as well, basically appending "uname" at the beginning of
the buffer, copying the rest of the message to the buffer and sending
the buffer to netpoll.

If the message needs fragmentation, basically keep "uname" as part of
the header, and the new header length (this_header) will be "header_len
+ uname_len"

This is the code that does it. How does it sound?

--

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 4f4f79532c6c..d26bd3b785c4 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -36,6 +36,7 @@
 #include <linux/inet.h>
 #include <linux/configfs.h>
 #include <linux/etherdevice.h>
+#include <linux/utsname.h>
 
 MODULE_AUTHOR("Maintainer: Matt Mackall <mpm@selenic.com>");
 MODULE_DESCRIPTION("Console driver for network interfaces");
@@ -772,8 +773,10 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
 	const char *header, *body;
 	int offset = 0;
 	int header_len, body_len;
+	int uname_len = 0;
 
-	if (msg_len <= MAX_PRINT_CHUNK) {
+	if (msg_len <= MAX_PRINT_CHUNK &&
+	    !IS_ENABLED(CONFIG_NETCONSOLE_UNAME)) {
 		netpoll_send_udp(&nt->np, msg, msg_len);
 		return;
 	}
@@ -788,14 +791,31 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
 	body_len = msg_len - header_len - 1;
 	body++;
 
+	if (IS_ENABLED(CONFIG_NETCONSOLE_UNAME)) {
+		/* Add uname at the beginning of buffer */
+		char *uname = init_utsname()->release;
+		/* uname_len contains the length of uname + ',' */
+		uname_len = strlen(uname) + 1;
+
+		if (uname_len + msg_len < MAX_PRINT_CHUNK) {
+			/* No fragmentation needed */
+			scnprintf(buf, MAX_PRINT_CHUNK, "%s,%s", uname, msg);
+			netpoll_send_udp(&nt->np, buf, uname_len + msg_len);
+			return;
+		}
+
+		/* The data will be fragment, prepending uname */
+		scnprintf(buf, MAX_PRINT_CHUNK, "%s,", uname);
+	}
+
 	/*
 	 * Transfer multiple chunks with the following extra header.
 	 * "ncfrag=<byte-offset>/<total-bytes>"
 	 */
-	memcpy(buf, header, header_len);
+	memcpy(buf + uname_len, header, header_len);
 
 	while (offset < body_len) {
-		int this_header = header_len;
+		int this_header = header_len + uname_len;
 		int this_chunk;
 
 		this_header += scnprintf(buf + this_header,
-- 
2.34.1

