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 <david@gibson.dropbear.id.au>
[sbrivio: Fixed minor detail in comment to struct flow_common]
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
This commit is contained in:
David Gibson 2024-07-17 14:52:19 +10:00 committed by Stefano Brivio
parent 9b125e7776
commit 71d7985188
3 changed files with 63 additions and 62 deletions

18
flow.h
View file

@ -132,8 +132,8 @@ extern const uint8_t flow_proto[];
#define SIDES 2 #define SIDES 2
#define INISIDE 0 /* Initiating side */ #define INISIDE 0 /* Initiating side index */
#define TGTSIDE 1 /* Target side */ #define TGTSIDE 1 /* Target side index */
/** /**
* struct flow_common - Common fields for packet flows * 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 * struct flow_sidx - ID for one side of a specific flow
* @side: Side referenced (0 or 1) * @sidei: Index of side referenced (0 or 1)
* @flow: Index of flow referenced * @flowi: Index of flow referenced
*/ */
typedef struct flow_sidx { typedef struct flow_sidx {
unsigned side :1; unsigned sidei :1;
unsigned flow :FLOW_INDEX_BITS; unsigned flowi :FLOW_INDEX_BITS;
} flow_sidx_t; } flow_sidx_t;
static_assert(sizeof(flow_sidx_t) <= sizeof(uint32_t), static_assert(sizeof(flow_sidx_t) <= sizeof(uint32_t),
"flow_sidx_t must fit within 32 bits"); "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 * 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) 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) 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; union flow;

View file

@ -75,7 +75,7 @@ static inline union flow *flow_at_sidx(flow_sidx_t sidx)
{ {
if (!flow_sidx_valid(sidx)) if (!flow_sidx_valid(sidx))
return NULL; return NULL;
return FLOW(sidx.flow); return FLOW(sidx.flowi);
} }
/** pif_at_sidx() - Interface for a given flow and side /** 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) if (!flow)
return PIF_NONE; 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 /** flow_sidx() - Index of one side of a flow from common structure
* @f: Common flow fields pointer * @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 * Return: index of @f and @side in the flow table
*/ */
static inline flow_sidx_t flow_sidx(const struct flow_common *f, static inline flow_sidx_t flow_sidx(const struct flow_common *f,
int side) unsigned sidei)
{ {
/* cppcheck-suppress [knownConditionTrueFalse, unmatchedSuppression] */ /* cppcheck-suppress [knownConditionTrueFalse, unmatchedSuppression] */
ASSERT(side == !!side); ASSERT(sidei == !!sidei);
return (flow_sidx_t){ return (flow_sidx_t){
.side = side, .sidei = sidei,
.flow = flow_idx(f), .flowi = flow_idx(f),
}; };
} }
/** FLOW_SIDX() - Find the index of one side of a flow /** FLOW_SIDX() - Find the index of one side of a flow
* @f_: Flow pointer, either union flow * or protocol specific * @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 * 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); union flow *flow_alloc(void);
void flow_alloc_cancel(union flow *flow); void flow_alloc_cancel(union flow *flow);

View file

@ -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) bool tcp_splice_flow_defer(struct tcp_splice_conn *conn)
{ {
unsigned side; unsigned sidei;
if (!(conn->flags & CLOSING)) if (!(conn->flags & CLOSING))
return false; return false;
for (side = 0; side < SIDES; side++) { for (sidei = 0; sidei < SIDES; sidei++) {
/* Flushing might need to block: don't recycle them. */ /* Flushing might need to block: don't recycle them. */
if (conn->pipe[side][0] >= 0) { if (conn->pipe[sidei][0] >= 0) {
close(conn->pipe[side][0]); close(conn->pipe[sidei][0]);
close(conn->pipe[side][1]); close(conn->pipe[sidei][1]);
conn->pipe[side][0] = conn->pipe[side][1] = -1; conn->pipe[sidei][0] = conn->pipe[sidei][1] = -1;
} }
if (conn->s[side] >= 0) { if (conn->s[sidei] >= 0) {
close(conn->s[side]); close(conn->s[sidei]);
conn->s[side] = -1; conn->s[sidei] = -1;
} }
conn->read[side] = conn->written[side] = 0; conn->read[sidei] = conn->written[sidei] = 0;
} }
conn->events = SPLICE_CLOSED; 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, static int tcp_splice_connect_finish(const struct ctx *c,
struct tcp_splice_conn *conn) struct tcp_splice_conn *conn)
{ {
unsigned side; unsigned sidei;
int i = 0; int i = 0;
for (side = 0; side < SIDES; side++) { for (sidei = 0; sidei < SIDES; sidei++) {
for (; i < TCP_SPLICE_PIPE_POOL_SIZE; i++) { for (; i < TCP_SPLICE_PIPE_POOL_SIZE; i++) {
if (splice_pipe_pool[i][0] >= 0) { if (splice_pipe_pool[i][0] >= 0) {
SWAP(conn->pipe[side][0], SWAP(conn->pipe[sidei][0],
splice_pipe_pool[i][0]); splice_pipe_pool[i][0]);
SWAP(conn->pipe[side][1], SWAP(conn->pipe[sidei][1],
splice_pipe_pool[i][1]); splice_pipe_pool[i][1]);
break; break;
} }
} }
if (conn->pipe[side][0] < 0) { if (conn->pipe[sidei][0] < 0) {
if (pipe2(conn->pipe[side], O_NONBLOCK | O_CLOEXEC)) { if (pipe2(conn->pipe[sidei], O_NONBLOCK | O_CLOEXEC)) {
flow_err(conn, "cannot create %d->%d pipe: %s", flow_err(conn, "cannot create %d->%d pipe: %s",
side, !side, strerror(errno)); sidei, !sidei, strerror(errno));
conn_flag(c, conn, CLOSING); conn_flag(c, conn, CLOSING);
return -EIO; return -EIO;
} }
if (fcntl(conn->pipe[side][0], F_SETPIPE_SZ, if (fcntl(conn->pipe[sidei][0], F_SETPIPE_SZ,
c->tcp.pipe_size)) { c->tcp.pipe_size)) {
flow_trace(conn, flow_trace(conn,
"cannot set %d->%d pipe size to %zu", "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) uint32_t events)
{ {
struct tcp_splice_conn *conn = conn_at_sidx(ref.flowside); 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; uint8_t lowat_set_flag, lowat_act_flag;
int eof, never_read; int eof, never_read;
@ -552,30 +552,31 @@ void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref,
} }
if (events & EPOLLOUT) { if (events & EPOLLOUT) {
fromside = !side; fromsidei = !evsidei;
conn_event(c, conn, side == 0 ? ~OUT_WAIT_0 : ~OUT_WAIT_1); conn_event(c, conn, evsidei == 0 ? ~OUT_WAIT_0 : ~OUT_WAIT_1);
} else { } else {
fromside = side; fromsidei = evsidei;
} }
if (events & EPOLLRDHUP) if (events & EPOLLRDHUP)
/* For side 0 this is fake, but implied */ /* 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: swap:
eof = 0; eof = 0;
never_read = 1; never_read = 1;
lowat_set_flag = fromside == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1; lowat_set_flag = fromsidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1;
lowat_act_flag = fromside == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1; lowat_act_flag = fromsidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1;
while (1) { while (1) {
ssize_t readlen, to_write = 0, written; ssize_t readlen, to_write = 0, written;
int more = 0; int more = 0;
retry: retry:
readlen = splice(conn->s[fromside], NULL, readlen = splice(conn->s[fromsidei], NULL,
conn->pipe[fromside][1], NULL, c->tcp.pipe_size, conn->pipe[fromsidei][1], NULL,
c->tcp.pipe_size,
SPLICE_F_MOVE | SPLICE_F_NONBLOCK); SPLICE_F_MOVE | SPLICE_F_NONBLOCK);
flow_trace(conn, "%zi from read-side call", readlen); flow_trace(conn, "%zi from read-side call", readlen);
if (readlen < 0) { if (readlen < 0) {
@ -600,8 +601,8 @@ retry:
} }
eintr: eintr:
written = splice(conn->pipe[fromside][0], NULL, written = splice(conn->pipe[fromsidei][0], NULL,
conn->s[!fromside], NULL, to_write, conn->s[!fromsidei], NULL, to_write,
SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK); SPLICE_F_MOVE | more | SPLICE_F_NONBLOCK);
flow_trace(conn, "%zi from write-side call (passed %zi)", flow_trace(conn, "%zi from write-side call (passed %zi)",
written, to_write); written, to_write);
@ -615,7 +616,7 @@ eintr:
readlen > (long)c->tcp.pipe_size / 10) { readlen > (long)c->tcp.pipe_size / 10) {
int lowat = c->tcp.pipe_size / 4; int lowat = c->tcp.pipe_size / 4;
if (setsockopt(conn->s[fromside], SOL_SOCKET, if (setsockopt(conn->s[fromsidei], SOL_SOCKET,
SO_RCVLOWAT, SO_RCVLOWAT,
&lowat, sizeof(lowat))) { &lowat, sizeof(lowat))) {
flow_trace(conn, flow_trace(conn,
@ -630,8 +631,8 @@ eintr:
break; break;
} }
conn->read[fromside] += readlen > 0 ? readlen : 0; conn->read[fromsidei] += readlen > 0 ? readlen : 0;
conn->written[fromside] += written > 0 ? written : 0; conn->written[fromsidei] += written > 0 ? written : 0;
if (written < 0) { if (written < 0) {
if (errno == EINTR) if (errno == EINTR)
@ -640,11 +641,11 @@ eintr:
if (errno != EAGAIN) if (errno != EAGAIN)
goto close; goto close;
if (conn->read[fromside] == conn->written[fromside]) if (conn->read[fromsidei] == conn->written[fromsidei])
break; break;
conn_event(c, conn, conn_event(c, conn,
fromside == 0 ? OUT_WAIT_1 : OUT_WAIT_0); fromsidei == 0 ? OUT_WAIT_1 : OUT_WAIT_0);
break; break;
} }
@ -661,14 +662,14 @@ eintr:
} }
if ((conn->events & FIN_RCVD_0) && !(conn->events & FIN_SENT_1)) { 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); shutdown(conn->s[1], SHUT_WR);
conn_event(c, conn, FIN_SENT_1); conn_event(c, conn, FIN_SENT_1);
} }
} }
if ((conn->events & FIN_RCVD_1) && !(conn->events & FIN_SENT_0)) { 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); shutdown(conn->s[0], SHUT_WR);
conn_event(c, conn, FIN_SENT_0); conn_event(c, conn, FIN_SENT_0);
} }
@ -680,7 +681,7 @@ eintr:
if ((events & (EPOLLIN | EPOLLOUT)) == (EPOLLIN | EPOLLOUT)) { if ((events & (EPOLLIN | EPOLLOUT)) == (EPOLLIN | EPOLLOUT)) {
events = EPOLLIN; events = EPOLLIN;
fromside = !fromside; fromsidei = !fromsidei;
goto swap; 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) void tcp_splice_timer(const struct ctx *c, struct tcp_splice_conn *conn)
{ {
int side; unsigned sidei;
ASSERT(!(conn->flags & CLOSING)); ASSERT(!(conn->flags & CLOSING));
for (side = 0; side < SIDES; side++) { for (sidei = 0; sidei < SIDES; sidei++) {
uint8_t set = side == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1; uint8_t set = sidei == 0 ? RCVLOWAT_SET_0 : RCVLOWAT_SET_1;
uint8_t act = side == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1; uint8_t act = sidei == 0 ? RCVLOWAT_ACT_0 : RCVLOWAT_ACT_1;
if ((conn->flags & set) && !(conn->flags & act)) { 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))) { &((int){ 1 }), sizeof(int))) {
flow_trace(conn, "can't set SO_RCVLOWAT on %d", flow_trace(conn, "can't set SO_RCVLOWAT on %d",
conn->s[side]); conn->s[sidei]);
} }
conn_flag(c, conn, ~set); conn_flag(c, conn, ~set);
} }