netdev - Re: [PATCH] tcp: replace head->tstamp with head->skb_mstamp_ns in tcp_tso_should

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1733310027-29602-1-git-send-email-mengensun@tencent.com>
Date: Wed,  4 Dec 2024 19:00:27 +0800
From: MengEn Sun <mengensun88@...il.com>
To: edumazet@...gle.com
Cc: dsahern@...nel.org,
	horms@...nel.org,
	kuba@...nel.org,
	linux-kernel@...r.kernel.org,
	mengensun88@...il.com,
	mengensun@...cent.com,
	netdev@...r.kernel.org,
	pabeni@...hat.com,
	yuehongwu@...cent.com
Subject: Re: [PATCH] tcp: replace head->tstamp with head->skb_mstamp_ns in tcp_tso_should_defer()

Thank you very much for your reply!

There is no functional issue with using tstamp here.

TCP does indeed use tstamp in many places, but it seems that most of
them are related to SO_TIMESTAMP*. The main interface functions that
set tstamp in the code include the following:
- __net_timestamp
- net_timestamp_set
- __skb_tstamp_tx
The points where this tstamp is modified are documented in the kernel
documentation: Documentation/networking/timestamping.txt.


The functions that mainly modify skb_mstamp_ns include the following:
- tcp_add_tx_delay
- __tcp_transmit_skb
- tcp_write_xmit
- tcp_make_synack
- tcp_send_syn_data
The tstamp in an skb on the RTX queue has indeed not been modified by
the first group of functions mentioned above; instead, it is set by
the second group of functions before cloning the skb using skb_mstamp_ns.

I spent quite a bit of time reading the code before realizing that
the use of tstamp here is actually intended to retrieve the meaning of
skb_mstamp_ns. Therefore, I think if skb_mstamp_ns is used here, it
might give newcomers reading the TCP code a hint that this is actually
the value set by the second group of functions.

Best regards
MengEn