From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id 36D0B5A026A for ; Thu, 16 Feb 2023 06:43:20 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4PHP4K56KMz4x5W; Thu, 16 Feb 2023 16:43:13 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1676526193; bh=Ary3H+vE1Y8N91rli8/2+gEGdtYv0Ro78n/1bg0LNI8=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=eY2xQ5nBy61M7lZWgKGfIud7yLojXHJhLJPHeXJtKxeHjyfxP8EB/1pVaQRwBiL/7 nA6L9W/kSuDvhNl0fJePzOpoNvBzsroEh/GGhyPumbbI6bjx/dLLocFzsKS1rkVs/G uorCU3xaVFQAswsRjts4Ah9a7rsIASR0mVYU/CYw= Date: Thu, 16 Feb 2023 15:42:54 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] conf, tcp, udp: Exit if we fail to bind sockets for all given ports Message-ID: References: <20230216010720.2224138-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="yMkmm3OTFjIqJ3tG" Content-Disposition: inline In-Reply-To: <20230216010720.2224138-1-sbrivio@redhat.com> Message-ID-Hash: PN36SIAC7J4KID5YPEJE4G4SYKGI55GT X-Message-ID-Hash: PN36SIAC7J4KID5YPEJE4G4SYKGI55GT 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, Yalan Zhang X-Mailman-Version: 3.3.3 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: --yMkmm3OTFjIqJ3tG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Feb 16, 2023 at 02:07:20AM +0100, Stefano Brivio wrote: > passt supports ranges of forwarded ports as well as 'all' for TCP and > UDP, so it might be convenient to proceed if we fail to bind only > some of the desired ports. >=20 > But if we fail to bind even a single port for a given specification, > we're clearly, unexpectedly, conflicting with another network > service. In that case, report failure and exit. >=20 > Reported-by: Yalan Zhang > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > conf.c | 47 ++++++++++++++++++++++++++++++++++------------- > tcp.c | 25 ++++++++++++++++++------- > tcp.h | 4 ++-- > udp.c | 16 +++++++++++++--- > udp.h | 4 ++-- > 5 files changed, 69 insertions(+), 27 deletions(-) >=20 > diff --git a/conf.c b/conf.c > index f175405..675d961 100644 > --- a/conf.c > +++ b/conf.c > @@ -179,9 +179,9 @@ static void conf_ports(const struct ctx *c, char optn= ame, const char *optarg, > { > char addr_buf[sizeof(struct in6_addr)] =3D { 0 }, *addr =3D addr_buf; > char buf[BUFSIZ], *spec, *ifname =3D NULL, *p; > + bool exclude_only =3D true, bound_one =3D false; > uint8_t exclude[PORT_BITMAP_SIZE] =3D { 0 }; > sa_family_t af =3D AF_UNSPEC; > - bool exclude_only =3D true; > =20 > if (!strcmp(optarg, "none")) { > if (fwd->mode) > @@ -215,12 +215,19 @@ static void conf_ports(const struct ctx *c, char op= tname, const char *optarg, > memset(fwd->map, 0xff, PORT_EPHEMERAL_MIN / 8); > =20 > for (i =3D 0; i < PORT_EPHEMERAL_MIN; i++) { > - if (optname =3D=3D 't') > - tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i); > - else if (optname =3D=3D 'u') > - udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, i); > + if (optname =3D=3D 't') { > + if (!tcp_sock_init(c, AF_UNSPEC, NULL, NULL, i)) > + bound_one =3D true; > + } else if (optname =3D=3D 'u') { > + if (!udp_sock_init(c, 0, AF_UNSPEC, NULL, NULL, > + i)) > + bound_one =3D true; > + } > } > =20 > + if (!bound_one) > + goto bind_fail; > + > return; > } > =20 > @@ -293,12 +300,18 @@ static void conf_ports(const struct ctx *c, char op= tname, const char *optarg, > =20 > bitmap_set(fwd->map, i); > =20 > - if (optname =3D=3D 't') > - tcp_sock_init(c, af, addr, ifname, i); > - else if (optname =3D=3D 'u') > - udp_sock_init(c, 0, af, addr, ifname, i); > + if (optname =3D=3D 't') { > + if (!tcp_sock_init(c, af, addr, ifname, i)) > + bound_one =3D true; > + } else if (optname =3D=3D 'u') { > + if (!udp_sock_init(c, 0, af, addr, ifname, i)) > + bound_one =3D true; > + } > } > =20 > + if (!bound_one) > + goto bind_fail; > + > return; > } > =20 > @@ -339,13 +352,19 @@ static void conf_ports(const struct ctx *c, char op= tname, const char *optarg, > =20 > fwd->delta[i] =3D mapped_range.first - orig_range.first; > =20 > - if (optname =3D=3D 't') > - tcp_sock_init(c, af, addr, ifname, i); > - else if (optname =3D=3D 'u') > - udp_sock_init(c, 0, af, addr, ifname, i); > + if (optname =3D=3D 't') { > + if (!tcp_sock_init(c, af, addr, ifname, i)) > + bound_one =3D true; > + } else if (optname =3D=3D 'u') { > + if (udp_sock_init(c, 0, af, addr, ifname, i)) > + bound_one =3D true; > + } > } > } while ((p =3D next_chunk(p, ','))); > =20 > + if (!bound_one) > + goto bind_fail; > + > return; > bad: > die("Invalid port specifier %s", optarg); > @@ -353,6 +372,8 @@ overlap: > die("Overlapping port specifier %s", optarg); > mode_conflict: > die("Port forwarding mode '%s' conflicts with previous mode", optarg); > +bind_fail: > + die("Failed to bind any port for '-%c %s', exiting", optname, optarg); > } > =20 > /** > diff --git a/tcp.c b/tcp.c > index f028e01..32ce1e9 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -2916,20 +2916,31 @@ static int tcp_sock_init_af(const struct ctx *c, = int af, in_port_t port, > * @addr: Pointer to address for binding, NULL if not configured > * @ifname: Name of interface to bind to, NULL if not configured > * @port: Port, host order > + * > + * Return: 0 on (partial) success, -1 on (complete) failure > */ > -void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, > - const char *ifname, in_port_t port) > +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; > + > if (af =3D=3D AF_UNSPEC && c->ifi4 && c->ifi6) > /* Attempt to get a dual stack socket */ > if (tcp_sock_init_af(c, AF_UNSPEC, port, addr, ifname) >=3D 0) > - return; > + return 0; > =20 > /* Otherwise create a socket per IP version */ > - if ((af =3D=3D AF_INET || af =3D=3D AF_UNSPEC) && c->ifi4) > - tcp_sock_init_af(c, AF_INET, port, addr, ifname); > - if ((af =3D=3D AF_INET6 || af =3D=3D AF_UNSPEC) && c->ifi6) > - tcp_sock_init_af(c, AF_INET6, port, addr, ifname); > + 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; > + } > + > + 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; > + } > + > + return ret; > } > =20 > /** > diff --git a/tcp.h b/tcp.h > index 236038f..5527c5b 100644 > --- a/tcp.h > +++ b/tcp.h > @@ -17,8 +17,8 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref re= f, uint32_t events, > const struct timespec *now); > int tcp_tap_handler(struct ctx *c, int af, const void *addr, > const struct pool *p, const struct timespec *now); > -void tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, > - const char *ifname, in_port_t port); > +int tcp_sock_init(const struct ctx *c, sa_family_t af, const void *addr, > + const char *ifname, in_port_t port); > int tcp_init(struct ctx *c); > void tcp_timer(struct ctx *c, const struct timespec *ts); > void tcp_defer_handler(struct ctx *c); > diff --git a/udp.c b/udp.c > index 2e9b7e6..c913d27 100644 > --- a/udp.c > +++ b/udp.c > @@ -955,12 +955,14 @@ int udp_tap_handler(struct ctx *c, int af, const vo= id *addr, > * @addr: Pointer to address for binding, NULL if not configured > * @ifname: Name of interface to bind to, NULL if not configured > * @port: Port, host order > + * > + * Return: 0 on (partial) success, -1 on (complete) failure > */ > -void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, > - const void *addr, const char *ifname, in_port_t port) > +int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, > + const void *addr, const char *ifname, in_port_t port) > { > union udp_epoll_ref uref =3D { .u32 =3D 0 }; > - int s; > + int s, ret =3D 0; > =20 > if (ns) { > uref.udp.port =3D (in_port_t)(port + > @@ -989,6 +991,9 @@ void udp_sock_init(const struct ctx *c, int ns, sa_fa= mily_t af, > ifname, port, uref.u32); > udp_splice_ns[V4][port].sock =3D s; > } > + > + if (s < 0) > + ret =3D -1; > } > =20 > if ((af =3D=3D AF_INET6 || af =3D=3D AF_UNSPEC) && c->ifi6) { > @@ -1009,7 +1014,12 @@ void udp_sock_init(const struct ctx *c, int ns, sa= _family_t af, > ifname, port, uref.u32); > udp_splice_ns[V6][port].sock =3D s; > } > + > + if (s < 0) > + ret =3D -1; > } > + > + return ret; > } > =20 > /** > diff --git a/udp.h b/udp.h > index 68082ea..0e81be8 100644 > --- a/udp.h > +++ b/udp.h > @@ -12,8 +12,8 @@ void udp_sock_handler(struct ctx *c, union epoll_ref re= f, uint32_t events, > const struct timespec *now); > int udp_tap_handler(struct ctx *c, int af, const void *addr, > const struct pool *p, const struct timespec *now); > -void udp_sock_init(const struct ctx *c, int ns, sa_family_t af, > - const void *addr, const char *ifname, in_port_t port); > +int udp_sock_init(const struct ctx *c, int ns, sa_family_t af, > + const void *addr, const char *ifname, in_port_t port); > int udp_init(struct ctx *c); > void udp_timer(struct ctx *c, const struct timespec *ts); > void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *= eth_s, --=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 --yMkmm3OTFjIqJ3tG Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmPttEgACgkQzQJF27ox 2GcwVg/6A0STFtjUZg/bJoNduzjsV+v2ZQGDJ3bp2TMTGsf9YYSoLi26ryH2o5WX eDhCmOc6njenv8gw/DpZlWskqbwo6j9DFBfabssN3yR8QlL2hPsHCi/THN36H2dD XRMOs5kDhQH9UnWH+1yeRPwIpsrS2CygkfMhiVqtx3IsnRJgVXAPoiVycNnsie7l 2jeJoKB6aQc1pwcqPrWZOE2cnmVZYQUlh2A8oYvQJNxQTtwp7DLSHoXAFpr6BttE hKRp47jruHOH2sMX+X1betdSBa504ZD+iWZaPR9ElEuri48eWcnRDq3Hdm2D2tKE 68gmEeGsTt9VY8urtBnwm9NxzMtrpD6ZmAw/lgsJY6lOZ9rKgeHlUY+ijUGUrlap Pp+W33qa8IA4ARza4wFYEpSssrS6ZjvCbFIMuyOUDsFnXietPFolFH5R41T76DJI jtIc9bJiyYhwhEcgo9OyNOMyIXyk1YrN4lV6s0b/yw2hLzd4OAf6WRWL7gwLu1EJ KxhjLuTo9MDtPTSNvvSyS+J0kxKnZK46z8tRMTOdo+e4/+HFLv2vqrPD1bDMc9wg xanL8cBzfYT3/I43U5ZN6PsFV8bW+wAzS6NFoCKHmhBZAaRelWjOiBqpxWwBfj1q bmQiiXHBxsIx/BHQmN/0CYMHptmWDiSIqws+x+mk/Ys8fl2+CtM= =BpIG -----END PGP SIGNATURE----- --yMkmm3OTFjIqJ3tG--