On Thu, 21 Dec 2023 18:02:23 +1100
David Gibson <david(a)gibson.dropbear.id.au> wrote:
Handling of each protocol needs some degree of
tracking of the addresses
and ports at the end of each connection or flow. Sometimes that's explicit
(as in the guest visible addresses for TCP connections), sometimes implicit
(the bound and connected addresses of sockets).
To allow more general and robust handling, and more consistency across
protocols we want to uniformly track the address and port at each end of
the connection. Furthermore, because we allow port remapping, and we
sometimes need to apply NAT, the addresses and ports can be different as
seen by the guest/namespace and as by the host.
Introduce 'struct flowside' to keep track of common information related to
one side of each flow. For now that's the addresses, ports and the pif id.
Store two of these in the common fields of a flow to track that information
for both sides.
For now we just introduce the structure and fields themselves, along with
a simple helper. Later patches will actually use these to store useful
information.
Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au>
---
flow.h | 33 +++++++++++++++++++++++++++++++++
passt.h | 2 ++
tcp_conn.h | 1 -
3 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/flow.h b/flow.h
index 48a0ab4..e090ba0 100644
--- a/flow.h
+++ b/flow.h
@@ -27,11 +27,44 @@ extern const char *flow_type_str[];
#define FLOW_TYPE(f) \
((f)->type < FLOW_NUM_TYPES ? flow_type_str[(f)->type] :
"?")
+/**
+ * struct flowside - Common information for one side of a flow
+ * @eaddr: Endpoint address (remote address from passt's PoV)
+ * @faddr: Forwarding address (local address from passt's PoV)
+ * @eport: Endpoint port
+ * @fport: Forwarding port
+ * @pif: pif ID on which this side of the flow exists
+ */
+struct flowside {
+ union inany_addr faddr;
+ union inany_addr eaddr;
+ in_port_t fport;
+ in_port_t eport;
+ uint8_t pif;
+};
+static_assert(_Alignof(struct flowside) == _Alignof(uint32_t),
+ "Unexpected alignment for struct flowside");
+
Nits:
+/** flowside_complete - Check if flowside is
fully initialized
flowside_complete(). By the way, shouldn't we call it something more
descriptive such as flowside_is_complete()? It's not obvious this is
a check otherwise.
+ * @fside: flowside to check
* Return: true if pif, addresses and ports are set
...or something of that sort.
+ */
+static inline bool flowside_complete(const struct flowside *fside)
+{
+ return fside->pif != PIF_NONE &&
+ !IN6_IS_ADDR_UNSPECIFIED(&fside->faddr) &&
+ !IN6_IS_ADDR_UNSPECIFIED(&fside->eaddr) &&
+ fside->fport != 0 && fside->eport != 0;
I would align everything after 'return ', that is:
return fside->pif != PIF_NONE &&
!IN6_IS_ADDR_UNSPECIFIED(&fside->faddr) &&
!IN6_IS_ADDR_UNSPECIFIED(&fside->eaddr) &&
fside->fport != 0 && fside->eport != 0;
mostly for consistency -- not a strong preference.
All seems reasonable, updated accordingly. I've also updated
'flow_complete' to 'flow_is_complete' later in the series.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!