* [PATCH] tcp: Add flags frames to payload frames arrays
@ 2025-09-10 7:46 Yumei Huang
2025-09-10 8:52 ` Stefano Brivio
0 siblings, 1 reply; 3+ messages in thread
From: Yumei Huang @ 2025-09-10 7:46 UTC (permalink / raw)
To: passt-dev; +Cc: sbrivio, dgibson, yuhuang, v, boleyn.su
There is an issue reported by Volker Diels-Grabsch and Boleyn Su.
A segmentation fault occurs when executing the following command:
(sleep 0.1; ssh -p 22000 127.0.0.1) & passt -f -t 22000:22
It's caused by commit 78da088f7bab ("tcp: unify payload and flags
l2 frames array"). Fix it by adding flags frames to payload frames
arrays.
Signed-off-by: Yumei Huang <yuhuang@redhat.com>
---
tcp_buf.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/tcp_buf.c b/tcp_buf.c
index bc898de..d351c20 100644
--- a/tcp_buf.c
+++ b/tcp_buf.c
@@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
if (ret <= 0)
return ret;
- tcp_payload_used++;
+ tcp_frame_conns[tcp_payload_used++] = conn;
l4len = optlen + sizeof(struct tcphdr);
iov[TCP_IOV_PAYLOAD].iov_len = l4len;
tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
if (flags & DUP_ACK) {
struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++];
+ tcp_frame_conns[tcp_payload_used - 1] = conn;
memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base,
iov[TCP_IOV_TAP].iov_len);
--
@@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
if (ret <= 0)
return ret;
- tcp_payload_used++;
+ tcp_frame_conns[tcp_payload_used++] = conn;
l4len = optlen + sizeof(struct tcphdr);
iov[TCP_IOV_PAYLOAD].iov_len = l4len;
tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
if (flags & DUP_ACK) {
struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++];
+ tcp_frame_conns[tcp_payload_used - 1] = conn;
memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base,
iov[TCP_IOV_TAP].iov_len);
--
2.47.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] tcp: Add flags frames to payload frames arrays
2025-09-10 7:46 [PATCH] tcp: Add flags frames to payload frames arrays Yumei Huang
@ 2025-09-10 8:52 ` Stefano Brivio
2025-09-10 9:25 ` Yumei Huang
0 siblings, 1 reply; 3+ messages in thread
From: Stefano Brivio @ 2025-09-10 8:52 UTC (permalink / raw)
To: Yumei Huang
Cc: passt-dev, David Gibson, Volker Diels-Grabsch, Boleyn Su, Jon Maloy
On Wed, 10 Sep 2025 15:46:30 +0800
Yumei Huang <yuhuang@redhat.com> wrote:
> There is an issue reported by Volker Diels-Grabsch and Boleyn Su.
> A segmentation fault occurs when executing the following command:
>
> (sleep 0.1; ssh -p 22000 127.0.0.1) & passt -f -t 22000:22
>
> It's caused by commit 78da088f7bab ("tcp: unify payload and flags
> l2 frames array"). Fix it by adding flags frames to payload frames
> arrays.
Ouch. Nice catch!
I guess in this sentence you meant "Fix it by setting ownership
information for flags frames"? The patch itself looks correct, the
commit message doesn't entirely match it though.
The commit title is also a bit mismatching. The description for
tcp_frame_conns[] is:
/* References tracking the owner connection of frames in the tap outqueue */
A couple of minor details you probably didn't know about:
- in this case we would typically use all these tags in the commit
message:
Reported-by: Volker Diels-Grabsch <v@njh.eu>
Reported-by: Boleyn Su <boleyn.su@gmail.com>
Fixes: 78da088f7bab ("tcp: unify payload and flags l2 frames array")
- you should Cc: Jon as the author of the faulty commit (I just did)
- David's upstream email address is david@gibson.dropbear.id.au (I just
changed it)
I can add the tags for you, but it's probably easier if you re-post a
v2 with a more accurate commit message. Thanks!
> Signed-off-by: Yumei Huang <yuhuang@redhat.com>
> ---
> tcp_buf.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/tcp_buf.c b/tcp_buf.c
> index bc898de..d351c20 100644
> --- a/tcp_buf.c
> +++ b/tcp_buf.c
> @@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
> if (ret <= 0)
> return ret;
>
> - tcp_payload_used++;
> + tcp_frame_conns[tcp_payload_used++] = conn;
> l4len = optlen + sizeof(struct tcphdr);
> iov[TCP_IOV_PAYLOAD].iov_len = l4len;
> tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
>
> if (flags & DUP_ACK) {
> struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++];
> + tcp_frame_conns[tcp_payload_used - 1] = conn;
>
> memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base,
> iov[TCP_IOV_TAP].iov_len);
--
Stefano
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] tcp: Add flags frames to payload frames arrays
2025-09-10 8:52 ` Stefano Brivio
@ 2025-09-10 9:25 ` Yumei Huang
0 siblings, 0 replies; 3+ messages in thread
From: Yumei Huang @ 2025-09-10 9:25 UTC (permalink / raw)
To: Stefano Brivio
Cc: passt-dev, David Gibson, Volker Diels-Grabsch, Boleyn Su, Jon Maloy
Thank you Stefano for all the information.
I will send a v2 soon.
On Wed, Sep 10, 2025 at 4:52 PM Stefano Brivio <sbrivio@redhat.com> wrote:
>
> On Wed, 10 Sep 2025 15:46:30 +0800
> Yumei Huang <yuhuang@redhat.com> wrote:
>
> > There is an issue reported by Volker Diels-Grabsch and Boleyn Su.
> > A segmentation fault occurs when executing the following command:
> >
> > (sleep 0.1; ssh -p 22000 127.0.0.1) & passt -f -t 22000:22
> >
> > It's caused by commit 78da088f7bab ("tcp: unify payload and flags
> > l2 frames array"). Fix it by adding flags frames to payload frames
> > arrays.
>
> Ouch. Nice catch!
>
> I guess in this sentence you meant "Fix it by setting ownership
> information for flags frames"? The patch itself looks correct, the
> commit message doesn't entirely match it though.
>
> The commit title is also a bit mismatching. The description for
> tcp_frame_conns[] is:
>
> /* References tracking the owner connection of frames in the tap outqueue */
>
> A couple of minor details you probably didn't know about:
>
> - in this case we would typically use all these tags in the commit
> message:
>
> Reported-by: Volker Diels-Grabsch <v@njh.eu>
> Reported-by: Boleyn Su <boleyn.su@gmail.com>
> Fixes: 78da088f7bab ("tcp: unify payload and flags l2 frames array")
>
> - you should Cc: Jon as the author of the faulty commit (I just did)
>
> - David's upstream email address is david@gibson.dropbear.id.au (I just
> changed it)
>
> I can add the tags for you, but it's probably easier if you re-post a
> v2 with a more accurate commit message. Thanks!
>
> > Signed-off-by: Yumei Huang <yuhuang@redhat.com>
> > ---
> > tcp_buf.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tcp_buf.c b/tcp_buf.c
> > index bc898de..d351c20 100644
> > --- a/tcp_buf.c
> > +++ b/tcp_buf.c
> > @@ -209,13 +209,14 @@ int tcp_buf_send_flag(const struct ctx *c, struct tcp_tap_conn *conn, int flags)
> > if (ret <= 0)
> > return ret;
> >
> > - tcp_payload_used++;
> > + tcp_frame_conns[tcp_payload_used++] = conn;
> > l4len = optlen + sizeof(struct tcphdr);
> > iov[TCP_IOV_PAYLOAD].iov_len = l4len;
> > tcp_l2_buf_fill_headers(conn, iov, NULL, seq, false);
> >
> > if (flags & DUP_ACK) {
> > struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used++];
> > + tcp_frame_conns[tcp_payload_used - 1] = conn;
> >
> > memcpy(dup_iov[TCP_IOV_TAP].iov_base, iov[TCP_IOV_TAP].iov_base,
> > iov[TCP_IOV_TAP].iov_len);
>
> --
> Stefano
>
--
Thanks,
Yumei Huang
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-09-10 9:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-09-10 7:46 [PATCH] tcp: Add flags frames to payload frames arrays Yumei Huang
2025-09-10 8:52 ` Stefano Brivio
2025-09-10 9:25 ` Yumei Huang
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).