On Thu, Sep 5, 2024 at 9:37 PM Willem de Bruijn <willemdebruijn.kernel@xxxxxxxxx> wrote: > > Jason Xing wrote: > > From: Jason Xing <kernelxing@xxxxxxxxxxx> > > > > 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@xxxxxxxxxx> > > Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx> > > 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
