TCP (both regular and spliced) and ICMP both have macros to retrieve the relevant protcol specific flow structure from a flow index. In most cases what we actually want is to get the specific flow from a sidx. Replace those simple macros with a more precise inline, which also asserts that the flow is of the type we expect. While we're they're also add a pif_at_sidx() helper to get the interface of a specific flow & side, which is useful in some places. Finally, fix some minor style issues in the comments on some of the existing sidx related helpers. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow_table.h | 26 ++++++++++++++++++++------ icmp.c | 22 +++++++++++++++++++--- tcp.c | 28 ++++++++++++++++++++++------ tcp_splice.c | 21 +++++++++++++++++++-- 4 files changed, 80 insertions(+), 17 deletions(-) diff --git a/flow_table.h b/flow_table.h index 226ddbdd..ab73e44a 100644 --- a/flow_table.h +++ b/flow_table.h @@ -42,7 +42,7 @@ extern unsigned flow_first_free; extern union flow flowtab[]; -/** flow_idx - Index of flow from common structure +/** flow_idx() - Index of flow from common structure * @f: Common flow fields pointer * * Return: index of @f in the flow table @@ -52,21 +52,21 @@ static inline unsigned flow_idx(const struct flow_common *f) return (union flow *)f - flowtab; } -/** FLOW_IDX - Find the index of a flow +/** FLOW_IDX() - Find the index of a flow * @f_: Flow pointer, either union flow * or protocol specific * * Return: index of @f in the flow table */ #define FLOW_IDX(f_) (flow_idx(&(f_)->f)) -/** FLOW - Flow entry at a given index +/** FLOW() - Flow entry at a given index * @idx: Flow index * * Return: pointer to entry @idx in the flow table */ #define FLOW(idx) (&flowtab[(idx)]) -/** flow_at_sidx - Flow entry for a given sidx +/** flow_at_sidx() - Flow entry for a given sidx * @sidx: Flow & side index * * Return: pointer to the corresponding flow entry, or NULL @@ -78,7 +78,21 @@ static inline union flow *flow_at_sidx(flow_sidx_t sidx) return FLOW(sidx.flow); } -/** flow_sidx_t - Index of one side of a flow from common structure +/** pif_at_sidx() - Interface for a given flow and side + * @sidx: Flow & side index + * + * Return: pif for the flow & side given by @sidx + */ +static inline uint8_t pif_at_sidx(flow_sidx_t sidx) +{ + const union flow *flow = flow_at_sidx(sidx); + + if (!flow) + return PIF_NONE; + return flow->f.pif[sidx.side]; +} + +/** 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) * @@ -96,7 +110,7 @@ static inline flow_sidx_t flow_sidx(const struct flow_common *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 * @side: Which side to index (0 or 1) * diff --git a/icmp.c b/icmp.c index d4ccc722..7cf31e60 100644 --- a/icmp.c +++ b/icmp.c @@ -45,11 +45,27 @@ #define ICMP_ECHO_TIMEOUT 60 /* s, timeout for ICMP socket activity */ #define ICMP_NUM_IDS (1U << 16) -#define PINGF(idx) (&(FLOW(idx)->ping)) - /* Indexed by ICMP echo identifier */ static struct icmp_ping_flow *icmp_id_map[IP_VERSIONS][ICMP_NUM_IDS]; +/** + * ping_at_sidx() - Get ping specific flow at given sidx + * @sidx: Flow and side to retrieve + * + * Return: ping specific flow at @sidx, or NULL of @sidx is invalid. Asserts if + * the flow at @sidx is not FLOW_PING4 or FLOW_PING6 + */ +static struct icmp_ping_flow *ping_at_sidx(flow_sidx_t sidx) +{ + union flow *flow = flow_at_sidx(sidx); + + if (!flow) + return NULL; + + ASSERT(flow->f.type == FLOW_PING4 || flow->f.type == FLOW_PING6); + return &flow->ping; +} + /** * icmp_sock_handler() - Handle new data from ICMP or ICMPv6 socket * @c: Execution context @@ -57,7 +73,7 @@ static struct icmp_ping_flow *icmp_id_map[IP_VERSIONS][ICMP_NUM_IDS]; */ void icmp_sock_handler(const struct ctx *c, union epoll_ref ref) { - struct icmp_ping_flow *pingf = PINGF(ref.flowside.flow); + struct icmp_ping_flow *pingf = ping_at_sidx(ref.flowside); union sockaddr_inany sr; socklen_t sl = sizeof(sr); char buf[USHRT_MAX]; diff --git a/tcp.c b/tcp.c index de3f9f64..04ea1f91 100644 --- a/tcp.c +++ b/tcp.c @@ -376,8 +376,6 @@ char tcp_buf_discard [MAX_WINDOW]; /* sendmsg() to socket */ static struct iovec tcp_iov [UIO_MAXIOV]; -#define CONN(idx) (&(FLOW(idx)->tcp)) - /* Table for lookup from remote address, local port, remote port */ static flow_sidx_t tc_hash[TCP_HASH_TABLE_SIZE]; @@ -388,6 +386,24 @@ static_assert(ARRAY_SIZE(tc_hash) >= FLOW_MAX, int init_sock_pool4 [TCP_SOCK_POOL_SIZE]; int init_sock_pool6 [TCP_SOCK_POOL_SIZE]; +/** + * conn_at_sidx() - Get TCP connection specific flow at given sidx + * @sidx: Flow and side to retrieve + * + * Return: TCP connection at @sidx, or NULL of @sidx is invalid. Asserts if the + * flow at @sidx is not FLOW_TCP. + */ +static struct tcp_tap_conn *conn_at_sidx(flow_sidx_t sidx) +{ + union flow *flow = flow_at_sidx(sidx); + + if (!flow) + return NULL; + + ASSERT(flow->f.type == FLOW_TCP); + return &flow->tcp; +} + /** * tcp_conn_epoll_events() - epoll events mask for given connection state * @events: Current connection events @@ -2339,9 +2355,10 @@ cancel: void tcp_timer_handler(struct ctx *c, union epoll_ref ref) { struct itimerspec check_armed = { { 0 }, { 0 } }; - struct tcp_tap_conn *conn = CONN(ref.flow); + struct tcp_tap_conn *conn = &FLOW(ref.flow)->tcp; ASSERT(!c->no_tcp); + ASSERT(conn->f.type == FLOW_TCP); /* We don't reset timers on ~ACK_FROM_TAP_DUE, ~ACK_TO_TAP_DUE. If the * timer is currently armed, this event came from a previous setting, @@ -2397,11 +2414,10 @@ void tcp_timer_handler(struct ctx *c, union epoll_ref ref) */ void tcp_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events) { - struct tcp_tap_conn *conn = CONN(ref.flowside.flow); + struct tcp_tap_conn *conn = conn_at_sidx(ref.flowside); ASSERT(!c->no_tcp); - ASSERT(conn->f.type == FLOW_TCP); - ASSERT(conn->f.pif[ref.flowside.side] != PIF_TAP); + ASSERT(pif_at_sidx(ref.flowside) != PIF_TAP); if (conn->events == CLOSED) return; diff --git a/tcp_splice.c b/tcp_splice.c index f2d4fc60..9fc56565 100644 --- a/tcp_splice.c +++ b/tcp_splice.c @@ -76,7 +76,6 @@ static int splice_pipe_pool [TCP_SPLICE_PIPE_POOL_SIZE][2]; #define CONN_V6(x) ((x)->flags & SPLICE_V6) #define CONN_V4(x) (!CONN_V6(x)) #define CONN_HAS(conn, set) (((conn)->events & (set)) == (set)) -#define CONN(idx) (&FLOW(idx)->tcp_splice) /* Display strings for connection events */ static const char *tcp_splice_event_str[] __attribute((__unused__)) = { @@ -94,6 +93,24 @@ static const char *tcp_splice_flag_str[] __attribute((__unused__)) = { static int tcp_sock_refill_ns(void *arg); static int tcp_conn_sock_ns(const struct ctx *c, sa_family_t af); +/** + * conn_at_sidx() - Get spliced TCP connection specific flow at given sidx + * @sidx: Flow and side to retrieve + * + * Return: Spliced TCP connection at @sidx, or NULL of @sidx is invalid. + * Asserts if the flow at @sidx is not FLOW_TCP_SPLICE. + */ +static struct tcp_splice_conn *conn_at_sidx(flow_sidx_t sidx) +{ + union flow *flow = flow_at_sidx(sidx); + + if (!flow) + return NULL; + + ASSERT(flow->f.type == FLOW_TCP_SPLICE); + return &flow->tcp_splice; +} + /** * tcp_splice_conn_epoll_events() - epoll events masks for given state * @events: Connection event flags @@ -502,7 +519,7 @@ bool tcp_splice_conn_from_sock(const struct ctx *c, void tcp_splice_sock_handler(struct ctx *c, union epoll_ref ref, uint32_t events) { - struct tcp_splice_conn *conn = CONN(ref.flowside.flow); + struct tcp_splice_conn *conn = conn_at_sidx(ref.flowside); unsigned side = ref.flowside.side, fromside; uint8_t lowat_set_flag, lowat_act_flag; int eof, never_read; -- 2.45.2