From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id A7CCC5A027B for ; Wed, 8 Mar 2023 23:45:15 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4PX6pF6Sg3z4xFM; Thu, 9 Mar 2023 09:45:09 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1678315509; bh=5/Dc8VZDPHw8BdAZzk1fWW2H7idazzW7x30mO+HQGgE=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=hUoRO31fvL6cxgiCdfS+TWHu3WoreVO5LeTSK2a/oHKXD6E6xbUNzB76YxJZUoqPy W1mGju9KKrrR+kno3uXXbZhe2txn6VGEFEbLjCvAnOp7i6DRycUskdDXyRxX15M2U+ /v7Hz1GDnwLBvXW3QILYCyKVl5h8ILxYSidzopCI= Date: Thu, 9 Mar 2023 09:16:03 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/3] tcp, udp, util: Pass socket creation errors all the way up Message-ID: References: <20230308123348.2232214-1-sbrivio@redhat.com> <20230308123348.2232214-2-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="Z6AtK+hMQ6unyMTB" Content-Disposition: inline In-Reply-To: <20230308123348.2232214-2-sbrivio@redhat.com> Message-ID-Hash: UZB6DINCYXXVHKPPMUZVTVRF5NUFK2G3 X-Message-ID-Hash: UZB6DINCYXXVHKPPMUZVTVRF5NUFK2G3 X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --Z6AtK+hMQ6unyMTB Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Mar 08, 2023 at 01:33:46PM +0100, Stefano Brivio wrote: > ...starting from sock_l4(), pass negative error (errno) codes instead > of -1. They will only be used in two commits from now, no functional > changes intended here. >=20 > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > tcp.c | 22 ++++++++++++---------- > udp.c | 18 +++++++++--------- > util.c | 31 ++++++++++++++++++------------- > 3 files changed, 39 insertions(+), 32 deletions(-) >=20 > diff --git a/tcp.c b/tcp.c > index 96ca5c7..e209483 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -2955,7 +2955,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_re= f ref, uint32_t events, > * @addr: Pointer to address for binding, NULL if not configured > * @ifname: Name of interface to bind to, NULL if not configured > * > - * Return: fd for the new listening socket, or -1 on failure > + * Return: fd for the new listening socket, negative error code on failu= re > */ > static int tcp_sock_init_af(const struct ctx *c, int af, in_port_t port, > const struct in_addr *addr, const char *ifname) > @@ -2968,13 +2968,13 @@ static int tcp_sock_init_af(const struct ctx *c, = int af, in_port_t port, > =20 > if (c->tcp.fwd_in.mode =3D=3D FWD_AUTO) { > if (af =3D=3D AF_INET || af =3D=3D AF_UNSPEC) > - tcp_sock_init_ext[port][V4] =3D s; > + tcp_sock_init_ext[port][V4] =3D s < 0 ? -1 : s; > if (af =3D=3D AF_INET6 || af =3D=3D AF_UNSPEC) > - tcp_sock_init_ext[port][V6] =3D s; > + tcp_sock_init_ext[port][V6] =3D s < 0 ? -1 : s; > } > =20 > if (s < 0) > - return -1; > + return s; > =20 > tcp_sock_set_bufsize(c, s); > return s; > @@ -2988,12 +2988,12 @@ static int tcp_sock_init_af(const struct ctx *c, = int af, in_port_t port, > * @ifname: Name of interface to bind to, NULL if not configured > * @port: Port, host order > * > - * Return: 0 on (partial) success, -1 on (complete) failure > + * Return: 0 on (partial) success, negative error code on (complete) fai= lure > */ > int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, > const char *ifname, in_port_t port) > { > - int ret =3D 0; > + int ret =3D 0, af_ret; > =20 > if (af =3D=3D AF_UNSPEC && c->ifi4 && c->ifi6) > /* Attempt to get a dual stack socket */ > @@ -3002,13 +3002,15 @@ int tcp_sock_init(const struct ctx *c, sa_family_= t af, const void *addr, > =20 > /* Otherwise create a socket per IP version */ > if ((af =3D=3D AF_INET || af =3D=3D AF_UNSPEC) && c->ifi4) { > - if (tcp_sock_init_af(c, AF_INET, port, addr, ifname) < 0) > - ret =3D -1; > + af_ret =3D tcp_sock_init_af(c, AF_INET, port, addr, ifname); > + if (af_ret < 0) > + ret =3D af_ret; > } > =20 > if ((af =3D=3D AF_INET6 || af =3D=3D AF_UNSPEC) && c->ifi6) { > - if (tcp_sock_init_af(c, AF_INET6, port, addr, ifname) < 0) > - ret =3D -1; > + af_ret =3D tcp_sock_init_af(c, AF_INET6, port, addr, ifname); > + if (af_ret < 0) > + ret =3D af_ret; > } > =20 > return ret; > diff --git a/udp.c b/udp.c > index 1077cde..41e0afd 100644 > --- a/udp.c > +++ b/udp.c > @@ -977,7 +977,7 @@ int udp_tap_handler(struct ctx *c, int af, const void= *addr, > * @ifname: Name of interface to bind to, NULL if not configured > * @port: Port, host order > * > - * Return: 0 on (partial) success, -1 on (complete) failure > + * Return: 0 on (partial) success, negative error code on (complete) fai= lure > */ > int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, > const void *addr, const char *ifname, in_port_t port) > @@ -1002,19 +1002,19 @@ int udp_sock_init(const struct ctx *c, int ns, sa= _family_t af, > s =3D sock_l4(c, AF_INET, IPPROTO_UDP, addr, ifname, > port, uref.u32); > =20 > - udp_tap_map[V4][uref.udp.port].sock =3D s; > - udp_splice_init[V4][port].sock =3D s; > + udp_tap_map[V4][uref.udp.port].sock =3D s < 0 ? -1 : s; > + udp_splice_init[V4][port].sock =3D s < 0 ? -1 : s; > } else { > struct in_addr loopback =3D { htonl(INADDR_LOOPBACK) }; > uref.udp.ns =3D true; > =20 > s =3D sock_l4(c, AF_INET, IPPROTO_UDP, &loopback, > ifname, port, uref.u32); > - udp_splice_ns[V4][port].sock =3D s; > + udp_splice_ns[V4][port].sock =3D s < 0 ? -1 : s; > } > =20 > if (s < 0) > - ret =3D -1; > + ret =3D s; > } > =20 > if ((af =3D=3D AF_INET6 || af =3D=3D AF_UNSPEC) && c->ifi6) { > @@ -1026,18 +1026,18 @@ int udp_sock_init(const struct ctx *c, int ns, sa= _family_t af, > s =3D sock_l4(c, AF_INET6, IPPROTO_UDP, addr, ifname, > port, uref.u32); > =20 > - udp_tap_map[V6][uref.udp.port].sock =3D s; > - udp_splice_init[V6][port].sock =3D s; > + udp_tap_map[V6][uref.udp.port].sock =3D s < 0 ? -1 : s; > + udp_splice_init[V6][port].sock =3D s < 0 ? -1 : s; > } else { > uref.udp.ns =3D true; > =20 > s =3D sock_l4(c, AF_INET6, IPPROTO_UDP, &in6addr_loopback, > ifname, port, uref.u32); > - udp_splice_ns[V6][port].sock =3D s; > + udp_splice_ns[V6][port].sock =3D s < 0 ? -1 : s; > } > =20 > if (s < 0) > - ret =3D -1; > + ret =3D s; > } > =20 > return ret; > diff --git a/util.c b/util.c > index 484889b..fddb5ed 100644 > --- a/util.c > +++ b/util.c > @@ -96,7 +96,7 @@ found: > * @port: Port, host order > * @data: epoll reference portion for protocol handlers > * > - * Return: newly created socket, -1 on error > + * Return: newly created socket, negative error code on failure > */ > int sock_l4(const struct ctx *c, int af, uint8_t proto, > const void *bind_addr, const char *ifname, uint16_t port, > @@ -115,16 +115,16 @@ int sock_l4(const struct ctx *c, int af, uint8_t pr= oto, > }; > const struct sockaddr *sa; > bool dual_stack =3D false; > + int fd, sl, y =3D 1, ret; > struct epoll_event ev; > - int fd, sl, y =3D 1; > =20 > if (proto !=3D IPPROTO_TCP && proto !=3D IPPROTO_UDP && > proto !=3D IPPROTO_ICMP && proto !=3D IPPROTO_ICMPV6) > - return -1; /* Not implemented. */ > + return -EPFNOSUPPORT; /* Not implemented. */ > =20 > if (af =3D=3D AF_UNSPEC) { > if (!DUAL_STACK_SOCKETS || bind_addr) > - return -1; > + return -EINVAL; > dual_stack =3D true; > af =3D AF_INET6; > } > @@ -134,14 +134,15 @@ int sock_l4(const struct ctx *c, int af, uint8_t pr= oto, > else > fd =3D socket(af, SOCK_DGRAM | SOCK_NONBLOCK, proto); > =20 > + ret =3D -errno; > if (fd < 0) { > - warn("L4 socket: %s", strerror(errno)); > - return -1; > + warn("L4 socket: %s", strerror(-ret)); > + return ret; > } > =20 > if (fd > SOCKET_MAX) { > close(fd); > - return -1; > + return -EBADF; > } > =20 > ref.r.s =3D fd; > @@ -186,10 +187,11 @@ int sock_l4(const struct ctx *c, int af, uint8_t pr= oto, > */ > if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, > ifname, strlen(ifname))) { > + ret =3D -errno; > warn("Can't bind socket for %s port %u to %s, closing", > ip_proto_str[proto], port, ifname); > close(fd); > - return -1; > + return ret; > } > } > =20 > @@ -200,22 +202,25 @@ int sock_l4(const struct ctx *c, int af, uint8_t pr= oto, > * broken SELinux policy, see icmp_tap_handler(). > */ > if (proto !=3D IPPROTO_ICMP && proto !=3D IPPROTO_ICMPV6) { > + ret =3D -errno; > close(fd); > - return -1; > + return ret; > } > } > =20 > if (proto =3D=3D IPPROTO_TCP && listen(fd, 128) < 0) { > - warn("TCP socket listen: %s", strerror(errno)); > + ret =3D -errno; > + warn("TCP socket listen: %s", strerror(-ret)); > close(fd); > - return -1; > + return ret; > } > =20 > ev.events =3D EPOLLIN; > ev.data.u64 =3D ref.u64; > if (epoll_ctl(c->epollfd, EPOLL_CTL_ADD, fd, &ev) =3D=3D -1) { > - warn("L4 epoll_ctl: %s", strerror(errno)); > - return -1; > + ret =3D -errno; > + warn("L4 epoll_ctl: %s", strerror(-ret)); > + return ret; > } > =20 > return fd; --=20 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 --Z6AtK+hMQ6unyMTB Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmQJCRwACgkQzQJF27ox 2GflUQ//QQ2OD7y3cggh6e2g41CQ2oNESoJjkpVKHLxOB4W6fzy9VOfxRoxqM/GD M3lRDkgtwqgfacuHXmzl4DkqM2OWmrTC5jufoLuj2KFymOKelOy8tGHh1wSspV3/ Zef2Bd4+y7huYnhXdR0uiKBJMHEBVfksq75owXnFYKhTu8ARAz/r6jtsZQ24qhMW 86dM1cRwUbJcJPWj4e3NBm2xfQTt67q5X6j/dqB+7D92o+M7Y/FAkKc9JSO4A4Qk SI7miYPtqlBq2UwnpeGi0qnfqInnkzID37DIc093U+4PXkfeFmYZIzeSmjPjpjvq lxwzyIqLGKAQg0cUSK/NE29kkUFePD16V9NK3icrSAzBq2rYoerTuIO8awxel7L6 RPANCyyBbOkLM0D06jigbwErkjsAXL/NWuCakkKmN/wxjhep+2cdRjmN0OJGG/B6 fY4+6xSGNzsAPRPgpMc0GNG9GCyhMG1/j1CcgLWLz9xv68Actu8+hyHlvbo4wJyw EOg8f25GA4GlLBRHzgFDZh0axvQAwg5B5bDKY1g9hfWBSBwLnSyCdYzA1g+xQza6 OC2pEQLlhpo2AkpH+u7lDX6/YNn3SwSH6ndl9a1yevOg8BrSJjPbR68O1tM8gxdv lUOPrYP9sA/mxY/XMV848jVUWwFKF09NZ7bEV9ykamPC6hTIRYE= =4AOJ -----END PGP SIGNATURE----- --Z6AtK+hMQ6unyMTB--