Re: [PATCH v2 11/22] flow: Clarify flow entry life cycle, introduce uniform logging

[Stefano Brivio <sbrivio@redhat.com>]

On Tue,  6 Feb 2024 12:17:23 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> 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@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/ ?

> + *            - It's not safe to return to the main epoll loop

..."before FLOW_START() is called on the entry"?

-- 
Stefano