From 71d7985188a7ef6e6a82efea0c87aa4d50ff8afa Mon Sep 17 00:00:00 2001 From: David Gibson Date: Wed, 17 Jul 2024 14:52:19 +1000 Subject: [PATCH] flow, tcp_splice: Prefer 'sidei' for variables referring to side index In various places we have variables named 'side' or similar which always have the value 0 or 1 (INISIDE or TGTSIDE). Given a flow, this refers to a specific side of it. Upcoming flow table work will make it more useful for "side" to refer to a specific side of a specific flow. To make things less confusing then, prefer the name term "side index" and name 'sidei' for variables with just the 0 or 1 value. Signed-off-by: David Gibson [sbrivio: Fixed minor detail in comment to struct flow_common] Signed-off-by: Stefano Brivio --- flow.h | 18 +++++------ flow_table.h | 18 +++++------ tcp_splice.c | 89 ++++++++++++++++++++++++++-------------------------- 3 files changed, 63 insertions(+), 62 deletions(-) diff --git a/flow.h b/flow.h index d1f49c6..b518904 100644 --- a/flow.h +++ b/flow.h @@ -132,8 +132,8 @@ extern const uint8_t flow_proto[]; #define SIDES 2 -#define INISIDE 0 /* Initiating side */ -#define TGTSIDE 1 /* Target side */ +#define INISIDE 0 /* Initiating side index */ +#define TGTSIDE 1 /* Target side index */ /** * struct flow_common - Common fields for packet flows @@ -164,17 +164,17 @@ struct flow_common { /** * struct flow_sidx - ID for one side of a specific flow - * @side: Side referenced (0 or 1) - * @flow: Index of flow referenced + * @sidei: Index of side referenced (0 or 1) + * @flowi: Index of flow referenced */ typedef struct flow_sidx { - unsigned side :1; - unsigned flow :FLOW_INDEX_BITS; + unsigned sidei :1; + unsigned flowi :FLOW_INDEX_BITS; } flow_sidx_t; static_assert(sizeof(flow_sidx_t) <= sizeof(uint32_t), "flow_sidx_t must fit within 32 bits"); -#define FLOW_SIDX_NONE ((flow_sidx_t){ .flow = FLOW_MAX }) +#define FLOW_SIDX_NONE ((flow_sidx_t){ .flowi = FLOW_MAX }) /** * flow_sidx_valid() - Test if a sidx is valid @@ -184,7 +184,7 @@ static_assert(sizeof(flow_sidx_t) <= sizeof(uint32_t), */ static inline bool flow_sidx_valid(flow_sidx_t sidx) { - return sidx.flow < FLOW_MAX; + return sidx.flowi < FLOW_MAX; } /** @@ -195,7 +195,7 @@ static inline bool flow_sidx_valid(flow_sidx_t sidx) */ static inline bool flow_sidx_eq(flow_sidx_t a, flow_sidx_t b) { - return (a.flow == b.flow) && (a.side == b.side); + return (a.flowi == b.flowi) && (a.sidei == b.sidei); } union flow; diff --git a/flow_table.h b/flow_table.h index ab73e44..8a5f8f2 100644 --- a/flow_table.h +++ b/flow_table.h @@ -75,7 +75,7 @@ static inline union flow *flow_at_sidx(flow_sidx_t sidx) { if (!flow_sidx_valid(sidx)) return NULL; - return FLOW(sidx.flow); + return FLOW(sidx.flowi); } /** pif_at_sidx() - Interface for a given flow and side @@ -89,34 +89,34 @@ static inline uint8_t pif_at_sidx(flow_sidx_t sidx) if (!flow) return PIF_NONE; - return flow->f.pif[sidx.side]; + return flow->f.pif[sidx.sidei]; } /** flow_sidx() - Index of one side of a flow from common structure * @f: Common flow fields pointer - * @side: Which side to refer to (0 or 1) + * @sidei: Which side to refer to (0 or 1) * * Return: index of @f and @side in the flow table */ static inline flow_sidx_t flow_sidx(const struct flow_common *f, - int side) + unsigned sidei) { /* cppcheck-suppress [knownConditionTrueFalse, unmatchedSuppression] */ - ASSERT(side == !!side); + ASSERT(sidei == !!sidei); return (flow_sidx_t){ - .side = side, - .flow = flow_idx(f), + .sidei = sidei, + .flowi = flow_idx(f), }; } /** FLOW_SIDX() - Find the index of one side of a flow * @f_: Flow pointer, either union flow * or protocol specific - * @side: Which side to index (0 or 1) + * @sidei: Which side to index (0 or 1) * * Return: index of @f and @side in the flow table */ -#define FLOW_SIDX(f_, side) (flow_sidx(&(f_)->f, (side))) +#define FLOW_SIDX(f_, sidei) (flow_sidx(&(f_)->f, (sidei))) union flow *flow_alloc(void); void flow_alloc_cancel(union flow *flow); diff --git a/tcp_splice.c b/tcp_splice.c index 9fc5656..8bc68a1 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -258,25 +258,25 @@ static void conn_event_do(const struct ctx *c, struct tcp_splice_conn *conn, */ bool tcp_splice_flow_defer(struct tcp_splice_conn *conn) { - unsigned side; + unsigned sidei; if (!(conn->flags & CLOSING)) return false; - for (side = 0; side < SIDES; side++) { + for (sidei = 0; sidei < SIDES; sidei++) { /* Flushing might need to block: don't recycle them. */ - if (conn->pipe[side][0] >= 0) { - close(conn->pipe[side][0]); - close(conn->pipe[side][1]); - conn->pipe[side][0] = conn->pipe[side][1] = -1; + if (conn->pipe[sidei][0] >= 0) { + close(conn->pipe[sidei][0]); + close(conn->pipe[sidei][1]); + conn->pipe[sidei][0] = conn->pipe[sidei][1] = -1; } - if (conn->s[side] >= 0) { - close(conn->s[side]); - conn->s[side] = -1; + if (conn->s[sidei] >= 0) { + close(conn->s[sidei]); + conn->s[sidei] = -1; } - conn->read[side] = conn->written[side] = 0; + conn->read[sidei] = conn->written[sidei] = 0; } conn->events = SPLICE_CLOSED; @@ -296,33 +296,33 @@ bool tcp_splice_flow_defer(struct tcp_splice_conn *conn) static int tcp_splice_connect_finish(const struct ctx *c, struct tcp_splice_conn *conn) { - unsigned side; + unsigned sidei; int i = 0; - for (side = 0; side < SIDES; side++) { + for (sidei = 0; sidei < SIDES; sidei++) { for (; i < TCP_SPLICE_PIPE_POOL_SIZE; i++) { if (splice_pipe_pool[i][0] >= 0) { - SWAP(conn->pipe[side][0], + SWAP(conn->pipe[sidei][0], splice_pipe_pool[i][0]); - SWAP(conn->pipe[side][1], + SWAP(conn->pipe[sidei][1], splice_pipe_pool[i][1]); break; } } - if (conn->pipe[side][0] < 0) { - if (pipe2(conn->pipe[side], O_NONBLOCK | O_CLOEXEC)) { + if (conn->pipe[sidei][0] < 0) { + if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) { flow_err(conn, "cannot create %d->%d pipe: %s", - side, !side, strerror(errno)); + sidei, !sidei, strerror(errno)); conn_flag(c, conn, CLOSING); return -EIO; } - if (fcntl(conn->pipe[side][0], F_SETPIPE_SZ, + if (fcntl(conn->pipe[sidei][0], F_SETPIPE_SZ, c->tcp.pipe_size)) { flow_trace(conn, "cannot set %d->%d pipe size to %zu", - side, !side, c->tcp.pipe_size); + sidei, !sidei, c->tcp.pipe_size); } } } @@ -520,7 +520,7 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events) { struct tcp_splice_conn *conn = conn_at_sidx(ref.flowside); - unsigned side = ref.flowside.side, fromside; + unsigned evsidei = ref.flowside.sidei, fromsidei; uint8_t lowat_set_flag, lowat_act_flag; int eof, never_read; @@ -552,30 +552,31 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref, } if (events & EPOLLOUT) { - fromside = !side; - conn_event(c, conn, side == 0 ? ~OUT_WAIT_0 : ~OUT_WAIT_1); + fromsidei = !evsidei; + conn_event(c, conn, evsidei == 0 ? ~OUT_WAIT_0 : ~OUT_WAIT_1); } else { - fromside = side; + fromsidei = evsidei; } if (events & EPOLLRDHUP) /* For side 0 this is fake, but implied */ - conn_event(c, conn, side == 0 ? FIN_RCVD_0 : FIN_RCVD_1); + conn_event(c, conn, evsidei == 0 ? FIN_RCVD_0 : FIN_RCVD_1); swap: eof = 0; never_read = 1; - lowat_set_flag = fromside == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1; - lowat_act_flag = fromside == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1; + lowat_set_flag = fromsidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1; + lowat_act_flag = fromsidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1; while (1) { ssize_t readlen, to_write = 0, written; int more = 0; retry: - readlen = splice(conn->s[fromside], NULL, - conn->pipe[fromside][1], NULL, c->tcp.pipe_size, + readlen = splice(conn->s[fromsidei], NULL, + conn->pipe[fromsidei][1], NULL, + c->tcp.pipe_size, SPLICE_F_MOVE | SPLICE_F_NONBLOCK); flow_trace(conn, "%zi from read-side call", readlen); if (readlen < 0) { @@ -600,8 +601,8 @@ retry: } eintr: - written = splice(conn->pipe[fromside][0], NULL, - conn->s[!fromside], NULL, to_write, + written = splice(conn->pipe[fromsidei][0], NULL, + conn->s[!fromsidei], NULL, to_write, SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); flow_trace(conn, "%zi from write-side call (passed %zi)", written, to_write); @@ -615,7 +616,7 @@ eintr: readlen > (long)c->tcp.pipe_size / 10) { int lowat = c->tcp.pipe_size / 4; - if (setsockopt(conn->s[fromside], SOL_SOCKET, + if (setsockopt(conn->s[fromsidei], SOL_SOCKET, SO_RCVLOWAT, &lowat, sizeof(lowat))) { flow_trace(conn, @@ -630,8 +631,8 @@ eintr: break; } - conn->read[fromside] += readlen > 0 ? readlen : 0; - conn->written[fromside] += written > 0 ? written : 0; + conn->read[fromsidei] += readlen > 0 ? readlen : 0; + conn->written[fromsidei] += written > 0 ? written : 0; if (written < 0) { if (errno == EINTR) @@ -640,11 +641,11 @@ eintr: if (errno != EAGAIN) goto close; - if (conn->read[fromside] == conn->written[fromside]) + if (conn->read[fromsidei] == conn->written[fromsidei]) break; conn_event(c, conn, - fromside == 0 ? OUT_WAIT_1 : OUT_WAIT_0); + fromsidei == 0 ? OUT_WAIT_1 : OUT_WAIT_0); break; } @@ -661,14 +662,14 @@ eintr: } if ((conn->events & FIN_RCVD_0) && !(conn->events & FIN_SENT_1)) { - if (conn->read[fromside] == conn->written[fromside] && eof) { + if (conn->read[fromsidei] == conn->written[fromsidei] && eof) { shutdown(conn->s[1], SHUT_WR); conn_event(c, conn, FIN_SENT_1); } } if ((conn->events & FIN_RCVD_1) && !(conn->events & FIN_SENT_0)) { - if (conn->read[fromside] == conn->written[fromside] && eof) { + if (conn->read[fromsidei] == conn->written[fromsidei] && eof) { shutdown(conn->s[0], SHUT_WR); conn_event(c, conn, FIN_SENT_0); } @@ -680,7 +681,7 @@ eintr: if ((events & (EPOLLIN | EPOLLOUT)) == (EPOLLIN | EPOLLOUT)) { events = EPOLLIN; - fromside = !fromside; + fromsidei = !fromsidei; goto swap; } @@ -815,19 +816,19 @@ void tcp_splice_init(struct ctx *c) */ void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn) { - int side; + unsigned sidei; ASSERT(!(conn->flags & CLOSING)); - for (side = 0; side < SIDES; side++) { - uint8_t set = side == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1; - uint8_t act = side == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1; + for (sidei = 0; sidei < SIDES; 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 (setsockopt(conn->s[side], SOL_SOCKET, SO_RCVLOWAT, + 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[side]); + conn->s[sidei]); } conn_flag(c, conn, ~set); }