linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata
@ 2025-06-11 14:36 Gustavo Luiz Duarte
  2025-06-11 14:36 ` [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field Gustavo Luiz Duarte
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-11 14:36 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman,
	Jonathan Corbet
  Cc: netdev, linux-kernel, linux-kselftest, linux-doc,
	Gustavo Luiz Duarte

This patch series introduces a new feature to netconsole which allows
appending a message ID to the userdata dictionary.

If the msgid feature is enabled, the message ID is built from a per-target 32
bit counter that is incremented and appended to every message sent to the target.

Example::
  echo 1 > "/sys/kernel/config/netconsole/cmdline0/userdata/msgid_enabled"
  echo "This is message #1" > /dev/kmsg
  echo "This is message #2" > /dev/kmsg
  13,434,54928466,-;This is message #1
   msgid=1
  13,435,54934019,-;This is message #2
   msgid=2

This feature can be used by the target to detect if messages were dropped or
reordered before reaching the target. This allows system administrators to
assess the reliability of their netconsole pipeline and detect loss of messages
due to network contention or temporary unavailability.

Suggested-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>

Note to maintainer:
This will conflict with a fix I sent recently to net:

c85bf1975108 netconsole: fix appending sysdata when sysdata_fields ==
SYSDATA_RELEASE

Please let me know if I should rebase at some point and send a v2.

---
Gustavo Luiz Duarte (5):
      netconsole: introduce 'msgid' as a new sysdata field
      netconsole: implement configfs for msgid_enabled
      netconsole: append msgid to sysdata
      selftests: netconsole: Add tests for 'msgid' feature in sysdata
      docs: netconsole: document msgid feature

 Documentation/networking/netconsole.rst            | 22 +++++++
 drivers/net/netconsole.c                           | 67 +++++++++++++++++++++-
 .../selftests/drivers/net/netcons_sysdata.sh       | 30 ++++++++++
 3 files changed, 118 insertions(+), 1 deletion(-)
---
base-commit: 0097c4195b1d0ca57d15979626c769c74747b5a0
change-id: 20250609-netconsole-msgid-b93c6f8e9c60

Best regards,
-- 
Gustavo Luiz Duarte <gustavold@gmail.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field
  2025-06-11 14:36 [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata Gustavo Luiz Duarte
@ 2025-06-11 14:36 ` Gustavo Luiz Duarte
  2025-06-11 15:14   ` Breno Leitao
  2025-06-11 14:36 ` [PATCH net-next 2/5] netconsole: implement configfs for msgid_enabled Gustavo Luiz Duarte
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-11 14:36 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman,
	Jonathan Corbet
  Cc: netdev, linux-kernel, linux-kselftest, linux-doc,
	Gustavo Luiz Duarte

