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 8B67D5A0082 for ; Mon, 21 Nov 2022 02:08:57 +0100 (CET) Received: by gandalf.ozlabs.org (Postfix, from userid 1007) id 4NFq5y0n7sz4xN5; Mon, 21 Nov 2022 12:08:54 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gibson.dropbear.id.au; s=201602; t=1668992934; bh=uNcFoSsrcO0TJbzflxCwMtoNDSngs6yjxStviKRXHJA=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Yz/N1aB9zUpw9aGodaugvJ5+MNEaKZA+6fQ+CfQqUQkfXDTZphHIEbq7/8gYV8TJK zDR8hyTr/YVhGj/RpvoQ4OnM3llT2QHTbE6mgEF7/Ge89CY9ju3AtyUQa5M4aqE/TN FxBrbzvBVINcebGEpSCjlsGK9cMIh9sVv7lVu7+E= Date: Mon, 21 Nov 2022 12:08:41 +1100 From: David Gibson To: Stefano Brivio Subject: Re: [PATCH] tcp: Pass union tcp_conn pointer to destroy and splice timer functions Message-ID: References: <20221119083929.3431109-1-sbrivio@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="6T7yKY2KTOABGU1n" Content-Disposition: inline In-Reply-To: <20221119083929.3431109-1-sbrivio@redhat.com> Message-ID-Hash: 54CONFX4OPZIUMLM6WE2RR6MWUBPXFWI X-Message-ID-Hash: 54CONFX4OPZIUMLM6WE2RR6MWUBPXFWI 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.3 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: --6T7yKY2KTOABGU1n Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sat, Nov 19, 2022 at 09:39:29AM +0100, Stefano Brivio wrote: > The pointers are actually the same, but we later pass the container > union to tcp_table_compact(), which might zero the size of the whole > union, and this confuses Coverity Scan. >=20 > Given that we have pointers to the container union to start with, > just pass those instead, all the way down to tcp_table_compact(). >=20 > Signed-off-by: Stefano Brivio Reviewed-by: David Gibson > --- > tcp.c | 17 +++++++++-------- > tcp_conn.h | 4 ++-- > tcp_splice.c | 16 ++++++++++------ > 3 files changed, 21 insertions(+), 16 deletions(-) >=20 > diff --git a/tcp.c b/tcp.c > index 8874789..ff12dfa 100644 > --- a/tcp.c > +++ b/tcp.c > @@ -1376,16 +1376,18 @@ void tcp_table_compact(struct ctx *c, union tcp_c= onn *hole) > /** > * tcp_conn_destroy() - Close sockets, trigger hash table removal and co= mpaction > * @c: Execution context > - * @conn: Connection pointer > + * @conn_union: Connection pointer (container union) > */ > -static void tcp_conn_destroy(struct ctx *c, struct tcp_tap_conn *conn) > +static void tcp_conn_destroy(struct ctx *c, union tcp_conn *conn_union) > { > + struct tcp_tap_conn *conn =3D &conn_union->tap; > + > close(conn->sock); > if (conn->timer !=3D -1) > close(conn->timer); > =20 > tcp_hash_remove(c, conn); > - tcp_table_compact(c, (union tcp_conn *)conn); > + tcp_table_compact(c, conn_union); > } > =20 > static void tcp_rst_do(struct ctx *c, struct tcp_tap_conn *conn); > @@ -1535,13 +1537,12 @@ void tcp_defer_handler(struct ctx *c) > for (conn =3D tc + c->tcp.conn_count - 1; conn >=3D tc; conn--) { > if (conn->c.spliced) { > if (conn->splice.flags & CLOSING) > - tcp_splice_destroy(c, &conn->splice); > + tcp_splice_destroy(c, conn); > } else { > if (conn->tap.events =3D=3D CLOSED) > - tcp_conn_destroy(c, &conn->tap); > + tcp_conn_destroy(c, conn); > } > } > - > } > =20 > /** > @@ -3395,10 +3396,10 @@ void tcp_timer(struct ctx *c, const struct timesp= ec *ts) > =20 > for (conn =3D tc + c->tcp.conn_count - 1; conn >=3D tc; conn--) { > if (conn->c.spliced) { > - tcp_splice_timer(c, &conn->splice); > + tcp_splice_timer(c, conn); > } else { > if (conn->tap.events =3D=3D CLOSED) > - tcp_conn_destroy(c, &conn->tap); > + tcp_conn_destroy(c, conn); > } > } > =20 > diff --git a/tcp_conn.h b/tcp_conn.h > index 4a8be29..a146276 100644 > --- a/tcp_conn.h > +++ b/tcp_conn.h > @@ -181,8 +181,8 @@ extern union tcp_conn tc[]; > =20 > void tcp_splice_conn_update(struct ctx *c, struct tcp_splice_conn *new); > void tcp_table_compact(struct ctx *c, union tcp_conn *hole); > -void tcp_splice_destroy(struct ctx *c, struct tcp_splice_conn *conn); > -void tcp_splice_timer(struct ctx *c, struct tcp_splice_conn *conn); > +void tcp_splice_destroy(struct ctx *c, union tcp_conn *conn_union); > +void tcp_splice_timer(struct ctx *c, union tcp_conn *conn_union); > void tcp_splice_pipe_refill(const struct ctx *c); > =20 > =20 > diff --git a/tcp_splice.c b/tcp_splice.c > index e2f0ce1..72b1672 100644 > --- a/tcp_splice.c > +++ b/tcp_splice.c > @@ -251,10 +251,12 @@ void tcp_splice_conn_update(struct ctx *c, struct t= cp_splice_conn *new) > /** > * tcp_splice_destroy() - Close spliced connection and pipes, clear > * @c: Execution context > - * @conn: Connection pointer > + * @conn_union: Spliced connection (container union) > */ > -void tcp_splice_destroy(struct ctx *c, struct tcp_splice_conn *conn) > +void tcp_splice_destroy(struct ctx *c, union tcp_conn *conn_union) > { > + struct tcp_splice_conn *conn =3D &conn_union->splice; > + > if (conn->events & SPLICE_ESTABLISHED) { > /* Flushing might need to block: don't recycle them. */ > if (conn->pipe_a_b[0] !=3D -1) { > @@ -283,7 +285,7 @@ void tcp_splice_destroy(struct ctx *c, struct tcp_spl= ice_conn *conn) > debug("TCP (spliced): index %li, CLOSED", CONN_IDX(conn)); > =20 > c->tcp.splice_conn_count--; > - tcp_table_compact(c, (union tcp_conn *)conn); > + tcp_table_compact(c, conn_union); > } > =20 > /** > @@ -824,12 +826,14 @@ void tcp_splice_init(struct ctx *c) > /** > * tcp_splice_timer() - Timer for spliced connections > * @c: Execution context > - * @conn: Spliced connection > + * @conn_union: Spliced connection (container union) > */ > -void tcp_splice_timer(struct ctx *c, struct tcp_splice_conn *conn) > +void tcp_splice_timer(struct ctx *c, union tcp_conn *conn_union) > { > + struct tcp_splice_conn *conn =3D &conn_union->splice; > + > if (conn->flags & CLOSING) { > - tcp_splice_destroy(c, conn); > + tcp_splice_destroy(c, conn_union); > return; > } > =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 --6T7yKY2KTOABGU1n Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEoULxWu4/Ws0dB+XtgypY4gEwYSIFAmN6z4EACgkQgypY4gEw YSIh4xAA00wQ096VAxigdNTcri5Qv79r92/AwM3hjx2oUUsalJYqi6lxaqWkHpx1 EdNbQA3cQoPx2aQrhaz/8VSxpwaXaOX5lw5IV8SWf2nWppT0zbpuo1cfcf7MG4uc BoUpP1nCVI50wsVkT2nFWy0tNyPUblKBX2SQy0arTelyKOD07UYljSeqA8j9y/F3 MU8dhiB0AZ2ptLJ+wJod+t2ja5kF2LuqrmaFwYJBIFrunGh2ABFk/MOktlFE/SWg gXAHzW88tY7cHxNDrqHKjtLkoD2zHouAdyB14jsV3q2eOwYuDoKFn/yzj1q7yAh7 WbNsIJrwWIRD3zoQ3SIHxRcZV+ujssshUFRauBmVBkvVIFoGNpL/qk016AfUJEJp recaLUHC4fq8mEJp5SHpjkmZ0Mv5kdwWlCqHLFI+7cr4/AqemooqPug4KvY2iNrq oMUb8pfYs5bgOqCTQIEiNPRHhWUOSG6mn9EbU4r0D2vGagO+v+BgFinnL4Dr2rOp D5O3EZCZ0Xi0JsAq+6n53in8e/ijTUqtBOAbhYzEWR1bbKVV4w6RpnnaF0IzM9O9 CpIBlN0d4NFP6+1144P+DpnJXF3jPBJZkAhkPqC38w77uCTHI3i7pR5qi/HnzmSh gxpdDfz8xoB1f3CLxvt48WSSTIqMMsgzal89aN7P2HI9abyr5bM= =Di7X -----END PGP SIGNATURE----- --6T7yKY2KTOABGU1n--