[PATCH net-next v4 0/4] net-timestamp: introduce a flag to filter out rx software and hardware report

[PATCH net-next v4 0/4] net-timestamp: introduce a flag to filter out rx software and hardware report

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

When one socket is set SOF_TIMESTAMPING_RX_SOFTWARE which means the
whole system turns on the netstamp_needed_key button, other sockets
that only have SOF_TIMESTAMPING_SOFTWARE will be affected and then
print the rx timestamp information even without setting
SOF_TIMESTAMPING_RX_SOFTWARE generation flag.

How to solve it without breaking users?
We introduce a new flag named SOF_TIMESTAMPING_OPT_RX_FILTER. Using
it together with SOF_TIMESTAMPING_SOFTWARE can stop reporting the
rx software timestamp.

Similarly, we also filter out the hardware case where one process
enables the rx hardware generation flag, then another process only
passing SOF_TIMESTAMPING_RAW_HARDWARE gets the timestamp. So we can set
both SOF_TIMESTAMPING_RAW_HARDWARE and SOF_TIMESTAMPING_OPT_RX_FILTER
to stop reporting rx hardware timestamp after this patch applied.

v4
Link: https://lore.kernel.org/all/20240830153751.86895-1-kerneljasonxing@gmail.com/
1. revise the doc and commit message (Willem)
2. add patch [2/4] to make the doc right (Willem)
3. add patch [3/4] to cover the hardware use (Willem)
4. add testcase for hardware use.
Note: the reason why I split into 4 patches is try to make each commit
clean, atomic, easy to review.

v3
Link: https://lore.kernel.org/all/20240828160145.68805-1-kerneljasonxing@gmail.com/
1. introduce a new flag to avoid application breakage, suggested by
Willem.
2. add it into the selftests.

v2
Link: https://lore.kernel.org/all/20240825152440.93054-1-kerneljasonxing@gmail.com/
Discussed with Willem
1. update the documentation accordingly
2. add more comments in each patch
3. remove the previous test statements in __sock_recv_timestamp()

Jason Xing (4):
  net-timestamp: filter out report when setting
    SOF_TIMESTAMPING_SOFTWARE
  net-timestamp: correct the use of SOF_TIMESTAMPING_RAW_HARDWARE
  net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use
  rxtimestamp.c: add the test for SOF_TIMESTAMPING_OPT_RX_FILTER

 Documentation/networking/timestamping.rst | 18 +++++++++++++++++-
 include/uapi/linux/net_tstamp.h           |  3 ++-
 net/core/sock.c                           |  5 +++++
 net/ethtool/common.c                      |  1 +
 net/ipv4/tcp.c                            |  7 +++++--
 net/socket.c                              |  7 +++++--
 tools/testing/selftests/net/rxtimestamp.c | 11 +++++++++++
 7 files changed, 46 insertions(+), 6 deletions(-)

-- 
2.37.3

[PATCH net-next v4 1/4] net-timestamp: filter out report when setting SOF_TIMESTAMPING_SOFTWARE

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

introduce a new flag SOF_TIMESTAMPING_OPT_RX_FILTER in the receive
path. User can set it with SOF_TIMESTAMPING_SOFTWARE to filter
out rx software timestamp report, especially after a process turns on
netstamp_needed_key which can time stamp every incoming skb.

Previously, we found out if an application starts first which turns on
netstamp_needed_key, then another one only passing SOF_TIMESTAMPING_SOFTWARE
could also get rx timestamp. Now we handle this case by introducing this
new flag without breaking users.

Quoting Willem to explain why we need the flag:
"why a process would want to request software timestamp reporting, but
not receive software timestamp generation. The only use I see is when
the application does request
SOF_TIMESTAMPING_SOFTWARE | SOF_TIMESTAMPING_TX_SOFTWARE."

In this way, we have two kinds of combination:
1. setting SOF_TIMESTAMPING_SOFTWARE|SOF_TIMESTAMPING_RX_SOFTWARE, it
will surely allow users to get the rx software timestamp report.
2. setting SOF_TIMESTAMPING_SOFTWARE|SOF_TIMESTAMPING_OPT_RX_FILTER
while the skb is timestamped, it will stop reporting the rx software
timestamp.

Another thing about errqueue in this patch I have a few words to say:
In this case, we need to handle the egress path carefully, or else
reporting the tx timestamp will fail. Egress path and ingress path will
finally call sock_recv_timestamp(). We have to distinguish them.
Errqueue is a good indicator to reflect the flow direction.

Suggested-by: Willem de Bruijn <willemb@google.com>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
v4
Link: https://lore.kernel.org/all/20240830153751.86895-2-kerneljasonxing@gmail.com/
1. revise the commit message and doc (Willem)
2. simplify the test statement (Jakub)
3. add Willem's reviewed-by tag (Willem)

v3
1. Willem suggested this alternative way to solve the issue, so I
added his Suggested-by tag here. Thanks!
---
 Documentation/networking/timestamping.rst | 12 ++++++++++++
 include/uapi/linux/net_tstamp.h           |  3 ++-
 net/core/sock.c                           |  4 ++++
 net/ethtool/common.c                      |  1 +
 net/ipv4/tcp.c                            |  6 ++++--
 net/socket.c                              |  4 +++-
 6 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
index 5e93cd71f99f..37ead02be3b1 100644
--- a/Documentation/networking/timestamping.rst
+++ b/Documentation/networking/timestamping.rst
@@ -266,6 +266,18 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
   two separate messages will be looped to the socket's error queue,
   each containing just one timestamp.
 
+SOF_TIMESTAMPING_OPT_RX_FILTER:
+  Used in the receive software timestamp. Enabling the flag along with
+  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
+  userspace so that it can filter out the case where one process starts
+  first which turns on netstamp_needed_key through setting generation
+  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
+  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
+
+  SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
+  influenced by others and let the application choose whether to report
+  the timestamp in the receive path or not.
+
 New applications are encouraged to pass SOF_TIMESTAMPING_OPT_ID to
 disambiguate timestamps and SOF_TIMESTAMPING_OPT_TSONLY to operate
 regardless of the setting of sysctl net.core.tstamp_allow_data.
diff --git a/include/uapi/linux/net_tstamp.h b/include/uapi/linux/net_tstamp.h
index a2c66b3d7f0f..858339d1c1c4 100644
--- a/include/uapi/linux/net_tstamp.h
+++ b/include/uapi/linux/net_tstamp.h
@@ -32,8 +32,9 @@ enum {
 	SOF_TIMESTAMPING_OPT_TX_SWHW = (1<<14),
 	SOF_TIMESTAMPING_BIND_PHC = (1 << 15),
 	SOF_TIMESTAMPING_OPT_ID_TCP = (1 << 16),
+	SOF_TIMESTAMPING_OPT_RX_FILTER = (1 << 17),
 
-	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_ID_TCP,
+	SOF_TIMESTAMPING_LAST = SOF_TIMESTAMPING_OPT_RX_FILTER,
 	SOF_TIMESTAMPING_MASK = (SOF_TIMESTAMPING_LAST - 1) |
 				 SOF_TIMESTAMPING_LAST
 };
diff --git a/net/core/sock.c b/net/core/sock.c
index 468b1239606c..6a93344e21cf 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -908,6 +908,10 @@ int sock_set_timestamping(struct sock *sk, int optname,
 	    !(val & SOF_TIMESTAMPING_OPT_ID))
 		return -EINVAL;
 
