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

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

Jason Xing wrote:

quoted

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"?

Yep, based on that sentence. If we find sk_tsflags is initialized,
then we will not do the same thing to sk_tskey again when we use bpf
method.

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.

This line "!(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {" was removed
and replaced in the new function sock_set_tskey(). So it could avoid
re-initialization.

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

Yep, either of the ways (bpf and non-bpf) can init it.