On Thu, Jan 12, 2023 at 07:09:25PM +0100, Stefano Brivio wrote:On Mon, 9 Jan 2023 11:50:24 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Hm, right.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 <david(a)gibson.dropbear.id.au> --- 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?Yeah, alright. I don't love this either, since just "namespace" and "init" are also kind of ambiguous without context. But, it's more consistent with the terminology in other places, so it will do for now. -- David Gibson | 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