+	if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
+	    val & SOF_TIMESTAMPING_OPT_RX_FILTER)
+		return -EINVAL;
+
 	if (val & SOF_TIMESTAMPING_OPT_ID &&
 	    !(sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)) {
 		if (sk_is_tcp(sk)) {
diff --git a/net/ethtool/common.c b/net/ethtool/common.c
index 781834ef57c3..6c245e59bbc1 100644
--- a/net/ethtool/common.c
+++ b/net/ethtool/common.c
@@ -427,6 +427,7 @@ const char sof_timestamping_names[][ETH_GSTRING_LEN] = {
 	[const_ilog2(SOF_TIMESTAMPING_OPT_TX_SWHW)]  = "option-tx-swhw",
 	[const_ilog2(SOF_TIMESTAMPING_BIND_PHC)]     = "bind-phc",
 	[const_ilog2(SOF_TIMESTAMPING_OPT_ID_TCP)]   = "option-id-tcp",
+	[const_ilog2(SOF_TIMESTAMPING_OPT_RX_FILTER)] = "option-rx-filter",
 };
 static_assert(ARRAY_SIZE(sof_timestamping_names) == __SOF_TIMESTAMPING_CNT);
 
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 8a5680b4e786..a0c57c8b77bd 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2235,6 +2235,7 @@ void tcp_recv_timestamp(struct msghdr *msg, const struct sock *sk,
 			struct scm_timestamping_internal *tss)
 {
 	int new_tstamp = sock_flag(sk, SOCK_TSTAMP_NEW);
+	u32 tsflags = READ_ONCE(sk->sk_tsflags);
 	bool has_timestamping = false;
 
 	if (tss->ts[0].tv_sec || tss->ts[0].tv_nsec) {
@@ -2274,14 +2275,15 @@ void tcp_recv_timestamp(struct msghdr *msg, const struct sock *sk,
 			}
 		}
 
-		if (READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_SOFTWARE)
+		if (tsflags & SOF_TIMESTAMPING_SOFTWARE &&
+		    !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))
 			has_timestamping = true;
 		else
 			tss->ts[0] = (struct timespec64) {0};
 	}
 
 	if (tss->ts[2].tv_sec || tss->ts[2].tv_nsec) {
-		if (READ_ONCE(sk->sk_tsflags) & SOF_TIMESTAMPING_RAW_HARDWARE)
+		if (tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)
 			has_timestamping = true;
 		else
 			tss->ts[2] = (struct timespec64) {0};
diff --git a/net/socket.c b/net/socket.c
index fcbdd5bc47ac..f8609d649ed3 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -946,7 +946,9 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 
 	memset(&tss, 0, sizeof(tss));
 	tsflags = READ_ONCE(sk->sk_tsflags);
-	if ((tsflags & SOF_TIMESTAMPING_SOFTWARE) &&
+	if ((tsflags & SOF_TIMESTAMPING_SOFTWARE &&
+	     (skb_is_err_queue(skb) ||
+	     !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))) &&
 	    ktime_to_timespec64_cond(skb->tstamp, tss.ts + 0))
 		empty = 0;
 	if (shhwtstamps &&
-- 
2.37.3

Re: [PATCH net-next v4 1/4] net-timestamp: filter out report when setting SOF_TIMESTAMPING_SOFTWARE

[Willem de Bruijn]

Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> introduce a new flag SOF_TIMESTAMPING_OPT_RX_FILTER in the receive
> path. User can set it with SOF_TIMESTAMPING_SOFTWARE to filter
> out rx software timestamp report, especially after a process turns on
> netstamp_needed_key which can time stamp every incoming skb.
> 
> Previously, we found out if an application starts first which turns on
> netstamp_needed_key, then another one only passing SOF_TIMESTAMPING_SOFTWARE
> could also get rx timestamp. Now we handle this case by introducing this
> new flag without breaking users.
> 
> Quoting Willem to explain why we need the flag:
> "why a process would want to request software timestamp reporting, but
> not receive software timestamp generation. The only use I see is when
> the application does request
> SOF_TIMESTAMPING_SOFTWARE | SOF_TIMESTAMPING_TX_SOFTWARE."
> 
> In this way, we have two kinds of combination:
> 1. setting SOF_TIMESTAMPING_SOFTWARE|SOF_TIMESTAMPING_RX_SOFTWARE, it
> will surely allow users to get the rx software timestamp report.
> 2. setting SOF_TIMESTAMPING_SOFTWARE|SOF_TIMESTAMPING_OPT_RX_FILTER
> while the skb is timestamped, it will stop reporting the rx software
> timestamp.
> 
> Another thing about errqueue in this patch I have a few words to say:
> In this case, we need to handle the egress path carefully, or else
> reporting the tx timestamp will fail. Egress path and ingress path will
> finally call sock_recv_timestamp(). We have to distinguish them.
> Errqueue is a good indicator to reflect the flow direction.
> 
> Suggested-by: Willem de Bruijn <willemb@google.com>
> Reviewed-by: Willem de Bruijn <willemb@google.com>

nit: Reviewed-by tags are only sticky if no changes are made.

> diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> index 5e93cd71f99f..37ead02be3b1 100644
> --- a/Documentation/networking/timestamping.rst
> +++ b/Documentation/networking/timestamping.rst
> @@ -266,6 +266,18 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
>    two separate messages will be looped to the socket's error queue,
>    each containing just one timestamp.
>  
> +SOF_TIMESTAMPING_OPT_RX_FILTER:
> +  Used in the receive software timestamp. Enabling the flag along with
> +  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> +  userspace so that it can filter out the case where one process starts
> +  first which turns on netstamp_needed_key through setting generation
> +  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> +  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.

This raises the question: why would a process request
report flag SOF_TIMESTAMPING_SOFTWARE without generate flag
SOF_TIMESTAMPING_RX_SOFTWARE? The only sensible use case I see is when
it sets SOF_TIMSETAMPING_TX_SOFTWARE. Probably good to mention that.

May also be good to mention that existing applications sometimes set
SOF_TIMESTAMPING_SOFTWARE only, because they implicitly came to depend
on another (usually daemon) process to enable rx timestamps systemwide.

> +
> +  SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> +  influenced by others and let the application choose whether to report
> +  the timestamp in the receive path or not.
> +

I'd drop this paragraph. It's more of a value statement.

Re: [PATCH net-next v4 1/4] net-timestamp: filter out report when setting SOF_TIMESTAMPING_SOFTWARE

[Jason Xing]

On Thu, Sep 5, 2024 at 9:37 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > introduce a new flag SOF_TIMESTAMPING_OPT_RX_FILTER in the receive
> > path. User can set it with SOF_TIMESTAMPING_SOFTWARE to filter
> > out rx software timestamp report, especially after a process turns on
> > netstamp_needed_key which can time stamp every incoming skb.
> >
> > Previously, we found out if an application starts first which turns on
> > netstamp_needed_key, then another one only passing SOF_TIMESTAMPING_SOFTWARE
> > could also get rx timestamp. Now we handle this case by introducing this
> > new flag without breaking users.
> >
> > Quoting Willem to explain why we need the flag:
> > "why a process would want to request software timestamp reporting, but
> > not receive software timestamp generation. The only use I see is when
> > the application does request
> > SOF_TIMESTAMPING_SOFTWARE | SOF_TIMESTAMPING_TX_SOFTWARE."
> >
> > In this way, we have two kinds of combination:
> > 1. setting SOF_TIMESTAMPING_SOFTWARE|SOF_TIMESTAMPING_RX_SOFTWARE, it
> > will surely allow users to get the rx software timestamp report.
> > 2. setting SOF_TIMESTAMPING_SOFTWARE|SOF_TIMESTAMPING_OPT_RX_FILTER
> > while the skb is timestamped, it will stop reporting the rx software
> > timestamp.
> >
> > Another thing about errqueue in this patch I have a few words to say:
> > In this case, we need to handle the egress path carefully, or else
> > reporting the tx timestamp will fail. Egress path and ingress path will
> > finally call sock_recv_timestamp(). We have to distinguish them.
> > Errqueue is a good indicator to reflect the flow direction.
> >
> > Suggested-by: Willem de Bruijn <willemb@google.com>
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> nit: Reviewed-by tags are only sticky if no changes are made.

I got it. I will remove it.

>
> > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> > index 5e93cd71f99f..37ead02be3b1 100644
> > --- a/Documentation/networking/timestamping.rst
> > +++ b/Documentation/networking/timestamping.rst
> > @@ -266,6 +266,18 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
> >    two separate messages will be looped to the socket's error queue,
> >    each containing just one timestamp.
> >
> > +SOF_TIMESTAMPING_OPT_RX_FILTER:
> > +  Used in the receive software timestamp. Enabling the flag along with
> > +  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> > +  userspace so that it can filter out the case where one process starts
> > +  first which turns on netstamp_needed_key through setting generation
> > +  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> > +  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
>
> This raises the question: why would a process request
> report flag SOF_TIMESTAMPING_SOFTWARE without generate flag
> SOF_TIMESTAMPING_RX_SOFTWARE? The only sensible use case I see is when
> it sets SOF_TIMSETAMPING_TX_SOFTWARE. Probably good to mention that.
>
> May also be good to mention that existing applications sometimes set
> SOF_TIMESTAMPING_SOFTWARE only, because they implicitly came to depend
> on another (usually daemon) process to enable rx timestamps systemwide.

Much better. Thanks. I will add them too.

>
> > +
> > +  SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> > +  influenced by others and let the application choose whether to report
> > +  the timestamp in the receive path or not.
> > +
>
> I'd drop this paragraph. It's more of a value statement.
>

I see. Will drop it.

Thanks,
Jason

[PATCH net-next v4 2/4] net-timestamp: correct the use of SOF_TIMESTAMPING_RAW_HARDWARE

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

SOF_TIMESTAMPING_RAW_HARDWARE is a report flag which passes the
timestamps generated by either SOF_TIMESTAMPING_TX_HARDWARE or
SOF_TIMESTAMPING_RX_HARDWARE to the userspace all the time.

So let us revise the doc here.

Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
Link: https://lore.kernel.org/all/66d8c21d3042a_163d93294cb@willemb.c.googlers.com.notmuch/
---
 Documentation/networking/timestamping.rst | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
index 37ead02be3b1..ac57d9de2f11 100644
--- a/Documentation/networking/timestamping.rst
+++ b/Documentation/networking/timestamping.rst
@@ -158,7 +158,8 @@ SOF_TIMESTAMPING_SYS_HARDWARE:
 
 SOF_TIMESTAMPING_RAW_HARDWARE:
   Report hardware timestamps as generated by
-  SOF_TIMESTAMPING_TX_HARDWARE when available.
+  SOF_TIMESTAMPING_TX_HARDWARE or SOF_TIMESTAMPING_RX_HARDWARE
+  when available.
 
 
 1.3.3 Timestamp Options
-- 
2.37.3

Re: [PATCH net-next v4 2/4] net-timestamp: correct the use of SOF_TIMESTAMPING_RAW_HARDWARE

[Willem de Bruijn]

Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> SOF_TIMESTAMPING_RAW_HARDWARE is a report flag which passes the
> timestamps generated by either SOF_TIMESTAMPING_TX_HARDWARE or
> SOF_TIMESTAMPING_RX_HARDWARE to the userspace all the time.
> 
> So let us revise the doc here.
> 
> Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

As an exception to the rule, as a one word fix, can be squashed into
the feature patch, I think.

> ---
> Link: https://lore.kernel.org/all/66d8c21d3042a_163d93294cb@willemb.c.googlers.com.notmuch/

Please put these at the top of the Suggested-by/Signed-off-by/..-by
block. Their more useful to future readers than to current followers
of the mailing list.

Re: [PATCH net-next v4 2/4] net-timestamp: correct the use of SOF_TIMESTAMPING_RAW_HARDWARE

[Jason Xing]

On Thu, Sep 5, 2024 at 9:38 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > SOF_TIMESTAMPING_RAW_HARDWARE is a report flag which passes the
> > timestamps generated by either SOF_TIMESTAMPING_TX_HARDWARE or
> > SOF_TIMESTAMPING_RX_HARDWARE to the userspace all the time.
> >
> > So let us revise the doc here.
> >
> > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> As an exception to the rule, as a one word fix, can be squashed into
> the feature patch, I think.
>
> > ---
> > Link: https://lore.kernel.org/all/66d8c21d3042a_163d93294cb@willemb.c.googlers.com.notmuch/
>
> Please put these at the top of the Suggested-by/Signed-off-by/..-by
> block. Their more useful to future readers than to current followers
> of the mailing list.

But I have to ask if it looks a little messy if the first patch will
grow much bigger than before with adding some suggested-by tags and
links, and a separate paragraph to explain why we touch the doc...

Cooking one patch which saves my much energy is surely easier for me
but not for readers probably, I assume.

Thanks,
Jason

Re: [PATCH net-next v4 2/4] net-timestamp: correct the use of SOF_TIMESTAMPING_RAW_HARDWARE

[Willem de Bruijn]

Jason Xing wrote:
> On Thu, Sep 5, 2024 at 9:38 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Jason Xing wrote:
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > SOF_TIMESTAMPING_RAW_HARDWARE is a report flag which passes the
> > > timestamps generated by either SOF_TIMESTAMPING_TX_HARDWARE or
> > > SOF_TIMESTAMPING_RX_HARDWARE to the userspace all the time.
> > >
> > > So let us revise the doc here.
> > >
> > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> >
> > Reviewed-by: Willem de Bruijn <willemb@google.com>
> >
> > As an exception to the rule, as a one word fix, can be squashed into
> > the feature patch, I think.
> >
> > > ---
> > > Link: https://lore.kernel.org/all/66d8c21d3042a_163d93294cb@willemb.c.googlers.com.notmuch/
> >
> > Please put these at the top of the Suggested-by/Signed-off-by/..-by
> > block. Their more useful to future readers than to current followers
> > of the mailing list.
> 
> But I have to ask if it looks a little messy if the first patch will
> grow much bigger than before with adding some suggested-by tags and
> links, and a separate paragraph to explain why we touch the doc...
> 
> Cooking one patch which saves my much energy is surely easier for me
> but not for readers probably, I assume.

Ok. This small patch can even be stand-alone. No need for a series.

Re: [PATCH net-next v4 2/4] net-timestamp: correct the use of SOF_TIMESTAMPING_RAW_HARDWARE

[Jason Xing]

On Thu, Sep 5, 2024 at 10:45 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > On Thu, Sep 5, 2024 at 9:38 PM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > >
> > > > SOF_TIMESTAMPING_RAW_HARDWARE is a report flag which passes the
> > > > timestamps generated by either SOF_TIMESTAMPING_TX_HARDWARE or
> > > > SOF_TIMESTAMPING_RX_HARDWARE to the userspace all the time.
> > > >
> > > > So let us revise the doc here.
> > > >
> > > > Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > >
> > > Reviewed-by: Willem de Bruijn <willemb@google.com>
> > >
> > > As an exception to the rule, as a one word fix, can be squashed into
> > > the feature patch, I think.
> > >
> > > > ---
> > > > Link: https://lore.kernel.org/all/66d8c21d3042a_163d93294cb@willemb.c.googlers.com.notmuch/
> > >
> > > Please put these at the top of the Suggested-by/Signed-off-by/..-by
> > > block. Their more useful to future readers than to current followers
> > > of the mailing list.
> >
> > But I have to ask if it looks a little messy if the first patch will
> > grow much bigger than before with adding some suggested-by tags and
> > links, and a separate paragraph to explain why we touch the doc...
> >
> > Cooking one patch which saves my much energy is surely easier for me
> > but not for readers probably, I assume.
>
> Ok. This small patch can even be stand-alone. No need for a series.
>

Got it. I will send it as a standalone patch tomorrow since I need to
wait more than 24 hours.

[PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

In the previous patch, we found things could happen in the rx software
timestamp. Here, we also noticed that, for rx hardware timestamp case,
it could happen when one process enables the rx hardware timestamp
generating flag first, then another process only setting
SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
timestamp.

In this patch, we extend the OPT_RX_FILTER flag to filter out the
above case for hardware use.

Suggested-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
---
 Documentation/networking/timestamping.rst | 15 +++++++++------
 net/core/sock.c                           |  5 +++--
 net/ipv4/tcp.c                            |  3 ++-
 net/socket.c                              |  3 ++-
 4 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
index ac57d9de2f11..55e79ea71f3e 100644
--- a/Documentation/networking/timestamping.rst
+++ b/Documentation/networking/timestamping.rst
@@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
   each containing just one timestamp.
 
 SOF_TIMESTAMPING_OPT_RX_FILTER:
-  Used in the receive software timestamp. Enabling the flag along with
-  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
-  userspace so that it can filter out the case where one process starts
-  first which turns on netstamp_needed_key through setting generation
-  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
-  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
+  Used in the receive software/hardware timestamp. Enabling the flag
+  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
+  will not report the rx timestamp to the userspace so that it can
+  filter out the cases where 1) one process starts first which turns
+  on netstamp_needed_key through setting generation flags like
+  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
+  generating the hardware timestamp already, then another one only
+  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
+  rx timestamp.
 
   SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
   influenced by others and let the application choose whether to report
diff --git a/net/core/sock.c b/net/core/sock.c
index 6a93344e21cf..dc4a43cfff59 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
 	    !(val & SOF_TIMESTAMPING_OPT_ID))
 		return -EINVAL;
 
-	if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
-	    val & SOF_TIMESTAMPING_OPT_RX_FILTER)
+	if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
+	    (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
+	     val & SOF_TIMESTAMPING_RX_HARDWARE))
 		return -EINVAL;
 
 	if (val & SOF_TIMESTAMPING_OPT_ID &&
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index a0c57c8b77bd..23f0722aa801 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2283,7 +2283,8 @@ void tcp_recv_timestamp(struct msghdr *msg, const struct sock *sk,
 	}
 
 	if (tss->ts[2].tv_sec || tss->ts[2].tv_nsec) {
-		if (tsflags & SOF_TIMESTAMPING_RAW_HARDWARE)
+		if (tsflags & SOF_TIMESTAMPING_RAW_HARDWARE &&
+		    !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER))
 			has_timestamping = true;
 		else
 			tss->ts[2] = (struct timespec64) {0};
diff --git a/net/socket.c b/net/socket.c
index f8609d649ed3..bfbae2069fbb 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -952,7 +952,8 @@ void __sock_recv_timestamp(struct msghdr *msg, struct sock *sk,
 	    ktime_to_timespec64_cond(skb->tstamp, tss.ts + 0))
 		empty = 0;
 	if (shhwtstamps &&
-	    (tsflags & SOF_TIMESTAMPING_RAW_HARDWARE) &&
+	    (tsflags & SOF_TIMESTAMPING_RAW_HARDWARE &&
+	    !(tsflags & SOF_TIMESTAMPING_OPT_RX_FILTER)) &&
 	    !skb_is_swtx_tstamp(skb, false_tstamp)) {
 		if_index = 0;
 		if (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP_NETDEV)
-- 
2.37.3

Re: [PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Willem de Bruijn]

Jason Xing wrote:
> From: Jason Xing <kernelxing@tencent.com>
> 
> In the previous patch, we found things could happen in the rx software
> timestamp. Here, we also noticed that, for rx hardware timestamp case,
> it could happen when one process enables the rx hardware timestamp
> generating flag first, then another process only setting
> SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
> timestamp.
> 
> In this patch, we extend the OPT_RX_FILTER flag to filter out the
> above case for hardware use.
> 
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
> Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
> ---
>  Documentation/networking/timestamping.rst | 15 +++++++++------
>  net/core/sock.c                           |  5 +++--
>  net/ipv4/tcp.c                            |  3 ++-
>  net/socket.c                              |  3 ++-
>  4 files changed, 16 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> index ac57d9de2f11..55e79ea71f3e 100644
> --- a/Documentation/networking/timestamping.rst
> +++ b/Documentation/networking/timestamping.rst
> @@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
>    each containing just one timestamp.
>  
>  SOF_TIMESTAMPING_OPT_RX_FILTER:
> -  Used in the receive software timestamp. Enabling the flag along with
> -  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> -  userspace so that it can filter out the case where one process starts
> -  first which turns on netstamp_needed_key through setting generation
> -  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> -  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
> +  Used in the receive software/hardware timestamp. Enabling the flag
> +  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
> +  will not report the rx timestamp to the userspace so that it can
> +  filter out the cases where 1) one process starts first which turns
> +  on netstamp_needed_key through setting generation flags like
> +  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
> +  generating the hardware timestamp already, then another one only
> +  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
> +  rx timestamp.

I think this patch should be squashed into patch 1.

Else SOF_TIMESTAMPING_OPT_RX_FILTER has two subtly different behaviors
across its lifetime. Even if it is only two SHA1s apart.

It also avoids such duplicate changes to the same code/text blocks.

More importantly, it matters for the behavior, see below.

>  
>    SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
>    influenced by others and let the application choose whether to report
> diff --git a/net/core/sock.c b/net/core/sock.c
> index 6a93344e21cf..dc4a43cfff59 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
>  	    !(val & SOF_TIMESTAMPING_OPT_ID))
>  		return -EINVAL;
>  
> -	if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> -	    val & SOF_TIMESTAMPING_OPT_RX_FILTER)
> +	if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
> +	    (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
> +	     val & SOF_TIMESTAMPING_RX_HARDWARE))
>  		return -EINVAL;

