From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id BA8195A004E for ; Fri, 14 Jun 2024 02:47:59 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1718326075; bh=YcA5yplOa2y7zTV6UUIOKhMiI+PuZcPCY2sPrt2U4Xg=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=qVsOXI93+ZDWhOWYPeDKXac0+Icpg+KJP7WuLbi4YmAIswSf1kTnobgs0QAPVUbDW iqhaUUrsJXXmimtCIEyVkCYHjPST7tJ5A9i2C8/XRDIBNG++cNaTVxHSCGByH/oF5s eQuwbPt8YYJXTYDS2P8ZHBt0AvGako4iTzbnOeLLxi9xYFltyq0/ova5eoMCCBSekd I7PIpkCDa7qZfCqXZr5Uv9hpxsgEDBtu9vT2do9kMedZtNBJWo7jR6Gtazm727DPCz Tlrn4NvVSwFX0gEqPBIb4rVtjhdfuhL0YSWpc78nHZn4SnCqfSKd+UIy96XJ96ub3H ZgTRROoQFzAGg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4W0gcC73khz4wb7; Fri, 14 Jun 2024 10:47:55 +1000 (AEST) Date: Fri, 14 Jun 2024 10:47:43 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH 1/4] util: Split construction of bind socket address from the rest of sock_l4() Message-ID: References: <20240605013903.3694452-1-david@gibson.dropbear.id.au> <20240605013903.3694452-2-david@gibson.dropbear.id.au> <20240613170625.49428bf6@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="KdtzeO9QQOhvCJUh" Content-Disposition: inline In-Reply-To: <20240613170625.49428bf6@elisabeth> Message-ID-Hash: ZKA5CGXBP2G422ZCVGXZGQL3IVA72LOZ X-Message-ID-Hash: ZKA5CGXBP2G422ZCVGXZGQL3IVA72LOZ 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: --KdtzeO9QQOhvCJUh Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 13, 2024 at 05:06:25PM +0200, Stefano Brivio wrote: > Sorry for the delay, nits only (I can fix them up on merge): Ok, I'll respin anyway, though, since it looks like there's a slightly non-trivial rebase needed on Laurent's stuff. > On Wed, 5 Jun 2024 11:39:00 +1000 > David Gibson wrote: >=20 > > sock_l4() creates, binds and otherwise prepares a new socket. It builds > > the socket address to bind from separately provided address and port. > > However, we have use cases coming up where it's more natural to constru= ct > > the socket address in the caller. > >=20 > > Prepare for this by adding sock_l4_sa() which takes a pre-constructed > > socket address, and rewriting sock_l4() in terms of it. > >=20 > > Signed-off-by: David Gibson > > --- > > util.c | 123 ++++++++++++++++++++++++++++++++------------------------- > > 1 file changed, 70 insertions(+), 53 deletions(-) > >=20 > > diff --git a/util.c b/util.c > > index cc1c73ba..4e5f6d23 100644 > > --- a/util.c > > +++ b/util.c > > @@ -33,36 +33,25 @@ > > #include "log.h" > > =20 > > /** > > - * sock_l4() - Create and bind socket for given L4, add to epoll list > > + * sock_l4_sa() - Create and bind socket for given L4, add to epoll li= st >=20 > That doesn't quite tell the difference from sock_l4(), perhaps: >=20 > * sock_l4_sa() - Create and bind socket given socket address, add to epo= ll list Good idea, done. > > * @c: Execution context > > - * @af: Address family, AF_INET or AF_INET6 > > * @proto: Protocol number > > - * @bind_addr: Address for binding, NULL for any > > + * @sa: Socket address to bind to > > + * @sl: Length of @sa > > * @ifname: Interface for binding, NULL for any > > - * @port: Port, host order > > + * @v6only: Set IPV6_V6ONLY socket option > > * @data: epoll reference portion for protocol handlers > > * > > * Return: newly created socket, negative error code on failure > > */ > > -int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, > > - const void *bind_addr, const char *ifname, uint16_t port, > > - uint32_t data) > > +static int sock_l4_sa(const struct ctx *c, uint8_t proto, > > + const void *sa, socklen_t sl, > > + const char *ifname, bool v6only, uint32_t data) > > { > > + sa_family_t af =3D((const struct sockaddr *)sa)->sa_family; >=20 > Missing whitespace after =3D. Oops. > > union epoll_ref ref =3D { .data =3D data }; > > - struct sockaddr_in addr4 =3D { > > - .sin_family =3D AF_INET, > > - .sin_port =3D htons(port), > > - { 0 }, { 0 }, > > - }; > > - struct sockaddr_in6 addr6 =3D { > > - .sin6_family =3D AF_INET6, > > - .sin6_port =3D htons(port), > > - 0, IN6ADDR_ANY_INIT, 0, > > - }; > > - const struct sockaddr *sa; > > - bool dual_stack =3D false; > > - int fd, sl, y =3D 1, ret; > > struct epoll_event ev; > > + int fd, y =3D 1, ret; > > =20 > > switch (proto) { > > case IPPROTO_TCP: > > @@ -79,13 +68,6 @@ int sock_l4(const struct ctx *c, sa_family_t af, uin= t8_t proto, > > return -EPFNOSUPPORT; /* Not implemented. */ > > } > > =20 > > - if (af =3D=3D AF_UNSPEC) { > > - if (!DUAL_STACK_SOCKETS || bind_addr) > > - return -EINVAL; > > - dual_stack =3D true; > > - af =3D AF_INET6; > > - } > > - > > if (proto =3D=3D IPPROTO_TCP) > > fd =3D socket(af, SOCK_STREAM | SOCK_NONBLOCK, proto); > > else > > @@ -104,30 +86,9 @@ int sock_l4(const struct ctx *c, sa_family_t af, ui= nt8_t proto, > > =20 > > ref.fd =3D fd; > > =20 > > - if (af =3D=3D AF_INET) { > > - if (bind_addr) > > - addr4.sin_addr =3D *(struct in_addr *)bind_addr; > > - > > - sa =3D (const struct sockaddr *)&addr4; > > - sl =3D sizeof(addr4); > > - } else { > > - if (bind_addr) { > > - addr6.sin6_addr =3D *(struct in6_addr *)bind_addr; > > - > > - if (!memcmp(bind_addr, &c->ip6.addr_ll, > > - sizeof(c->ip6.addr_ll))) > > - addr6.sin6_scope_id =3D c->ifi6; > > - } > > - > > - sa =3D (const struct sockaddr *)&addr6; > > - sl =3D sizeof(addr6); > > - > > - if (!dual_stack) > > - if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, > > - &y, sizeof(y))) > > - debug("Failed to set IPV6_V6ONLY on socket %i", > > - fd); > > - } > > + if (v6only) > > + if (setsockopt(fd, IPPROTO_IPV6, IPV6_V6ONLY, &y, sizeof(y))) > > + debug("Failed to set IPV6_V6ONLY on socket %i", fd); > > =20 > > if (setsockopt(fd, SOL_SOCKET, SO_REUSEADDR, &y, sizeof(y))) > > debug("Failed to set SO_REUSEADDR on socket %i", fd); > > @@ -140,9 +101,12 @@ int sock_l4(const struct ctx *c, sa_family_t af, u= int8_t proto, > > */ > > if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, > > ifname, strlen(ifname))) { > > + char str[SOCKADDR_STRLEN]; > > + > > ret =3D -errno; > > - warn("Can't bind %s socket for port %u to %s, closing", > > - EPOLL_TYPE_STR(proto), port, ifname); > > + warn("Can't bind %s socket for %s to %s, closing", > > + EPOLL_TYPE_STR(proto), > > + sockaddr_ntop(sa, str, sizeof(str)), ifname); > > close(fd); > > return ret; > > } > > @@ -178,6 +142,59 @@ int sock_l4(const struct ctx *c, sa_family_t af, u= int8_t proto, > > =20 > > return fd; > > } > > +/** > > + * sock_l4() - Create and bind socket for given L4, add to epoll list > > + * @c: Execution context > > + * @af: Address family, AF_INET or AF_INET6 > > + * @proto: Protocol number > > + * @bind_addr: Address for binding, NULL for any > > + * @ifname: Interface for binding, NULL for any > > + * @port: Port, host order > > + * @data: epoll reference portion for protocol handlers > > + * > > + * Return: newly created socket, negative error code on failure > > + */ > > +int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, > > + const void *bind_addr, const char *ifname, uint16_t port, > > + uint32_t data) > > +{ > > + switch (af) { > > + case AF_INET: { > > + struct sockaddr_in addr4 =3D { > > + .sin_family =3D AF_INET, > > + .sin_port =3D htons(port), > > + { 0 }, { 0 }, > > + }; > > + if (bind_addr) > > + addr4.sin_addr =3D *(struct in_addr *)bind_addr; > > + return sock_l4_sa(c, proto, &addr4, sizeof(addr4), ifname, > > + false, data); > > + } > > + > > + case AF_UNSPEC: > > + if (!DUAL_STACK_SOCKETS || bind_addr) > > + return -EINVAL; > > + /* fallthrough */ > > + case AF_INET6: { > > + struct sockaddr_in6 addr6 =3D { > > + .sin6_family =3D AF_INET6, > > + .sin6_port =3D htons(port), > > + 0, IN6ADDR_ANY_INIT, 0, > > + }; > > + if (bind_addr) { > > + addr6.sin6_addr =3D *(struct in6_addr *)bind_addr; > > + > > + if (!memcmp(bind_addr, &c->ip6.addr_ll, > > + sizeof(c->ip6.addr_ll))) > > + addr6.sin6_scope_id =3D c->ifi6; > > + } > > + return sock_l4_sa(c, proto, &addr6, sizeof(addr6), ifname, > > + af =3D=3D AF_INET6, data); > > + } > > + default: > > + return -EINVAL; > > + } > > +} > > =20 > > /** > > * sock_probe_mem() - Check if setting high SO_SNDBUF and SO_RCVBUF is= allowed >=20 --=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 --KdtzeO9QQOhvCJUh Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmZrkxoACgkQzQJF27ox 2GdK7xAAop3TH1AmZqKpvnYLQOQw7/hMPZp3Dnt01F0l/AZAdInsjZew5mgTnmRF 6PYkUOGUgLi6lpyef1crGTgk5iDNeplipfzvhIUpI5+4WHFdoBBmKf3L8nRP1tx/ 47a/VMQkdoNCaP8XOAkTHvC8NutOQS0gNzEAMP0hSmqwQbPr+xa4KVItOd3SLwYB LPK7dTrtYSwKXWknh/WhFQ4zZiPNkclvFizZLgQlfNjF7bh/x68kvLes7qlqpZZv I/t1dkybR1PVJ+nsSjKfjbLXIbnP4rATmKnH2TNGfVBAyQjWi8LVL++QVn+fW9UD S23TMdYIsZfZ31682s/0M+3uALEns+JMdVLPctbs3bgfpss4QlRKZ8jWjlGNuE8Y ngjYBYiWeZNukWcAYCSW0Na9N7dQ5CbeLdrTjAoXpSWbbHHIyn5L/UHCPhJP3aFY is5C1AdokMIwyY4qPuSxVyXGoTxDT56r6n9TfXqHvPHJ2q2P7ZfySDiiG0i9XEeJ Lbr+PlOef8VlZqo8ep3L8LWPPWJbnMqWlUKA6pxJAvOL4t0sS1iq7nB2bZpfOuRe EwtduYREqZUfjeD3KFanq2vrsULvN2oIiV92ATM73TvQjR9zlpy2BN7FD5dhRxSq EoIRqkhok7U0fk6LcTolX5diIoEOeOyTohwiTBhsm3CZSuZw4NA= =4BlT -----END PGP SIGNATURE----- --KdtzeO9QQOhvCJUh--