Re: Broken SOF_TIMESTAMPING_OPT_ID in linux-4.19.y and earlier stable branches

[Vladimir Oltean <olteanv@gmail.com>]

Hi Willem,

Thanks for the reply.

On Fri, Mar 25, 2022 at 09:15:30AM -0400, Willem de Bruijn wrote:
> On Thu, Mar 24, 2022 at 5:43 PM Vladimir Oltean <olteanv@gmail.com> wrote:
> >
> > Hello Willem,
> >
> > I have an application which makes use of SOF_TIMESTAMPING_OPT_ID, and I
> > received reports from multiple users that all timestamps are delivered
> > with a tskey of 0 for all stable kernel branches earlier than, and
> > including, 4.19.
> >
> > I bisected this issue down to:
> >
> > | commit 8f932f762e7928d250e21006b00ff9b7718b0a64 (HEAD)
> > | Author: Willem de Bruijn <willemb@google.com>
> > | Date:   Mon Dec 17 12:24:00 2018 -0500
> > |
> > |     net: add missing SOF_TIMESTAMPING_OPT_ID support
> > |
> > |     SOF_TIMESTAMPING_OPT_ID is supported on TCP, UDP and RAW sockets.
> > |     But it was missing on RAW with IPPROTO_IP, PF_PACKET and CAN.
> > |
> > |     Add skb_setup_tx_timestamp that configures both tx_flags and tskey
> > |     for these paths that do not need corking or use bytestream keys.
> > |
> > |     Fixes: 09c2d251b707 ("net-timestamp: add key to disambiguate concurrent datagrams")
> > |     Signed-off-by: Willem de Bruijn <willemb@google.com>
> > |     Acked-by: Soheil Hassas Yeganeh <soheil@google.com>
> > |     Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > and, interestingly, I found this discussion on the topic:
> > https://www.spinics.net/lists/netdev/msg540752.html
> > (copied here in case the link rots in the future)
> >
> > | > Series applied.
> > | >
> > | > What is your opinion about -stable for this?
> > |
> > | Thanks David. Since these are just missing features that no one has
> > | reported as actually having been missing a whole lot, I don't think
> > | that they are worth the effort or risk.
> >
> > So I have 2 questions:
> >
> > Is there a way for user space to validate functional kernel support for
> > SOF_TIMESTAMPING_OPT_ID? What I'm noticing is that (at least with
> > AF_PACKET sockets) the "level == SOL_PACKET && type == PACKET_TX_TIMESTAMP"
> > cmsg is _not_ missing, but instead contains a valid sock_err->ee_data
> > (tskey) of 0.
> 
> The commit only fixes missing OPT_ID support for PF_PACKET and various SOCK_RAW.
> 
> The cmsg structure returned for timestamps is the same regardless of
> whether the option is set configured. It just uses an otherwise constant field.
> 
> On these kernels the feature is supported, and should work on TCP and UDP.
> So a feature check would give the wrong answer.

Ok, I read this as "user space can't detect whether OPT_ID works on PF_PACKET sockets",
except by retroactively looking at the tskeys, and if they're all zero, say
"hmm, something's not right". Pretty complicated.

So we probably need to fix the stable kernels. For the particular case
of my application, I have just about zero control of what kernel the
users are running, so the more stable branches we could cover, the better.

> > If it's not possible, could you please consider sending these fixes as
> > patches to linux-stable?
> 
> The first of the two fixes
> 
>     fbfb2321e9509 ("ipv6: add missing tx timestamping on IPPROTO_RAW")
> 
> is in 4.19.y as of 4.19.99
> 
> The follow-on fix that you want
> 
>     8f932f762e79 ("net: add missing SOF_TIMESTAMPING_OPT_ID support")
> 
> applies cleanly to 4.19.236.
> 
> I think it's fine to cherry-pick. Not sure how to go about that.

Do you have any particular concerns about sending this patch to the
linux-stable branches for 4.19, 4.14 and 4.9? From https://www.kernel.org/
I see those are the only stable branches left.