Re: [PATCH] pasta: Add fallback timer mechanism to check if namespace is gone

[Stefano Brivio <sbrivio@redhat.com>]

On Fri, 16 Feb 2024 14:00:41 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Thu, Feb 15, 2024 at 11:39:11PM +0100, Stefano Brivio wrote:
> > We don't know how frequently this happens, but hitting
> > fs.inotify.max_user_watches or similar sysctl limits is definitely
> > not out of question, and Paul mentioned that, for example, Podman's
> > CI environments hit similar issues in the past.
> > 
> > Introduce a fallback mechanism based on a timer file descriptor: we
> > grab the directory handle at startup, and we can then use openat(),
> > triggered periodically, to check if the (network) namespace directory
> > still exists. If openat() fails at some point, exit.
> > 
> > Link: https://github.com/containers/podman/pull/21563#issuecomment-1943505707
> > Reported-by: Paul Holzinger <pholzing@redhat.com>
> > Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
> > ---
> >  passt.c |  6 ++--
> >  passt.h |  2 ++
> >  pasta.c | 85 +++++++++++++++++++++++++++++++++++++++++++++------------
> >  pasta.h |  2 +-
> >  4 files changed, 73 insertions(+), 22 deletions(-)
> > 
> > diff --git a/passt.c b/passt.c
> > index aaa8e58..13670b9 100644
> > --- a/passt.c
> > +++ b/passt.c
> > @@ -201,7 +201,7 @@ void exit_handler(int signal)
> >   */
> >  int main(int argc, char **argv)
> >  {
> > -	int nfds, i, devnull_fd = -1, pidfile_fd = -1, quit_fd;
> > +	int nfds, i, devnull_fd = -1, pidfile_fd = -1;
> >  	struct epoll_event events[EPOLL_EVENTS];
> >  	char *log_name, argv0[PATH_MAX], *name;
> >  	struct ctx c = { 0 };
> > @@ -274,7 +274,7 @@ int main(int argc, char **argv)
> >  	if (c.force_stderr || isatty(fileno(stdout)))
> >  		__openlog(log_name, LOG_PERROR, LOG_DAEMON);
> >  
> > -	quit_fd = pasta_netns_quit_init(&c);
> > +	pasta_netns_quit_init(&c);
> >  
> >  	tap_sock_init(&c);
> >  
> > @@ -371,7 +371,7 @@ loop:
> >  			tap_listen_handler(&c, eventmask);
> >  			break;
> >  		case EPOLL_TYPE_NSQUIT:
> > -			pasta_netns_quit_handler(&c, quit_fd);
> > +			pasta_netns_quit_handler(&c, ref);  
> 
> Hm.  As a rule, I've been trying to use a separate EPOLL_TYPE for each
> different handler, rather than having secondary dispatch based on
> other details, even if those different handlers are accomplishing
> similar purposes (e.g. TAP_PASTA vs. TAP_PASST).

Yes, I have to admit that the (ref.nsdir_fd != -1) if clause in the
handler is a bit of a hack, and a rather gratuitous one. Changed in v2
to use a new epoll reference type.

-- 
Stefano