From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTP id 4B07B5A004E for ; Thu, 13 Jun 2024 17:07:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1718291230; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=PS1vimuBc5sGVfLDCp3uGj7B+7DjZtCb0oef5ewqIEY=; b=AbkEsEPDByG5jDmym1HTCG5MVWhhRYyPedUFt+3Ja1PQDKweOckTkC4c16CSDAii5mFugv 7Tjawm5pAcqtIzZzC29YpSSkmT5CzvDTa7RYQkK7eXlfAcuXIlfKqoFFAAjWUtRVeUig++ 8uCWEXcsdSQpM0HfckOZ30NWrMrlET4= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-460-8iD5l8iSNUCdyJT8np1C1w-1; Thu, 13 Jun 2024 11:07:05 -0400 X-MC-Unique: 8iD5l8iSNUCdyJT8np1C1w-1 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-44055f6d991so34680201cf.0 for ; Thu, 13 Jun 2024 08:07:05 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1718291224; x=1718896024; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=PS1vimuBc5sGVfLDCp3uGj7B+7DjZtCb0oef5ewqIEY=; b=H0NtGqR2uhbaVksdVBZ6OB+LYqticA+0E1W+aKM6pUV513MS+Qtf1AlYo2QSex9cXN 1rKKmUNZKhQ0dyvIgBjluqP2M7IkyyLgwRFs+RcNGFu2GT32eR9emW+ovHnyRxQJhip6 IQxF2OXelnDjuo2W6rPbl/7MYUeAl5ekKJRkImC4KtLftLHNDw2Ohzgq2Pk7ae8+6h27 SFLGvpRRIr1UEd6Y2jkqc0t8+737pN1W8nL8yYDSB3O1HhRO9w1sZjlEzTNQfYI/rkL2 A6OiOi1cfwjZuv8p7//lJEyb2FqcqMXGQKiJzyQ44H8Hsm1HunpdMnJQpUaq8nEu8bPf MNkA== X-Gm-Message-State: AOJu0Yzsi3CyOZ0g2KUyAtgsy9w3huJvF2IGVE0YnTwBzL+V41++M1or ZN49YkvuuLupNxYAyFZju3RJV07s1ndPJdYtKrgPcgfFKtlYAl0PhSB+5zKEmJ7rTFDsi8QWBsJ P4VWi0w4lVyBokrjmz90v3BcQIwY4kIe/SNINJh/GAej5UPgv00bryXzZTZM6 X-Received: by 2002:a05:620a:4384:b0:795:4b87:b2c9 with SMTP id af79cd13be357-798d04e5e1emr12723685a.39.1718291224379; Thu, 13 Jun 2024 08:07:04 -0700 (PDT) X-Google-Smtp-Source: AGHT+IE/+zmhP3U8p09n+YwTc7WUzg06P8Q2EH7KdPdJEGdfGzakulI0pw7tvu9CNfjS/PNslnL9Vg== X-Received: by 2002:a05:620a:4384:b0:795:4b87:b2c9 with SMTP id af79cd13be357-798d04e5e1emr12718485a.39.1718291223661; Thu, 13 Jun 2024 08:07:03 -0700 (PDT) Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id af79cd13be357-798c874f958sm24939385a.63.2024.06.13.08.07.02 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 13 Jun 2024 08:07:03 -0700 (PDT) Date: Thu, 13 Jun 2024 17:06:25 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 1/4] util: Split construction of bind socket address from the rest of sock_l4() Message-ID: <20240613170625.49428bf6@elisabeth> In-Reply-To: <20240605013903.3694452-2-david@gibson.dropbear.id.au> References: <20240605013903.3694452-1-david@gibson.dropbear.id.au> <20240605013903.3694452-2-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.41; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: DBCOMOC7P2KCFSVHVN7AGNIQQJ22PBOA X-Message-ID-Hash: DBCOMOC7P2KCFSVHVN7AGNIQQJ22PBOA X-MailFrom: sbrivio@redhat.com 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: Sorry for the delay, nits only (I can fix them up on merge): On Wed, 5 Jun 2024 11:39:00 +1000 David Gibson wrote: > 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 construct > the socket address in the caller. > > Prepare for this by adding sock_l4_sa() which takes a pre-constructed > socket address, and rewriting sock_l4() in terms of it. > > Signed-off-by: David Gibson > --- > util.c | 123 ++++++++++++++++++++++++++++++++------------------------- > 1 file changed, 70 insertions(+), 53 deletions(-) > > 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" > > /** > - * 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 list That doesn't quite tell the difference from sock_l4(), perhaps: * sock_l4_sa() - Create and bind socket given socket address, 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 > + * @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 =((const struct sockaddr *)sa)->sa_family; Missing whitespace after =. > union epoll_ref ref = { .data = data }; > - struct sockaddr_in addr4 = { > - .sin_family = AF_INET, > - .sin_port = htons(port), > - { 0 }, { 0 }, > - }; > - struct sockaddr_in6 addr6 = { > - .sin6_family = AF_INET6, > - .sin6_port = htons(port), > - 0, IN6ADDR_ANY_INIT, 0, > - }; > - const struct sockaddr *sa; > - bool dual_stack = false; > - int fd, sl, y = 1, ret; > struct epoll_event ev; > + int fd, y = 1, ret; > > switch (proto) { > case IPPROTO_TCP: > @@ -79,13 +68,6 @@ int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, > return -EPFNOSUPPORT; /* Not implemented. */ > } > > - if (af == AF_UNSPEC) { > - if (!DUAL_STACK_SOCKETS || bind_addr) > - return -EINVAL; > - dual_stack = true; > - af = AF_INET6; > - } > - > if (proto == IPPROTO_TCP) > fd = socket(af, SOCK_STREAM | SOCK_NONBLOCK, proto); > else > @@ -104,30 +86,9 @@ int sock_l4(const struct ctx *c, sa_family_t af, uint8_t proto, > > ref.fd = fd; > > - if (af == AF_INET) { > - if (bind_addr) > - addr4.sin_addr = *(struct in_addr *)bind_addr; > - > - sa = (const struct sockaddr *)&addr4; > - sl = sizeof(addr4); > - } else { > - if (bind_addr) { > - addr6.sin6_addr = *(struct in6_addr *)bind_addr; > - > - if (!memcmp(bind_addr, &c->ip6.addr_ll, > - sizeof(c->ip6.addr_ll))) > - addr6.sin6_scope_id = c->ifi6; > - } > - > - sa = (const struct sockaddr *)&addr6; > - sl = 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); > > 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, uint8_t proto, > */ > if (setsockopt(fd, SOL_SOCKET, SO_BINDTODEVICE, > ifname, strlen(ifname))) { > + char str[SOCKADDR_STRLEN]; > + > ret = -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, uint8_t proto, > > 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 = { > + .sin_family = AF_INET, > + .sin_port = htons(port), > + { 0 }, { 0 }, > + }; > + if (bind_addr) > + addr4.sin_addr = *(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 = { > + .sin6_family = AF_INET6, > + .sin6_port = htons(port), > + 0, IN6ADDR_ANY_INIT, 0, > + }; > + if (bind_addr) { > + addr6.sin6_addr = *(struct in6_addr *)bind_addr; > + > + if (!memcmp(bind_addr, &c->ip6.addr_ll, > + sizeof(c->ip6.addr_ll))) > + addr6.sin6_scope_id = c->ifi6; > + } > + return sock_l4_sa(c, proto, &addr6, sizeof(addr6), ifname, > + af == AF_INET6, data); > + } > + default: > + return -EINVAL; > + } > +} > > /** > * sock_probe_mem() - Check if setting high SO_SNDBUF and SO_RCVBUF is allowed -- Stefano