From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: passt.top; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: passt.top; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=HMHSP7Wt; dkim-atps=neutral Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by passt.top (Postfix) with ESMTPS id 5A8715A0BC2 for ; Thu, 13 Nov 2025 07:33:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1763015599; 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=RyJqfPMcc4hzvUYtUNHMwK5MCpNLbOf4KQ4pmNShw0A=; b=HMHSP7WtxSVMjtxot+MhDfilgAUKazkB4HuSofQ1u0pmubjuxJWVN2EScyfV/4WsY4NzDN usna+VJ1R78Nm10jXnKg6C1AAhdAAqbxnU0cdyjc+9/GTQ4M3rTjKOyCh07wqkLExNvNNX 7d6wbG7Kbk84QA6RouNLnKss1Fy9h9Q= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-64-yoBvf_JhOhyY2QemZY073w-1; Thu, 13 Nov 2025 01:33:17 -0500 X-MC-Unique: yoBvf_JhOhyY2QemZY073w-1 X-Mimecast-MFC-AGG-ID: yoBvf_JhOhyY2QemZY073w_1763015596 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-477563e531cso3980265e9.1 for ; Wed, 12 Nov 2025 22:33:17 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1763015596; x=1763620396; h=content-transfer-encoding:mime-version:organization:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=RyJqfPMcc4hzvUYtUNHMwK5MCpNLbOf4KQ4pmNShw0A=; b=bReh44qonIiZj1SfwVGqauDPHohMJNNdSiY5FsWb4sXrCLRcUrg/H+dDMjm/U1SYBy oGl4+dquQAHoATaOvzyGRjJvlRXefzzoYkpV757Y2xeMmmLhItpF4jj+gYk0vxYai6Vh kRfRhToM5LqqayZx6VZ7Z5w7eSPQgYi4SndLYpdhA8wiiyNXXUnId21ukuFcsHqEjZIA x9FR+O3wEyVbBF3avubIgtB/NpHpgziofgDHjQWkQMCEGoGbpUgmmutZuX2wggbc/YlO 0MRlf29X9LK6XJDIqpLvf5wkSVEh6V2cRKlqcBlByqrw04IynqZTh7ieJlGWtw/TjjU5 kblg== X-Gm-Message-State: AOJu0Yz1UtNbxZNPtk/vUKX8PeZmWaV36e/Y5ozyrLFRBs2a0tfjdXZU pFCBYzLcZA7mT7TPG3AU2wp+59nhYZJ6vkFd8bFcKlK+BH9p2X4wyDhk9iuJMyX4x006n58vR0g H74dexZ6IdSdypVkA0QIxGJcVt3TFjZ3+2wJl+kVVwveJtpAehMYoeVfGOSw+Bg== X-Gm-Gg: ASbGncs6m7scbnpTAeeWBxeakf15kFDsmRNubRTWcuPwDsE2SJvMxn+eKEQeocmxv2d 8da+clT9sNzByi6zeUPULmI0kPLcra4dLKITd5t078RfvnEZ+P13ruLTmpCPK71FI/Sw/MYYuqg DGlx4wdUtM8PMH5D4jTf1C+UUVtzH/2cEc7ys+r6LFoSD4eogjqTVpE/DypY+2jGoSnyzuWr882 9ZVyl3G2UEgQeyWvFbtSjvsFAW0toipJ5fhmyTRUNMfjDvYTiMgPRITkK1IJ8oCDWmIAWSLMpjW vvwEfUuMassVwXdpL3uswGgYACZQlRWdL3Axa8m6FmdRCx1OhSe/O2oON0F6RBZDu5PSxfBBIzL uiZvRkGrKN3s+JLHUbbPTmXnFuzA= X-Received: by 2002:a05:600c:3587:b0:477:63db:c718 with SMTP id 5b1f17b1804b1-477870857f9mr43791705e9.16.1763015595849; Wed, 12 Nov 2025 22:33:15 -0800 (PST) X-Google-Smtp-Source: AGHT+IE7zdmyIchFFNKyxqdy6oMTssCYNiyuv7m6Px5JH+9kZDVDbZdrlcTwWfeTFs1ymeYTqlCXYg== X-Received: by 2002:a05:600c:3587:b0:477:63db:c718 with SMTP id 5b1f17b1804b1-477870857f9mr43791555e9.16.1763015595297; Wed, 12 Nov 2025 22:33:15 -0800 (PST) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [176.103.220.4]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-42b53f0b57asm1860547f8f.27.2025.11.12.22.33.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 12 Nov 2025 22:33:14 -0800 (PST) Date: Thu, 13 Nov 2025 07:33:13 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v3 2/8] util, flow, pif: Simplify sock_l4_sa() interface Message-ID: <20251113073313.1287b4dc@elisabeth> In-Reply-To: <20251029062628.1647051-3-david@gibson.dropbear.id.au> References: <20251029062628.1647051-1-david@gibson.dropbear.id.au> <20251029062628.1647051-3-david@gibson.dropbear.id.au> Organization: Red Hat X-Mailer: Claws Mail 4.2.0 (GTK 3.24.49; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: iLoFNjIgjC0BOkBTENWNffhuGMldzp89Q7kOeX4R48Y_1763015596 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: FR2R6KNNRQCD5XCUJFQ77ON7MSEHLS2P X-Message-ID-Hash: FR2R6KNNRQCD5XCUJFQ77ON7MSEHLS2P 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: On Wed, 29 Oct 2025 17:26:22 +1100 David Gibson wrote: > sock_l4_sa() has a somewhat confusing 'v6only' option controlling whether > to set the IPV6_V6ONLY socket option. Usually it's set when the given > address is IPv6, but not when we want to create a dual stack listening > socket. The latter only makes sense when the address is :: however. > > Clarify this by only keeping the v6only option in an internal helper > sock_l4_(). External users will call either sock_l4() which always creates > a socket bound to a specific IP version, or sock_l4_dualstack() which > creates a dual stack socket, but takes only a port not an address. I'm not sure if we'll ever need anything different, but I guess that this is not the only obvious semantic of sock_l4_dualstack(), as it could take a sockaddr_inany eventually, and bind() IPv6 address and its v4-mapped equivalent (...does that even work?). > We drop the '_sa' suffix while we're at it - it exists because this used > to be an internal version with a sock_l4() wrapper. The wrapper no longer > exists so the '_sa' is no longer useful. > > Signed-off-by: David Gibson > --- > flow.c | 6 ++---- > pif.c | 10 +++------- > util.c | 27 +++++++++++++++++++++++---- > util.h | 8 +++++--- > 4 files changed, 33 insertions(+), 18 deletions(-) > > diff --git a/flow.c b/flow.c > index 9926f408..fd530ddb 100644 > --- a/flow.c > +++ b/flow.c > @@ -186,8 +186,7 @@ static int flowside_sock_splice(void *arg) > > ns_enter(a->c); > > - a->fd = sock_l4_sa(a->c, a->type, a->sa, NULL, > - a->sa->sa_family == AF_INET6, a->data); > + a->fd = sock_l4(a->c, a->type, a->sa, NULL, a->data); > a->err = errno; > > return 0; > @@ -222,8 +221,7 @@ int flowside_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, > else if (sa.sa_family == AF_INET6) > ifname = c->ip6.ifname_out; > > - return sock_l4_sa(c, type, &sa, ifname, > - sa.sa_family == AF_INET6, data); > + return sock_l4(c, type, &sa, ifname, data); > > case PIF_SPLICE: { > struct flowside_sock_args args = { > diff --git a/pif.c b/pif.c > index 31723b29..5fb1f455 100644 > --- a/pif.c > +++ b/pif.c > @@ -75,11 +75,7 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, > const union inany_addr *addr, const char *ifname, > in_port_t port, uint32_t data) > { > - union sockaddr_inany sa = { > - .sa6.sin6_family = AF_INET6, > - .sa6.sin6_addr = in6addr_any, > - .sa6.sin6_port = htons(port), > - }; > + union sockaddr_inany sa; > > ASSERT(pif_is_socket(pif)); > > @@ -90,8 +86,8 @@ int pif_sock_l4(const struct ctx *c, enum epoll_type type, uint8_t pif, > } > > if (!addr) > - return sock_l4_sa(c, type, &sa, ifname, false, data); > + return sock_l4_dualstack(c, type, port, ifname, data); > > pif_sockaddr(c, &sa, pif, addr, port); > - return sock_l4_sa(c, type, &sa, ifname, sa.sa_family == AF_INET6, data); > + return sock_l4(c, type, &sa, ifname, data); > } > diff --git a/util.c b/util.c > index 976fcabe..c94efae4 100644 > --- a/util.c > +++ b/util.c > @@ -40,7 +40,7 @@ > #endif > > /** > - * sock_l4_sa() - Create and bind socket to socket address, add to epoll list > + * sock_l4_() - Create and bind socket to socket address, add to epoll list > * @c: Execution context > * @type: epoll type > * @sa: Socket address to bind to > @@ -50,9 +50,9 @@ > * > * Return: newly created socket, negative error code on failure > */ > -int sock_l4_sa(const struct ctx *c, enum epoll_type type, > - const union sockaddr_inany *sa, const char *ifname, > - bool v6only, uint32_t data) > +static int sock_l4_(const struct ctx *c, enum epoll_type type, > + const union sockaddr_inany *sa, const char *ifname, > + bool v6only, uint32_t data) > { > sa_family_t af = sa->sa_family; > union epoll_ref ref = { .type = type, .data = data }; > @@ -182,6 +182,25 @@ int sock_l4_sa(const struct ctx *c, enum epoll_type type, > return fd; > } > > +int sock_l4(const struct ctx *c, enum epoll_type type, > + const union sockaddr_inany *sa, const char *ifname, > + uint32_t data) Not extremely useful but it saves one "lookup": /** * sock_l4() - Create and bind socket to given address, add to epoll list * @c: Execution context * @type: epoll type * @sa: Socket address to bind to * @ifname: Interface for binding, NULL for any * * Return: newly created socket, negative error code on failure */ > +{ > + return sock_l4_(c, type, sa, ifname, sa->sa_family == AF_INET6, data); > +} > + > +int sock_l4_dualstack(const struct ctx *c, enum epoll_type type, > + in_port_t port, const char *ifname, uint32_t data) ...same here, and the comment might be used to clarify the functionality. > +{ > + union sockaddr_inany sa = { > + .sa6.sin6_family = AF_INET6, > + .sa6.sin6_addr = in6addr_any, > + .sa6.sin6_port = htons(port), > + }; > + > + return sock_l4_(c, type, &sa, ifname, 0, data); > +} > + > /** > * sock_unix() - Create and bind AF_UNIX socket > * @sock_path: Socket path. If empty, set on return (UNIX_SOCK_PATH as prefix) > diff --git a/util.h b/util.h > index e1a1ebc9..7f0cf686 100644 > --- a/util.h > +++ b/util.h > @@ -203,9 +203,11 @@ int do_clone(int (*fn)(void *), char *stack_area, size_t stack_size, int flags, > struct ctx; > union sockaddr_inany; > > -int sock_l4_sa(const struct ctx *c, enum epoll_type type, > - const union sockaddr_inany *sa, const char *ifname, > - bool v6only, uint32_t data); > +int sock_l4(const struct ctx *c, enum epoll_type type, > + const union sockaddr_inany *sa, const char *ifname, > + uint32_t data); > +int sock_l4_dualstack(const struct ctx *c, enum epoll_type type, > + in_port_t port, const char *ifname, uint32_t data); > int sock_unix(char *sock_path); > void sock_probe_mem(struct ctx *c); > long timespec_diff_ms(const struct timespec *a, const struct timespec *b); -- Stefano