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

[Breno Leitao]

Hello Jakub,

On Mon, Jan 20, 2025 at 11:06:53AM -0800, Jakub Kicinski wrote:
> >
> > 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

> > 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@xxxxxxxxxx/

Again, thanks for reviewing this change,
--breno