This adds a new sysdata field to enable assigning a per-target unique id
to each message sent to that target. This id can later be appended as
part of sysdata, allowing targets to detect dropped netconsole messages.
Update count_extradata_entries() to take the new field into account.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 21077aff061c..787d170c3a0b 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -113,6 +113,8 @@ enum sysdata_feature {
 	SYSDATA_TASKNAME = BIT(1),
 	/* Kernel release/version as part of sysdata */
 	SYSDATA_RELEASE = BIT(2),
+	/* Include a per-target message ID as part of sysdata */
+	SYSDATA_MSGID = BIT(3),
 };
 
 /**
@@ -782,6 +784,8 @@ static size_t count_extradata_entries(struct netconsole_target *nt)
 		entries += 1;
 	if (nt->sysdata_fields & SYSDATA_RELEASE)
 		entries += 1;
+	if (nt->sysdata_fields & SYSDATA_MSGID)
+		entries += 1;
 
 	return entries;
 }

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 2/5] netconsole: implement configfs for msgid_enabled
  2025-06-11 14:36 [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata Gustavo Luiz Duarte
  2025-06-11 14:36 ` [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field Gustavo Luiz Duarte
@ 2025-06-11 14:36 ` Gustavo Luiz Duarte
  2025-06-11 15:22   ` Breno Leitao
  2025-06-11 14:36 ` [PATCH net-next 3/5] netconsole: append msgid to sysdata Gustavo Luiz Duarte
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-11 14:36 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman,
	Jonathan Corbet
  Cc: netdev, linux-kernel, linux-kselftest, linux-doc,
	Gustavo Luiz Duarte

Implement the _show and _store functions for the msgid_enabled configfs
attribute under userdata.
Set the sysdata_fields bit accordingly.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 787d170c3a0b..813f50abaf9f 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -489,6 +489,19 @@ static void unregister_netcons_consoles(void)
 		unregister_console(&netconsole);
 }
 
+static ssize_t sysdata_msgid_enabled_show(struct config_item *item,
+					  char *buf)
+{
+	struct netconsole_target *nt = to_target(item->ci_parent);
+	bool msgid_enabled;
+
+	mutex_lock(&dynamic_netconsole_mutex);
+	msgid_enabled = !!(nt->sysdata_fields & SYSDATA_MSGID);
+	mutex_unlock(&dynamic_netconsole_mutex);
+
+	return sysfs_emit(buf, "%d\n", msgid_enabled);
+}
+
 /*
  * This one is special -- targets created through the configfs interface
  * are not enabled (and the corresponding netpoll activated) by default.
@@ -922,6 +935,40 @@ static void disable_sysdata_feature(struct netconsole_target *nt,
 	nt->extradata_complete[nt->userdata_length] = 0;
 }
 
+static ssize_t sysdata_msgid_enabled_store(struct config_item *item,
+					   const char *buf, size_t count)
+{
+	struct netconsole_target *nt = to_target(item->ci_parent);
+	bool msgid_enabled, curr;
+	ssize_t ret;
+
+	ret = kstrtobool(buf, &msgid_enabled);
+	if (ret)
+		return ret;
+
+	mutex_lock(&dynamic_netconsole_mutex);
+	curr = !!(nt->sysdata_fields & SYSDATA_MSGID);
+	if (msgid_enabled == curr)
+		goto unlock_ok;
+
+	if (msgid_enabled &&
+	    count_extradata_entries(nt) >= MAX_EXTRADATA_ITEMS) {
+		ret = -ENOSPC;
+		goto unlock;
+	}
+
+	if (msgid_enabled)
+		nt->sysdata_fields |= SYSDATA_MSGID;
+	else
+		disable_sysdata_feature(nt, SYSDATA_MSGID);
+
+unlock_ok:
+	ret = strnlen(buf, count);
+unlock:
+	mutex_unlock(&dynamic_netconsole_mutex);
+	return ret;
+}
+
 static ssize_t sysdata_release_enabled_store(struct config_item *item,
 					     const char *buf, size_t count)
 {
@@ -1037,6 +1084,7 @@ CONFIGFS_ATTR(userdatum_, value);
 CONFIGFS_ATTR(sysdata_, cpu_nr_enabled);
 CONFIGFS_ATTR(sysdata_, taskname_enabled);
 CONFIGFS_ATTR(sysdata_, release_enabled);
+CONFIGFS_ATTR(sysdata_, msgid_enabled);
 
 static struct configfs_attribute *userdatum_attrs[] = {
 	&userdatum_attr_value,
@@ -1099,6 +1147,7 @@ static struct configfs_attribute *userdata_attrs[] = {
 	&sysdata_attr_cpu_nr_enabled,
 	&sysdata_attr_taskname_enabled,
 	&sysdata_attr_release_enabled,
+	&sysdata_attr_msgid_enabled,
 	NULL,
 };
 

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 3/5] netconsole: append msgid to sysdata
  2025-06-11 14:36 [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata Gustavo Luiz Duarte
  2025-06-11 14:36 ` [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field Gustavo Luiz Duarte
  2025-06-11 14:36 ` [PATCH net-next 2/5] netconsole: implement configfs for msgid_enabled Gustavo Luiz Duarte
@ 2025-06-11 14:36 ` Gustavo Luiz Duarte
  2025-06-11 15:30   ` Breno Leitao
  2025-06-11 14:36 ` [PATCH net-next 4/5] selftests: netconsole: Add tests for 'msgid' feature in sysdata Gustavo Luiz Duarte
  2025-06-11 14:36 ` [PATCH net-next 5/5] docs: netconsole: document msgid feature Gustavo Luiz Duarte
  4 siblings, 1 reply; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-11 14:36 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman,
	Jonathan Corbet
  Cc: netdev, linux-kernel, linux-kselftest, linux-doc,
	Gustavo Luiz Duarte

Add msgcounter to the netconsole_target struct to generate message IDs.
If the msgid_enabled attribute is true, increment msgcounter and append
msgid=<msgcounter> to sysdata buffer before sending the message.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 drivers/net/netconsole.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
index 813f50abaf9f..34b61e299eeb 100644
--- a/drivers/net/netconsole.c
+++ b/drivers/net/netconsole.c
@@ -155,6 +155,8 @@ struct netconsole_target {
 	size_t			userdata_length;
 	/* bit-wise with sysdata_feature bits */
 	u32			sysdata_fields;