There may be legitimate use cases of wanting to receive hardware
receive timestamps, plus software transmit timestamp, but
suppress spurious software timestamps (or vice versa):

    SOF_TIMESTAMPING_RAW_HARDWARE | \
    SOF_TIMESTAMPING_RX_HARDWARE | \
    SOF_TIMESTAMPING_SOFTWARE | \
    SOF_TIMESTAMPING_TX_SOFTWARE | \
    SOF_TIMESTAMPING_OPT_RX_FILTER

Admittedly this seems a bit contrived. But it's little hassle to
support it?

We just can no longer use the branch simplification that Jakub
pointed out.

Re: [PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Jason Xing]

On Thu, Sep 5, 2024 at 9:45 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > From: Jason Xing <kernelxing@tencent.com>
> >
> > In the previous patch, we found things could happen in the rx software
> > timestamp. Here, we also noticed that, for rx hardware timestamp case,
> > it could happen when one process enables the rx hardware timestamp
> > generating flag first, then another process only setting
> > SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
> > timestamp.
> >
> > In this patch, we extend the OPT_RX_FILTER flag to filter out the
> > above case for hardware use.
> >
> > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > ---
> > Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
> > ---
> >  Documentation/networking/timestamping.rst | 15 +++++++++------
> >  net/core/sock.c                           |  5 +++--
> >  net/ipv4/tcp.c                            |  3 ++-
> >  net/socket.c                              |  3 ++-
> >  4 files changed, 16 insertions(+), 10 deletions(-)
> >
> > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> > index ac57d9de2f11..55e79ea71f3e 100644
> > --- a/Documentation/networking/timestamping.rst
> > +++ b/Documentation/networking/timestamping.rst
> > @@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
> >    each containing just one timestamp.
> >
> >  SOF_TIMESTAMPING_OPT_RX_FILTER:
> > -  Used in the receive software timestamp. Enabling the flag along with
> > -  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> > -  userspace so that it can filter out the case where one process starts
> > -  first which turns on netstamp_needed_key through setting generation
> > -  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> > -  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
> > +  Used in the receive software/hardware timestamp. Enabling the flag
> > +  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
> > +  will not report the rx timestamp to the userspace so that it can
> > +  filter out the cases where 1) one process starts first which turns
> > +  on netstamp_needed_key through setting generation flags like
> > +  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
> > +  generating the hardware timestamp already, then another one only
> > +  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
> > +  rx timestamp.
>
> I think this patch should be squashed into patch 1.
>
> Else SOF_TIMESTAMPING_OPT_RX_FILTER has two subtly different behaviors
> across its lifetime. Even if it is only two SHA1s apart.

