* [PATCH] tcp: Pass union tcp_conn pointer to destroy and splice timer functions
@ 2022-11-19 8:39 Stefano Brivio
2022-11-21 1:08 ` David Gibson
0 siblings, 1 reply; 2+ messages in thread
From: Stefano Brivio @ 2022-11-19 8:39 UTC (permalink / raw)
To: passt-dev; +Cc: David Gibson
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.
Given that we have pointers to the container union to start with,
just pass those instead, all the way down to tcp_table_compact().
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tcp.c | 17 +++++++++--------
tcp_conn.h | 4 ++--
tcp_splice.c | 16 ++++++++++------
3 files changed, 21 insertions(+), 16 deletions(-)
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_conn *hole)
/**
* tcp_conn_destroy() - Close sockets, trigger hash table removal and compaction
* @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 = &conn_union->tap;
+
close(conn->sock);
if (conn->timer != -1)
close(conn->timer);
tcp_hash_remove(c, conn);
- tcp_table_compact(c, (union tcp_conn *)conn);
+ tcp_table_compact(c, conn_union);
}
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 = tc + c->tcp.conn_count - 1; conn >= 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 == CLOSED)
- tcp_conn_destroy(c, &conn->tap);
+ tcp_conn_destroy(c, conn);
}
}
-
}
/**
@@ -3395,10 +3396,10 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
for (conn = tc + c->tcp.conn_count - 1; conn >= tc; conn--) {
if (conn->c.spliced) {
- tcp_splice_timer(c, &conn->splice);
+ tcp_splice_timer(c, conn);
} else {
if (conn->tap.events == CLOSED)
- tcp_conn_destroy(c, &conn->tap);
+ tcp_conn_destroy(c, conn);
}
}
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[];
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);
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 tcp_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 = &conn_union->splice;
+
if (conn->events & SPLICE_ESTABLISHED) {
/* Flushing might need to block: don't recycle them. */
if (conn->pipe_a_b[0] != -1) {
@@ -283,7 +285,7 @@ void tcp_splice_destroy(struct ctx *c, struct tcp_splice_conn *conn)
debug("TCP (spliced): index %li, CLOSED", CONN_IDX(conn));
c->tcp.splice_conn_count--;
- tcp_table_compact(c, (union tcp_conn *)conn);
+ tcp_table_compact(c, conn_union);
}
/**
@@ -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 = &conn_union->splice;
+
if (conn->flags & CLOSING) {
- tcp_splice_destroy(c, conn);
+ tcp_splice_destroy(c, conn_union);
return;
}
--
@@ -251,10 +251,12 @@ void tcp_splice_conn_update(struct ctx *c, struct tcp_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 = &conn_union->splice;
+
if (conn->events & SPLICE_ESTABLISHED) {
/* Flushing might need to block: don't recycle them. */
if (conn->pipe_a_b[0] != -1) {
@@ -283,7 +285,7 @@ void tcp_splice_destroy(struct ctx *c, struct tcp_splice_conn *conn)
debug("TCP (spliced): index %li, CLOSED", CONN_IDX(conn));
c->tcp.splice_conn_count--;
- tcp_table_compact(c, (union tcp_conn *)conn);
+ tcp_table_compact(c, conn_union);
}
/**
@@ -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 = &conn_union->splice;
+
if (conn->flags & CLOSING) {
- tcp_splice_destroy(c, conn);
+ tcp_splice_destroy(c, conn_union);
return;
}
--
2.35.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] tcp: Pass union tcp_conn pointer to destroy and splice timer functions
2022-11-19 8:39 [PATCH] tcp: Pass union tcp_conn pointer to destroy and splice timer functions Stefano Brivio
@ 2022-11-21 1:08 ` David Gibson
0 siblings, 0 replies; 2+ messages in thread
From: David Gibson @ 2022-11-21 1:08 UTC (permalink / raw)
To: Stefano Brivio; +Cc: passt-dev
[-- Attachment #1: Type: text/plain, Size: 4993 bytes --]
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.
>
> Given that we have pointers to the container union to start with,
> just pass those instead, all the way down to tcp_table_compact().
>
> Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> tcp.c | 17 +++++++++--------
> tcp_conn.h | 4 ++--
> tcp_splice.c | 16 ++++++++++------
> 3 files changed, 21 insertions(+), 16 deletions(-)
>
> 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_conn *hole)
> /**
> * tcp_conn_destroy() - Close sockets, trigger hash table removal and compaction
> * @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 = &conn_union->tap;
> +
> close(conn->sock);
> if (conn->timer != -1)
> close(conn->timer);
>
> tcp_hash_remove(c, conn);
> - tcp_table_compact(c, (union tcp_conn *)conn);
> + tcp_table_compact(c, conn_union);
> }
>
> 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 = tc + c->tcp.conn_count - 1; conn >= 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 == CLOSED)
> - tcp_conn_destroy(c, &conn->tap);
> + tcp_conn_destroy(c, conn);
> }
> }
> -
> }
>
> /**
> @@ -3395,10 +3396,10 @@ void tcp_timer(struct ctx *c, const struct timespec *ts)
>
> for (conn = tc + c->tcp.conn_count - 1; conn >= tc; conn--) {
> if (conn->c.spliced) {
> - tcp_splice_timer(c, &conn->splice);
> + tcp_splice_timer(c, conn);
> } else {
> if (conn->tap.events == CLOSED)
> - tcp_conn_destroy(c, &conn->tap);
> + tcp_conn_destroy(c, conn);
> }
> }
>
> 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[];
>
> 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);
>
>
> 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 tcp_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 = &conn_union->splice;
> +
> if (conn->events & SPLICE_ESTABLISHED) {
> /* Flushing might need to block: don't recycle them. */
> if (conn->pipe_a_b[0] != -1) {
> @@ -283,7 +285,7 @@ void tcp_splice_destroy(struct ctx *c, struct tcp_splice_conn *conn)
> debug("TCP (spliced): index %li, CLOSED", CONN_IDX(conn));
>
> c->tcp.splice_conn_count--;
> - tcp_table_compact(c, (union tcp_conn *)conn);
> + tcp_table_compact(c, conn_union);
> }
>
> /**
> @@ -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 = &conn_union->splice;
> +
> if (conn->flags & CLOSING) {
> - tcp_splice_destroy(c, conn);
> + tcp_splice_destroy(c, conn_union);
> return;
> }
>
--
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-11-21 1:08 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-19 8:39 [PATCH] tcp: Pass union tcp_conn pointer to destroy and splice timer functions Stefano Brivio
2022-11-21 1:08 ` David Gibson
Code repositories for project(s) associated with this public inbox
https://passt.top/passt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for IMAP folder(s).