+	/* protected by target_list_lock */
+	u32			msgcounter;
 #endif
 	struct netconsole_target_stats stats;
 	bool			enabled;
@@ -1345,13 +1347,21 @@ static int sysdata_append_release(struct netconsole_target *nt, int offset)
 			 init_utsname()->release);
 }
 
+static int sysdata_append_msgid(struct netconsole_target *nt, int offset)
+{
+	nt->msgcounter++;
+	return scnprintf(&nt->extradata_complete[offset],
+			 MAX_EXTRADATA_ENTRY_LEN, " msgid=%u\n",
+			 nt->msgcounter);
+}
+
 /*
  * prepare_extradata - append sysdata at extradata_complete in runtime
  * @nt: target to send message to
  */
 static int prepare_extradata(struct netconsole_target *nt)
 {
-	u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME;
+	u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME | SYSDATA_MSGID;
 	int extradata_len;
 
 	/* userdata was appended when configfs write helper was called
@@ -1368,6 +1378,8 @@ static int prepare_extradata(struct netconsole_target *nt)
 		extradata_len += sysdata_append_taskname(nt, extradata_len);
 	if (nt->sysdata_fields & SYSDATA_RELEASE)
 		extradata_len += sysdata_append_release(nt, extradata_len);
+	if (nt->sysdata_fields & SYSDATA_MSGID)
+		extradata_len += sysdata_append_msgid(nt, extradata_len);
 
 	WARN_ON_ONCE(extradata_len >
 		     MAX_EXTRADATA_ENTRY_LEN * MAX_EXTRADATA_ITEMS);

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 4/5] selftests: netconsole: Add tests for 'msgid' feature in sysdata
  2025-06-11 14:36 [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata Gustavo Luiz Duarte
                   ` (2 preceding siblings ...)
  2025-06-11 14:36 ` [PATCH net-next 3/5] netconsole: append msgid to sysdata Gustavo Luiz Duarte
@ 2025-06-11 14:36 ` Gustavo Luiz Duarte
  2025-06-11 14:36 ` [PATCH net-next 5/5] docs: netconsole: document msgid feature Gustavo Luiz Duarte
  4 siblings, 0 replies; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-11 14:36 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman,
	Jonathan Corbet
  Cc: netdev, linux-kernel, linux-kselftest, linux-doc,
	Gustavo Luiz Duarte

Extend the self-tests to cover the 'msgid' feature in sysdata.

Verify that msgid is appended to the message when the feature is enabled
and that it is not appended when the feature is disabled.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 .../selftests/drivers/net/netcons_sysdata.sh       | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/netcons_sysdata.sh b/tools/testing/selftests/drivers/net/netcons_sysdata.sh
index a737e377bf08..baf69031089e 100755
--- a/tools/testing/selftests/drivers/net/netcons_sysdata.sh
+++ b/tools/testing/selftests/drivers/net/netcons_sysdata.sh
@@ -53,6 +53,17 @@ function set_release() {
 	echo 1 > "${NETCONS_PATH}/userdata/release_enabled"
 }
 
+# Enable the msgid to be appended to sysdata
+function set_msgid() {
+	if [[ ! -f "${NETCONS_PATH}/userdata/msgid_enabled" ]]
+	then
+		echo "Not able to enable msgid sysdata append. Configfs not available in ${NETCONS_PATH}/userdata/msgid_enabled" >&2
+		exit "${ksft_skip}"
+	fi
+
+	echo 1 > "${NETCONS_PATH}/userdata/msgid_enabled"
+}
+
 # Disable the sysdata cpu_nr feature
 function unset_cpu_nr() {
 	echo 0 > "${NETCONS_PATH}/userdata/cpu_nr_enabled"
@@ -67,6 +78,10 @@ function unset_release() {
 	echo 0 > "${NETCONS_PATH}/userdata/release_enabled"
 }
 
+function unset_msgid() {
+	echo 0 > "${NETCONS_PATH}/userdata/msgid_enabled"
+}
+
 # Test if MSG contains sysdata
 function validate_sysdata() {
 	# OUTPUT_FILE will contain something like:
@@ -74,6 +89,7 @@ function validate_sysdata() {
 	#  userdatakey=userdatavalue
 	#  cpu=X
 	#  taskname=<taskname>
+	#  msgid=<id>
 
 	# Echo is what this test uses to create the message. See runtest()
 	# function
@@ -104,6 +120,12 @@ function validate_sysdata() {
 		exit "${ksft_fail}"
 	fi
 
+	if ! grep -q "msgid=[0-9]\+$" "${OUTPUT_FILE}"; then
+		echo "FAIL: 'msgid=<id>' not found in ${OUTPUT_FILE}" >&2
+		cat "${OUTPUT_FILE}" >&2
+		exit "${ksft_fail}"
+	fi
+
 	rm "${OUTPUT_FILE}"
 	pkill_socat
 }
@@ -155,6 +177,12 @@ function validate_no_sysdata() {
 		exit "${ksft_fail}"
 	fi
 
+	if grep -q "msgid=" "${OUTPUT_FILE}"; then
+		echo "FAIL: 'msgid=  found in ${OUTPUT_FILE}" >&2
+		cat "${OUTPUT_FILE}" >&2
+		exit "${ksft_fail}"
+	fi
+
 	rm "${OUTPUT_FILE}"
 }
 
@@ -206,6 +234,7 @@ set_cpu_nr
 # Enable taskname to be appended to sysdata
 set_taskname
 set_release
+set_msgid
 runtest
 # Make sure the message was received in the dst part
 # and exit
@@ -235,6 +264,7 @@ MSG="Test #3 from CPU${CPU}"
 unset_cpu_nr
 unset_taskname
 unset_release
+unset_msgid
 runtest
 # At this time, cpu= shouldn't be present in the msg
 validate_no_sysdata

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH net-next 5/5] docs: netconsole: document msgid feature
  2025-06-11 14:36 [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata Gustavo Luiz Duarte
                   ` (3 preceding siblings ...)
  2025-06-11 14:36 ` [PATCH net-next 4/5] selftests: netconsole: Add tests for 'msgid' feature in sysdata Gustavo Luiz Duarte
@ 2025-06-11 14:36 ` Gustavo Luiz Duarte
  2025-06-11 15:33   ` Breno Leitao
  4 siblings, 1 reply; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-11 14:36 UTC (permalink / raw)
  To: Breno Leitao, Andrew Lunn, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Shuah Khan, Simon Horman,
	Jonathan Corbet
  Cc: netdev, linux-kernel, linux-kselftest, linux-doc,
	Gustavo Luiz Duarte

Add documentation explaining the msgid feature in netconsole.

This feature appends unique id to the userdata dictionary. The message
ID is populated from a per-target 32 bit counter which is incremented
for each message sent to the target. This allows a target to detect if
messages are dropped before reaching the target.

Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
---
 Documentation/networking/netconsole.rst | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
index a0076b542e9c..42a0acf2eb5e 100644
--- a/Documentation/networking/netconsole.rst
+++ b/Documentation/networking/netconsole.rst
@@ -340,6 +340,28 @@ In this example, the message was sent by CPU 42.
       cpu=42    # kernel-populated value
 
 
+Message ID auto population in userdata
+--------------------------------------
+
+Within the netconsole configfs hierarchy, there is a file named `msgid_enabled`
+located in the `userdata` directory. This file controls the message ID
+auto-population feature, which assigns a unique id to each message sent to a
+given target and appends the ID to userdata dictionary in every message sent.
+
+The message ID is built from a per-target 32 bit counter that is incremented
+for every message sent to the target. This ID can be used by the target to
+detect if messages were dropped before reaching the target.
+
+Example::
+
+  echo "This is message #1" > /dev/kmsg
+  echo "This is message #2" > /dev/kmsg
+  13,434,54928466,-;This is message #1
+   msgid=1
+  13,435,54934019,-;This is message #2
+   msgid=2
+
+
 Extended console:
 =================
 

-- 
2.47.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field
  2025-06-11 14:36 ` [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field Gustavo Luiz Duarte
@ 2025-06-11 15:14   ` Breno Leitao
  0 siblings, 0 replies; 12+ messages in thread
From: Breno Leitao @ 2025-06-11 15:14 UTC (permalink / raw)
  To: Gustavo Luiz Duarte
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, Jonathan Corbet, netdev,
	linux-kernel, linux-kselftest, linux-doc

On Wed, Jun 11, 2025 at 07:36:03AM -0700, Gustavo Luiz Duarte wrote:
> This adds a new sysdata field to enable assigning a per-target unique id
> to each message sent to that target. This id can later be appended as
> part of sysdata, allowing targets to detect dropped netconsole messages.
> Update count_extradata_entries() to take the new field into account.
> 
> Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 2/5] netconsole: implement configfs for msgid_enabled
  2025-06-11 14:36 ` [PATCH net-next 2/5] netconsole: implement configfs for msgid_enabled Gustavo Luiz Duarte
@ 2025-06-11 15:22   ` Breno Leitao
  0 siblings, 0 replies; 12+ messages in thread
From: Breno Leitao @ 2025-06-11 15:22 UTC (permalink / raw)
  To: Gustavo Luiz Duarte
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, Jonathan Corbet, netdev,
	linux-kernel, linux-kselftest, linux-doc

On Wed, Jun 11, 2025 at 07:36:04AM -0700, Gustavo Luiz Duarte wrote:
> Implement the _show and _store functions for the msgid_enabled configfs
> attribute under userdata.
> Set the sysdata_fields bit accordingly.
> 
> Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>

Reviewed-by: Breno Leitao <leitao@debian.org>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 3/5] netconsole: append msgid to sysdata
  2025-06-11 14:36 ` [PATCH net-next 3/5] netconsole: append msgid to sysdata Gustavo Luiz Duarte
@ 2025-06-11 15:30   ` Breno Leitao
  2025-06-11 20:40     ` Jakub Kicinski
  2025-06-12 13:06     ` Gustavo Luiz Duarte
  0 siblings, 2 replies; 12+ messages in thread
From: Breno Leitao @ 2025-06-11 15:30 UTC (permalink / raw)
  To: Gustavo Luiz Duarte
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, Jonathan Corbet, netdev,
	linux-kernel, linux-kselftest, linux-doc

On Wed, Jun 11, 2025 at 07:36:05AM -0700, Gustavo Luiz Duarte wrote:
> Add msgcounter to the netconsole_target struct to generate message IDs.
> If the msgid_enabled attribute is true, increment msgcounter and append
> msgid=<msgcounter> to sysdata buffer before sending the message.
> 
> Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
> ---
>  drivers/net/netconsole.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> index 813f50abaf9f..34b61e299eeb 100644
> --- a/drivers/net/netconsole.c
> +++ b/drivers/net/netconsole.c
> @@ -155,6 +155,8 @@ struct netconsole_target {
>  	size_t			userdata_length;
>  	/* bit-wise with sysdata_feature bits */
>  	u32			sysdata_fields;
> +	/* protected by target_list_lock */
> +	u32			msgcounter;
>  #endif
>  	struct netconsole_target_stats stats;
>  	bool			enabled;
> @@ -1345,13 +1347,21 @@ static int sysdata_append_release(struct netconsole_target *nt, int offset)
>  			 init_utsname()->release);
>  }
>  
> +static int sysdata_append_msgid(struct netconsole_target *nt, int offset)
> +{
> +	nt->msgcounter++;

This will eventually wrap. I am wondering if you should use the
overflow.h helpers to avoid warnings in UBSAN and friends.

Quick glanced over that filed, I found:

	/**
	* wrapping_add() - Intentionally perform a wrapping addition
	* @type: type for result of calculation
	* @a: first addend
	* @b: second addend
	*
	* Return the potentially wrapped-around addition without
	* tripping any wrap-around sanitizers that may be enabled.
	*/

> +	return scnprintf(&nt->extradata_complete[offset],
> +			 MAX_EXTRADATA_ENTRY_LEN, " msgid=%u\n",
> +			 nt->msgcounter);
> +}
> +
>  /*
>   * prepare_extradata - append sysdata at extradata_complete in runtime
>   * @nt: target to send message to
>   */
>  static int prepare_extradata(struct netconsole_target *nt)
>  {
> -	u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME;
> +	u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME | SYSDATA_MSGID;

This might be gone now, according to your last patch.,

LGTM.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 5/5] docs: netconsole: document msgid feature
  2025-06-11 14:36 ` [PATCH net-next 5/5] docs: netconsole: document msgid feature Gustavo Luiz Duarte
@ 2025-06-11 15:33   ` Breno Leitao
  0 siblings, 0 replies; 12+ messages in thread
