Message-ID: <0422c2f707746427a8542888d0058b6487324249.camel@alliedtelesis.co.nz>
Date:   Tue, 14 Jul 2020 05:19:04 +0000
From:   Matt Bennett <Matt.Bennett@...iedtelesis.co.nz>
To:     "ebiederm@...ssion.com" <ebiederm@...ssion.com>,
        "cyphar@...har.com" <cyphar@...har.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "christian.brauner@...ntu.com" <christian.brauner@...ntu.com>,
        "zbr@...emap.net" <zbr@...emap.net>,
        "containers@...ts.linux-foundation.org" 
        <containers@...ts.linux-foundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 0/5] RFC: connector: Add network namespace awareness

On Tue, 2020-07-14 at 15:03 +1000, Aleksa Sarai wrote:
> On 2020-07-13, Eric W. Biederman <ebiederm@...ssion.com> wrote:
> > Matt Bennett <Matt.Bennett@...iedtelesis.co.nz> writes:
> > 
> > > On Thu, 2020-07-02 at 21:10 +0200, Christian Brauner wrote:
> > > > On Thu, Jul 02, 2020 at 08:17:38AM -0500, Eric W. Biederman wrote:
> > > > > Matt Bennett <matt.bennett@...iedtelesis.co.nz> writes:
> > > > > 
> > > > > > Previously the connector functionality could only be used by processes running in the
> > > > > > default network namespace. This meant that any process that uses the connector functionality
> > > > > > could not operate correctly when run inside a container. This is a draft patch series that
> > > > > > attempts to now allow this functionality outside of the default network namespace.
> > > > > > 
> > > > > > I see this has been discussed previously [1], but am not sure how my changes relate to all
> > > > > > of the topics discussed there and/or if there are any unintended side effects from my draft
> > > > > > changes.
> > > > > 
> > > > > Is there a piece of software that uses connector that you want to get
> > > > > working in containers?
> > > 
> > > We have an IPC system [1] where processes can register their socket
> > > details (unix, tcp, tipc, ...) to a 'monitor' process. Processes can
> > > then get notified when other processes they are interested in
> > > start/stop their servers and use the registered details to connect to
> > > them. Everything works unless a process crashes, in which case the
> > > monitoring process never removes their details. Therefore the
> > > monitoring process uses the connector functionality with
> > > PROC_EVENT_EXIT to detect when a process crashes and removes the
> > > details if it is a previously registered PID.
> > > 
> > > This was working for us until we tried to run our system in a container.
> > > 
> > > > > 
> > > > > I am curious what the motivation is because up until now there has been
> > > > > nothing very interesting using this functionality.  So it hasn't been
> > > > > worth anyone's time to make the necessary changes to the code.
> > > > 
> > > > Imho, we should just state once and for all that the proc connector will
> > > > not be namespaced. This is such a corner-case thing and has been
> > > > non-namespaced for such a long time without consistent push for it to be
> > > > namespaced combined with the fact that this needs quite some code to
> > > > make it work correctly that I fear we end up buying more bugs than we're
> > > > selling features. And realistically, you and I will end up maintaining
> > > > this and I feel this is not worth the time(?). Maybe I'm being too
> > > > pessimistic though.
> > > > 
> > > 
> > > Fair enough. I can certainly look for another way to detect process
> > > crashes. Interestingly I found a patch set [2] on the mailing list
> > > that attempts to solve the problem I wish to solve, but it doesn't
> > > look like the patches were ever developed further. From reading the
> > > discussion thread on that patch set it appears that I should be doing
> > > some form of polling on the /proc files.
> > 
> > Recently Christian Brauner implemented pidfd complete with a poll
> > operation that reports when a process terminates.
> > 
> > If you are willing to change your userspace code switching to pidfd
> > should be all that you need.
> 
> While this does solve the problem of getting exit notifications in
> general, you cannot get the exit code. But if they don't care about that
> then we can solve that problem another time. :D
> 

From first glance using pidfd will do exactly what we need. Not being able
to get the exit code will not be an issue. In fact I think it will be an
improvement over the connector as the listener will now only be waiting for
the PIDs we actually care about - rather than getting woken up on every single
process exit and having to check if it cares about the PID.

Many thanks Eric and others,
Matt

Powered by blists - more mailing lists