Vadim Fedorenko wrote:

On 04/09/2024 12:31, Vadim Fedorenko wrote:

quoted

TCP sockets have different flow for providing timestamp OPT_ID value.
Adjust the code to support SCM_TS_OPT_ID option for TCP sockets.

Signed-off-by: Vadim Fedorenko <redacted>
---
  net/ipv4/tcp.c | 13 +++++++++----
  1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 8a5680b4e786..5553a8aeee80 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c

@@ -474,9 +474,10 @@ void tcp_init_sock(struct sock *sk)
  }
  EXPORT_SYMBOL(tcp_init_sock);
  
-static void tcp_tx_timestamp(struct sock *sk, u16 tsflags)
+static void tcp_tx_timestamp(struct sock *sk, struct sockcm_cookie *sockc)
  {
  	struct sk_buff *skb = tcp_write_queue_tail(sk);
+	u32 tsflags = sockc->tsflags;
  
  	if (tsflags && skb) {
  		struct skb_shared_info *shinfo = skb_shinfo(skb);

@@ -485,8 +486,12 @@ static void tcp_tx_timestamp(struct sock *sk, u16 tsflags)
  		sock_tx_timestamp(sk, tsflags, &shinfo->tx_flags);
  		if (tsflags & SOF_TIMESTAMPING_TX_ACK)
  			tcb->txstamp_ack = 1;
-		if (tsflags & SOF_TIMESTAMPING_TX_RECORD_MASK)
-			shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
+		if (tsflags & SOF_TIMESTAMPING_TX_RECORD_MASK) {
+			if (tsflags & SOCKCM_FLAG_TS_OPT_ID)
+				shinfo->tskey = sockc->ts_opt_id;
+			else
+				shinfo->tskey = TCP_SKB_CB(skb)->seq + skb->len - 1;
+		}
  	}
  }
  

@@ -1318,7 +1323,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
  
  out:
  	if (copied) {
-		tcp_tx_timestamp(sk, sockc.tsflags);
+		tcp_tx_timestamp(sk, &sockc);
  		tcp_push(sk, flags, mss_now, tp->nonagle, size_goal);
  	}
  out_nopush:

Hi Willem,

Unfortunately, these changes are not enough to enable custom OPT_ID for
TCP sockets. There are some functions which rewrite shinfo->tskey in TCP
flow:

tcp_skb_collapse_tstamp()
tcp_fragment_tstamp()
tcp_gso_tstamp()

I believe the last one breaks tests, but the problem is that there is no
easy way to provide the flag of constant tskey to it. Only
shinfo::tx_flags are available at the caller side and we have already
discussed that we shouldn't use the last bit of this field.

So, how should we deal with the problem? Or is it better to postpone
support for TCP sockets in this case?

Are you sure that this is a problem. These functions pass on the
skb_shinfo(skb)->ts_key from one skb to another.

As long as tcp_tx_timestamp sets the skb_shinfo(skb)->ts_key of the
original skb correctly, either from the cmsg or sk_tskey, then I don't
immediate see how this passing on from one skb to another would break
the intent.