From: Breno Leitao @ 2025-06-11 15:33 UTC (permalink / raw)
  To: Gustavo Luiz Duarte
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, Jonathan Corbet, netdev,
	linux-kernel, linux-kselftest, linux-doc

On Wed, Jun 11, 2025 at 07:36:07AM -0700, Gustavo Luiz Duarte wrote:
> Add documentation explaining the msgid feature in netconsole.
> 
> This feature appends unique id to the userdata dictionary. The message
> ID is populated from a per-target 32 bit counter which is incremented
> for each message sent to the target. This allows a target to detect if
> messages are dropped before reaching the target.
> 
> Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
> ---
>  Documentation/networking/netconsole.rst | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/Documentation/networking/netconsole.rst b/Documentation/networking/netconsole.rst
> index a0076b542e9c..42a0acf2eb5e 100644
> --- a/Documentation/networking/netconsole.rst
> +++ b/Documentation/networking/netconsole.rst
> @@ -340,6 +340,28 @@ In this example, the message was sent by CPU 42.
>        cpu=42    # kernel-populated value
>  
>  
> +Message ID auto population in userdata
> +--------------------------------------
> +
> +Within the netconsole configfs hierarchy, there is a file named `msgid_enabled`
> +located in the `userdata` directory. This file controls the message ID
> +auto-population feature, which assigns a unique id to each message sent to a