I thought about last night as well. Like the patch [2/4] and this
patch, the reason why I wanted to split is because I have to explain a
lot for both hw and sw in one patch. One patch mixes different things.

No strong preference. If you still think so, I definitely can squash
them as you said :)

>
> It also avoids such duplicate changes to the same code/text blocks.
>
> More importantly, it matters for the behavior, see below.
>
> >
> >    SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> >    influenced by others and let the application choose whether to report
> > diff --git a/net/core/sock.c b/net/core/sock.c
> > index 6a93344e21cf..dc4a43cfff59 100644
> > --- a/net/core/sock.c
> > +++ b/net/core/sock.c
> > @@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
> >           !(val & SOF_TIMESTAMPING_OPT_ID))
> >               return -EINVAL;
> >
> > -     if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> > -         val & SOF_TIMESTAMPING_OPT_RX_FILTER)
> > +     if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
> > +         (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > +          val & SOF_TIMESTAMPING_RX_HARDWARE))
> >               return -EINVAL;
>
> There may be legitimate use cases of wanting to receive hardware
> receive timestamps, plus software transmit timestamp, but
> suppress spurious software timestamps (or vice versa):
>
>     SOF_TIMESTAMPING_RAW_HARDWARE | \
>     SOF_TIMESTAMPING_RX_HARDWARE | \
>     SOF_TIMESTAMPING_SOFTWARE | \
>     SOF_TIMESTAMPING_TX_SOFTWARE | \
>     SOF_TIMESTAMPING_OPT_RX_FILTER

