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.129.124]) by passt.top (Postfix) with ESMTP id 0A0FF5A0082 for ; Fri, 25 Nov 2022 02:47:51 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1669340870; 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=p7QStlzc2S5wyVQmVsgGmsC16+1k5OtYAWlBPKy4+9I=; b=dZLPv44OImtkfGCl/tlVjKapPWbjhX+vJXYCkAA3wOQTiaeFVe2f6CE4c4IUgbQ5zpVSGd H25beGCgJlz45aYP3AMcU3ZYfI5HDB5woGSzPcgu2weAd4qJCZbMARTHzFIGlG5qTPlmsZ sB2W9DZI8JjxxIon+OJn1ag0bbIIp2k= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-249-n7AtqoVENOGuiKfwtPJHIA-1; Thu, 24 Nov 2022 20:47:49 -0500 X-MC-Unique: n7AtqoVENOGuiKfwtPJHIA-1 Received: from smtp.corp.redhat.com (int-mx10.intmail.prod.int.rdu2.redhat.com [10.11.54.10]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C0F223C01D8F; Fri, 25 Nov 2022 01:47:48 +0000 (UTC) Received: from maya.cloud.tilaa.com (ovpn-208-30.brq.redhat.com [10.40.208.30]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 28152492B04; Fri, 25 Nov 2022 01:47:47 +0000 (UTC) Date: Fri, 25 Nov 2022 02:47:45 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH v2 02/16] udp: Separate tracking of inbound and outbound packet flows Message-ID: <20221125024745.2240314e@elisabeth> In-Reply-To: <20221124011659.1024901-3-david@gibson.dropbear.id.au> References: <20221124011659.1024901-1-david@gibson.dropbear.id.au> <20221124011659.1024901-3-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.10 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: WVEP52BZDB7GNGSOYY7HGIQ3HJVEXFYE X-Message-ID-Hash: WVEP52BZDB7GNGSOYY7HGIQ3HJVEXFYE 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.3 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: Just two nits here: On Thu, 24 Nov 2022 12:16:45 +1100 David Gibson wrote: > Each entry udp_splice_map[v6][N] keeps information about two essentially > unrelated packet flows. @ns_conn_sock, @ns_conn_ts and @init_bound_sock > track a packet flow from port N in the host init namespace to some other > port in the pasta namespace (the one @ns_conn_sock is connected to). > @init_conn_sock, @init_conn_ts and @ns_bound_sock track packet flow from > port N in the pasta namespace to some other port in the host init namespace > (the one @init_conn_sock is connected to). > > Split udp_splice_map[][] into two separate tables for the two directions. > Each entry in each table is a 'struct udp_splice_flow' with @orig_sock > (previously the bound socket), @target_sock (previously the connected > socket) and @ts (the timeout for the target socket). > > Signed-off-by: David Gibson > --- > udp.c | 111 +++++++++++++++++++++++++++++----------------------------- > 1 file changed, 56 insertions(+), 55 deletions(-) > > diff --git a/udp.c b/udp.c > index a025a48..4caf73e 100644 > --- a/udp.c > +++ b/udp.c > @@ -47,44 +47,44 @@ > * This comment still references struct udp_splice_port, it should now say "see struct udp_spliced_flow" instead. > * - forward direction: 127.0.0.1:5000 -> 127.0.0.1:80 in init from bound > * socket s, with epoll reference: index = 80, splice = UDP_TO_NS > - * - if udp_splice_map[V4][5000].ns_conn_sock: > - * - send packet to udp4_splice_map[5000].ns_conn_sock > + * - if udp_splice_to_ns[V4][5000].target_sock: > + * - send packet to udp_splice_to_ns[V4][5000].target_sock > * - otherwise: > - * - create new socket udp_splice_map[V4][5000].ns_conn_sock > + * - create new socket udp_splice_to_ns[V4][5000].target_sock > * - bind in namespace to 127.0.0.1:5000 > * - connect in namespace to 127.0.0.1:80 (note: this destination port > * might be remapped to another port instead) > * - add to epoll with reference: index = 5000, splice: UDP_BACK_TO_INIT > - * - set udp_splice_map[V4][5000].init_bound_sock to s > - * - update udp_splice_map[V4][5000].ns_conn_ts with current time > + * - set udp_splice_to_ns[V4][5000].orig_sock to s > + * - update udp_splice_to_ns[V4][5000].ts with current time > * > * - reverse direction: 127.0.0.1:80 -> 127.0.0.1:5000 in namespace from > * connected socket s, having epoll reference: index = 5000, > * splice = UDP_BACK_TO_INIT > - * - if udp_splice_map[V4][5000].init_bound_sock: > - * - send to udp_splice_map[V4][5000].init_bound_sock, with destination > - * port 5000 > + * - if udp_splice_to_ns[V4][5000].orig_sock: > + * - send to udp_splice_to_ns[V4][5000].orig_sock, with destination port > + * 5000 > * - otherwise, discard > * > * - from namespace to init: > * > * - forward direction: 127.0.0.1:2000 -> 127.0.0.1:22 in namespace from bound > * socket s, with epoll reference: index = 22, splice = UDP_TO_INIT > - * - if udp4_splice_map[V4][2000].init_conn_sock: > - * - send packet to udp4_splice_map[2000].init_conn_sock > + * - if udp4_splice_to_init[V4][2000].target_sock: > + * - send packet to udp_splice_to_init[V4][2000].target_sock > * - otherwise: > - * - create new socket udp_splice_map[V4][2000].init_conn_sock > + * - create new socket udp_splice_to_init[V4][2000].target_sock > * - bind in init to 127.0.0.1:2000 > * - connect in init to 127.0.0.1:22 (note: this destination port > * might be remapped to another port instead) > * - add to epoll with reference: index = 2000, splice = UDP_BACK_TO_NS > - * - set udp_splice_map[V4][2000].ns_bound_sock to s > - * - update udp_splice_map[V4][2000].init_conn_ts with current time > + * - set udp_splice_to_init[V4][2000].orig_sock to s > + * - update udp_splice_to_init[V4][2000].ts with current time > * > * - reverse direction: 127.0.0.1:22 -> 127.0.0.1:2000 in init from connected > * socket s, having epoll reference: index = 2000, splice = UDP_BACK_TO_NS > - * - if udp_splice_map[V4][2000].ns_bound_sock: > - * - send to udp_splice_map[V4][2000].ns_bound_sock, with destination port > + * - if udp_splice_to_init[V4][2000].orig_sock: > + * - send to udp_splice_to_init[V4][2000].orig_sock, with destination port > * 2000 > * - otherwise, discard > */ > @@ -138,28 +138,26 @@ struct udp_tap_port { > }; > > /** > - * struct udp_splice_port - Source port tracking for traffic between namespaces > - * @ns_conn_sock: Socket connected in namespace for init source port > - * @init_conn_sock: Socket connected in init for namespace source port > - * @ns_conn_ts: Timestamp of activity for socket connected in namespace > - * @init_conn_ts: Timestamp of activity for socket connceted in init > - * @ns_bound_sock: Bound socket in namespace for this source port in init > - * @init_bound_sock: Bound socket in init for this source port in namespace > + * struct udp_splice_flow - Spliced "connection" > + * @orig_sock: Originating socket, bound to dest port in source ns of > + * originating datagram > + * @target_sock: Target socket, bound to source port of originating > + * datagram in dest ns, connected to dest port of > + * originating datagram in dest ns > + * @ts: Activity timestamp > */ > -struct udp_splice_port { > - int ns_conn_sock; > - int init_conn_sock; > - > - time_t ns_conn_ts; > - time_t init_conn_ts; > - > - int ns_bound_sock; > - int init_bound_sock; > +struct udp_splice_flow { > + int orig_sock; > + int target_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]; > -static struct udp_splice_port udp_splice_map [IP_VERSIONS][NUM_PORTS]; > + > +/* Spliced "connections" indexed by originating source port (host order) */ > +static struct udp_splice_flow udp_splice_to_ns [IP_VERSIONS][NUM_PORTS]; > +static struct udp_splice_flow udp_splice_to_init[IP_VERSIONS][NUM_PORTS]; > > enum udp_act_type { > UDP_ACT_TAP, > @@ -421,8 +419,17 @@ int udp_splice_connect(const struct ctx *c, int v6, int bound_sock, > .r.p.udp.udp = { .splice = splice, .v6 = v6, > .port = src } > }; > - struct udp_splice_port *sp = &udp_splice_map[v6 ? V6 : V4][src]; > + struct udp_splice_flow *flow; > int s; > + int act; ...and this should go before 'int s;'. -- Stefano