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 79FF85A026D for ; Wed, 3 Jan 2024 04:55:37 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=202312; t=1704254134; bh=bD7l/fQEV0ITbweyuBpTuqMSPqPhys+RA1tuftcGW2Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=bQrRlDiK7oDnXmuxcjPK6HBROkoIwOBv9DAkOS9Y5IdDZ8BRWvppPKTqcqFTnjb/5 9dnRQsLXZ4RydTXVvfrdHs7Fh8QIhc7NE8toZKOpi4INEm+Lr49tp59IfzPOvYYSTg oFDBhoteY1wpVuduA9kE7C5OkG9QegSdhkj699rU9uW/57n+1/e2OoHnOkEegQyGP/ o8qiG07uyP0uexN/fSa27UtLRgBNoyATguCnZym3oqtHOq21XigRlftBCrjf7+m/l+ Mbr2NY2UQPtZdT4AeqzsvxWxyWJlMrC5+XLYEBuliYHeaAwGDZMFmwc7192pEDijqV g6UOvCKLMj1wg== Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4T4bTy54t9z4wxX; Wed, 3 Jan 2024 14:55:34 +1100 (AEDT) Date: Wed, 3 Jan 2024 14:45:37 +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> <20240102191328.206f0fe4@elisabeth> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="USIr0eEbVK4xi/0f" Content-Disposition: inline In-Reply-To: <20240102191328.206f0fe4@elisabeth> Message-ID-Hash: SSCWXYYMUO63YYNBW7ZY2ABQTEEJZVMA X-Message-ID-Hash: SSCWXYYMUO63YYNBW7ZY2ABQTEEJZVMA 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: --USIr0eEbVK4xi/0f Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jan 02, 2024 at 07:13:28PM +0100, Stefano Brivio wrote: > On Sun, 31 Dec 2023 16:56:30 +1100 > David Gibson wrote: >=20 > > 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 protoco= ls to > > > > the flow table, they're likely to want some similar scanning. It m= akes > > > > more sense for cache friendliness to perform a single scan of the f= low > > > > table and dispatch to the protocol specific handlers, rather than h= aving > > > > 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; flo= w--) { > > > > + 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, fl= ow_sidx_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 *f= mt, ...) > > > > __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 s= truct timespec *now) > > > > /* NOLINTNEXTLINE(bugprone-branch-clone): intervals can be the sa= me */ > > > > 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(c= onst 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 str= uct ctx *c) > > > > * tcp_defer_handler() - Handler for TCP deferred tasks > > > > * @c: Execution context > > > > */ > > > > +/* cppcheck-suppress constParameterPointer */ =20 > > >=20 > > > This needs to be: > > >=20 > > > /* cppcheck-suppress [constParameterPointer, unmatchedSuppression] = */ > > >=20 > > > otherwise we get warnings with cppcheck 2.10, =20 > >=20 > > 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. >=20 > Yeah... go figure. On the other hand it's not really a new type in the > sense that this _should_ have been covered by a "constParameter" > warning, before 2.11. Dang. Oh well, updated. > > > and we'll get warnings if > > > cppcheck's behaviour ever changes again. =20 > >=20 > > 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 > Oops, I didn't realise. Well, in any case, then we don't expect > cppcheck's behaviour to ever change in this regard, so I don't see any > advantage omitting unmatchedSuppression here. >=20 --=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 --USIr0eEbVK4xi/0f Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEO+dNsU4E3yXUXRK2zQJF27ox2GcFAmWU2FAACgkQzQJF27ox 2Gc71g//c2xpOJ0/BRj0lGE3zG8YYIWGfP9aq3/ZwKDNPS1Y4VYhgX1+DQc97JRY 4+IveHdG/B+1V0jwPnLAf1oq1d0s3xWHp1tPlPbJggDKsTb0pvh/I7zbFSqeN8GE Zut62N2MqclmevYwxyyaZ8YH/3M4hWM5kVx0U34VQ4F+4eD4gTDEzELQKtzQ6F/7 aYFi9Ep7xxHOgXapKw/+Y3BDphHc1wjAvYxo5Ld2U0aS/Prf36ji1I1AajUnjo0Q lXDEz9U6c77w5FijTMvoTuBwAhYPt71r9xoBFGsy7j9w7Am2bNz7ZD2INiUOxllg kBYDnuN8ylQa/Bl1dI8bynytJIRJ5I7GmVhn9Zi6llPKYkdVljSumvjLdJHoNf5L DZNWy8k2Iy5uggORtCaNDGgGahazWqAkmWjF3UzqaM0CULvUTA1rnViSiS9ngohU cksyqp/N8Xiwtm0rlO98TAfizbIJ8E3IMTJC9AdgjEu3UCvmO5vzX3lwugauKZU0 EIc/eeyPHoSlW0utBEKv4kLbj/2spcIKkmYie2pwy2fK/HSnKsccs1VAiXS3hptv wO4I+19jDy5/MmE+/OdZOo/1dbLO/5akTlUsI+dp75vTsYF2te07dpud1mMTqxK0 ZwTlfkaZ8rq3u9OP9sRaB9umSBh8sFOgvzNOkkSXhkXWp1Gp+z0= =DwBq -----END PGP SIGNATURE----- --USIr0eEbVK4xi/0f--