Following the previous change, where the non-fragmented case was moved
to its own function, this update introduces a new function called
send_msg_fragmented to specifically manage scenarios where message
fragmentation is required.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/netconsole.c | 129 ++++++++++++++++++++++-----------------
 1 file changed, 74 insertions(+), 55 deletions(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 0e43dacbd291..176ce6c616cb 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -1058,66 +1058,20 @@ static struct notifier_block netconsole_netdev_notifier = {
 	.notifier_call  = netconsole_netdev_event,
 };
 
-static void send_msg_no_fragmentation(struct netconsole_target *nt,
-				      const char *msg,
-				      const char *userdata,
-				      int msg_len,
-				      int release_len)
-{
-	static char buf[MAX_PRINT_CHUNK]; /* protected by target_list_lock */
-	const char *release;
-
-	if (release_len) {
-		release = init_utsname()->release;
-
-		scnprintf(buf, MAX_PRINT_CHUNK, "%s,%s", release, msg);
-		msg_len += release_len;
-	} else {
-		memcpy(buf, msg, msg_len);
-	}
-
-	if (userdata)
-		msg_len += scnprintf(&buf[msg_len],
-				     MAX_PRINT_CHUNK - msg_len,
-				     "%s", userdata);
-
-	netpoll_send_udp(&nt->np, buf, msg_len);
-}
-
-/**
- * send_ext_msg_udp - send extended log message to target
- * @nt: target to send message to
- * @msg: extended log message to send
- * @msg_len: length of message
- *
- * Transfer extended log @msg to @nt.  If @msg is longer than
- * MAX_PRINT_CHUNK, it'll be split and transmitted in multiple chunks with
- * ncfrag header field added to identify them.
- */
-static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
-			     int msg_len)
+static void send_msg_fragmented(struct netconsole_target *nt,
+				const char *msg,
+				const char *userdata,
+				int msg_len,
+				int release_len)
 {
 	static char buf[MAX_PRINT_CHUNK]; /* protected by target_list_lock */
+	int offset = 0, userdata_len = 0;
 	const char *header, *body;
-	int offset = 0;
 	int header_len, body_len;
 	const char *release;
-	int release_len = 0;
-	int userdata_len = 0;
-	char *userdata = NULL;
-
-#ifdef CONFIG_NETCONSOLE_DYNAMIC
-	userdata = nt->userdata_complete;
-	userdata_len = nt->userdata_length;
-#endif
-
-	if (nt->release) {
-		release = init_utsname()->release;
-		release_len = strlen(release) + 1;
-	}
 
-	if (msg_len + release_len + userdata_len <= MAX_PRINT_CHUNK)
-		return send_msg_no_fragmentation(nt, msg, userdata, msg_len, release_len);
+	if (userdata)
+		userdata_len = nt->userdata_length;
 
 	/* need to insert extra header fields, detect header and body */
 	header = msg;
@@ -1133,11 +1087,18 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
 	 * Transfer multiple chunks with the following extra header.
 	 * "ncfrag=<byte-offset>/<total-bytes>"
 	 */
-	if (nt->release)
+	if (release_len) {
+		release = init_utsname()->release;
 		scnprintf(buf, MAX_PRINT_CHUNK, "%s,", release);
+	}
+
+	/* Copy the header into the buffer */
 	memcpy(buf + release_len, header, header_len);
 	header_len += release_len;
 
+	/* for now on, the header will be persisted, and the body
+	 * will be replaced
+	 */
 	while (offset < body_len + userdata_len) {
 		int this_header = header_len;
 		int this_offset = 0;
@@ -1183,6 +1144,64 @@ static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
 	}
 }
 
+static void send_msg_no_fragmentation(struct netconsole_target *nt,
+				      const char *msg,
+				      const char *userdata,
+				      int msg_len,
+				      int release_len)
+{
+	static char buf[MAX_PRINT_CHUNK]; /* protected by target_list_lock */
+	const char *release;
+
+	if (release_len) {
+		release = init_utsname()->release;
+
+		scnprintf(buf, MAX_PRINT_CHUNK, "%s,%s", release, msg);
+		msg_len += release_len;
+	} else {
+		memcpy(buf, msg, msg_len);
+	}
+
+	if (userdata)
+		msg_len += scnprintf(&buf[msg_len],
+				     MAX_PRINT_CHUNK - msg_len,
+				     "%s", userdata);
+
+	netpoll_send_udp(&nt->np, buf, msg_len);
+}
+
+/**
+ * send_ext_msg_udp - send extended log message to target
+ * @nt: target to send message to
+ * @msg: extended log message to send
+ * @msg_len: length of message
+ *
+ * Transfer extended log @msg to @nt.  If @msg is longer than
+ * MAX_PRINT_CHUNK, it'll be split and transmitted in multiple chunks with
+ * ncfrag header field added to identify them.
+ */
+static void send_ext_msg_udp(struct netconsole_target *nt, const char *msg,
+			     int msg_len)
+{
+	char *userdata = NULL;
+	int userdata_len = 0;
+	int release_len = 0;
+
+#ifdef CONFIG_NETCONSOLE_DYNAMIC
+	userdata = nt->userdata_complete;
+	userdata_len = nt->userdata_length;
+#endif
+
+	if (nt->release)
+		release_len = strlen(init_utsname()->release) + 1;
+
+	if (msg_len + release_len + userdata_len <= MAX_PRINT_CHUNK)
+		return send_msg_no_fragmentation(nt, msg, userdata, msg_len,
+						 release_len);
+
+	return send_msg_fragmented(nt, msg, userdata, msg_len, release_len);
+}
+
 static void write_ext_msg(struct console *con, const char *msg,
 			  unsigned int len)
 {
-- 
2.43.5

On Tue, Sep 03, 2024 at 07:07:46AM -0700, Breno Leitao wrote:
> Following the previous change, where the non-fragmented case was moved
> to its own function, this update introduces a new function called
> send_msg_fragmented to specifically manage scenarios where message
> fragmentation is required.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>

Due to tooling the diff below seems to more verbose than the change
warrants. Perhaps some diff flags would alleviate this, but anyone viewing
the patch using git with default flags, would see what is below anyway.

So I wonder if you could consider moving send_msg_fragmented()
to above send_msg_no_fragmentation(). Locally this lead to an entirely
more reasonable diff to review.

I did review this change using that technique, and it looks good to me.

Reviewed-by: Simon Horman <horms@kernel.org>

On Wed, Sep 04, 2024 at 11:59:20AM +0100, Simon Horman wrote:
> On Tue, Sep 03, 2024 at 07:07:46AM -0700, Breno Leitao wrote:
> > Following the previous change, where the non-fragmented case was moved
> > to its own function, this update introduces a new function called
> > send_msg_fragmented to specifically manage scenarios where message
> > fragmentation is required.
> > 
> > Signed-off-by: Breno Leitao <leitao@debian.org>
> 
> Due to tooling the diff below seems to more verbose than the change
> warrants. Perhaps some diff flags would alleviate this, but anyone viewing
> the patch using git with default flags, would see what is below anyway.
> 
> So I wonder if you could consider moving send_msg_fragmented()
> to above send_msg_no_fragmentation(). Locally this lead to an entirely
> more reasonable diff to review.

I agree. Let me move the functions around aiming to generate an
easy-to-review diff.

Thanks for the feedback.