Re: [PATCH 2/2] netcons: Add udp send fail statistics to netconsole

Hey Jakub,
thanks for looking into this.

PS. A couple more email send mistakes and I'll go install mutt, sorry
for the noise :)

On Tue, Aug 27, 2024 at 9:59 AM Jakub Kicinski [off-list ref] wrote:

On Mon, 26 Aug 2024 19:55:36 -0400 Maksym Kutsevol wrote:

quoted

quoted

quoted

+static ssize_t stats_show(struct config_item *item, char *buf)
+{
+     struct netconsole_target *nt = to_target(item);
+
+     return
+             nt->stats.xmit_drop_count, nt->stats.enomem_count);

does configfs require value per file like sysfs or this is okay?

Docs say (Documentation/filesystems/sysfs.txt):

Attributes should be ASCII text files, preferably with only one value
per file. It is noted that it may not be efficient to contain only one
value per file, so it is socially acceptable to express an array of
values of the same type.

Right, but this is for sysfs, main question is whether configfs has
the same expectations.

Eh, my bad, thank you :)

Docs on configfs (Documentation/filesystems/configfs.rst) say approximately
the same, quote:
* Normal attributes, which similar to sysfs attributes, are small ASCII text
  files, with a maximum size of one page (PAGE_SIZE, 4096 on i386).  Preferably
  only one value per file should be used, and the same caveats from sysfs apply.
  Configfs expects write(2) to store the entire buffer at once.  When writing to
  normal configfs attributes, userspace processes should first read the entire
  file, modify the portions they wish to change, and then write the entire
  buffer back.

so based on sysfs+configfs docs it looks ok to do so. What do you think?

Regarding the overall idea of exposing stats via configfs I found this:
https://github.com/torvalds/linux/blob/master/drivers/target/iscsi/iscsi_target_stat.c#L82-L87
as an example of another place doing it, which exposes the number of
active sessions.

quoted

Given those are of the same type, I thought it's ok. To make it less
"fancy" maybe move to
just values separated by whitespace + a block in
Documentation/networking/netconsole.rst describing the format?
E.g. sysfs_emit(buf, "%lu %lu\n", .....) ? I really don't want to have
multiple files for it.
What do you think?

Stats as an array are quite hard to read / understand

I agree with that.
I couldn't find examples of multiple values exported as stats from
configfs. Only from sysfs,
e.g. https://www.kernel.org/doc/Documentation/block/stat.txt, which
describes a whitespace
separated file with stats.

I want to lean on the opinion of someone more experienced in kernel
dev on how to proceed here.
- as is
- whitespace separated like blockdev stats
- multiple files and stop talking about it? :)