Jason Xing wrote: > From: Jason Xing <kernelxing@xxxxxxxxxxx> > > 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@xxxxxxxxxx> > Signed-off-by: Jason Xing <kernelxing@xxxxxxxxxxx> > --- > Link: https://lore.kernel.org/all/20240903121940.6390b958@xxxxxxxxxx/ > --- > 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.
