Re: [PATCH v2] feat: Pass open files to child in pasta mode

[David Gibson <david@gibson.dropbear.id.au>]

On Fri, Aug 14, 2026 at 11:43:19PM +0200, Stefano Brivio wrote:
> Hi Richard,
> 
> On Fri, 14 Aug 2026 18:12:56 +0000
> "Lawrence, Richard E" <rlawrence@tamu.edu> wrote:
> 
> > Howdy Stefano, David,
> > 
> > I don't understand the plan. Are you waiting on me? Am I waiting on you?
> > 
> > I could implement any of these ideas, but I don't wish to implement **all** of them. Can we reach a consensus?
> 
> Sorry for the confusion and the lack of updates. David mentionen, in
> the latest weekly call (see also https://pad.passt.top/p/weekly), that
> he plans to look into a way to avoid closing files we open altogether,
> but in a more lightweight way than the approaches I was suggesting, that
> is, he wanted to look into whether we can reshuffle the order of some
> initial operations instead.

Right, sorry for the lack of updates.  I kind of have a bunch going on
right now.

I started looking into this.  I still think it's possible, but it's
trickier than I initiially thought.  Then, in the process of figuring
out how to reorganise things, I discovered some potentially nasty
preexisting bugs.  So, I was diverted into working out to fix those.
... and that's where I've got so far.

I've still got a bunch going on, so I'm not sure when, or if, I'll
finish this.  I'll probably habe a better idea in a week.

> If that works out, then we should be able to use just the command line
> part of your patch. If not, I guess it might make sense to *also* look
> into the approaches I suggested (any comments from your side?) and reach
> a conclusion. But maybe we don't have to go there at all.
> 
> -- 
> Stefano
> 
> > ________________________________
> > From: Stefano Brivio <sbrivio@redhat.com>
> > Sent: Thursday, August 6, 2026 4:54 PM
> > To: David Gibson <david@gibson.dropbear.id.au>
> > Cc: Lawrence, Richard E <rlawrence@tamu.edu>; passt-dev@passt.top <passt-dev@passt.top>
> > Subject: Re: [PATCH v2] feat: Pass open files to child in pasta mode
> > 
> > A couple of ideas, rigorously not tested: On Thu, 6 Aug 2026 13: 20: 27 +1000 David Gibson <david@ gibson. dropbear. id. au> wrote: > On Mon, Aug 03, 2026 at 08: 34: 19PM +0000, Lawrence, Richard E wrote: > > Howdy David, > > >
> > ZjQcmQRYFpfptBannerStart
> > This Message Is From an External Sender
> > This message came from outside your organization.
> > 
> > ZjQcmQRYFpfptBannerEnd
> > 
> > A couple of ideas, rigorously not tested:
> > 
> > On Thu, 6 Aug 2026 13:20:27 +1000
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > On Mon, Aug 03, 2026 at 08:34:19PM +0000, Lawrence, Richard E wrote:  
> > > > Howdy David,
> > > >
> > > > Why enumerate inherited fds?
> > > >     Well, I first tried using close_range() from the tap fd up to
> > > > max, as you suggested, and I found that it was closing file
> > > > descriptors that passt had itself opened (like the log file), which
> > > > is obviously not correct.  
> > >
> > > Oh.  Right.  Of course.  Because this has moved later, we've now
> > > opened a bunch of stuff before it runs, meaning that closing
> > > everything no longer works.
> > >  
> > > >     I considered enumerating all the possible file descriptors that
> > > > passt *might* have opened, and pass them to isolate_fds as
> > > > fds_to_keep: it turns out that there are around 10 or so file
> > > > descriptors that may or may not have been opened before the call to
> > > > isolate_fds. I decided I didn't like that strategy, because it would
> > > > create a maintenance burden:  
> > >
> > > Right, that doesn't seem practical.  
> > 
> > ...maybe we could make it slightly more practical by, either:
> > 
> > 1. keeping all the files we open for configuration / logging / control
> >    path purposes (it's not many and they are clearly documented in
> >    struct ctx, unless I missed some) in a separate struct living inside
> >    ctx, maybe even as an array indexed by an enum.
> > 
> >    At that point, it would be clear what those files are, and if
> >    somebody forgets to use that struct or array in the future, they'll
> >    see their change presumably not working at all as we'll close
> >    whatever new file descriptor right away. So I'm not that worried by
> >    the maintenance burden because it looks rather fool-proof (this is
> >    regardless of the separate struct / array I'm proposing)
> > 
> > 2. introducing a mandatory (or optional) wrapper around open(), which
> >    would open a file and store its descriptor number in an array, along
> >    with a wrapper for close() which would check that array and drop the
> >    item if it's present
> > 
> > Otherwise:
> > 
> > > > every time someone tinkers with a file
> > > > descriptor or reorders the startup sequence, they would have to
> > > > remember to also check the list of file descriptors to keep. And in
> > > > particular, some of the file descriptors are non-intuitive. Maybe
> > > > not everyone realizes that a socket counts a file descriptor. What
> > > > if there's a bug in someone's new feature where sockets are just
> > > > disappearing without a trace? I would hate to waste someone's time
> > > > chasing a subtle bug like that. So, I decided that enumerating
> > > > fds_to_close at runtime time would be less of a maintenance burden
> > > > than enumerating fds_to_keep in code.  
> > >
> > >  
> > > >     I also considered reordering the startup so that forking happens
> > > > before opening any files, so isolate fds can run with only the tap
> > > > fd as its exclusion list, but I didn't see how to make that change
> > > > without significant refactoring. It might end up being necessary,
> > > > but I can't make that decision on my own. I don't understand the
> > > > code well enough. Interested in talking that through?  
> > >
> > > Fair enough.  My best guess is that this would be the best approach,
> > > but as you say it needs pretty in depth understanding of the code.
> > > And, as you've seen I've now guessed wrong several times about the
> > > best way to approach things.
> > >
> > > Let me have a look into this and see if I can come up with something
> > > that actually works.  
> > 
> > ...if this is becoming too complicated, I would almost suggest adding a
> > command line option to keep all files open *only* in the case where
> > pasta spawns a command. I'm not enthusiastic about it, but given the
> > complexity we risk introducing otherwise, I wouldn't find it outrageous
> > either.
> > 
> > > > I don't understand your comment about conf_tap_fd becoming
> > > > static. My implementation parses -F at the same time as the other
> > > > arguments (not early anymore). Did you mean that you would inline
> > > > the string to int conversion in conf()? That seems reasonable. I
> > > > just felt that conf_tap_fd was overall too complex to inline in its
> > > > current form.  
> > >
> > > I just meant that since it is now only used in conf.c, it can become
> > > local to that module, a static function in C terminology.
> > >  
> > > > While relocating tap fd to 3+ could easily happen later, I still
> > > > don't think it should happen during isolation, because it actually
> > > > conceptually has nothing to do with isolation. It's about avoiding a
> > > > corner case that would cause tap device chatter accidentally being
> > > > printed to std err or something like that. Isolation should be
> > > > focused on closing unused fds, not on managing fds that are in
> > > > use. Perhaps we could relocate the tap fd to 3+ in main, around the
> > > > same time that we are populating 0—2, for clarity (since those two
> > > > steps need to coordinate).
> > > >
> > > > I was not aware that dynamic memory would be off the table. My main
> > > > reason for doing it that way is because I didn't want a very large
> > > > array (max fds) to be always in memory even though most of the time
> > > > it would be unused. I will have to re-think the strategy. Maybe
> > > > there is another way to avoid a large unused array.  
> > >
> > > That array isn't particularly large compared to many others we already
> > > have.  And even those are usually only a small fraction of our total
> > > effective memory usage - most of that comes in the form of kernel
> > > memory for sockets and buffers.  In principle we could also use
> > > MADV_DONTNEED to discard the allocated memory once we're done with it.
> > >  
> > > > Thank you for pointing out the flaw in my logic regarding the max
> > > > open files limit. I will have to think harder about a correct way to
> > > > close fds.
> > > >
> > > > I do not believe that my patch disables your feature of populating
> > > > fds 0—2 with devnull. That happens in main, not in isolate fds. My
> > > > implementation of enumerating the inherited fds excludes 0—2.  
> > >
> > > You're right, sorry.  I got muddled because it was inside
> > > isolate_fds() in a bunch of draft versions of my patches (I eventually
> > > realised that wouldn't quite work).
> > >  
> > > >
> > > > I hope to hear from you again soon
> > > > Richard
> > > >
> > > > PS. In case anyone else who's reading along has concerns about the
> > > > potential performance hit caused by abandoning close_range(), I have
> > > > an argument prepared to explain why that is not a serious concern.  
> > >
> > > So, fwiw, the concern isn't the cost of close() on the actually open
> > > files - as a one time cose that will generally be trivial.  The conern
> > > is discovering the open fds: 2^31 close()s or other syscalls to
> > > discover if there's an fd there _would_ be too much.  Using
> > > /proc/self/fd is an interesting approach.  It's not portable, but then
> > > neither is close_range() (although FreeBSD does have close_range()
> > > apparently).  Working out how to size things is the tricky bit with
> > > the /proc/self/fd approach, though.
> > >
> > > [...]  
> > 
> > --
> > Stefano
> > 
> > 
> 

-- 
David Gibson (he or they)	| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you, not the other way
				| around.
http://www.ozlabs.org/~dgibson