Important to say that the message id will eventually wrap, thus it is
not very unique.

> +given target and appends the ID to userdata dictionary in every message sent.
> +
> +The message ID is built from a per-target 32 bit counter that is incremented
> +for every message sent to the target. This ID can be used by the target to
> +detect if messages were dropped before reaching the target.

Please also add that we cannot rely on console ids, given some messages
never make netconsole, thus, we never now if the message was never sent,
or, never received.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 3/5] netconsole: append msgid to sysdata
  2025-06-11 15:30   ` Breno Leitao
@ 2025-06-11 20:40     ` Jakub Kicinski
  2025-06-12 13:06     ` Gustavo Luiz Duarte
  1 sibling, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2025-06-11 20:40 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Gustavo Luiz Duarte, Andrew Lunn, David S. Miller, Eric Dumazet,
	Paolo Abeni, Shuah Khan, Simon Horman, Jonathan Corbet, netdev,
	linux-kernel, linux-kselftest, linux-doc

On Wed, 11 Jun 2025 08:30:11 -0700 Breno Leitao wrote:
> >  static int prepare_extradata(struct netconsole_target *nt)
> >  {
> > -	u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME;
> > +	u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME | SYSDATA_MSGID;  
> 
> This might be gone now, according to your last patch.,

Right, please wait until tomorrow afternoon (Pacific Time), then rebase
and repost. We will merge the net tree into net-next after we send the
pending fixes to Linus. Otherwise by the time we try to merge this it
will no longer apply..

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next 3/5] netconsole: append msgid to sysdata
  2025-06-11 15:30   ` Breno Leitao
  2025-06-11 20:40     ` Jakub Kicinski
@ 2025-06-12 13:06     ` Gustavo Luiz Duarte
  1 sibling, 0 replies; 12+ messages in thread
