Re: [PATCH v2 1/3] Fix handling of netlink multipart route dumps.

[Stefano Brivio <sbrivio@redhat.com>]

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 <ms+git@mur.at> wrote:

> 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-your-work-the-developer-s-certificate-of-origin

...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