flow, icmp, tcp: Clean up helpers for getting flow from index

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@gibson.dropbear.id.au>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>

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)
  *

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];

tcp.c

@@ -379,8 +379,6 @@ bool peek_offset_cap;
 /* 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];
 
@@ -391,6 +389,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_set_peek_offset() - Set SO_PEEK_OFF offset on a socket if supported
  * @s:          Socket to update
@@ -2379,9 +2395,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,
@@ -2441,11 +2458,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;

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;