From mboxrd@z Thu Jan  1 00:00:00 1970
Authentication-Results: passt.top; dmarc=none (p=none dis=none) header.from=njh.eu
Received: from mail.notjusthosting.com (mail.notjusthosting.com [IPv6:2a01:4f8:a0:516f:1::1])
	by passt.top (Postfix) with ESMTPS id 7E9495A0279
	for <passt-dev@passt.top>; Wed, 10 Sep 2025 12:49:59 +0200 (CEST)
Received: from dynamic-2a02-3100-2db2-7a00-cc57-1656-4cdd-2e19.310.pool.telefonica.de ([2a02:3100:2db2:7a00:cc57:1656:4cdd:2e19] helo=vlap)
	by mail.notjusthosting.com with esmtpsa (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.89)
	(envelope-from <v@njh.eu>)
	id 1uwIOc-0007dM-0U; Wed, 10 Sep 2025 12:49:50 +0200
Date: Wed, 10 Sep 2025 12:49:49 +0200
From: Volker Diels-Grabsch <v@njh.eu>
To: Yumei Huang <yuhuang@redhat.com>
Subject: Re: [PATCH v2] tcp: Store the owner connections for flags frames
Message-ID: <aMFXzY5rWd7NEVI9@6153f789-1cf1-4ca3-8cea-6fa7ae195a8b.njh.eu>
References: <20250910095647.25279-1-yuhuang@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20250910095647.25279-1-yuhuang@redhat.com>
Message-ID-Hash: T4EXHHCLNXQNXIL4ALSWTYHYYZF346EU
X-Message-ID-Hash: T4EXHHCLNXQNXIL4ALSWTYHYYZF346EU
X-MailFrom: v@njh.eu
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, sbrivio@redhat.com, dgibson@redhat.com, boleyn.su@gmail.com, david@gibson.dropbear.id.au, jmaloy@redhat.com
X-Mailman-Version: 3.3.8
Precedence: list
List-Id: Development discussion and patches for passt <passt-dev.passt.top>
Archived-At: <https://archives.passt.top/passt-dev/aMFXzY5rWd7NEVI9@6153f789-1cf1-4ca3-8cea-6fa7ae195a8b.njh.eu/>
Archived-At: <https://passt.top/hyperkitty/list/passt-dev@passt.top/message/T4EXHHCLNXQNXIL4ALSWTYHYYZF346EU/>
List-Archive: <https://archives.passt.top/passt-dev/>
List-Archive: <https://passt.top/hyperkitty/list/passt-dev@passt.top/>
List-Help: <mailto:passt-dev-request@passt.top?subject=help>
List-Owner: <mailto:passt-dev-owner@passt.top>
List-Post: <mailto:passt-dev@passt.top>
List-Subscribe: <mailto:passt-dev-join@passt.top>
List-Unsubscribe: <mailto:passt-dev-leave@passt.top>

Dear Yumei,

Thanks a lot for providing a proper fix for that issue.  Just a minor
nitpick from my side:

Yumei Huang wrote:
> @@ -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;

I find it a bit strange to read that way, incrementing tcp_payload_used
just to subtract one from it in the next step.  I, personally, would
find it easier to read and to understand that way around:

 		struct iovec *dup_iov = tcp_l2_iov[tcp_payload_used];
		tcp_frame_conns[tcp_payload_used++] = conn;

But maybe it's just me.


Best regards,
Volker

-- 
.---<<<((()))>>>---.
|      [[||]]      |
'---<<<((()))>>>---'