Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
Hi Florian, On Mon, Jul 24, 2023 at 01:08:16PM +0200, Florian Bezdeka wrote: > On Thu, 2023-05-25 at 15:55 +0200, Salvatore Bonaccorso wrote: > > > > > Thanks for confirming, so this change can go into the master branch > > first. > > I expected to have this bug fixed in the next point release, which was > released recently. The "version graph" visible at [1] (upper right > corner) mentions the 6.1.38 release as well but it seems the kernel > configuration file stayed unmodified. > > Any additional action expected from my side? Right, now that the change is in testing, we can pick it up as well for bookworm, which got almost forgotten, thanks for your ping. I picked it up https://salsa.debian.org/kernel-team/linux/-/commit/81db00d5a849c316aeb02311f15dbd0640ca3302 which means if no surprises arises, it will be in 12.2. Regards, Salvatore
Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
Control: tags -1 - moreinfo Control: tags -1 + confirmed Hi Ben, On Thu, May 25, 2023 at 03:07:24PM +0200, Ben Hutchings wrote: > On Thu, 2023-05-25 at 10:37 +0200, Salvatore Bonaccorso wrote: > > Hi Florian, > > > > [dropping a typoed mail from my to not cause further bounces] > > > > On Thu, May 25, 2023 at 10:18:46AM +0200, Florian Bezdeka wrote: > > > On Thu, 2023-05-25 at 10:03 +0200, Salvatore Bonaccorso wrote: > [...] > > > > > > > Ben, would you agree on make the PTP support. > > > > > > > > > It would be possible that we provide a merge request on > > > > > salsa.debian.org. Just tell me the correct target branch. We would be > > > > > very happy to have it in 6.1 (bookworm) and of course in all kernel > > > > > flavors/feature sets. > > > > > > > > I do not plan do accept any further bookworm targetting updates *right > > > > now*, mabye later for bookworm point releases as we are now in full > > > > freeze. There is no further upload planned for the 6.1.y series to > > > > unstable, following to be migrated in testing before the bookworm > > > > release. We can first apply to master and experimental upload if > > > > agreed on the following change above and then consider it later for > > > > bookworm. > > > > > > I think a point release would be acceptable for us. Sounds like a plan! > > > Thanks! > > > > Let's see if there are comments from Ben or Arnd on the proposed > > change. > > I agree we should make this built-in (except on armel/marvell). Thanks for confirming, so this change can go into the master branch first. Regards, Salvatore
Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
On Thu, 2023-05-25 at 10:37 +0200, Salvatore Bonaccorso wrote: > Hi Florian, > > [dropping a typoed mail from my to not cause further bounces] > > On Thu, May 25, 2023 at 10:18:46AM +0200, Florian Bezdeka wrote: > > On Thu, 2023-05-25 at 10:03 +0200, Salvatore Bonaccorso wrote: [...] > > > > > Ben, would you agree on make the PTP support. > > > > > > > It would be possible that we provide a merge request on > > > > salsa.debian.org. Just tell me the correct target branch. We would be > > > > very happy to have it in 6.1 (bookworm) and of course in all kernel > > > > flavors/feature sets. > > > > > > I do not plan do accept any further bookworm targetting updates *right > > > now*, mabye later for bookworm point releases as we are now in full > > > freeze. There is no further upload planned for the 6.1.y series to > > > unstable, following to be migrated in testing before the bookworm > > > release. We can first apply to master and experimental upload if > > > agreed on the following change above and then consider it later for > > > bookworm. > > > > I think a point release would be acceptable for us. Sounds like a plan! > > Thanks! > > Let's see if there are comments from Ben or Arnd on the proposed > change. I agree we should make this built-in (except on armel/marvell). Ben. -- Ben Hutchings - Debian developer, member of kernel, installer and LTS teams
Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
Hi Florian,
[dropping a typoed mail from my to not cause further bounces]
On Thu, May 25, 2023 at 10:18:46AM +0200, Florian Bezdeka wrote:
> On Thu, 2023-05-25 at 10:03 +0200, Salvatore Bonaccorso wrote:
> > Control: tags -1 + moreinfo
> >
> > On Thu, May 25, 2023 at 07:21:41AM +, Bezdeka, Florian wrote:
> > > Package: linux-image-amd64
> > > Version: 6.1.27-1
> > >
> > > Hi all,
> > >
> > > we did some investigations regarding time synchronization on Debian.
> > > Background is industrial communication on Linux in general.
> > >
> > > We found out that the PTP subsystem is only partially usable in
> > > combination with virtual clocks on Debian. While the setup was running
> > > fine on other distributions (like Ubuntu and Fedora) we had problems to
> > > bring it up and running on Debian.
> > >
> > > When starting ptp4l in combination with a vclock, the vclock could be
> > > created but not attached to a physical clock:
> > >
> > > ptp4l[127.805]: selected /dev/ptp4 as PTP clock
> > > ptp4l[127.805]: port 1 (enptp): PHC device mismatch
> > > ptp4l[127.805]: port 1 (enptp): /dev/ptp4 requested, ptp0 attached
> > > ptp4l[127.805]: failed to open port enptp
> > > failed to create a clock
> > >
> > > It turned out that CONFIG_PTP_1588_CLOCK makes the difference. When
> > > enabled (built-in, =y) our setup works, when set to "m" we run into
> > > problems. The "problematic" code can be found at [1].
> > >
> > > Might I request to set CONFIG_PTP_1588_CLOCK to y in the Debian kernel
> > > configuration?
> >
> > Question: Is this a bug, why does the code needs to expect it to be
> > builtin? (I'm not meaning what is present actually in the code, but
> > why it *needs* to be builtin on upstream side)? AFAICS the requirement
> > is added with e5f31552674e ("ethernet: fix PTP_1588_CLOCK
> > dependencies") in 5.15-rc1. As explained in the commit in the
> > following:
> >
> > > However, the two recently added ptp_get_vclocks_index() and
> > > ptp_convert_timestamp() interfaces are only called from builtin code
> > > with
> > > ethtool and socket timestamps, so keep the current behavior by
> > > stubbing
> > > those out completely when PTP is in a loadable module. This should be
> > > addressed properly in a follow-up.
> > >
> > > As Richard suggested, we may want to actually turn PTP support into a
> > > 'bool' option later on, preventing it from being a loadable module
> > > altogether, which would be one way to solve the problem with the
> > > ethtool
> > > interface.
> >
>
> I re-visited the changes from Arnd as well. Seems this "follow up"
> never happened.
Ok great for confirming.
>
> > Ben, would you agree on make the PTP support.
> >
> > > It would be possible that we provide a merge request on
> > > salsa.debian.org. Just tell me the correct target branch. We would be
> > > very happy to have it in 6.1 (bookworm) and of course in all kernel
> > > flavors/feature sets.
> >
> > I do not plan do accept any further bookworm targetting updates *right
> > now*, mabye later for bookworm point releases as we are now in full
> > freeze. There is no further upload planned for the 6.1.y series to
> > unstable, following to be migrated in testing before the bookworm
> > release. We can first apply to master and experimental upload if
> > agreed on the following change above and then consider it later for
> > bookworm.
>
> I think a point release would be acceptable for us. Sounds like a plan!
> Thanks!
Let's see if there are comments from Ben or Arnd on the proposed
change.
Regards,
Salvatore
Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
On Thu, 2023-05-25 at 10:03 +0200, Salvatore Bonaccorso wrote:
> Control: tags -1 + moreinfo
>
> On Thu, May 25, 2023 at 07:21:41AM +, Bezdeka, Florian wrote:
> > Package: linux-image-amd64
> > Version: 6.1.27-1
> >
> > Hi all,
> >
> > we did some investigations regarding time synchronization on Debian.
> > Background is industrial communication on Linux in general.
> >
> > We found out that the PTP subsystem is only partially usable in
> > combination with virtual clocks on Debian. While the setup was running
> > fine on other distributions (like Ubuntu and Fedora) we had problems to
> > bring it up and running on Debian.
> >
> > When starting ptp4l in combination with a vclock, the vclock could be
> > created but not attached to a physical clock:
> >
> > ptp4l[127.805]: selected /dev/ptp4 as PTP clock
> > ptp4l[127.805]: port 1 (enptp): PHC device mismatch
> > ptp4l[127.805]: port 1 (enptp): /dev/ptp4 requested, ptp0 attached
> > ptp4l[127.805]: failed to open port enptp
> > failed to create a clock
> >
> > It turned out that CONFIG_PTP_1588_CLOCK makes the difference. When
> > enabled (built-in, =y) our setup works, when set to "m" we run into
> > problems. The "problematic" code can be found at [1].
> >
> > Might I request to set CONFIG_PTP_1588_CLOCK to y in the Debian kernel
> > configuration?
>
> Question: Is this a bug, why does the code needs to expect it to be
> builtin? (I'm not meaning what is present actually in the code, but
> why it *needs* to be builtin on upstream side)? AFAICS the requirement
> is added with e5f31552674e ("ethernet: fix PTP_1588_CLOCK
> dependencies") in 5.15-rc1. As explained in the commit in the
> following:
>
> > However, the two recently added ptp_get_vclocks_index() and
> > ptp_convert_timestamp() interfaces are only called from builtin code
> > with
> > ethtool and socket timestamps, so keep the current behavior by stubbing
> > those out completely when PTP is in a loadable module. This should be
> > addressed properly in a follow-up.
> >
> > As Richard suggested, we may want to actually turn PTP support into a
> > 'bool' option later on, preventing it from being a loadable module
> > altogether, which would be one way to solve the problem with the ethtool
> > interface.
>
I re-visited the changes from Arnd as well. Seems this "follow up"
never happened.
> Ben, would you agree on make the PTP support.
>
> > It would be possible that we provide a merge request on
> > salsa.debian.org. Just tell me the correct target branch. We would be
> > very happy to have it in 6.1 (bookworm) and of course in all kernel
> > flavors/feature sets.
>
> I do not plan do accept any further bookworm targetting updates *right
> now*, mabye later for bookworm point releases as we are now in full
> freeze. There is no further upload planned for the 6.1.y series to
> unstable, following to be migrated in testing before the bookworm
> release. We can first apply to master and experimental upload if
> agreed on the following change above and then consider it later for
> bookworm.
I think a point release would be acceptable for us. Sounds like a plan!
Thanks!
>
> Hope this helps already
>
> Regards,
> Salvatore
Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
Control: tags -1 + moreinfo
On Thu, May 25, 2023 at 07:21:41AM +, Bezdeka, Florian wrote:
> Package: linux-image-amd64
> Version: 6.1.27-1
>
> Hi all,
>
> we did some investigations regarding time synchronization on Debian.
> Background is industrial communication on Linux in general.
>
> We found out that the PTP subsystem is only partially usable in
> combination with virtual clocks on Debian. While the setup was running
> fine on other distributions (like Ubuntu and Fedora) we had problems to
> bring it up and running on Debian.
>
> When starting ptp4l in combination with a vclock, the vclock could be
> created but not attached to a physical clock:
>
> ptp4l[127.805]: selected /dev/ptp4 as PTP clock
> ptp4l[127.805]: port 1 (enptp): PHC device mismatch
> ptp4l[127.805]: port 1 (enptp): /dev/ptp4 requested, ptp0 attached
> ptp4l[127.805]: failed to open port enptp
> failed to create a clock
>
> It turned out that CONFIG_PTP_1588_CLOCK makes the difference. When
> enabled (built-in, =y) our setup works, when set to "m" we run into
> problems. The "problematic" code can be found at [1].
>
> Might I request to set CONFIG_PTP_1588_CLOCK to y in the Debian kernel
> configuration?
Question: Is this a bug, why does the code needs to expect it to be
builtin? (I'm not meaning what is present actually in the code, but
why it *needs* to be builtin on upstream side)? AFAICS the requirement
is added with e5f31552674e ("ethernet: fix PTP_1588_CLOCK
dependencies") in 5.15-rc1. As explained in the commit in the
following:
| However, the two recently added ptp_get_vclocks_index() and
| ptp_convert_timestamp() interfaces are only called from builtin code with
| ethtool and socket timestamps, so keep the current behavior by stubbing
| those out completely when PTP is in a loadable module. This should be
| addressed properly in a follow-up.
|
| As Richard suggested, we may want to actually turn PTP support into a
| 'bool' option later on, preventing it from being a loadable module
| altogether, which would be one way to solve the problem with the ethtool
| interface.
Ben, would you agree on make the PTP support.
> It would be possible that we provide a merge request on
> salsa.debian.org. Just tell me the correct target branch. We would be
> very happy to have it in 6.1 (bookworm) and of course in all kernel
> flavors/feature sets.
I do not plan do accept any further bookworm targetting updates *right
now*, mabye later for bookworm point releases as we are now in full
freeze. There is no further upload planned for the 6.1.y series to
unstable, following to be migrated in testing before the bookworm
release. We can first apply to master and experimental upload if
agreed on the following change above and then consider it later for
bookworm.
Hope this helps already
Regards,
Salvatore
Bug#1036744: PTP in combination with vclocks partially broken on Debian kernels
Package: linux-image-amd64 Version: 6.1.27-1 Hi all, we did some investigations regarding time synchronization on Debian. Background is industrial communication on Linux in general. We found out that the PTP subsystem is only partially usable in combination with virtual clocks on Debian. While the setup was running fine on other distributions (like Ubuntu and Fedora) we had problems to bring it up and running on Debian. When starting ptp4l in combination with a vclock, the vclock could be created but not attached to a physical clock: ptp4l[127.805]: selected /dev/ptp4 as PTP clock ptp4l[127.805]: port 1 (enptp): PHC device mismatch ptp4l[127.805]: port 1 (enptp): /dev/ptp4 requested, ptp0 attached ptp4l[127.805]: failed to open port enptp failed to create a clock It turned out that CONFIG_PTP_1588_CLOCK makes the difference. When enabled (built-in, =y) our setup works, when set to "m" we run into problems. The "problematic" code can be found at [1]. Might I request to set CONFIG_PTP_1588_CLOCK to y in the Debian kernel configuration? It would be possible that we provide a merge request on salsa.debian.org. Just tell me the correct target branch. We would be very happy to have it in 6.1 (bookworm) and of course in all kernel flavors/feature sets. Best regards, Florian [1] https://elixir.bootlin.com/linux/v6.1.27/source/drivers/ptp/ptp_vclock.c#L232