Oh, right, it can happen! RAW_HARDWARE is a little bit different,
covering both ingress and egress path.

>
> Admittedly this seems a bit contrived. But it's little hassle to
> support it?
>
> We just can no longer use the branch simplification that Jakub
> pointed out.
>

I see. I'm going to do two things as you said:
1) restore the simplification branch
2) only take care of software case in sock_set_timestamping()

Thanks for pointing this out!

Re: [PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Willem de Bruijn]

Jason Xing wrote:
> On Thu, Sep 5, 2024 at 9:45 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Jason Xing wrote:
> > > From: Jason Xing <kernelxing@tencent.com>
> > >
> > > In the previous patch, we found things could happen in the rx software
> > > timestamp. Here, we also noticed that, for rx hardware timestamp case,
> > > it could happen when one process enables the rx hardware timestamp
> > > generating flag first, then another process only setting
> > > SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
> > > timestamp.
> > >
> > > In this patch, we extend the OPT_RX_FILTER flag to filter out the
> > > above case for hardware use.
> > >
> > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > ---
> > > Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
> > > ---
> > >  Documentation/networking/timestamping.rst | 15 +++++++++------
> > >  net/core/sock.c                           |  5 +++--
> > >  net/ipv4/tcp.c                            |  3 ++-
> > >  net/socket.c                              |  3 ++-
> > >  4 files changed, 16 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> > > index ac57d9de2f11..55e79ea71f3e 100644
> > > --- a/Documentation/networking/timestamping.rst
> > > +++ b/Documentation/networking/timestamping.rst
> > > @@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
> > >    each containing just one timestamp.
> > >
> > >  SOF_TIMESTAMPING_OPT_RX_FILTER:
> > > -  Used in the receive software timestamp. Enabling the flag along with
> > > -  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> > > -  userspace so that it can filter out the case where one process starts
> > > -  first which turns on netstamp_needed_key through setting generation
> > > -  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> > > -  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
> > > +  Used in the receive software/hardware timestamp. Enabling the flag
> > > +  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
> > > +  will not report the rx timestamp to the userspace so that it can
> > > +  filter out the cases where 1) one process starts first which turns
> > > +  on netstamp_needed_key through setting generation flags like
> > > +  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
> > > +  generating the hardware timestamp already, then another one only
> > > +  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
> > > +  rx timestamp.
> >
> > I think this patch should be squashed into patch 1.
> >
> > Else SOF_TIMESTAMPING_OPT_RX_FILTER has two subtly different behaviors
> > across its lifetime. Even if it is only two SHA1s apart.
> 
> I thought about last night as well. Like the patch [2/4] and this
> patch, the reason why I wanted to split is because I have to explain a
> lot for both hw and sw in one patch. One patch mixes different things.
> 
> No strong preference. If you still think so, I definitely can squash
> them as you said :)

No strong preference on 2/4. See other reply.

In this case, patch 1/4 introduces some behavior and 3/4 immediately
updates it. I think it makes more sense to combine them.

> >
> > It also avoids such duplicate changes to the same code/text blocks.
> >
> > More importantly, it matters for the behavior, see below.
> >
> > >
> > >    SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> > >    influenced by others and let the application choose whether to report
> > > diff --git a/net/core/sock.c b/net/core/sock.c
> > > index 6a93344e21cf..dc4a43cfff59 100644
> > > --- a/net/core/sock.c
> > > +++ b/net/core/sock.c
> > > @@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
> > >           !(val & SOF_TIMESTAMPING_OPT_ID))
> > >               return -EINVAL;
> > >
> > > -     if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> > > -         val & SOF_TIMESTAMPING_OPT_RX_FILTER)
> > > +     if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
> > > +         (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > > +          val & SOF_TIMESTAMPING_RX_HARDWARE))
> > >               return -EINVAL;
> >
> > There may be legitimate use cases of wanting to receive hardware
> > receive timestamps, plus software transmit timestamp, but
> > suppress spurious software timestamps (or vice versa):
> >
> >     SOF_TIMESTAMPING_RAW_HARDWARE | \
> >     SOF_TIMESTAMPING_RX_HARDWARE | \
> >     SOF_TIMESTAMPING_SOFTWARE | \
> >     SOF_TIMESTAMPING_TX_SOFTWARE | \
> >     SOF_TIMESTAMPING_OPT_RX_FILTER
> 
> Oh, right, it can happen! RAW_HARDWARE is a little bit different,
> covering both ingress and egress path.

As said, it is a bit contrived. Feel free to disagree and keep as is
too.
 
> >
> > Admittedly this seems a bit contrived. But it's little hassle to
> > support it?
> >
> > We just can no longer use the branch simplification that Jakub
> > pointed out.
> >
> 
> I see. I'm going to do two things as you said:
> 1) restore the simplification branch
> 2) only take care of software case in sock_set_timestamping()
> 
> Thanks for pointing this out!

Re: [PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Jason Xing]

