[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+FuTSeJCZ1F3b9rrLpdcp6sbok8OXBA40jSmtxbJ7cnQayr+w@mail.gmail.com>
Date: Fri, 25 Mar 2022 09:48:41 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: Vladimir Oltean <olteanv@...il.com>
Cc: Willem de Bruijn <willemdebruijn.kernel@...il.com>,
netdev@...r.kernel.org, Paolo Abeni <pabeni@...hat.com>,
Jakub Kicinski <kuba@...nel.org>,
"David S. Miller" <davem@...emloft.net>
Subject: Re: Broken SOF_TIMESTAMPING_OPT_ID in linux-4.19.y and earlier stable branches
On Fri, Mar 25, 2022 at 9:37 AM Vladimir Oltean <olteanv@...il.com> wrote:
>
> 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@...il.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@...gle.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@...gle.com>
> > > | Acked-by: Soheil Hassas Yeganeh <soheil@...gle.com>
> > > | Signed-off-by: David S. Miller <davem@...emloft.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.
The second patch does not apply cleanly to 4.14.y and even the first
(one-liner) has a conflict on 4.9.y.
It would be good to verify by running the expanded
tools/testing/selftests/net/txtimestamp.c against the patched kernels
first. That should serve as a good test whether the feature works on a
kernel, re: that previous point.
If you want to test and send the 4.19.y patch, please go ahead. Or I
can do it, but it will take some time.
Powered by blists - more mailing lists