From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from gandalf.ozlabs.org (mail.ozlabs.org [IPv6:2404:9400:2221:ea00::3]) by passt.top (Postfix) with ESMTPS id B16105A026D for ; Mon, 1 Jan 2024 00:19:56 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704064793; bh=ZggL7vHkzo73SS/Xi1KKUpOlbraiJPGw/29CrsAEF2E=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=WG37rq3m0Ll9QEZHNqQ9h/d43sXRRLXiNPXT1FMJtpoC0+Re/hyxbU+mzRQCEr8C8 9Nn+GdEB99VEDkEWLZKk7zj33zWYTY/qkO0uTZr9j/ph6RP8n4v+jmhph2aaLCa2ml njyqhay9WmHEsJLQABc+M8+42KfJJq2esPAF6SVYeyW8/WYeXpmo22Nkx6i15YEKbN +Sd7wZRTxmG07sDGmodb3W0Xrttwt5RDrIQBAJPrOTLKSXB3l//XyoC4FGrDPN839K S2JAcAS+Emmq9I9nmEVUIrQBELyxrHOyM6FJcFgmFSGIwGWQv0faxMlOhfXW4HCqO1 TtUyfVKKKHK1Q== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T3FSn2gzcz4wdF; Mon, 1 Jan 2024 10:19:53 +1100 (AEDT) Date: Sun, 31 Dec 2023 16:56:30 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH v3 05/13] flow, tcp: Add flow-centric dispatch for deferred flow handling Message-ID: References: <20231221061549.976358-1-david@gibson.dropbear.id.au> <20231221061549.976358-6-david@gibson.dropbear.id.au> <20231228192446.66102e44@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ciMlbyKKxT0zJBBY" Content-Disposition: inline In-Reply-To: <20231228192446.66102e44@elisabeth> Message-ID-Hash: CCZMDW5RWOQHAPYHP4C7W53PGLLMRUV3 X-Message-ID-Hash: CCZMDW5RWOQHAPYHP4C7W53PGLLMRUV3 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: --ciMlbyKKxT0zJBBY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Dec 28, 2023 at 07:24:46PM +0100, Stefano Brivio wrote: > On Thu, 21 Dec 2023 17:15:41 +1100 > David Gibson wrote: >=20 > > tcp_defer_handler(), amongst other things, scans the flow table and does > > some processing for each TCP connection. When we add other protocols to > > the flow table, they're likely to want some similar scanning. It makes > > more sense for cache friendliness to perform a single scan of the flow > > table and dispatch to the protocol specific handlers, rather than having > > each protocol separately scan the table. > >=20 > > To that end, add a new flow_defer_handler() handling all flow-linked > > deferred operations. > >=20 > > Signed-off-by: David Gibson > > --- > > flow.c | 23 +++++++++++++++++++++++ > > flow.h | 1 + > > passt.c | 1 + > > tcp.c | 19 ++----------------- > > tcp_conn.h | 1 + > > 5 files changed, 28 insertions(+), 17 deletions(-) > >=20 > > diff --git a/flow.c b/flow.c > > index a1c0a34..0a0402d 100644 > > --- a/flow.c > > +++ b/flow.c > > @@ -83,3 +83,26 @@ void flow_log_(const struct flow_common *f, int pri,= const char *fmt, ...) > > =20 > > logmsg(pri, "Flow %u (%s): %s", flow_idx(f), FLOW_TYPE(f), msg); > > } > > + > > +/** > > + * flow_defer_handler() - Handler for per-flow deferred tasks > > + * @c: Execution context > > + */ > > +void flow_defer_handler(struct ctx *c) > > +{ > > + union flow *flow; > > + > > + for (flow =3D flowtab + c->flow_count - 1; flow >=3D flowtab; flow--)= { > > + switch (flow->f.type) { > > + case FLOW_TCP: > > + tcp_flow_defer(c, flow); > > + break; > > + case FLOW_TCP_SPLICE: > > + tcp_splice_flow_defer(c, flow); > > + break; > > + default: > > + /* Assume other flow types don't need any handling */ > > + ; > > + } > > + } > > +} > > diff --git a/flow.h b/flow.h > > index 959b461..6b17fa8 100644 > > --- a/flow.h > > +++ b/flow.h > > @@ -67,6 +67,7 @@ static inline bool flow_sidx_eq(flow_sidx_t a, flow_s= idx_t b) > > union flow; > > =20 > > void flow_table_compact(struct ctx *c, union flow *hole); > > +void flow_defer_handler(struct ctx *c); > > =20 > > void flow_log_(const struct flow_common *f, int pri, const char *fmt, = =2E..) > > __attribute__((format(printf, 3, 4))); > > diff --git a/passt.c b/passt.c > > index 0246b04..5f72a28 100644 > > --- a/passt.c > > +++ b/passt.c > > @@ -103,6 +103,7 @@ static void post_handler(struct ctx *c, const struc= t timespec *now) > > /* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the same */ > > CALL_PROTO_HANDLER(c, now, icmp, ICMP); > > =20 > > + flow_defer_handler(c); > > #undef CALL_PROTO_HANDLER > > } > > =20 > > diff --git a/tcp.c b/tcp.c > > index ad1a70d..9230d80 100644 > > --- a/tcp.c > > +++ b/tcp.c > > @@ -1306,7 +1306,7 @@ static struct tcp_tap_conn *tcp_hash_lookup(const= struct ctx *c, > > * @c: Execution context > > * @flow: Flow table entry for this connection > > */ > > -static void tcp_flow_defer(struct ctx *c, union flow *flow) > > +void tcp_flow_defer(struct ctx *c, union flow *flow) > > { > > const struct tcp_tap_conn *conn =3D &flow->tcp; > > =20 > > @@ -1364,26 +1364,11 @@ static void tcp_l2_data_buf_flush(const struct = ctx *c) > > * tcp_defer_handler() - Handler for TCP deferred tasks > > * @c: Execution context > > */ > > +/* cppcheck-suppress constParameterPointer */ >=20 > This needs to be: >=20 > /* cppcheck-suppress [constParameterPointer, unmatchedSuppression] */ >=20 > otherwise we get warnings with cppcheck 2.10, Drat, do we? I was hoping this was a new warning type with the newer cppcheck, and it would ignore the suppression if it was for a warning type it didn't know about. > and we'll get warnings if > cppcheck's behaviour ever changes again. That's actually a good thing. This one isn't a workaround for a cppcheck false positive or weird semantic that we hope will go away. Rhe warning is real and correct as far as it goes. The problem is that the signature needs to match that of other deferred handlers because of how we generate the calls from a macro. Some of those others need write access to the context. --=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 --ciMlbyKKxT0zJBBY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWRAnEACgkQzQJF27ox 2GcN+w//RCdKawxBk+sIJny+j+cUmjaH+eEYGD2Z+0W5rw53uh0hwP3WX//K0Lzp gq7QNLYYIrSwSweRm8uc22hjMSd7i7wZGALII1g/UPyYSEpD9qD7AIEmKk9o/re+ gjne4j+zCtAjLKXLweaCPOiQ+IufXJZznVplDp0lYwrcgz6p4ACoLB26dcJVAFga FOy6iBDXiwjGcIbsR60TBUUWq+46L+6apQlovA8FcN1oK6/6WgSONUttynoT5dsk DwKjkA4Bg1tx0jfpCmlfnS8O7wKwKcACE8CpcT1jBOqMCLLHIAXTqW5pj8T8yZRc ZF1CvhxtMNVCqUVBUCJ1SClfboDGDcMeIUySD9B6tdXAzqExlmHC9G6ctlpHjizA 6fOR31l2NmfBD7+nIJCkVfKCop4/HbfR6vFWMbVM8nSgJ0WHWjx6pCzomqLDhi/u HpYgT8JLuPdh2kheqWJwsKtakW5mZHzH9ljI4T/+YWV+H93V+4v6O7dby5sKTXGb ePgUnt7Tch8mt+KtzjOjVwXmvMMRmUWzPTorep7XnQl2vvJkLZpD/fTlI05MX/8B pm5sbU4+Opc+UOReOXhiAlesy2X8vDjFthO8MDJSTm7jFkXg6h8vOXKjBKpHFpsl VoTVb8car+EJNKondgybI1RDPG3vIbO4P0Y85g4GK6aU261dbAg= =A/TA -----END PGP SIGNATURE----- --ciMlbyKKxT0zJBBY--