On Thu, Sep 5, 2024 at 10:46 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > On Thu, Sep 5, 2024 at 9:45 PM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > Jason Xing wrote:
> > > > From: Jason Xing <kernelxing@tencent.com>
> > > >
> > > > In the previous patch, we found things could happen in the rx software
> > > > timestamp. Here, we also noticed that, for rx hardware timestamp case,
> > > > it could happen when one process enables the rx hardware timestamp
> > > > generating flag first, then another process only setting
> > > > SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
> > > > timestamp.
> > > >
> > > > In this patch, we extend the OPT_RX_FILTER flag to filter out the
> > > > above case for hardware use.
> > > >
> > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > ---
> > > > Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
> > > > ---
> > > >  Documentation/networking/timestamping.rst | 15 +++++++++------
> > > >  net/core/sock.c                           |  5 +++--
> > > >  net/ipv4/tcp.c                            |  3 ++-
> > > >  net/socket.c                              |  3 ++-
> > > >  4 files changed, 16 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> > > > index ac57d9de2f11..55e79ea71f3e 100644
> > > > --- a/Documentation/networking/timestamping.rst
> > > > +++ b/Documentation/networking/timestamping.rst
> > > > @@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
> > > >    each containing just one timestamp.
> > > >
> > > >  SOF_TIMESTAMPING_OPT_RX_FILTER:
> > > > -  Used in the receive software timestamp. Enabling the flag along with
> > > > -  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> > > > -  userspace so that it can filter out the case where one process starts
> > > > -  first which turns on netstamp_needed_key through setting generation
> > > > -  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> > > > -  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
> > > > +  Used in the receive software/hardware timestamp. Enabling the flag
> > > > +  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
> > > > +  will not report the rx timestamp to the userspace so that it can
> > > > +  filter out the cases where 1) one process starts first which turns
> > > > +  on netstamp_needed_key through setting generation flags like
> > > > +  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
> > > > +  generating the hardware timestamp already, then another one only
> > > > +  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
> > > > +  rx timestamp.
> > >
> > > I think this patch should be squashed into patch 1.
> > >
> > > Else SOF_TIMESTAMPING_OPT_RX_FILTER has two subtly different behaviors
> > > across its lifetime. Even if it is only two SHA1s apart.
> >
> > I thought about last night as well. Like the patch [2/4] and this
> > patch, the reason why I wanted to split is because I have to explain a
> > lot for both hw and sw in one patch. One patch mixes different things.
> >
> > No strong preference. If you still think so, I definitely can squash
> > them as you said :)
>
> No strong preference on 2/4. See other reply.
>
> In this case, patch 1/4 introduces some behavior and 3/4 immediately
> updates it. I think it makes more sense to combine them.

Roger that. Will squash this one:)

>
> > >
> > > It also avoids such duplicate changes to the same code/text blocks.
> > >
> > > More importantly, it matters for the behavior, see below.
> > >
> > > >
> > > >    SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> > > >    influenced by others and let the application choose whether to report
> > > > diff --git a/net/core/sock.c b/net/core/sock.c
> > > > index 6a93344e21cf..dc4a43cfff59 100644
> > > > --- a/net/core/sock.c
> > > > +++ b/net/core/sock.c
> > > > @@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
> > > >           !(val & SOF_TIMESTAMPING_OPT_ID))
> > > >               return -EINVAL;
> > > >
> > > > -     if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> > > > -         val & SOF_TIMESTAMPING_OPT_RX_FILTER)
> > > > +     if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
> > > > +         (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > > > +          val & SOF_TIMESTAMPING_RX_HARDWARE))
> > > >               return -EINVAL;
> > >
> > > There may be legitimate use cases of wanting to receive hardware
> > > receive timestamps, plus software transmit timestamp, but
> > > suppress spurious software timestamps (or vice versa):
> > >
> > >     SOF_TIMESTAMPING_RAW_HARDWARE | \
> > >     SOF_TIMESTAMPING_RX_HARDWARE | \
> > >     SOF_TIMESTAMPING_SOFTWARE | \
> > >     SOF_TIMESTAMPING_TX_SOFTWARE | \
> > >     SOF_TIMESTAMPING_OPT_RX_FILTER

Sorry, I think my initial understanding at first read is not right. I
was thinking you want this combination to pass the check in
sock_set_timestamping().

If the users insist on receiving "hardware receive timestamps" with
OPT_RX_FILTER enabled in this case, I think we should implement
another new flag, say, OPT_RX_HARDWARE_FILTER...

> >
> > Oh, right, it can happen! RAW_HARDWARE is a little bit different,
> > covering both ingress and egress path.
>
> As said, it is a bit contrived. Feel free to disagree and keep as is
> too.

Well, I can keep it as is. It's easy for me, saving much energy,
but...you already pointed out/ noticed this kind of use case that is
not invalid.

If we want to tackle it well, we need to add a new flag for the
hardware case, then we can individually control each of them, which is
a more fine-grained control honestly. I'm totally fine with it as long
as it will be good for users in the long run :)

If so, adding a new patch into this series (like patch [3/4]) seems
inevitable. It won't take much time, I feel.

Any further thoughts?

Thanks,
Jason

Re: [PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Willem de Bruijn]

Jason Xing wrote:
> On Thu, Sep 5, 2024 at 10:46 PM Willem de Bruijn
> <willemdebruijn.kernel@gmail.com> wrote:
> >
> > Jason Xing wrote:
> > > On Thu, Sep 5, 2024 at 9:45 PM Willem de Bruijn
> > > <willemdebruijn.kernel@gmail.com> wrote:
> > > >
> > > > Jason Xing wrote:
> > > > > From: Jason Xing <kernelxing@tencent.com>
> > > > >
> > > > > In the previous patch, we found things could happen in the rx software
> > > > > timestamp. Here, we also noticed that, for rx hardware timestamp case,
> > > > > it could happen when one process enables the rx hardware timestamp
> > > > > generating flag first, then another process only setting
> > > > > SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
> > > > > timestamp.
> > > > >
> > > > > In this patch, we extend the OPT_RX_FILTER flag to filter out the
> > > > > above case for hardware use.
> > > > >
> > > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > > ---
> > > > > Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
> > > > > ---
> > > > >  Documentation/networking/timestamping.rst | 15 +++++++++------
> > > > >  net/core/sock.c                           |  5 +++--
> > > > >  net/ipv4/tcp.c                            |  3 ++-
> > > > >  net/socket.c                              |  3 ++-
> > > > >  4 files changed, 16 insertions(+), 10 deletions(-)
> > > > >
> > > > > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> > > > > index ac57d9de2f11..55e79ea71f3e 100644
> > > > > --- a/Documentation/networking/timestamping.rst
> > > > > +++ b/Documentation/networking/timestamping.rst
> > > > > @@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
> > > > >    each containing just one timestamp.
> > > > >
> > > > >  SOF_TIMESTAMPING_OPT_RX_FILTER:
> > > > > -  Used in the receive software timestamp. Enabling the flag along with
> > > > > -  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> > > > > -  userspace so that it can filter out the case where one process starts
> > > > > -  first which turns on netstamp_needed_key through setting generation
> > > > > -  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> > > > > -  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
> > > > > +  Used in the receive software/hardware timestamp. Enabling the flag
> > > > > +  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
> > > > > +  will not report the rx timestamp to the userspace so that it can
> > > > > +  filter out the cases where 1) one process starts first which turns
> > > > > +  on netstamp_needed_key through setting generation flags like
> > > > > +  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
> > > > > +  generating the hardware timestamp already, then another one only
> > > > > +  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
> > > > > +  rx timestamp.
> > > >
> > > > I think this patch should be squashed into patch 1.
> > > >
> > > > Else SOF_TIMESTAMPING_OPT_RX_FILTER has two subtly different behaviors
> > > > across its lifetime. Even if it is only two SHA1s apart.
> > >
> > > I thought about last night as well. Like the patch [2/4] and this
> > > patch, the reason why I wanted to split is because I have to explain a
> > > lot for both hw and sw in one patch. One patch mixes different things.
> > >
> > > No strong preference. If you still think so, I definitely can squash
> > > them as you said :)
> >
> > No strong preference on 2/4. See other reply.
> >
> > In this case, patch 1/4 introduces some behavior and 3/4 immediately
> > updates it. I think it makes more sense to combine them.
> 
> Roger that. Will squash this one:)
> 
> >
> > > >
> > > > It also avoids such duplicate changes to the same code/text blocks.
> > > >
> > > > More importantly, it matters for the behavior, see below.
> > > >
> > > > >
> > > > >    SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> > > > >    influenced by others and let the application choose whether to report
> > > > > diff --git a/net/core/sock.c b/net/core/sock.c
> > > > > index 6a93344e21cf..dc4a43cfff59 100644
> > > > > --- a/net/core/sock.c
> > > > > +++ b/net/core/sock.c
> > > > > @@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
> > > > >           !(val & SOF_TIMESTAMPING_OPT_ID))
> > > > >               return -EINVAL;
> > > > >
> > > > > -     if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> > > > > -         val & SOF_TIMESTAMPING_OPT_RX_FILTER)
> > > > > +     if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
> > > > > +         (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > > > > +          val & SOF_TIMESTAMPING_RX_HARDWARE))
> > > > >               return -EINVAL;
> > > >
> > > > There may be legitimate use cases of wanting to receive hardware
> > > > receive timestamps, plus software transmit timestamp, but
> > > > suppress spurious software timestamps (or vice versa):
> > > >
> > > >     SOF_TIMESTAMPING_RAW_HARDWARE | \
> > > >     SOF_TIMESTAMPING_RX_HARDWARE | \
> > > >     SOF_TIMESTAMPING_SOFTWARE | \
> > > >     SOF_TIMESTAMPING_TX_SOFTWARE | \
> > > >     SOF_TIMESTAMPING_OPT_RX_FILTER
> 
> Sorry, I think my initial understanding at first read is not right. I
> was thinking you want this combination to pass the check in
> sock_set_timestamping().
> 
> If the users insist on receiving "hardware receive timestamps" with
> OPT_RX_FILTER enabled in this case, I think we should implement
> another new flag, say, OPT_RX_HARDWARE_FILTER...

