Re: [PATCH] tap: include errno in error when tap_ns_tun() fails

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

[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]

On Wed, Aug 02, 2023 at 11:51:02AM +0200, Paul Holzinger wrote:
> 
> On 02/08/2023 03:39, David Gibson wrote:
> > On Tue, Aug 01, 2023 at 01:50:17PM +0200, Paul Holzinger wrote:
> > > It is important to know why a syscall failed so pasta should include the
> > > errno in the error message. This is still not perfect as we do not know
> > > which of functions (open, ioctl, if_nametoindex) failed but it should at
> > > least include more important context.
> > Uh.. we certainly want this, but I don't think this implementation
> > will quite do it.
> > 
> > > This change was inspiered by a podman issue[1].
> > > 
> > > [1] https://github.com/containers/podman/issues/19428
> > > 
> > > Signed-off-by: Paul Holzinger <pholzing@redhat.com>
> > > ---
> > >   tap.c | 3 ++-
> > >   1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/tap.c b/tap.c
> > > index a6a73d3..c212616 100644
> > > --- a/tap.c
> > > +++ b/tap.c
> > > @@ -1205,7 +1205,8 @@ static void tap_sock_tun_init(struct ctx *c)
> > >   	NS_CALL(tap_ns_tun, c);
> > NS_CALL means we're running the function in an ephemeral
> > thread/process/thingy..
> > 
> > >   	if (tun_ns_fd == -1)
> > > -		die("Failed to open tun socket in namespace");
> > > +		die("Failed to open tun socket in namespace: %s",
> > > +		    strerror(errno));
> > ..which means we can't rely on it actually setting errno in the
> > original process.  The ephemeral thread does share memory, but modern
> > errno is a weird per-thread thingy, so I'm not entirely sure what will
> > happen here, but I'm certainly not confident about it working as we'd
> > like.
> > 
> > I'll have a crack at a more robust approach today.
> 
> Thanks, I only did a quick test with chmod 600 /dev/net/tun and got the
> expected EACCES so I assumed it worked fine

Huh, interesting, maybe it does work.  Unless some earlier syscall
happened to set errno to EACCES.

> but anyway your series looks
> much better.

Yeah, I realized as I was working on it that that approach has the
additional advantage of clearly showing which operation failed.

-- 
David Gibson			| 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]