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 D0CE05A004F for ; Thu, 11 Jul 2024 03:54:13 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1720662836; bh=n0BaWkHrbp6xtz23Xq7lu83KzjanhRup3+Px13iDdwY=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=o4tZPHVwdO+P342x2TmuknFSTJGf0KuwW7d5m6BpXW+ET1FPcdqgS4nhJxh9yhsSo h9OOU3HFJhHj4k6royKAKHNq1Di7HwMb6DXpUj5CgZ17aPC/YddSNU2V7jDi5uEoSi zKE946Cyz+T74ePptxung2WBHMeQsC0n71KSrgjppeMN7VQea01OLSuCukB+UGPSQG sFt5tC+GeSwcPvozpf2uWEKAwvkGkkQZXqvGAiaSO4Me3pLdLOegvOH2N1+g/fErH7 3BON7xINBI/Datme9a3o5xMJjjcxz03JMNg/+ktYtGCNrMchG0y5MFuU7XqkY/sbSv 7Eg7JazkbWkIQ== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4WKHnw4HHlz4wxk; Thu, 11 Jul 2024 11:53:56 +1000 (AEST) Date: Thu, 11 Jul 2024 10:43:43 +1000 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v7 22/27] udp: Remove obsolete splice tracking Message-ID: References: <20240705020724.3447719-1-david@gibson.dropbear.id.au> <20240705020724.3447719-23-david@gibson.dropbear.id.au> <20240710233606.06e57ee9@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="W5d5nRiHXfw/uw12" Content-Disposition: inline In-Reply-To: <20240710233606.06e57ee9@elisabeth> Message-ID-Hash: APDX6ZTNSVX4H5ZKSKPJXOGFF3NUWYSC X-Message-ID-Hash: APDX6ZTNSVX4H5ZKSKPJXOGFF3NUWYSC 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, jmaloy@redhat.com 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: --W5d5nRiHXfw/uw12 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Jul 10, 2024 at 11:36:06PM +0200, Stefano Brivio wrote: > On Fri, 5 Jul 2024 12:07:19 +1000 > David Gibson wrote: >=20 > > Now that spliced datagrams are managed via the flow table, remove > > UDP_ACT_SPLICE_NS and UDP_ACT_SPLICE_INIT which are no longer used. Wi= th > > those removed, the 'ts' field in udp_splice_port is also no longer used. > > struct udp_splice_port now contains just a socket fd, so replace it with > > a plain int in udp_splice_ns[] and udp_splice_init[]. The latter are s= till > > used for tracking of automatic port forwarding. > >=20 > > Finally, the 'splice' field of union udp_epoll_ref is no longer used so > > remove it as well. > >=20 > > Signed-off-by: David Gibson > > --- > > udp.c | 65 +++++++++++++++++------------------------------------------ > > udp.h | 3 +-- > > 2 files changed, 19 insertions(+), 49 deletions(-) > >=20 > > diff --git a/udp.c b/udp.c > > index f4c696db..a4eb6d0f 100644 > > --- a/udp.c > > +++ b/udp.c > > @@ -136,27 +136,15 @@ struct udp_tap_port { > > time_t ts; > > }; > > =20 > > -/** > > - * struct udp_splice_port - Bound socket for spliced communication > > - * @sock: Socket bound to index port > > - * @ts: Activity timestamp > > - */ > > -struct udp_splice_port { > > - int sock; > > - time_t ts; > > -}; > > - > > /* Port tracking, arrays indexed by packet source port (host order) */ > > static struct udp_tap_port udp_tap_map [IP_VERSIONS][NUM_PORTS]; > > =20 > > /* "Spliced" sockets indexed by bound port (host order) */ > > -static struct udp_splice_port udp_splice_ns [IP_VERSIONS][NUM_PORTS]; > > -static struct udp_splice_port udp_splice_init[IP_VERSIONS][NUM_PORTS]; > > +static int udp_splice_ns [IP_VERSIONS][NUM_PORTS]; > > +static int udp_splice_init[IP_VERSIONS][NUM_PORTS]; > > =20 > > enum udp_act_type { > > UDP_ACT_TAP, > > - UDP_ACT_SPLICE_NS, > > - UDP_ACT_SPLICE_INIT, > > UDP_ACT_TYPE_MAX, > > }; > > =20 > > @@ -246,8 +234,8 @@ void udp_portmap_clear(void) > > =20 > > for (i =3D 0; i < NUM_PORTS; i++) { > > udp_tap_map[V4][i].sock =3D udp_tap_map[V6][i].sock =3D -1; > > - udp_splice_ns[V4][i].sock =3D udp_splice_ns[V6][i].sock =3D -1; > > - udp_splice_init[V4][i].sock =3D udp_splice_init[V6][i].sock =3D -1; > > + udp_splice_ns[V4][i] =3D udp_splice_ns[V6][i] =3D -1; > > + udp_splice_init[V4][i] =3D udp_splice_init[V6][i] =3D -1; > > } > > } > > =20 > > @@ -1050,8 +1038,7 @@ int udp_tap_handler(struct ctx *c, uint8_t pif, > > 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 { .splice =3D (c->mode =3D=3D MODE_PASTA= ), > > - .orig =3D true, .port =3D port }; > > + union udp_epoll_ref uref =3D { .orig =3D true, .port =3D port }; > > int s, r4 =3D FD_REF_MAX + 1, r6 =3D FD_REF_MAX + 1; > > =20 > > if (ns) > > @@ -1067,12 +1054,12 @@ int udp_sock_init(const struct ctx *c, int ns, = sa_family_t af, > > ifname, port, uref.u32); > > =20 > > udp_tap_map[V4][port].sock =3D s < 0 ? -1 : s; > > - udp_splice_init[V4][port].sock =3D s < 0 ? -1 : s; > > + udp_splice_init[V4][port] =3D s < 0 ? -1 : s; > > } else { > > r4 =3D s =3D sock_l4(c, AF_INET, EPOLL_TYPE_UDP, > > &in4addr_loopback, > > ifname, port, uref.u32); > > - udp_splice_ns[V4][port].sock =3D s < 0 ? -1 : s; > > + udp_splice_ns[V4][port] =3D s < 0 ? -1 : s; > > } > > } > > =20 > > @@ -1084,12 +1071,12 @@ int udp_sock_init(const struct ctx *c, int ns, = sa_family_t af, > > ifname, port, uref.u32); > > =20 > > udp_tap_map[V6][port].sock =3D s < 0 ? -1 : s; > > - udp_splice_init[V6][port].sock =3D s < 0 ? -1 : s; > > + udp_splice_init[V6][port] =3D s < 0 ? -1 : s; > > } else { > > r6 =3D s =3D sock_l4(c, AF_INET6, EPOLL_TYPE_UDP, > > &in6addr_loopback, > > ifname, port, uref.u32); > > - udp_splice_ns[V6][port].sock =3D s < 0 ? -1 : s; > > + udp_splice_ns[V6][port] =3D s < 0 ? -1 : s; > > } > > } > > =20 > > @@ -1130,7 +1117,6 @@ static void udp_splice_iov_init(void) > > static void udp_timer_one(struct ctx *c, int v6, enum udp_act_type typ= e, > > in_port_t port, const struct timespec *now) > > { > > - struct udp_splice_port *sp; > > struct udp_tap_port *tp; > > int *sockp =3D NULL; > > =20 > > @@ -1143,20 +1129,6 @@ static void udp_timer_one(struct ctx *c, int v6,= enum udp_act_type type, > > tp->flags =3D 0; > > } > > =20 > > - break; > > - case UDP_ACT_SPLICE_INIT: > > - sp =3D &udp_splice_init[v6 ? V6 : V4][port]; > > - > > - if (now->tv_sec - sp->ts > UDP_CONN_TIMEOUT) > > - sockp =3D &sp->sock; > > - > > - break; > > - case UDP_ACT_SPLICE_NS: > > - sp =3D &udp_splice_ns[v6 ? V6 : V4][port]; > > - > > - if (now->tv_sec - sp->ts > UDP_CONN_TIMEOUT) > > - sockp =3D &sp->sock; > > - > > break; > > default: > > return; > > @@ -1184,20 +1156,19 @@ static void udp_port_rebind(struct ctx *c, bool= outbound) > > =3D outbound ? c->udp.fwd_out.f.map : c->udp.fwd_in.f.map; > > const uint8_t *rmap > > =3D outbound ? c->udp.fwd_in.f.map : c->udp.fwd_out.f.map; > > - struct udp_splice_port (*socks)[NUM_PORTS] > > - =3D outbound ? udp_splice_ns : udp_splice_init; > > + int (*socks)[NUM_PORTS] =3D outbound ? udp_splice_ns : udp_splice_ini= t; >=20 > Nit: this should be moved up now, before the declaration of 'fmap'. Done. > > unsigned port; > > =20 > > for (port =3D 0; port < NUM_PORTS; port++) { > > if (!bitmap_isset(fmap, port)) { > > - if (socks[V4][port].sock >=3D 0) { > > - close(socks[V4][port].sock); > > - socks[V4][port].sock =3D -1; > > + if (socks[V4][port] >=3D 0) { > > + close(socks[V4][port]); > > + socks[V4][port] =3D -1; > > } > > =20 > > - if (socks[V6][port].sock >=3D 0) { > > - close(socks[V6][port].sock); > > - socks[V6][port].sock =3D -1; > > + if (socks[V6][port] >=3D 0) { > > + close(socks[V6][port]); > > + socks[V6][port] =3D -1; > > } > > =20 > > continue; > > @@ -1207,8 +1178,8 @@ static void udp_port_rebind(struct ctx *c, bool o= utbound) > > if (bitmap_isset(rmap, port)) > > continue; > > =20 > > - if ((c->ifi4 && socks[V4][port].sock =3D=3D -1) || > > - (c->ifi6 && socks[V6][port].sock =3D=3D -1)) > > + if ((c->ifi4 && socks[V4][port] =3D=3D -1) || > > + (c->ifi6 && socks[V6][port] =3D=3D -1)) > > udp_sock_init(c, outbound, AF_UNSPEC, NULL, NULL, port); > > } > > } > > diff --git a/udp.h b/udp.h > > index db5e546e..310f42fd 100644 > > --- a/udp.h > > +++ b/udp.h > > @@ -36,8 +36,7 @@ union udp_epoll_ref { > > struct { > > in_port_t port; > > uint8_t pif; > > - bool splice:1, > > - orig:1, > > + bool orig:1, >=20 > The comment to the union should be updated, removing 'splice'. While at > it, I guess you could also drop 'bound' (removed in 851723924356 ("udp: > Remove the @bound field from union udp_epoll_ref"), but the comment wasn't > updated). Done. --=20 David Gibson (he or they) | 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 --W5d5nRiHXfw/uw12 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmaPKr4ACgkQzQJF27ox 2GeYnw//TVeDgQT8egGSHWG5xHxOPiZiQahFSgwsulaog5hZQjYYycw3U9dlinle eQoEOYrcsxyv2lWgoObxlsGok6klhjV7/rD5Caj3GzAtRpbSOQbgz/T4KsK/F3y+ o/NNkdUlE1NTCD+ybPaEYRkKtkQf6oYeae22ZelG+5XQmHqKOd4D3fbM0ZB4qnZj 0uSyLb/HCSGrDczinxDtDfBGJcVko/6+Ym7P55iUz5ROpRHsHUYR256ko5IHT6p+ QjFQXEGBygFcPKLbPvMRqkMJoypLdEHoUkcyQz957nZ1mqrz/AmtbwFw6Nf5hjes C4RZFTuT9rJHz+2VfPnfehFpszGxlToVEJ629KSQOgy2ngYU2hdWDuzVKRPD4zsc LJ0GwU6q2Ymkg7m8gbA6qmw5fk4j1zAEFiNRPmmbXhwaaC3Do0ccqkdXVKAGvet/ zA4pIeIG2QsRggK/aQf20e5FB12eHFs6cFbomRFAkJG96q0BWWZvAuNwfqfV2Csj FdSnz8uG86NvmBnyPo285p2oZY4pvzZtO50wwnXNUuGiefmvG3jSGXnzZXN0bs3G dfubzu1o5a7LJVd6RsookWtFVnSqwHyrC50V620NMCMxIu2fe2SmpwtUin7k6AA2 Ag4Vb+sls2dFh7reUtH2jNL5b1lSrWxUtr+qrJ865h0c+1BPkMo= =TY55 -----END PGP SIGNATURE----- --W5d5nRiHXfw/uw12--