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=gjFre0xc; 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 06E555A026F for ; Tue, 21 Oct 2025 23:51:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1761083473; 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=rnTv/J3PmmVG6ETUTSD7DWoLvkQSKJnyQJUw8Ylsejc=; b=gjFre0xcG++ARSbwbKLMsqcB8jdYd7JsfTN3DmJOTPvSW2dEOuL7CWqReUUGPdHTa80TRe pptaErLtnTQBc7T9JLPTkh0Yjrq9we/jG5IE373JfxkZ+Pjags9tb+OjvKFky2mhKV1qEE PpxgYyYp7NSlKEBAnp22mx8dkaFtqRQ= 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-536-_Zmtt-sSNceSrUSJiNCLtQ-1; Tue, 21 Oct 2025 17:51:12 -0400 X-MC-Unique: _Zmtt-sSNceSrUSJiNCLtQ-1 X-Mimecast-MFC-AGG-ID: _Zmtt-sSNceSrUSJiNCLtQ_1761083471 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-47106a388cfso28067375e9.0 for ; Tue, 21 Oct 2025 14:51:12 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1761083470; x=1761688270; 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=rnTv/J3PmmVG6ETUTSD7DWoLvkQSKJnyQJUw8Ylsejc=; b=I92XQcjgSvZ28enleW3QVzum4al559MQQPDwd1cOpoC09DagQQkS5NCDC/VTPEWY+w nvN4orVVVRpNI+4rmnciHMoYABvR7mjSwZ/zEzS2Nyee5lx2waEtjO67AJv81TuCGHa9 5qsILDhaDysiy4J6jI0HNbvpxdxVy1xL+ardOwSNxnzR3ZI/0mGWA5jMbAt3EM/WLP3s zprzWUY5svcRKAh7HGOUkGosBfWSjVJcp1vw3DdXN6uK2mTBVD2jvqktSkiG960J2zQu SN6kp4XAuEb04Omv2XNuFblWPqfiH7GmFqF/+uAzkPp0RKJh8Y208HRcYuRTaCkVxeF6 XCNA== X-Gm-Message-State: AOJu0YwZtU7x9XXsi+FbaoXD73/78UdYSOAbElNTObgRYm9NgQWmesNr 3KMxiPFgg4mvkBmXwWCPjlai9pdYWv/TMNL5xA/ynX4TgJdxA9trhaUhKgjuBYyoVCcukXDixAS /lsx68fGzk0IzvCrapadYiebUFT7okG1IisrR53tmw3RBJwdqq6fspp6lutRviw== X-Gm-Gg: ASbGncsmQEfTe9suYDEwOSD0pI7s3tdWuEy4cPAkuwlpCfEhqv64LRaqUSPnTU1M+aM z+k9EOpn565O3whj+qmJx4u7obrtbDyC/RlbbDT+oaWkyJyIN1olZcRxKDOTJ+SuADMdSby2d46 aR0QBfXEbSF6/JB87wv700A87l3Pl5NGIy3mH7NuxIsLKedpZ0dqMEhtsMhDVn/UBd4MpGshzqP NAAP+RVh438Aju4GyaSXMr3hDoZqTgzPg/NkGwnqKaXKiQOfYa/1hdRT+qM6gSqjTLqcGyZxBOp wx7SAsfgJCY6MjgegL68C1BI95vD/gdApcBPlrn/BMLxxzN7LYYQYWhuiuqcWEuR+7VBLQZ+y6p VrexrciogMg== X-Received: by 2002:a05:600c:1493:b0:471:1c48:7c5a with SMTP id 5b1f17b1804b1-4711c487d74mr90173665e9.9.1761083470147; Tue, 21 Oct 2025 14:51:10 -0700 (PDT) X-Google-Smtp-Source: AGHT+IG5MsjUyCp2zhCkEVHtOA0SUkRU+78aca7JsyXzalu5ZhahkhsoOLhhI3ERPY/xtBuB4bO+aw== X-Received: by 2002:a05:600c:1493:b0:471:1c48:7c5a with SMTP id 5b1f17b1804b1-4711c487d74mr90173595e9.9.1761083469690; Tue, 21 Oct 2025 14:51:09 -0700 (PDT) Received: from maya.myfinge.rs (ifcgrfdd.trafficplex.cloud. [2a10:fc81:a806:d6a9::1]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-475c42b4867sm11678295e9.14.2025.10.21.14.51.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 21 Oct 2025 14:51:08 -0700 (PDT) Date: Tue, 21 Oct 2025 23:51:07 +0200 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 2/3] udp: Unify some more inbound/outbound parts of udp_sock_init() Message-ID: <20251021235107.105dd98c@elisabeth> In-Reply-To: <20251017003447.414103-3-david@gibson.dropbear.id.au> References: <20251017003447.414103-1-david@gibson.dropbear.id.au> <20251017003447.414103-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: nfSlUMurRlVkqbUTU4gCIEcPgkELrmXMvDpTdHAbX-4_1761083471 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: BXAYAJ5LQWREOUBLIDWSWRCFJFEJTAMI X-Message-ID-Hash: BXAYAJ5LQWREOUBLIDWSWRCFJFEJTAMI 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 Fri, 17 Oct 2025 11:34:46 +1100 David Gibson wrote: > udp_sock_init() takes an 'ns' parameter determining if it creates a socket > in the guest namespace or host namespace. Alter it to take a pif > parameter instead, like tcp_sock_init(), and use that change to slightly > reduce code duplication between the HOST and SPLICE cases. > > Signed-off-by: David Gibson > --- > conf.c | 2 +- > udp.c | 60 +++++++++++++++++++++++++++++----------------------------- > udp.h | 5 +++-- > 3 files changed, 34 insertions(+), 33 deletions(-) > > diff --git a/conf.c b/conf.c > index 26f1bcc0..08cb50aa 100644 > --- a/conf.c > +++ b/conf.c > @@ -171,7 +171,7 @@ static void conf_ports_range_except(const struct ctx *c, char optname, > if (optname == 't') > ret = tcp_sock_init(c, PIF_HOST, addr, ifname, i); > else if (optname == 'u') > - ret = udp_sock_init(c, 0, addr, ifname, i); > + ret = udp_sock_init(c, PIF_HOST, addr, ifname, i); > else > /* No way to check in advance for -T and -U */ > ret = 0; > diff --git a/udp.c b/udp.c > index 86585b7e..49dd0144 100644 > --- a/udp.c > +++ b/udp.c > @@ -1093,64 +1093,63 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, > /** > * udp_sock_init() - Initialise listening sockets for a given port > * @c: Execution context > - * @ns: In pasta mode, if set, bind with loopback address in namespace > + * @pif: Interface to open the socket for (PIF_HOST or PIF_SPLICE) > * @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, negative error code on (complete) failure > */ > -int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, > - const char *ifname, in_port_t port) > +int udp_sock_init(const struct ctx *c, uint8_t pif, > + const union inany_addr *addr, const char *ifname, > + in_port_t port) > { > + int (*socks)[NUM_PORTS] = pif == PIF_HOST ? > + udp_splice_init : udp_splice_ns; Same as on v1: I'd rather avoid cramping ternary operators like this. > union udp_listen_epoll_ref uref = { > - .pif = ns ? PIF_SPLICE : PIF_HOST, > + .pif = pif, > .port = port, > }; > int r4 = FD_REF_MAX + 1, r6 = FD_REF_MAX + 1; > > ASSERT(!c->no_udp); > > - if (!addr && c->ifi4 && c->ifi6 && !ns) { > + if (!addr && c->ifi4 && c->ifi6 && (pif == PIF_HOST)) { I think it's more readable without the extra parentheses around the comparison, because when I see those I automatically think of an assignment we want to use in a conditional clause, but that's just a comparison. > int s; > > /* Attempt to get a dual stack socket */ > s = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST, > NULL, ifname, port, uref.u32); > - udp_splice_init[V4][port] = s < 0 ? -1 : s; > - udp_splice_init[V6][port] = s < 0 ? -1 : s; > + socks[V4][port] = s < 0 ? -1 : s; > + socks[V6][port] = s < 0 ? -1 : s; > if (IN_INTERVAL(0, FD_REF_MAX, s)) > return 0; > } > > if ((!addr || inany_v4(addr)) && c->ifi4) { > - if (!ns) { > - r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST, > - addr ? addr : &inany_any4, ifname, > - port, uref.u32); > + const union inany_addr *a = addr ? > + addr : &inany_any4; > > - udp_splice_init[V4][port] = r4 < 0 ? -1 : r4; > - } else { > - r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_SPLICE, > - &inany_loopback4, ifname, > - port, uref.u32); > - udp_splice_ns[V4][port] = r4 < 0 ? -1 : r4; > - } > + if (pif == PIF_SPLICE) > + a = &inany_loopback4; > + > + r4 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, pif, a, ifname, > + port, uref.u32); > + > + socks[V4][port] = r4 < 0 ? -1 : r4; > } > > if ((!addr || !inany_v4(addr)) && c->ifi6) { > - if (!ns) { > - r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_HOST, > - addr ? addr : &inany_any6, ifname, > - port, uref.u32); > + const union inany_addr *a = addr ? > + addr : &inany_any6; > > - udp_splice_init[V6][port] = r6 < 0 ? -1 : r6; > - } else { > - r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, PIF_SPLICE, > - &inany_loopback6, ifname, > - port, uref.u32); > - udp_splice_ns[V6][port] = r6 < 0 ? -1 : r6; > - } > + if (pif == PIF_SPLICE) > + a = &inany_loopback6; > + > + r6 = pif_sock_l4(c, EPOLL_TYPE_UDP_LISTEN, pif, a, ifname, > + port, uref.u32); > + > + socks[V6][port] = r6 < 0 ? -1 : r6; > } > > if (IN_INTERVAL(0, FD_REF_MAX, r4) || IN_INTERVAL(0, FD_REF_MAX, r6)) > @@ -1216,7 +1215,8 @@ static void udp_port_rebind(struct ctx *c, bool outbound) > > if ((c->ifi4 && socks[V4][port] == -1) || > (c->ifi6 && socks[V6][port] == -1)) > - udp_sock_init(c, outbound, NULL, NULL, port); > + udp_sock_init(c, outbound ? PIF_SPLICE : PIF_HOST, > + NULL, NULL, port); > } > } > > diff --git a/udp.h b/udp.h > index 8f8531ad..f78dc528 100644 > --- a/udp.h > +++ b/udp.h > @@ -17,8 +17,9 @@ int udp_tap_handler(const struct ctx *c, uint8_t pif, > sa_family_t af, const void *saddr, const void *daddr, > uint8_t ttl, const struct pool *p, int idx, > const struct timespec *now); > -int udp_sock_init(const struct ctx *c, int ns, const union inany_addr *addr, > - const char *ifname, in_port_t port); > +int udp_sock_init(const struct ctx *c, uint8_t pif, > + const union inany_addr *addr, const char *ifname, > + in_port_t port); > int udp_init(struct ctx *c); > void udp_timer(struct ctx *c, const struct timespec *now); > void udp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s); -- Stefano