I just replied to your v4, some left-over comments from here that might
still be relevant:
On Sat, 19 Sep 2026 14:56:36 +0200
Martin Schitter
On 9/17/26 21:08, Stefano Brivio wrote:
[...]
Here, and in following patches: missing Signed-off-by: tag (see CONTRIBUTING.md).
o.k. I'll add it -- although I'm not convinced that it makes much sense in a modern jj based patch description workflow.
jj meaning... Jujutsu? But we don't use it? Anyway, the idea behind it is described here: https://www.kernel.org/doc/html/next/process/submitting-patches.html#sign-yo... ...it's not so much about the workflow. It's about a concise way of certifying all that, and it might be something we'll need at some point in the future.
[...]
+ nh = nl_next(s, &buf[collected], NULL, &n); + debug("collect: add chunk of size n=%ld at offset=%ld, seq=%d nlmsg_len=%d", + n, collected, seq, ((const struct nlmsghdr *)&buf[collected])->nlmsg_len);
At some point, we should get a netlink monitor (see https://bugs.passt.top/show_bug.cgi?id=141) which might run this function quite frequently at runtime, and having this as debug() message would risk flooding debug logs and making them otherwise unusable.
You can eliminate all my debug print commands just as you like.
But as long as we do not have suitable unit tests to control at least a subset of common cases in advance, debug logs could be very helpful to iron out remaining mistakes and false assumptions.
Well, you can create tests if you want (see test/README.md). Not unit tests, but you could easily build a test building a large number of routes (within a pasta environment) and then run pasta --config-net into it. And yes, I don't doubt of the value this kind of debug print, but still the downsides seems to outweigh the benefits in this case.
btw.: I don't think you should run this kind of routines frequently.
I don't expect we'll need to duplicate hundreds of routes per second, so that's not the kind of overhead I meant. It's mostly routes coming and going frequently and spamming logs.
Kernel-userspace-transfers are rather expensive. Perhaps you could try to utilize eBPF to handle these route table duplication resp. transfer between namespaces in a more efficient manner.
No root, no eBPF. That's kind of the whole point of this project. :)
I think we could consider trace(). My preference would actually to drop this altogether because it seems to be specific to some particular phase of your development and not something of general relevance.
That's o.k. for me.
But please consider, that we do not have much control about many details of the actual kernel behavior in respect to this complex netlink multipart dumps. It can be rather challenging to reproduce and solve all related issues without useful debug log capabilities.
Yeah, I see, on the other hand if users report something specific they can also try out a special build with added debugging.
If we want to keep this for some reason: "collect" isn't very descriptive (mind that it will just be printed among other messages coming from completely different parts of the code). I would rather use __func__.
__func__ and similar more precise code location prefixes would IMHO make the output even more dense, confusing, and cluttering.
Maybe it doesn't look good, but at least it tells you what it is about, instead of "collect:". -- Stefano