From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (gandalf.ozlabs.org [150.107.74.76]) by passt.top (Postfix) with ESMTPS id 68BCD5A0278 for ; Mon, 19 Feb 2024 03:03:18 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1708308192; bh=JMZi0WyErkkRIFVSlQk3cdGC0ZBBeIwzmWZFrqjm8oo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=MApDeSHpkj6wDwML9spGV7lYAdTRB02vVLLdSYEn4YUho0gjFcWheSh500lvt7rYV RSJxmZ831ovqa98np/CbgTuOsz+Czh3Be5FiLHrAlCVqPWwaG9/ca7Qc51iaPGk//2 9Dzb74du3Q+WDBWw5KNHNcO1tw//p2mHDM4OIKCZYMa9mJpsWw/58gJSOCqvutnZgl w0sNXcuBMgfJ8Pe3yYu1UAHbnK4koN1ZrzW87qTmH4aSlz6NoDfUa61Fmd/X6hNcEr Crh8Og55DzF1zxSnWa+S5/PvgmwxtW8qzY7872oZYKsFi0TsoPnmpyhXvoe1X0J661 AGBLxZqkYucBA== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4TdQmc4ZMHz4wcK; Mon, 19 Feb 2024 13:03:12 +1100 (AEDT) Date: Mon, 19 Feb 2024 12:58:23 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v2 11/22] flow: Clarify flow entry life cycle, introduce uniform logging Message-ID: References: <20240206011734.884138-1-david@gibson.dropbear.id.au> <20240206011734.884138-12-david@gibson.dropbear.id.au> <20240218220004.2ed89b84@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="3VIKxg5RlUC8nxeV" Content-Disposition: inline In-Reply-To: <20240218220004.2ed89b84@elisabeth> Message-ID-Hash: LW3RI53YJ2YBVZBM2PAPQVYBKSYZ6OPO X-Message-ID-Hash: LW3RI53YJ2YBVZBM2PAPQVYBKSYZ6OPO X-MailFrom: dgibson@gandalf.ozlabs.org X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: passt-dev@passt.top X-Mailman-Version: 3.3.8 Precedence: list List-Id: Development discussion and patches for passt Archived-At: Archived-At: List-Archive: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --3VIKxg5RlUC8nxeV Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Feb 18, 2024 at 10:00:04PM +0100, Stefano Brivio wrote: > On Tue, 6 Feb 2024 12:17:23 +1100 > David Gibson wrote: >=20 > > 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 d= oc > > comment explaining the life cycle. > >=20 > > In addition, make a FLOW_START() macro to mark one of the important > > transitions. This encourages correct usage, by making it natural to on= ly > > access the flow type specific structure after calling it. It also logs > > that a new flow has been created, which is useful for debugging. > >=20 > > 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 f= rom > > the flow deferred handler. > >=20 > > Signed-off-by: David Gibson > > --- > > flow.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > > flow.h | 5 ++++ > > tcp.c | 15 +++++------ > > tcp_splice.c | 11 ++++---- > > tcp_splice.h | 5 ++-- > > 5 files changed, 94 insertions(+), 18 deletions(-) > >=20 > > 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) =3D=3D FLOW_NUM= _TYPES, > > =20 > > /* Global Flow Table */ > > =20 > > +/** > > + * 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 s= tate > > + * Caveats: > > + * - It's not safe to write fields in the flow entry > > + * - It's not safe to allocate other entries with flow_allo= c() >=20 > I'm not entirely sure what you mean here -- is this in the sense of > s/other/further/ ? Yes. I've changed the word to "further", which I agree is clearer. > > + * - It's not safe to return to the main epoll loop >=20 > ..."before FLOW_START() is called on the entry"? 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. --=20 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 --3VIKxg5RlUC8nxeV Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmXStb4ACgkQzQJF27ox 2GdogxAAmRBfNu+7aI2nZ3gHtFoxce66vVds70qRQKniFzf7/j7Up+CHvknwBCSr dmPy4tBB1v8KIoNQR+mwDZOswEggoqn3oj4g11T4FrJozvxyruHUyKRU18KlNW3+ hsZ/HGCgG6NPsMD4fID1Akkc8BtzFkJ7qWv9SxbusYCEqTyhJNuSDiGpercIzpS+ BMb1Hwboa8GcW2ZzBTPUMWXIsFPFQYP97p/lFiOO/dl5rNxgOa35JuzeRU6wGhoK 54L3D93GhSWPtMcbsr8xafHDylKN0tGBTcms92DIswoK2p//YI1mmq9SNwvUSHQ7 s7hPEH/7eY99Jh1NgF1neJrLgRTwtN2SGkr/TAR1hr1uHgLW29DUNu6Xr2CtJijO mqfi+VZU5Z3DuzTfIsYz9oYlO1c9J9CtwnxT1vW02UdmQQ0dorsfkUueQgI4HFaI 9O0elJXCPrVaDXPBozXCPDZFe9KoVjFg+PJyUhOKAjdWNns0hp0samm2Q5+qgCIs bT/xEilCKqvK21qGNgyjpsrRVt/aCCj+rQLqSYGVGXgDuqy9g+AoudG2+KAzwv7E oMBYCsRAf7dyEnPK/t3rNEp1IIGqcI9PinM2xtfZNvOeM6NnVTi0XR6aoqMdLUdB v/edFykNAb3rFxOa7SYw/krmFaO7dZmiXvaEgGyJFDy4NAi5c+o= =hiJo -----END PGP SIGNATURE----- --3VIKxg5RlUC8nxeV--