From: Gustavo Luiz Duarte @ 2025-06-12 13:06 UTC (permalink / raw)
  To: Breno Leitao
  Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Shuah Khan, Simon Horman, Jonathan Corbet, netdev,
	linux-kernel, linux-kselftest, linux-doc

On Wed, Jun 11, 2025 at 4:30 PM Breno Leitao <leitao@debian.org> wrote:
>
> On Wed, Jun 11, 2025 at 07:36:05AM -0700, Gustavo Luiz Duarte wrote:
> > Add msgcounter to the netconsole_target struct to generate message IDs.
> > If the msgid_enabled attribute is true, increment msgcounter and append
> > msgid=<msgcounter> to sysdata buffer before sending the message.
> >
> > Signed-off-by: Gustavo Luiz Duarte <gustavold@gmail.com>
> > ---
> >  drivers/net/netconsole.c | 14 +++++++++++++-
> >  1 file changed, 13 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/netconsole.c b/drivers/net/netconsole.c
> > index 813f50abaf9f..34b61e299eeb 100644
> > --- a/drivers/net/netconsole.c
> > +++ b/drivers/net/netconsole.c
> > @@ -155,6 +155,8 @@ struct netconsole_target {
> >       size_t                  userdata_length;
> >       /* bit-wise with sysdata_feature bits */
> >       u32                     sysdata_fields;
> > +     /* protected by target_list_lock */
> > +     u32                     msgcounter;
> >  #endif
> >       struct netconsole_target_stats stats;
> >       bool                    enabled;
> > @@ -1345,13 +1347,21 @@ static int sysdata_append_release(struct netconsole_target *nt, int offset)
> >                        init_utsname()->release);
> >  }
> >
> > +static int sysdata_append_msgid(struct netconsole_target *nt, int offset)
> > +{
> > +     nt->msgcounter++;
>
> This will eventually wrap. I am wondering if you should use the
> overflow.h helpers to avoid warnings in UBSAN and friends.

Good point. I will send v2 using one of the overflow safe macros.
Thanks for reviewing!

>
> Quick glanced over that filed, I found:
>
>         /**
>         * wrapping_add() - Intentionally perform a wrapping addition
>         * @type: type for result of calculation
>         * @a: first addend
>         * @b: second addend
>         *
>         * Return the potentially wrapped-around addition without
>         * tripping any wrap-around sanitizers that may be enabled.
>         */
>
> > +     return scnprintf(&nt->extradata_complete[offset],
> > +                      MAX_EXTRADATA_ENTRY_LEN, " msgid=%u\n",
> > +                      nt->msgcounter);
> > +}
> > +
> >  /*
> >   * prepare_extradata - append sysdata at extradata_complete in runtime
> >   * @nt: target to send message to
> >   */
> >  static int prepare_extradata(struct netconsole_target *nt)
> >  {
> > -     u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME;
> > +     u32 fields = SYSDATA_CPU_NR | SYSDATA_TASKNAME | SYSDATA_MSGID;
>
> This might be gone now, according to your last patch.,
>
> LGTM.

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-06-12 13:07 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11 14:36 [PATCH net-next 0/5] netconsole: Add support for msgid in sysdata Gustavo Luiz Duarte
2025-06-11 14:36 ` [PATCH net-next 1/5] netconsole: introduce 'msgid' as a new sysdata field Gustavo Luiz Duarte
2025-06-11 15:14   ` Breno Leitao
2025-06-11 14:36 ` [PATCH net-next 2/5] netconsole: implement configfs for msgid_enabled Gustavo Luiz Duarte
2025-06-11 15:22   ` Breno Leitao
2025-06-11 14:36 ` [PATCH net-next 3/5] netconsole: append msgid to sysdata Gustavo Luiz Duarte
2025-06-11 15:30   ` Breno Leitao
2025-06-11 20:40     ` Jakub Kicinski
2025-06-12 13:06     ` Gustavo Luiz Duarte
2025-06-11 14:36 ` [PATCH net-next 4/5] selftests: netconsole: Add tests for 'msgid' feature in sysdata Gustavo Luiz Duarte
2025-06-11 14:36 ` [PATCH net-next 5/5] docs: netconsole: document msgid feature Gustavo Luiz Duarte
2025-06-11 15:33   ` Breno Leitao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).