From: David Gibson <david@gibson.dropbear.id.au>
To: passt-dev@passt.top, Stefano Brivio <sbrivio@redhat.com>
Cc: David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 4/6] tcp_splice: Use parameterised macros for per-side event/flag bits
Date: Wed, 17 Jul 2024 14:52:21 +1000 [thread overview]
Message-ID: <20240717045223.2309975-5-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <20240717045223.2309975-1-david@gibson.dropbear.id.au>
Both the events and flags fields in tcp_splice_conn have several bits
which are per-side, e.g. OUT_WAIT_0 for side 0 and OUT_WAIT_1 for side 1.
This necessitates some rather awkward ternary expressions when we need
to get the relevant bit for a particular side.
Simplify this by using a parameterised macro for the bit values. This
needs a ternary expression inside the macros, but makes the places we use
it substantially clearer.
That simplification in turn allows us to use a loop across each side to
implement several things which are currently open coded to do equivalent
things for each side in turn.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
tcp_conn.h | 15 +++++--------
tcp_splice.c | 60 +++++++++++++++++++++++++---------------------------
2 files changed, 34 insertions(+), 41 deletions(-)
diff --git a/tcp_conn.h b/tcp_conn.h
index 5f8c8fb6..f80ef67b 100644
--- a/tcp_conn.h
+++ b/tcp_conn.h
@@ -129,19 +129,14 @@ struct tcp_splice_conn {
#define SPLICE_CLOSED 0
#define SPLICE_CONNECT BIT(0)
#define SPLICE_ESTABLISHED BIT(1)
-#define OUT_WAIT_0 BIT(2)
-#define OUT_WAIT_1 BIT(3)
-#define FIN_RCVD_0 BIT(4)
-#define FIN_RCVD_1 BIT(5)
-#define FIN_SENT_0 BIT(6)
-#define FIN_SENT_1 BIT(7)
+#define OUT_WAIT(sidei_) ((sidei_) ? BIT(3) : BIT(2))
+#define FIN_RCVD(sidei_) ((sidei_) ? BIT(5) : BIT(4))
+#define FIN_SENT(sidei_) ((sidei_) ? BIT(7) : BIT(6))
uint8_t flags;
#define SPLICE_V6 BIT(0)
-#define RCVLOWAT_SET_0 BIT(1)
-#define RCVLOWAT_SET_1 BIT(2)
-#define RCVLOWAT_ACT_0 BIT(3)
-#define RCVLOWAT_ACT_1 BIT(4)
+#define RCVLOWAT_SET(sidei_) ((sidei_) ? BIT(2) : BIT(1))
+#define RCVLOWAT_ACT(sidei_) ((sidei_) ? BIT(4) : BIT(3))
#define CLOSING BIT(5)
uint32_t read[SIDES];
diff --git a/tcp_splice.c b/tcp_splice.c
index 5a9325b1..ffddc853 100644
--- a/tcp_splice.c
+++ b/tcp_splice.c
@@ -119,19 +119,22 @@ static struct tcp_splice_conn *conn_at_sidx(flow_sidx_t sidx)
static void tcp_splice_conn_epoll_events(uint16_t events,
struct epoll_event ev[])
{
- ev[0].events = ev[1].events = 0;
+ unsigned sidei;
+
+ flow_foreach_sidei(sidei)
+ ev[sidei].events = 0;
if (events & SPLICE_ESTABLISHED) {
- if (!(events & FIN_SENT_1))
- ev[0].events = EPOLLIN | EPOLLRDHUP;
- if (!(events & FIN_SENT_0))
- ev[1].events = EPOLLIN | EPOLLRDHUP;
+ flow_foreach_sidei(sidei) {
+ if (!(events & FIN_SENT(!sidei)))
+ ev[sidei].events = EPOLLIN | EPOLLRDHUP;
+ }
} else if (events & SPLICE_CONNECT) {
ev[1].events = EPOLLOUT;
}
- ev[0].events |= (events & OUT_WAIT_0) ? EPOLLOUT : 0;
- ev[1].events |= (events & OUT_WAIT_1) ? EPOLLOUT : 0;
+ flow_foreach_sidei(sidei)
+ ev[sidei].events |= (events & OUT_WAIT(sidei)) ? EPOLLOUT : 0;
}
/**
@@ -553,21 +556,21 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
if (events & EPOLLOUT) {
fromsidei = !evsidei;
- conn_event(c, conn, evsidei == 0 ? ~OUT_WAIT_0 : ~OUT_WAIT_1);
+ conn_event(c, conn, ~OUT_WAIT(evsidei));
} else {
fromsidei = evsidei;
}
if (events & EPOLLRDHUP)
/* For side 0 this is fake, but implied */
- conn_event(c, conn, evsidei == 0 ? FIN_RCVD_0 : FIN_RCVD_1);
+ conn_event(c, conn, FIN_RCVD(evsidei));
swap:
eof = 0;
never_read = 1;
- lowat_set_flag = fromsidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1;
- lowat_act_flag = fromsidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1;
+ lowat_set_flag = RCVLOWAT_SET(fromsidei);
+ lowat_act_flag = RCVLOWAT_ACT(fromsidei);
while (1) {
ssize_t readlen, to_write = 0, written;
@@ -644,8 +647,7 @@ eintr:
if (conn->read[fromsidei] == conn->written[fromsidei])
break;
- conn_event(c, conn,
- fromsidei == 0 ? OUT_WAIT_1 : OUT_WAIT_0);
+ conn_event(c, conn, OUT_WAIT(fromsidei));
break;
}
@@ -661,21 +663,19 @@ eintr:
break;
}
- if ((conn->events & FIN_RCVD_0) && !(conn->events & FIN_SENT_1)) {
- if (conn->read[fromsidei] == conn->written[fromsidei] && eof) {
- shutdown(conn->s[1], SHUT_WR);
- conn_event(c, conn, FIN_SENT_1);
- }
- }
+ if (conn->read[fromsidei] == conn->written[fromsidei] && eof) {
+ unsigned sidei;
- if ((conn->events & FIN_RCVD_1) && !(conn->events & FIN_SENT_0)) {
- if (conn->read[fromsidei] == conn->written[fromsidei] && eof) {
- shutdown(conn->s[0], SHUT_WR);
- conn_event(c, conn, FIN_SENT_0);
+ flow_foreach_sidei(sidei) {
+ if ((conn->events & FIN_RCVD(sidei)) &&
+ !(conn->events & FIN_SENT(!sidei))) {
+ shutdown(conn->s[!sidei], SHUT_WR);
+ conn_event(c, conn, FIN_SENT(!sidei));
+ }
}
}
- if (CONN_HAS(conn, FIN_SENT_0 | FIN_SENT_1))
+ if (CONN_HAS(conn, FIN_SENT(0) | FIN_SENT(1)))
goto close;
if ((events & (EPOLLIN | EPOLLOUT)) == (EPOLLIN | EPOLLOUT)) {
@@ -821,19 +821,17 @@ void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn)
ASSERT(!(conn->flags & CLOSING));
flow_foreach_sidei(sidei) {
- uint8_t set = sidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1;
- uint8_t act = sidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1;
-
- if ((conn->flags & set) && !(conn->flags & act)) {
+ if ((conn->flags & RCVLOWAT_SET(sidei)) &&
+ !(conn->flags & RCVLOWAT_ACT(sidei))) {
if (setsockopt(conn->s[sidei], SOL_SOCKET, SO_RCVLOWAT,
&((int){ 1 }), sizeof(int))) {
flow_trace(conn, "can't set SO_RCVLOWAT on %d",
conn->s[sidei]);
}
- conn_flag(c, conn, ~set);
+ conn_flag(c, conn, ~RCVLOWAT_SET(sidei));
}
}
- conn_flag(c, conn, ~RCVLOWAT_ACT_0);
- conn_flag(c, conn, ~RCVLOWAT_ACT_1);
+ flow_foreach_sidei(sidei)
+ conn_flag(c, conn, ~RCVLOWAT_ACT(sidei));
}
--
@@ -119,19 +119,22 @@ static struct tcp_splice_conn *conn_at_sidx(flow_sidx_t sidx)
static void tcp_splice_conn_epoll_events(uint16_t events,
struct epoll_event ev[])
{
- ev[0].events = ev[1].events = 0;
+ unsigned sidei;
+
+ flow_foreach_sidei(sidei)
+ ev[sidei].events = 0;
if (events & SPLICE_ESTABLISHED) {
- if (!(events & FIN_SENT_1))
- ev[0].events = EPOLLIN | EPOLLRDHUP;
- if (!(events & FIN_SENT_0))
- ev[1].events = EPOLLIN | EPOLLRDHUP;
+ flow_foreach_sidei(sidei) {
+ if (!(events & FIN_SENT(!sidei)))
+ ev[sidei].events = EPOLLIN | EPOLLRDHUP;
+ }
} else if (events & SPLICE_CONNECT) {
ev[1].events = EPOLLOUT;
}
- ev[0].events |= (events & OUT_WAIT_0) ? EPOLLOUT : 0;
- ev[1].events |= (events & OUT_WAIT_1) ? EPOLLOUT : 0;
+ flow_foreach_sidei(sidei)
+ ev[sidei].events |= (events & OUT_WAIT(sidei)) ? EPOLLOUT : 0;
}
/**
@@ -553,21 +556,21 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
if (events & EPOLLOUT) {
fromsidei = !evsidei;
- conn_event(c, conn, evsidei == 0 ? ~OUT_WAIT_0 : ~OUT_WAIT_1);
+ conn_event(c, conn, ~OUT_WAIT(evsidei));
} else {
fromsidei = evsidei;
}
if (events & EPOLLRDHUP)
/* For side 0 this is fake, but implied */
- conn_event(c, conn, evsidei == 0 ? FIN_RCVD_0 : FIN_RCVD_1);
+ conn_event(c, conn, FIN_RCVD(evsidei));
swap:
eof = 0;
never_read = 1;
- lowat_set_flag = fromsidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1;
- lowat_act_flag = fromsidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1;
+ lowat_set_flag = RCVLOWAT_SET(fromsidei);
+ lowat_act_flag = RCVLOWAT_ACT(fromsidei);
while (1) {
ssize_t readlen, to_write = 0, written;
@@ -644,8 +647,7 @@ eintr:
if (conn->read[fromsidei] == conn->written[fromsidei])
break;
- conn_event(c, conn,
- fromsidei == 0 ? OUT_WAIT_1 : OUT_WAIT_0);
+ conn_event(c, conn, OUT_WAIT(fromsidei));
break;
}
@@ -661,21 +663,19 @@ eintr:
break;
}
- if ((conn->events & FIN_RCVD_0) && !(conn->events & FIN_SENT_1)) {
- if (conn->read[fromsidei] == conn->written[fromsidei] && eof) {
- shutdown(conn->s[1], SHUT_WR);
- conn_event(c, conn, FIN_SENT_1);
- }
- }
+ if (conn->read[fromsidei] == conn->written[fromsidei] && eof) {
+ unsigned sidei;
- if ((conn->events & FIN_RCVD_1) && !(conn->events & FIN_SENT_0)) {
- if (conn->read[fromsidei] == conn->written[fromsidei] && eof) {
- shutdown(conn->s[0], SHUT_WR);
- conn_event(c, conn, FIN_SENT_0);
+ flow_foreach_sidei(sidei) {
+ if ((conn->events & FIN_RCVD(sidei)) &&
+ !(conn->events & FIN_SENT(!sidei))) {
+ shutdown(conn->s[!sidei], SHUT_WR);
+ conn_event(c, conn, FIN_SENT(!sidei));
+ }
}
}
- if (CONN_HAS(conn, FIN_SENT_0 | FIN_SENT_1))
+ if (CONN_HAS(conn, FIN_SENT(0) | FIN_SENT(1)))
goto close;
if ((events & (EPOLLIN | EPOLLOUT)) == (EPOLLIN | EPOLLOUT)) {
@@ -821,19 +821,17 @@ void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn)
ASSERT(!(conn->flags & CLOSING));
flow_foreach_sidei(sidei) {
- uint8_t set = sidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1;
- uint8_t act = sidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1;
-
- if ((conn->flags & set) && !(conn->flags & act)) {
+ if ((conn->flags & RCVLOWAT_SET(sidei)) &&
+ !(conn->flags & RCVLOWAT_ACT(sidei))) {
if (setsockopt(conn->s[sidei], SOL_SOCKET, SO_RCVLOWAT,
&((int){ 1 }), sizeof(int))) {
flow_trace(conn, "can't set SO_RCVLOWAT on %d",
conn->s[sidei]);
}
- conn_flag(c, conn, ~set);
+ conn_flag(c, conn, ~RCVLOWAT_SET(sidei));
}
}
- conn_flag(c, conn, ~RCVLOWAT_ACT_0);
- conn_flag(c, conn, ~RCVLOWAT_ACT_1);
+ flow_foreach_sidei(sidei)
+ conn_flag(c, conn, ~RCVLOWAT_ACT(sidei));
}
--
2.45.2
next prev parent reply other threads:[~2024-07-17 4:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-17 4:52 [PATCH 0/6] Of course there are more flow table preliminaries David Gibson
2024-07-17 4:52 ` [PATCH 1/6] flow, icmp, tcp: Clean up helpers for getting flow from index David Gibson
2024-07-17 4:52 ` [PATCH 2/6] flow, tcp_splice: Prefer 'sidei' for variables referring to side index David Gibson
2024-07-17 5:20 ` Stefano Brivio
2024-07-17 5:28 ` David Gibson
2024-07-17 5:50 ` David Gibson
2024-07-17 7:20 ` Stefano Brivio
2024-07-17 7:27 ` David Gibson
2024-07-17 4:52 ` [PATCH 3/6] flow: Introduce flow_foreach_sidei() macro David Gibson
2024-07-17 4:52 ` David Gibson [this message]
2024-07-17 4:52 ` [PATCH 5/6] doc: Test behaviour of closing duplicate UDP sockets David Gibson
2024-07-17 4:52 ` [PATCH 6/6] doc: Extend zero-recv test with methods using msghdr David Gibson
2024-07-17 15:06 ` Stefano Brivio
2024-07-17 15:07 ` [PATCH 0/6] Of course there are more flow table preliminaries Stefano Brivio
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240717045223.2309975-5-david@gibson.dropbear.id.au \
--to=david@gibson.dropbear.id.au \
--cc=passt-dev@passt.top \
--cc=sbrivio@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).