Re: [PATCH net-next v2 3/5] netconsole: add support for sysdata and CPU population

Hello Jakub,

On Mon, Jan 20, 2025 at 11:06:53AM -0800, Jakub Kicinski wrote:

quoted

The only way to do it properly is having a extra buffer where we
have `cpu=42` and copy 5 bytes from there, and then copy the last one in
the next iteration. I am not sure we can do it in one shot.

FWIW to simplify reasoning about the length I thought we could take the
worst case, assume we'll need len(cpu=) + log10(nr_cpu_ids) of space.

We can do that, but, we are going to come back to this discussion again
as soon we expand sysdata. For instance, I have plans to expand it to
have task_struct->comm, release, etc.

For that, we need to know the length of the struct ahead of time

quoted

1) Average messages size will become bigger. Thus, memcpy will be needed
one way or another.

2) Unless we can come up with a smart solution, this solution will be
harder to reason about.

If you want to invest more time in this direction, I am more than happy
to create a PoC, so we can discuss more concretely. 

I don't feel super strongly about this. But hacking around is always
good to get a sense of how hairy the implementation ends up being.


To rephrase my concern is that we have some data as static on the
stack, some dynamically appended at the send_*() stage, now we're
adding a third way of handling things. Perhaps the simplest way to
make me happy would be to move the bufs which are currently static 
into nt.

I've hacked it, and I think I addressed most of these concerns. This is
how the new RFC is:

1) moved the buffer to netconsole_target. no more static buffer.
2) created a function called prepare_extradata(), which will handle
   sysdata and userdata.
	2.1) to be fair, userdata is already in the temporary buffer
	  (extradata_complete) since it doesn't change frequently, only
	  when configfs helpers are called. We can parse configfs nodes
	  to generate it in runtime, but, this will be unnecessary.

3) prepare_extradata() is called once at the send path.

I've just sent an RFC (v3) with the full changes, let's see if it
improves your concerns.

https://lore.kernel.org/all/20250124-netcon_cpu-v3-0-12a0d286ba1d@debian.org/ (local)

Again, thanks for reviewing this change,
--breno