My interpretation of the OPT_RX_FILTER flag is:

Only return RX timestamps if the socket also has the corresponding
reporting flag set.

So it is valid to have

  SOF_TIMESTAMPING_RAW_HARDWARE |
  SOF_TIMESTAMPING_RX_HARDWARE |
  SOF_TIMESTAMPING_SOFTWARE |
  SOF_TIMESTAMPING_OPT_RX_FILTER

To filter out the software rx timestamps, but let through the
hardware rx timestamps.

> 
> > >
> > > Oh, right, it can happen! RAW_HARDWARE is a little bit different,
> > > covering both ingress and egress path.
> >
> > As said, it is a bit contrived. Feel free to disagree and keep as is
> > too.
> 
> Well, I can keep it as is. It's easy for me, saving much energy,
> but...you already pointed out/ noticed this kind of use case that is
> not invalid.
> 
> If we want to tackle it well, we need to add a new flag for the
> hardware case, then we can individually control each of them, which is
> a more fine-grained control honestly. I'm totally fine with it as long
> as it will be good for users in the long run :)
> 
> If so, adding a new patch into this series (like patch [3/4]) seems
> inevitable. It won't take much time, I feel.
> 
> Any further thoughts?
> 
> Thanks,
> Jason

Re: [PATCH net-next v4 3/4] net-timestamp: extend SOF_TIMESTAMPING_OPT_RX_FILTER for hardware use

[Jason Xing]

On Fri, Sep 6, 2024 at 12:41 AM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Jason Xing wrote:
> > On Thu, Sep 5, 2024 at 10:46 PM Willem de Bruijn
> > <willemdebruijn.kernel@gmail.com> wrote:
> > >
> > > Jason Xing wrote:
> > > > On Thu, Sep 5, 2024 at 9:45 PM Willem de Bruijn
> > > > <willemdebruijn.kernel@gmail.com> wrote:
> > > > >
> > > > > Jason Xing wrote:
> > > > > > From: Jason Xing <kernelxing@tencent.com>
> > > > > >
> > > > > > In the previous patch, we found things could happen in the rx software
> > > > > > timestamp. Here, we also noticed that, for rx hardware timestamp case,
> > > > > > it could happen when one process enables the rx hardware timestamp
> > > > > > generating flag first, then another process only setting
> > > > > > SOF_TIMESTAMPING_RAW_HARDWARE report flag can still get the hardware
> > > > > > timestamp.
> > > > > >
> > > > > > In this patch, we extend the OPT_RX_FILTER flag to filter out the
> > > > > > above case for hardware use.
> > > > > >
> > > > > > Suggested-by: Jakub Kicinski <kuba@kernel.org>
> > > > > > Signed-off-by: Jason Xing <kernelxing@tencent.com>
> > > > > > ---
> > > > > > Link: https://lore.kernel.org/all/20240903121940.6390b958@kernel.org/
> > > > > > ---
> > > > > >  Documentation/networking/timestamping.rst | 15 +++++++++------
> > > > > >  net/core/sock.c                           |  5 +++--
> > > > > >  net/ipv4/tcp.c                            |  3 ++-
> > > > > >  net/socket.c                              |  3 ++-
> > > > > >  4 files changed, 16 insertions(+), 10 deletions(-)
> > > > > >
> > > > > > diff --git a/Documentation/networking/timestamping.rst b/Documentation/networking/timestamping.rst
> > > > > > index ac57d9de2f11..55e79ea71f3e 100644
> > > > > > --- a/Documentation/networking/timestamping.rst
> > > > > > +++ b/Documentation/networking/timestamping.rst
> > > > > > @@ -268,12 +268,15 @@ SOF_TIMESTAMPING_OPT_TX_SWHW:
> > > > > >    each containing just one timestamp.
> > > > > >
> > > > > >  SOF_TIMESTAMPING_OPT_RX_FILTER:
> > > > > > -  Used in the receive software timestamp. Enabling the flag along with
> > > > > > -  SOF_TIMESTAMPING_SOFTWARE will not report the rx timestamp to the
> > > > > > -  userspace so that it can filter out the case where one process starts
> > > > > > -  first which turns on netstamp_needed_key through setting generation
> > > > > > -  flags like SOF_TIMESTAMPING_RX_SOFTWARE, then another one only passing
> > > > > > -  SOF_TIMESTAMPING_SOFTWARE report flag could also get the rx timestamp.
> > > > > > +  Used in the receive software/hardware timestamp. Enabling the flag
> > > > > > +  along with SOF_TIMESTAMPING_SOFTWARE/SOF_TIMESTAMPING_RAW_HARDWARE
> > > > > > +  will not report the rx timestamp to the userspace so that it can
> > > > > > +  filter out the cases where 1) one process starts first which turns
> > > > > > +  on netstamp_needed_key through setting generation flags like
> > > > > > +  SOF_TIMESTAMPING_RX_SOFTWARE, or 2) similarly one process enables
> > > > > > +  generating the hardware timestamp already, then another one only
> > > > > > +  passing SOF_TIMESTAMPING_SOFTWARE report flag could also get the
> > > > > > +  rx timestamp.
> > > > >
> > > > > I think this patch should be squashed into patch 1.
> > > > >
> > > > > Else SOF_TIMESTAMPING_OPT_RX_FILTER has two subtly different behaviors
> > > > > across its lifetime. Even if it is only two SHA1s apart.
> > > >
> > > > I thought about last night as well. Like the patch [2/4] and this
> > > > patch, the reason why I wanted to split is because I have to explain a
> > > > lot for both hw and sw in one patch. One patch mixes different things.
> > > >
> > > > No strong preference. If you still think so, I definitely can squash
> > > > them as you said :)
> > >
> > > No strong preference on 2/4. See other reply.
> > >
> > > In this case, patch 1/4 introduces some behavior and 3/4 immediately
> > > updates it. I think it makes more sense to combine them.
> >
> > Roger that. Will squash this one:)
> >
> > >
> > > > >
> > > > > It also avoids such duplicate changes to the same code/text blocks.
> > > > >
> > > > > More importantly, it matters for the behavior, see below.
> > > > >
> > > > > >
> > > > > >    SOF_TIMESTAMPING_OPT_RX_FILTER prevents the application from being
> > > > > >    influenced by others and let the application choose whether to report
> > > > > > diff --git a/net/core/sock.c b/net/core/sock.c
> > > > > > index 6a93344e21cf..dc4a43cfff59 100644
> > > > > > --- a/net/core/sock.c
> > > > > > +++ b/net/core/sock.c
> > > > > > @@ -908,8 +908,9 @@ int sock_set_timestamping(struct sock *sk, int optname,
> > > > > >           !(val & SOF_TIMESTAMPING_OPT_ID))
> > > > > >               return -EINVAL;
> > > > > >
> > > > > > -     if (val & SOF_TIMESTAMPING_RX_SOFTWARE &&
> > > > > > -         val & SOF_TIMESTAMPING_OPT_RX_FILTER)
> > > > > > +     if (val & SOF_TIMESTAMPING_OPT_RX_FILTER &&
> > > > > > +         (val & SOF_TIMESTAMPING_RX_SOFTWARE ||
> > > > > > +          val & SOF_TIMESTAMPING_RX_HARDWARE))
> > > > > >               return -EINVAL;
> > > > >
> > > > > There may be legitimate use cases of wanting to receive hardware
> > > > > receive timestamps, plus software transmit timestamp, but
> > > > > suppress spurious software timestamps (or vice versa):
> > > > >
> > > > >     SOF_TIMESTAMPING_RAW_HARDWARE | \
> > > > >     SOF_TIMESTAMPING_RX_HARDWARE | \
> > > > >     SOF_TIMESTAMPING_SOFTWARE | \
> > > > >     SOF_TIMESTAMPING_TX_SOFTWARE | \
> > > > >     SOF_TIMESTAMPING_OPT_RX_FILTER
> >
> > Sorry, I think my initial understanding at first read is not right. I
> > was thinking you want this combination to pass the check in
> > sock_set_timestamping().
> >
> > If the users insist on receiving "hardware receive timestamps" with
> > OPT_RX_FILTER enabled in this case, I think we should implement
> > another new flag, say, OPT_RX_HARDWARE_FILTER...
>
> My interpretation of the OPT_RX_FILTER flag is:
>
> Only return RX timestamps if the socket also has the corresponding
> reporting flag set.
>
> So it is valid to have
>
>   SOF_TIMESTAMPING_RAW_HARDWARE |
>   SOF_TIMESTAMPING_RX_HARDWARE |
>   SOF_TIMESTAMPING_SOFTWARE |
>   SOF_TIMESTAMPING_OPT_RX_FILTER
>
> To filter out the software rx timestamps, but let through the
> hardware rx timestamps.

