On Sun, Feb 18, 2024 at 10:00:04PM +0100, Stefano Brivio wrote:On Tue, 6 Feb 2024 12:17:23 +1100 David Gibson <david(a)gibson.dropbear.id.au> wrote:Yes. I've changed the word to "further", which I agree is clearer.Our allocation scheme for flow entries means there are some non-obvious constraints on when what things can be done with an entry. Add a big doc comment explaining the life cycle. In addition, make a FLOW_START() macro to mark one of the important transitions. This encourages correct usage, by making it natural to only access the flow type specific structure after calling it. It also logs that a new flow has been created, which is useful for debugging. We also add logging when a flow's lifecycle ends. This doesn't need a new helper, because it can only happen either from flow_alloc_cancel() or from the flow deferred handler. Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- flow.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++-- flow.h | 5 ++++ tcp.c | 15 +++++------ tcp_splice.c | 11 ++++---- tcp_splice.h | 5 ++-- 5 files changed, 94 insertions(+), 18 deletions(-) diff --git a/flow.c b/flow.c index beb9749c..a155b54b 100644 --- a/flow.c +++ b/flow.c @@ -34,6 +34,45 @@ static_assert(ARRAY_SIZE(flow_proto) == FLOW_NUM_TYPES, /* Global Flow Table */ +/** + * DOC: Theory of Operation - flow entry life cycle + * + * An individual flow table entry moves through these logical states, usually in + * this order. + * + * FREE - Part of the general pool of free flow table entries + * Operations: + * - flow_alloc() finds an entry and moves it to ALLOC state + * + * ALLOC - A tentatively allocated entry + * Operations: + * - flow_alloc_cancel() returns the entry to FREE state + * - FLOW_START() set the entry's type and moves to START state + * Caveats: + * - It's not safe to write fields in the flow entry + * - It's not safe to allocate other entries with flow_alloc()I'm not entirely sure what you mean here -- is this in the sense of s/other/further/ ?Right, because FLOW_START() moves to START state, where returning to the epoll loop *is* allowed. I've added a note which I hope makes that clearer. -- 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_! http://www.ozlabs.org/~dgibson+ * - It's not safe to return to the main epoll loop..."before FLOW_START() is called on the entry"?