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 8D4105A026D for ; Thu, 8 Feb 2024 18:11:08 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707412267; 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=vR2yoScsus7+qGyGKwvBfAmjFMvwKEbmftX5nYZeTco=; b=ZJxwrSoWSz2+ehsUV3wO/mYP3aJ4e8Eeudh1n1BscsAha0cx/J8MFpAilgzev7wTE9uISo iXnoFc07U9ex9Qqu3n72eHEhwKx4HEA41SfqwJOhQKWW0PvQvipCpZqTLnxsnDcb4ZO2da FlpmvMCsKcKeksDYb0TPuuGgaFejLQ0= Received: from mail-ed1-f71.google.com (mail-ed1-f71.google.com [209.85.208.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-628-JIIUSRIEOx-y20XDr4_7Pg-1; Thu, 08 Feb 2024 12:11:05 -0500 X-MC-Unique: JIIUSRIEOx-y20XDr4_7Pg-1 Received: by mail-ed1-f71.google.com with SMTP id 4fb4d7f45d1cf-54554ea191bso64789a12.2 for ; Thu, 08 Feb 2024 09:11:05 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707412264; x=1708017064; 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=vR2yoScsus7+qGyGKwvBfAmjFMvwKEbmftX5nYZeTco=; b=BZFGKGhzBHyihBcjIX975QYHM8/MiT0yw1QVaVy8y4qhyfH3nL2fUv0ibxl/hV7Qd+ fm81Xx1d68fwF6juE2kR2UHf71zY/wCoj2yfnsLQlnwaXejnIAFV4eVTu/Y7XKNB8s9F jE2MIr1KVpI9atYg6//c0oj0TCDcqTDEe2uCc7Iyp8PrVL7GeaUYzpCXTT5a/0qFnwDc sE3ehDElFuWXBsG4me5qV2ydv8/lUNjXLzXjeGVn2JapIt1XQSpS4sxYtVQIxtT5EgKl F+rE7MgeEqohf+eTVUmVyhe6cGvoS4fU5p0GbCbIQt1bwTwsEst3n9WUhe1NwhQBlsJF U5mg== X-Gm-Message-State: AOJu0YzuwP7vxhRDiqqLsWXbtWSuflQj/tyIge2tQucB1qLA51RgliPn Sojnmd9CCbWgxpegIaGzVDllyz27Qe8b/fP/xLOeW3EW1//29zypimg7EtDWVOmDbepNax/Ul0R OCIqgYs2daqLSxwgQ8yO5uILhjj/eNgackyoIymXrHFu+XFjysA== X-Received: by 2002:aa7:d5c6:0:b0:55f:e574:4ea6 with SMTP id d6-20020aa7d5c6000000b0055fe5744ea6mr7387474eds.2.1707412264192; Thu, 08 Feb 2024 09:11:04 -0800 (PST) X-Google-Smtp-Source: AGHT+IFx04czdstCdM5oU6EAoNA/Zf9GU8IY9Jgv7mqmda6J2azRcREdA2Yj2MVvFR74J++dga2cEg== X-Received: by 2002:aa7:d5c6:0:b0:55f:e574:4ea6 with SMTP id d6-20020aa7d5c6000000b0055fe5744ea6mr7387457eds.2.1707412263802; Thu, 08 Feb 2024 09:11:03 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCUQSNgbNLJs6aS+NNqRGupyTJmtIYjgFY73P+/TlXcGbCEEYRhm1M/JKdiDj4a/fo9qC2Y1O4Qlqy4vsjBnpc1I8wIN Received: from maya.cloud.tilaa.com (maya.cloud.tilaa.com. [164.138.29.33]) by smtp.gmail.com with ESMTPSA id ek6-20020a056402370600b0056104738371sm863486edb.65.2024.02.08.09.11.03 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Feb 2024 09:11:03 -0800 (PST) Date: Thu, 8 Feb 2024 18:10:13 +0100 From: Stefano Brivio To: David Gibson Subject: Re: [PATCH 10/24] tcp: rename functions that manage buffers Message-ID: <20240208181013.1e8ec1fa@elisabeth> In-Reply-To: References: <20240202141151.3762941-1-lvivier@redhat.com> <20240202141151.3762941-11-lvivier@redhat.com> Organization: Red Hat X-Mailer: Claws Mail 4.1.1 (GTK 3.24.36; x86_64-pc-linux-gnu) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Message-ID-Hash: M6HHZGJUVGWISWH5YSPKGZCTK4HTQ6KE X-Message-ID-Hash: M6HHZGJUVGWISWH5YSPKGZCTK4HTQ6KE 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: Laurent Vivier , 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 Tue, 6 Feb 2024 12:48:30 +1100 David Gibson wrote: > On Fri, Feb 02, 2024 at 03:11:37PM +0100, Laurent Vivier wrote: > > To separate these functions from the ones specific to TCP management, > > we are going to move it to a new file, but before that update their names > > to reflect their role. > > > > Signed-off-by: Laurent Vivier > > --- > > passt.c | 2 +- > > tcp.c | 84 ++++++++++++++++++++++++++++----------------------------- > > tcp.h | 2 +- > > 3 files changed, 44 insertions(+), 44 deletions(-) > > > > diff --git a/passt.c b/passt.c > > index 44d3a0b0548c..10042a9b9789 100644 > > --- a/passt.c > > +++ b/passt.c > > @@ -164,7 +164,7 @@ static void timer_init(struct ctx *c, const struct timespec *now) > > */ > > void proto_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s) > > { > > - tcp_update_l2_buf(eth_d, eth_s); > > + tcp_buf_update_l2(eth_d, eth_s); > > udp_update_l2_buf(eth_d, eth_s); > > } > > > > diff --git a/tcp.c b/tcp.c > > index cdbceed65033..640209533772 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -383,7 +383,7 @@ struct tcp6_l2_head { /* For MSS6 macro: keep in sync with tcp6_l2_buf_t */ > > #define ACK (1 << 4) > > /* Flags for internal usage */ > > #define DUP_ACK (1 << 5) > > -#define ACK_IF_NEEDED 0 /* See tcp_send_flag() */ > > +#define ACK_IF_NEEDED 0 /* See tcp_buf_send_flag() */ > > > > #define OPT_EOL 0 > > #define OPT_NOP 1 > > @@ -960,11 +960,11 @@ static uint16_t tcp_update_check_tcp6(struct ipv6hdr *ip6h) > > } > > > > /** > > - * tcp_update_l2_buf() - Update L2 buffers with Ethernet and IPv4 addresses > > + * tcp_buf_update_l2() - Update L2 buffers with Ethernet and IPv4 addresses > > * @eth_d: Ethernet destination address, NULL if unchanged > > * @eth_s: Ethernet source address, NULL if unchanged > > */ > > -void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s) > > +void tcp_buf_update_l2(const unsigned char *eth_d, const unsigned char *eth_s) > > { > > int i; > > > > @@ -982,10 +982,10 @@ void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s) > > } > > > > /** > > - * tcp_sock4_iov_init() - Initialise scatter-gather L2 buffers for IPv4 sockets > > + * tcp_buf_sock4_iov_init() - Initialise scatter-gather L2 buffers for IPv4 sockets > > * @c: Execution context > > */ > > -static void tcp_sock4_iov_init(const struct ctx *c) > > +static void tcp_buf_sock4_iov_init(const struct ctx *c) > > { > > struct iphdr iph = L2_BUF_IP4_INIT(IPPROTO_TCP); > > struct iovec *iov; > > @@ -1014,10 +1014,10 @@ static void tcp_sock4_iov_init(const struct ctx *c) > > } > > > > /** > > - * tcp_sock6_iov_init() - Initialise scatter-gather L2 buffers for IPv6 sockets > > + * tcp_buf_sock6_iov_init() - Initialise scatter-gather L2 buffers for IPv6 sockets > > * @c: Execution context > > */ > > -static void tcp_sock6_iov_init(const struct ctx *c) > > +static void tcp_buf_sock6_iov_init(const struct ctx *c) > > { > > struct iovec *iov; > > int i; > > @@ -1277,10 +1277,10 @@ static void tcp_rst_do(struct ctx *c, struct tcp_tap_conn *conn); > > } while (0) > > > > /** > > - * tcp_l2_flags_buf_flush() - Send out buffers for segments with no data (flags) > > + * tcp_buf_l2_flags_flush() - Send out buffers for segments with no data (flags) > > * @c: Execution context > > */ > > -static void tcp_l2_flags_buf_flush(const struct ctx *c) > > +static void tcp_buf_l2_flags_flush(const struct ctx *c) > > { > > tap_send_frames(c, tcp6_l2_flags_iov, tcp6_l2_flags_buf_used); > > tcp6_l2_flags_buf_used = 0; > > @@ -1290,10 +1290,10 @@ static void tcp_l2_flags_buf_flush(const struct ctx *c) > > } > > > > /** > > - * tcp_l2_data_buf_flush() - Send out buffers for segments with data > > + * tcp_buf_l2_data_flush() - Send out buffers for segments with data > > * @c: Execution context > > */ > > -static void tcp_l2_data_buf_flush(const struct ctx *c) > > +static void tcp_buf_l2_data_flush(const struct ctx *c) > > { > > unsigned i; > > size_t m; > > @@ -1316,8 +1316,8 @@ static void tcp_l2_data_buf_flush(const struct ctx *c) > > /* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ > > void tcp_defer_handler(struct ctx *c) > > { > > - tcp_l2_flags_buf_flush(c); > > - tcp_l2_data_buf_flush(c); > > + tcp_buf_l2_flags_flush(c); > > + tcp_buf_l2_data_flush(c); > > } > > > > static void tcp_set_tcp_header(struct tcphdr *th, > > @@ -1629,7 +1629,7 @@ static int do_tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags, > > return 1; > > } > > > > -static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) > > +static int tcp_buf_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) > > The functions above could reasonably be said to be part of the buffer > management, but I'm not convinced on this one - it's primary purpose > is to, well, send a flag, so it uses the buffers, but I wouldn't > really say it manages them. Right, this and tcp_data_from_sock() below really implement TCP logic. I'd be happy if we could avoid this patch and 11/24, but I didn't reach the point where you need them, yet. > > { > > size_t optlen = 0; > > struct iovec *iov; > > @@ -1664,7 +1664,7 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) > > } > > > > if (tcp4_l2_flags_buf_used > ARRAY_SIZE(tcp4_l2_flags_buf) - 2) > > - tcp_l2_flags_buf_flush(c); > > + tcp_buf_l2_flags_flush(c); > > } else { > > struct tcp6_l2_flags_buf_t *b6; > > > > @@ -1688,7 +1688,7 @@ static int tcp_send_flag(struct ctx *c, struct tcp_tap_conn *conn, int flags) > > } > > > > if (tcp6_l2_flags_buf_used > ARRAY_SIZE(tcp6_l2_flags_buf) - 2) > > - tcp_l2_flags_buf_flush(c); > > + tcp_buf_l2_flags_flush(c); > > } > > > > return 0; > > @@ -1704,7 +1704,7 @@ static void tcp_rst_do(struct ctx *c, struct tcp_tap_conn *conn) > > if (conn->events == CLOSED) > > return; > > > > - if (!tcp_send_flag(c, conn, RST)) > > + if (!tcp_buf_send_flag(c, conn, RST)) > > conn_event(c, conn, CLOSED); > > } > > > > @@ -2024,7 +2024,7 @@ static void tcp_conn_from_tap(struct ctx *c, > > } else { > > tcp_get_sndbuf(conn); > > > > - if (tcp_send_flag(c, conn, SYN | ACK)) > > + if (tcp_buf_send_flag(c, conn, SYN | ACK)) > > return; > > > > conn_event(c, conn, TAP_SYN_ACK_SENT); > > @@ -2100,7 +2100,7 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, > > iov = tcp4_l2_iov + tcp4_l2_buf_used++; > > iov->iov_len = tap_iov_len(c, &b->taph, ip_len); > > if (tcp4_l2_buf_used > ARRAY_SIZE(tcp4_l2_buf) - 1) > > - tcp_l2_data_buf_flush(c); > > + tcp_buf_l2_data_flush(c); > > } else if (CONN_V6(conn)) { > > struct tcp6_l2_buf_t *b = &tcp6_l2_buf[tcp6_l2_buf_used]; > > > > @@ -2112,12 +2112,12 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, > > iov = tcp6_l2_iov + tcp6_l2_buf_used++; > > iov->iov_len = tap_iov_len(c, &b->taph, ip_len); > > if (tcp6_l2_buf_used > ARRAY_SIZE(tcp6_l2_buf) - 1) > > - tcp_l2_data_buf_flush(c); > > + tcp_buf_l2_data_flush(c); > > } > > } > > > > /** > > - * tcp_data_from_sock() - Handle new data from socket, queue to tap, in window > > + * tcp_buf_data_from_sock() - Handle new data from socket, queue to tap, in window > > Same with this one. > > > * @c: Execution context > > * @conn: Connection pointer > > * > > @@ -2125,7 +2125,7 @@ static void tcp_data_to_tap(const struct ctx *c, struct tcp_tap_conn *conn, > > * > > * #syscalls recvmsg > > */ > > -static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) > > +static int tcp_buf_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) > > { > > uint32_t wnd_scaled = conn->wnd_from_tap << conn->ws_from_tap; > > int fill_bufs, send_bufs = 0, last_len, iov_rem = 0; > > @@ -2169,7 +2169,7 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) > > > > if (( v4 && tcp4_l2_buf_used + fill_bufs > ARRAY_SIZE(tcp4_l2_buf)) || > > (!v4 && tcp6_l2_buf_used + fill_bufs > ARRAY_SIZE(tcp6_l2_buf))) { > > - tcp_l2_data_buf_flush(c); > > + tcp_buf_l2_data_flush(c); > > > > /* Silence Coverity CWE-125 false positive */ > > tcp4_l2_buf_used = tcp6_l2_buf_used = 0; > > @@ -2195,7 +2195,7 @@ static int tcp_data_from_sock(struct ctx *c, struct tcp_tap_conn *conn) > > > > if (!len) { > > if ((conn->events & (SOCK_FIN_RCVD | TAP_FIN_SENT)) == SOCK_FIN_RCVD) { > > - if ((ret = tcp_send_flag(c, conn, FIN | ACK))) { > > + if ((ret = tcp_buf_send_flag(c, conn, FIN | ACK))) { > > tcp_rst(c, conn); > > return ret; > > } > > @@ -2378,7 +2378,7 @@ static int tcp_data_from_tap(struct ctx *c, struct tcp_tap_conn *conn, > > max_ack_seq, conn->seq_to_tap); > > conn->seq_ack_from_tap = max_ack_seq; > > conn->seq_to_tap = max_ack_seq; > > - tcp_data_from_sock(c, conn); > > + tcp_buf_data_from_sock(c, conn); > > } > > > > if (!iov_i) > > @@ -2394,14 +2394,14 @@ eintr: > > * Then swiftly looked away and left. > > */ > > conn->seq_from_tap = seq_from_tap; > > - tcp_send_flag(c, conn, ACK); > > + tcp_buf_send_flag(c, conn, ACK); > > } > > > > if (errno == EINTR) > > goto eintr; > > > > if (errno == EAGAIN || errno == EWOULDBLOCK) { > > - tcp_send_flag(c, conn, ACK_IF_NEEDED); > > + tcp_buf_send_flag(c, conn, ACK_IF_NEEDED); > > return p->count - idx; > > > > } > > @@ -2411,7 +2411,7 @@ eintr: > > if (n < (int)(seq_from_tap - conn->seq_from_tap)) { > > partial_send = 1; > > conn->seq_from_tap += n; > > - tcp_send_flag(c, conn, ACK_IF_NEEDED); > > + tcp_buf_send_flag(c, conn, ACK_IF_NEEDED); > > } else { > > conn->seq_from_tap += n; > > } > > @@ -2424,7 +2424,7 @@ out: > > */ > > if (conn->seq_dup_ack_approx != (conn->seq_from_tap & 0xff)) { > > conn->seq_dup_ack_approx = conn->seq_from_tap & 0xff; > > - tcp_send_flag(c, conn, DUP_ACK); > > + tcp_buf_send_flag(c, conn, DUP_ACK); > > } > > return p->count - idx; > > } > > @@ -2438,7 +2438,7 @@ out: > > > > conn_event(c, conn, TAP_FIN_RCVD); > > } else { > > - tcp_send_flag(c, conn, ACK_IF_NEEDED); > > + tcp_buf_send_flag(c, conn, ACK_IF_NEEDED); > > } > > > > return p->count - idx; > > @@ -2474,8 +2474,8 @@ static void tcp_conn_from_sock_finish(struct ctx *c, struct tcp_tap_conn *conn, > > /* The client might have sent data already, which we didn't > > * dequeue waiting for SYN,ACK from tap -- check now. > > */ > > - tcp_data_from_sock(c, conn); > > - tcp_send_flag(c, conn, ACK); > > + tcp_buf_data_from_sock(c, conn); > > + tcp_buf_send_flag(c, conn, ACK); > > } > > > > /** > > @@ -2555,7 +2555,7 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, int af, > > conn->seq_from_tap++; > > > > shutdown(conn->sock, SHUT_WR); > > - tcp_send_flag(c, conn, ACK); > > + tcp_buf_send_flag(c, conn, ACK); > > conn_event(c, conn, SOCK_FIN_SENT); > > > > return 1; > > @@ -2566,7 +2566,7 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, int af, > > > > tcp_tap_window_update(conn, ntohs(th->window)); > > > > - tcp_data_from_sock(c, conn); > > + tcp_buf_data_from_sock(c, conn); > > > > if (p->count - idx == 1) > > return 1; > > @@ -2596,7 +2596,7 @@ int tcp_tap_handler(struct ctx *c, uint8_t pif, int af, > > if ((conn->events & TAP_FIN_RCVD) && !(conn->events & SOCK_FIN_SENT)) { > > shutdown(conn->sock, SHUT_WR); > > conn_event(c, conn, SOCK_FIN_SENT); > > - tcp_send_flag(c, conn, ACK); > > + tcp_buf_send_flag(c, conn, ACK); > > ack_due = 0; > > } > > > > @@ -2630,7 +2630,7 @@ static void tcp_connect_finish(struct ctx *c, struct tcp_tap_conn *conn) > > return; > > } > > > > - if (tcp_send_flag(c, conn, SYN | ACK)) > > + if (tcp_buf_send_flag(c, conn, SYN | ACK)) > > return; > > > > conn_event(c, conn, TAP_SYN_ACK_SENT); > > @@ -2698,7 +2698,7 @@ static void tcp_tap_conn_from_sock(struct ctx *c, > > > > conn->wnd_from_tap = WINDOW_DEFAULT; > > > > - tcp_send_flag(c, conn, SYN); > > + tcp_buf_send_flag(c, conn, SYN); > > conn_flag(c, conn, ACK_FROM_TAP_DUE); > > > > tcp_get_sndbuf(conn); > > @@ -2762,7 +2762,7 @@ void tcp_timer_handler(struct ctx *c, union epoll_ref ref) > > return; > > > > if (conn->flags & ACK_TO_TAP_DUE) { > > - tcp_send_flag(c, conn, ACK_IF_NEEDED); > > + tcp_buf_send_flag(c, conn, ACK_IF_NEEDED); > > tcp_timer_ctl(c, conn); > > } else if (conn->flags & ACK_FROM_TAP_DUE) { > > if (!(conn->events & ESTABLISHED)) { > > @@ -2778,7 +2778,7 @@ void tcp_timer_handler(struct ctx *c, union epoll_ref ref) > > flow_dbg(conn, "ACK timeout, retry"); > > conn->retrans++; > > conn->seq_to_tap = conn->seq_ack_from_tap; > > - tcp_data_from_sock(c, conn); > > + tcp_buf_data_from_sock(c, conn); > > tcp_timer_ctl(c, conn); > > } > > } else { > > @@ -2833,7 +2833,7 @@ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events) > > conn_event(c, conn, SOCK_FIN_RCVD); > > > > if (events & EPOLLIN) > > - tcp_data_from_sock(c, conn); > > + tcp_buf_data_from_sock(c, conn); > > > > if (events & EPOLLOUT) > > tcp_update_seqack_wnd(c, conn, 0, NULL); > > @@ -3058,10 +3058,10 @@ int tcp_init(struct ctx *c) > > tc_hash[b] = FLOW_SIDX_NONE; > > > > if (c->ifi4) > > - tcp_sock4_iov_init(c); > > + tcp_buf_sock4_iov_init(c); > > > > if (c->ifi6) > > - tcp_sock6_iov_init(c); > > + tcp_buf_sock6_iov_init(c); > > > > memset(init_sock_pool4, 0xff, sizeof(init_sock_pool4)); > > memset(init_sock_pool6, 0xff, sizeof(init_sock_pool6)); > > diff --git a/tcp.h b/tcp.h > > index b9f546d31002..e7dbcfa2ddbd 100644 > > --- a/tcp.h > > +++ b/tcp.h > > @@ -23,7 +23,7 @@ int tcp_init(struct ctx *c); > > void tcp_timer(struct ctx *c, const struct timespec *now); > > void tcp_defer_handler(struct ctx *c); > > > > -void tcp_update_l2_buf(const unsigned char *eth_d, const unsigned char *eth_s); > > +void tcp_buf_update_l2(const unsigned char *eth_d, const unsigned char *eth_s); > > > > /** > > * union tcp_epoll_ref - epoll reference portion for TCP connections > -- Stefano