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 2963D5A026B for ; Thu, 12 Jan 2023 19:09:35 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673546974; 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=iM/cBNdoTj1wZtXmm90fclDJs2oKcyRFteukRz0dtdE=; b=Vj6JLrEEGKZVS3IEGTdwXuR73453whp9/2AsTxAM8Zm/RacF7UJ2oROjqT0tAUb2x+g/u3 UMEr4GX2gBoFCa89aT35/nyMBT9R4AnlgGA7FsATRp0PpB4mn+FzebSY8pdZsuxuanAvqo 3JLRszoLYHxGN9TSLp2Xk8XOQpEiikc= 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-244-n51ZKRhHNWCQJ4bIVY76ow-1; Thu, 12 Jan 2023 13:09:28 -0500 X-MC-Unique: n51ZKRhHNWCQJ4bIVY76ow-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 14C303C0D18C; Thu, 12 Jan 2023 18:09:28 +0000 (UTC) Received: from maya.cloud.tilaa.com (ovpn-208-4.brq.redhat.com [10.40.208.4]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9F4101121314; Thu, 12 Jan 2023 18:09:27 +0000 (UTC) Date: Thu, 12 Jan 2023 19:09:25 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 3/5] tcp: Move socket pool declarations around Message-ID: <20230112190925.73f2c128@elisabeth> In-Reply-To: <20230109005026.24512-4-david@gibson.dropbear.id.au> References: <20230109005026.24512-1-david@gibson.dropbear.id.au> <20230109005026.24512-4-david@gibson.dropbear.id.au> Organization: Red Hat MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: XZRMHTGAG27ZQACCPOL5QOIATNM5BIMO X-Message-ID-Hash: XZRMHTGAG27ZQACCPOL5QOIATNM5BIMO 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: On Mon, 9 Jan 2023 11:50:24 +1100 David Gibson wrote: > tcp_splice.c has some explicit extern declarations to access the socket > pools. This is pretty dangerous - if we changed the type of these > variables in tcp.c, we'd have tcp.c and tcp_splice.c using the memory > differently with no compiler error. So, move the extern declarations to > tcp_conn.h so they're visible to both tcp.c and tcp_splice.c, but not the > rest of pasta. > > In fact the pools for the guest namespace are necessarily only used by > tcp_splice.c - we have no sockets on the guest side if we're not splicing. > So move those declarations - and the functions that deal exclusively with > them - to tcp_splice.c > > Signed-off-by: David Gibson > --- > tcp.c | 41 ++++------------------------------------- > tcp.h | 2 -- > tcp_conn.h | 10 ++++++++-- > tcp_splice.c | 50 +++++++++++++++++++++++++++++++++++++++++++------- > 4 files changed, 55 insertions(+), 48 deletions(-) > > diff --git a/tcp.c b/tcp.c > index 4da823c..19fd17b 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -370,8 +370,6 @@ struct tcp6_l2_head { /* For MSS6 macro: keep in sync with tcp6_l2_buf_t */ > #define FIN_TIMEOUT 60 > #define ACT_TIMEOUT 7200 > > -#define TCP_SOCK_POOL_TSH 16 /* Refill in ns if > x used */ > - > #define LOW_RTT_TABLE_SIZE 8 > #define LOW_RTT_THRESHOLD 10 /* us */ > > @@ -595,11 +593,9 @@ static inline struct tcp_tap_conn *conn_at_idx(int index) > /* Table for lookup from remote address, local port, remote port */ > static struct tcp_tap_conn *tc_hash[TCP_HASH_TABLE_SIZE]; > > -/* Pools for pre-opened sockets */ > +/* Pools for pre-opened sockets (init namespace) */ > int init_sock_pool4 [TCP_SOCK_POOL_SIZE]; > int init_sock_pool6 [TCP_SOCK_POOL_SIZE]; > -int ns_sock_pool4 [TCP_SOCK_POOL_SIZE]; > -int ns_sock_pool6 [TCP_SOCK_POOL_SIZE]; > > /** > * tcp_conn_epoll_events() - epoll events mask for given connection state > @@ -2988,7 +2984,7 @@ static int tcp_ns_socks_init(void *arg) > * @pool: Pool of sockets to refill > * @af: Address family to use > */ > -static void tcp_sock_refill_pool(const struct ctx *c, int pool[], int af) > +void tcp_sock_refill_pool(const struct ctx *c, int pool[], int af) > { > int i; > > @@ -3022,26 +3018,6 @@ static void tcp_sock_refill_init(const struct ctx *c) > tcp_sock_refill_pool(c, init_sock_pool6, AF_INET6); > } > > -/** > - * tcp_sock_refill_ns() - Refill pools of pre-opened sockets in namespace > - * @arg: Execution context cast to void * > - * > - * Return: 0 > - */ > -static int tcp_sock_refill_ns(void *arg) > -{ > - const struct ctx *c = (const struct ctx *)arg; > - > - ns_enter(c); > - > - if (c->ifi4) > - tcp_sock_refill_pool(c, ns_sock_pool4, AF_INET); > - if (c->ifi6) > - tcp_sock_refill_pool(c, ns_sock_pool6, AF_INET6); > - > - return 0; > -} > - > /** > * tcp_init() - Get initial sequence, hash secret, initialise per-socket data > * @c: Execution context > @@ -3090,8 +3066,6 @@ int tcp_init(struct ctx *c) > > memset(init_sock_pool4, 0xff, sizeof(init_sock_pool4)); > memset(init_sock_pool6, 0xff, sizeof(init_sock_pool6)); > - memset(ns_sock_pool4, 0xff, sizeof(ns_sock_pool4)); > - memset(ns_sock_pool6, 0xff, sizeof(ns_sock_pool6)); > memset(tcp_sock_init_ext, 0xff, sizeof(tcp_sock_init_ext)); > memset(tcp_sock_ns, 0xff, sizeof(tcp_sock_ns)); > > @@ -3101,8 +3075,6 @@ int tcp_init(struct ctx *c) > tcp_splice_init(c); > > NS_CALL(tcp_ns_socks_init, c); > - > - NS_CALL(tcp_sock_refill_ns, c); > } > > return 0; > @@ -3255,11 +3227,6 @@ void tcp_timer(struct ctx *c, const struct timespec *ts) > } > > tcp_sock_refill_init(c); > - if (c->mode == MODE_PASTA) { > - if ((c->ifi4 && ns_sock_pool4[TCP_SOCK_POOL_TSH] < 0) || > - (c->ifi6 && ns_sock_pool6[TCP_SOCK_POOL_TSH] < 0)) > - NS_CALL(tcp_sock_refill_ns, c); > - > - tcp_splice_pipe_refill(c); > - } > + if (c->mode == MODE_PASTA) > + tcp_splice_refill(c); > } > diff --git a/tcp.h b/tcp.h > index 739b451..236038f 100644 > --- a/tcp.h > +++ b/tcp.h > @@ -11,8 +11,6 @@ > #define TCP_CONN_INDEX_BITS 17 /* 128k */ > #define TCP_MAX_CONNS (1 << TCP_CONN_INDEX_BITS) > > -#define TCP_SOCK_POOL_SIZE 32 > - > struct ctx; > > void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events, > diff --git a/tcp_conn.h b/tcp_conn.h > index 70f4a7c..9951b0a 100644 > --- a/tcp_conn.h > +++ b/tcp_conn.h > @@ -182,11 +182,17 @@ union tcp_conn { > /* TCP connections */ > extern union tcp_conn tc[]; > > +/* Socket pools */ > +#define TCP_SOCK_POOL_SIZE 32 > + > +extern int init_sock_pool4 [TCP_SOCK_POOL_SIZE]; > +extern int init_sock_pool6 [TCP_SOCK_POOL_SIZE]; > + > void tcp_splice_conn_update(struct ctx *c, struct tcp_splice_conn *new); > void tcp_table_compact(struct ctx *c, union tcp_conn *hole); > void tcp_splice_destroy(struct ctx *c, union tcp_conn *conn_union); > void tcp_splice_timer(struct ctx *c, union tcp_conn *conn_union); > -void tcp_splice_pipe_refill(const struct ctx *c); > - > +void tcp_sock_refill_pool(const struct ctx *c, int pool[], int af); > +void tcp_splice_refill(const struct ctx *c); > > #endif /* TCP_CONN_H */ > diff --git a/tcp_splice.c b/tcp_splice.c > index 72b1672..688d776 100644 > --- a/tcp_splice.c > +++ b/tcp_splice.c > @@ -61,11 +61,11 @@ > #define TCP_SPLICE_CONN_PRESSURE 30 /* % of conn_count */ > #define TCP_SPLICE_FILE_PRESSURE 30 /* % of c->nofile */ > > -/* From tcp.c */ > -extern int init_sock_pool4 [TCP_SOCK_POOL_SIZE]; > -extern int init_sock_pool6 [TCP_SOCK_POOL_SIZE]; > -extern int ns_sock_pool4 [TCP_SOCK_POOL_SIZE]; > -extern int ns_sock_pool6 [TCP_SOCK_POOL_SIZE]; > +/* Pools for pre-opened sockets (guest namespace) */ What you mean by "guest namespace" is perfectly clear, but strictly speaking, that's not a thing, and I'm a bit concerned that it might cause confusion (especially with passt). What about using "init" (instead of "init namespace") above, and "namespace" here? -- Stefano