Re: [PATCH net-next v2 09/12] net-timestamp: add tx OPT_ID_TCP support for bpf case

Jason Xing wrote:

On Tue, Oct 15, 2024 at 9:38 AM Willem de Bruijn
[off-list ref] wrote:

quoted

Jason Xing wrote:

quoted

From: Jason Xing <kernelxing@tencent.com>

We can set OPT_ID|OPT_ID_TCP before we initialize the last skb
from each sendmsg. We only set the socket once like how we use
setsockopt() with OPT_ID|OPT_ID_TCP flags.

Note: we will check if non-bpf _and_ bpf sk_tsflags have OPT_ID
flag. If either of them has been set before, we will not initialize
the key any more,

Where and how is this achieved?

Please see this patch and you will find the following codes.
+       tsflags |= (sk->sk_tsflags[SOCKETOPT_TS_REQUESTOR] |
+                   sk->sk_tsflags[BPFPROG_TS_REQUESTOR]);

I saw that, but it's not a condition that stops reinitializing. Which
I think is the intent, based on "If either of them has been set
before, we will not initialize the key anymore"?

Reinitialization is actually supported behavior.

                if (val & SOF_TIMESTAMPING_OPT_ID &&
                    !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {

But the sk_tsflags bit may be repeatedly set and cleared.

Anyway, the current patch sets it if either requests it?

+	tsflags |= (sk->sk_tsflags[SOCKETOPT_TS_REQUESTOR] |
+		    sk->sk_tsflags[BPFPROG_TS_REQUESTOR]);
 	if (val & SOF_TIMESTAMPING_OPT_ID &&
-	    !(sk->sk_tsflags[SOCKETOPT_TS_REQUESTOR] & SOF_TIMESTAMPING_OPT_ID)) {
+	    !(tsflags & SOF_TIMESTAMPING_OPT_ID)) {

 

But the difference/problem is that the non-bpf feature only init it
when connect() is done, but the bpf feature could do it at the
beginning of connect(). If running txtimestamp -l 1000, the former
will generate 999 for turkey while the latter 1000.