I see. Thanks for your advice.

If both SOF_TIMESTAMPING_RX_SOFTWARE and
SOF_TIMESTAMPING_OPT_RX_FILTER are set at once, the latter will not
take any effect.

I will 1) completely remove the limitation in sock_set_timestamping(),
2) restore those simplification branches.

[PATCH net-next v4 4/4] rxtimestamp.c: add the test for SOF_TIMESTAMPING_OPT_RX_FILTER

[Jason Xing]

From: Jason Xing <kernelxing@tencent.com>

Test when we use SOF_TIMESTAMPING_OPT_RX_FILTER with software
or hardware report flag. The expected result is no rx timestamp
report.

Reviewed-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: Jason Xing <kernelxing@tencent.com>
---
 tools/testing/selftests/net/rxtimestamp.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/tools/testing/selftests/net/rxtimestamp.c b/tools/testing/selftests/net/rxtimestamp.c
index 9eb42570294d..9760abdb6e05 100644
--- a/tools/testing/selftests/net/rxtimestamp.c
+++ b/tools/testing/selftests/net/rxtimestamp.c
@@ -57,6 +57,7 @@ static struct sof_flag sof_flags[] = {
 	SOF_FLAG(SOF_TIMESTAMPING_SOFTWARE),
 	SOF_FLAG(SOF_TIMESTAMPING_RX_SOFTWARE),
 	SOF_FLAG(SOF_TIMESTAMPING_RX_HARDWARE),
+	SOF_FLAG(SOF_TIMESTAMPING_OPT_RX_FILTER),
 };
 
 static struct socket_type socket_types[] = {
@@ -97,6 +98,16 @@ static struct test_case test_cases[] = {
 			| SOF_TIMESTAMPING_RX_HARDWARE },
 		{}
 	},
+	{
+		{ .so_timestamping = SOF_TIMESTAMPING_RAW_HARDWARE
+			| SOF_TIMESTAMPING_OPT_RX_FILTER },
+		{}
+	},
+	{
+		{ .so_timestamping = SOF_TIMESTAMPING_SOFTWARE
+			| SOF_TIMESTAMPING_OPT_RX_FILTER },
+		{}
+	},
 	{
 		{ .so_timestamping = SOF_TIMESTAMPING_SOFTWARE
 			| SOF_TIMESTAMPING_RX_SOFTWARE },
-- 
2.37.3

Re: [PATCH net-next v4 4/4] rxtimestamp.c: add the test for SOF_TIMESTAMPING_OPT_RX_FILTER

[Jason Xing]

On Thu, Sep 5, 2024 at 3:18 PM Jason Xing <kerneljasonxing@gmail.com> wrote:
>
> From: Jason Xing <kernelxing@tencent.com>
>
> Test when we use SOF_TIMESTAMPING_OPT_RX_FILTER with software
> or hardware report flag. The expected result is no rx timestamp
> report.
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
> Signed-off-by: Jason Xing <kernelxing@tencent.com>
> ---
>  tools/testing/selftests/net/rxtimestamp.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/tools/testing/selftests/net/rxtimestamp.c b/tools/testing/selftests/net/rxtimestamp.c
> index 9eb42570294d..9760abdb6e05 100644
> --- a/tools/testing/selftests/net/rxtimestamp.c
> +++ b/tools/testing/selftests/net/rxtimestamp.c
> @@ -57,6 +57,7 @@ static struct sof_flag sof_flags[] = {
>         SOF_FLAG(SOF_TIMESTAMPING_SOFTWARE),
>         SOF_FLAG(SOF_TIMESTAMPING_RX_SOFTWARE),
>         SOF_FLAG(SOF_TIMESTAMPING_RX_HARDWARE),
> +       SOF_FLAG(SOF_TIMESTAMPING_OPT_RX_FILTER),

Ah.. I missed adding SOF_TIMESTAMPING_RAW_HARDWARE :S

I'll repost it in 24 hour.

>  };
>
>  static struct socket_type socket_types[] = {
> @@ -97,6 +98,16 @@ static struct test_case test_cases[] = {
>                         | SOF_TIMESTAMPING_RX_HARDWARE },
>                 {}
>         },
> +       {
> +               { .so_timestamping = SOF_TIMESTAMPING_RAW_HARDWARE
> +                       | SOF_TIMESTAMPING_OPT_RX_FILTER },
> +               {}
> +       },
> +       {
> +               { .so_timestamping = SOF_TIMESTAMPING_SOFTWARE
> +                       | SOF_TIMESTAMPING_OPT_RX_FILTER },
> +               {}
> +       },
>         {
>                 { .so_timestamping = SOF_TIMESTAMPING_SOFTWARE
>                         | SOF_TIMESTAMPING_RX_SOFTWARE },
> --
> 2.37.3
>