[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

Interface configuration is hard coded in the config file. This will
throw an error when this interace (eth0) is not present.

This patch removes the interface to be used from the config and appends it
to the PTP4L_ARGS. With this the custom arguments can be set via
/etc/defaults/ptp4l.

Cc: Michael Walle <michael@walle.cc>
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
---
 package/linuxptp/S65ptp4l      | 2 +-
 package/linuxptp/linuxptp.cfg  | 2 --
 package/linuxptp/ptp4l.service | 2 +-
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/package/linuxptp/S65ptp4l b/package/linuxptp/S65ptp4l
index 1b9e3c9300..f888c1026c 100644
--- a/package/linuxptp/S65ptp4l
+++ b/package/linuxptp/S65ptp4l
@@ -7,7 +7,7 @@ DAEMON="ptp4l"
 
 PIDFILE="/var/run/$DAEMON.pid"
 
-PTP4L_ARGS="-f /etc/linuxptp.cfg"
+PTP4L_ARGS="-f /etc/linuxptp.cfg -i eth0"
 
 # shellcheck source=/dev/null
 [ -r "/etc/default/ptp4l" ] && . "/etc/default/ptp4l"
diff --git a/package/linuxptp/linuxptp.cfg b/package/linuxptp/linuxptp.cfg
index f9d02e8d97..376fc85ac7 100644
--- a/package/linuxptp/linuxptp.cfg
+++ b/package/linuxptp/linuxptp.cfg
@@ -15,5 +15,3 @@ delay_mechanism		Auto
 network_transport	UDPv4
 time_stamping		hardware
 step_threshold		1.0
-
-[eth0]
diff --git a/package/linuxptp/ptp4l.service b/package/linuxptp/ptp4l.service
index 07f0b68fad..fb39ee3d94 100644
--- a/package/linuxptp/ptp4l.service
+++ b/package/linuxptp/ptp4l.service
@@ -6,7 +6,7 @@ Wants=time-sync.target
 Wants=phc2sys.service
 
 [Service]
-ExecStart=/usr/sbin/ptp4l -f /etc/linuxptp.cfg
+ExecStart=/usr/sbin/ptp4l -f /etc/linuxptp.cfg -i eth0
 Restart=always
 
 [Install]
-- 
2.20.1

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Michael Walle]

Am 2020-05-20 07:16, schrieb Heiko Thiery:
> Interface configuration is hard coded in the config file. This will
> throw an error when this interace (eth0) is not present.
> 
> This patch removes the interface to be used from the config and appends 
> it
> to the PTP4L_ARGS. With this the custom arguments can be set via
> /etc/defaults/ptp4l.
> 
> Cc: Michael Walle <michael@walle.cc>
> Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

Reviewed-by: Michael Walle <michael@walle.cc>

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Thomas Petazzoni]

Hello Heiko,

On Wed, 20 May 2020 07:16:48 +0200
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> Interface configuration is hard coded in the config file. This will
> throw an error when this interace (eth0) is not present.
> 
> This patch removes the interface to be used from the config and appends it
> to the PTP4L_ARGS. With this the custom arguments can be set via
> /etc/defaults/ptp4l.

Well, you can also just as well provide your custom linuxptp.cfg in a
rootfs overlay, no?

Also, your change to linuxptp.cfg kind of breaks what is explained in
linuxptp.cfg:

# By default synchronize time in slave-only mode using UDP and hardware time
# stamps on eth0. If the difference to master is >1.0 second correct by
# stepping the clock instead of adjusting the frequency.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

Hi Thomas,


> > Interface configuration is hard coded in the config file. This will
> > throw an error when this interace (eth0) is not present.
> >
> > This patch removes the interface to be used from the config and appends it
> > to the PTP4L_ARGS. With this the custom arguments can be set via
> > /etc/defaults/ptp4l.
>
> Well, you can also just as well provide your custom linuxptp.cfg in a
> rootfs overlay, no?
>

Yes this is for sure a valid solution. You mean it doesn't matter
adding a custom linuxptp.cfg or a /etc/defaults/ptp4l to change the
interface to be used because both ways need to add a file to the
rootfs overlay?

> Also, your change to linuxptp.cfg kind of breaks what is explained in
> linuxptp.cfg:
>

I think it does not break the functionality. The patch only changes
the location of the interface to be used. ptp4l can set the interface
either in the config or in the arguments.


> # By default synchronize time in slave-only mode using UDP and hardware time
> # stamps on eth0. If the difference to master is >1.0 second correct by
> # stepping the clock instead of adjusting the frequency.

I read over the mention of the interface name eth0 in the comment.


-- 
Heiko

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Michael Walle]

Hi all,

Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> Hello Heiko,
> 
> On Wed, 20 May 2020 07:16:48 +0200
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
> 
>> Interface configuration is hard coded in the config file. This will
>> throw an error when this interace (eth0) is not present.
>> 
>> This patch removes the interface to be used from the config and 
>> appends it
>> to the PTP4L_ARGS. With this the custom arguments can be set via
>> /etc/defaults/ptp4l.
> 
> Well, you can also just as well provide your custom linuxptp.cfg in a
> rootfs overlay, no?

You can, but then you'd have to copy the configuration on each and every
board which has another interface name than "eth0". That being said, I'd
also prefer it to have the normal default config, shipped with linuxptp.
Like at least debian does too.

-michael

> 
> Also, your change to linuxptp.cfg kind of breaks what is explained in
> linuxptp.cfg:
> 
> # By default synchronize time in slave-only mode using UDP and hardware 
> time
> # stamps on eth0. If the difference to master is >1.0 second correct by
> # stepping the clock instead of adjusting the frequency.
> 
> Thomas

-- 
-michael

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Thomas Petazzoni]

Hello,

On Thu, 21 May 2020 22:52:07 +0200
Heiko Thiery <heiko.thiery@gmail.com> wrote:

> > Well, you can also just as well provide your custom linuxptp.cfg in a
> > rootfs overlay, no?
> 
> Yes this is for sure a valid solution. You mean it doesn't matter
> adding a custom linuxptp.cfg or a /etc/defaults/ptp4l to change the
> interface to be used because both ways need to add a file to the
> rootfs overlay?

Exactly what I meant indeed.

> > Also, your change to linuxptp.cfg kind of breaks what is explained in
> > linuxptp.cfg:
> 
> I think it does not break the functionality. The patch only changes
> the location of the interface to be used. ptp4l can set the interface
> either in the config or in the arguments.

No, it does not break the functionality, but the explanation in the
comment becomes a bit weird, as the fact that eth0 is used is no longer
configured through that config file.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

Hi Michael, Hi Petr,

Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
>
> Hi all,
>
> Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> > Hello Heiko,
> >
> > On Wed, 20 May 2020 07:16:48 +0200
> > Heiko Thiery <heiko.thiery@gmail.com> wrote:
> >
> >> Interface configuration is hard coded in the config file. This will
> >> throw an error when this interace (eth0) is not present.
> >>
> >> This patch removes the interface to be used from the config and
> >> appends it
> >> to the PTP4L_ARGS. With this the custom arguments can be set via
> >> /etc/defaults/ptp4l.
> >
> > Well, you can also just as well provide your custom linuxptp.cfg in a
> > rootfs overlay, no?
>
> You can, but then you'd have to copy the configuration on each and every
> board which has another interface name than "eth0". That being said, I'd
> also prefer it to have the normal default config, shipped with linuxptp.
> Like at least debian does too.

A replacement of the currently used configuration could be a break in
backward compatibility? But as you said using the default linuxptp
configuraiton sounds to be a better solution. Since the currently used
config seems a use case from the initial committer (Petr Kulhavy).

-- 
Heiko

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

Hi Thomas,

Am Mo., 25. Mai 2020 um 09:40 Uhr schrieb Thomas Petazzoni
<thomas.petazzoni@bootlin.com>:
>
> Hello,
>
> On Thu, 21 May 2020 22:52:07 +0200
> Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> > > Well, you can also just as well provide your custom linuxptp.cfg in a
> > > rootfs overlay, no?
> >
> > Yes this is for sure a valid solution. You mean it doesn't matter
> > adding a custom linuxptp.cfg or a /etc/defaults/ptp4l to change the
> > interface to be used because both ways need to add a file to the
> > rootfs overlay?
>
> Exactly what I meant indeed.

Ok. But then the user needs to sync the config when changes are made
in the default of ptp4l/linuxptp unlike only changing the used
interface.

> > > Also, your change to linuxptp.cfg kind of breaks what is explained in
> > > linuxptp.cfg:
> >
> > I think it does not break the functionality. The patch only changes
> > the location of the interface to be used. ptp4l can set the interface
> > either in the config or in the arguments.
>
> No, it does not break the functionality, but the explanation in the
> comment becomes a bit weird, as the fact that eth0 is used is no longer
> configured through that config file.

When submitting a new patch version I will change the comment.

-- 
Heiko

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

Hi Petr,


Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
>
> Hi all,
>
> Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> > Hello Heiko,
> >
> > On Wed, 20 May 2020 07:16:48 +0200
> > Heiko Thiery <heiko.thiery@gmail.com> wrote:
> >
> >> Interface configuration is hard coded in the config file. This will
> >> throw an error when this interace (eth0) is not present.
> >>
> >> This patch removes the interface to be used from the config and
> >> appends it
> >> to the PTP4L_ARGS. With this the custom arguments can be set via
> >> /etc/defaults/ptp4l.
> >
> > Well, you can also just as well provide your custom linuxptp.cfg in a
> > rootfs overlay, no?
>
> You can, but then you'd have to copy the configuration on each and every
> board which has another interface name than "eth0". That being said, I'd
> also prefer it to have the normal default config, shipped with linuxptp.
> Like at least debian does too.

What do you think? Can we change the default configuration to the one
coming from the package? As Michael mentioned this is also the case
e.g. for debian.

-- 
Heiko

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

Hi Thomas, Hi Michael,

Am Mi., 27. Mai 2020 um 08:51 Uhr schrieb Heiko Thiery <heiko.thiery@gmail.com>:
>
> Hi Petr,
>
>
> Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
> >
> > Hi all,
> >
> > Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> > > Hello Heiko,
> > >
> > > On Wed, 20 May 2020 07:16:48 +0200
> > > Heiko Thiery <heiko.thiery@gmail.com> wrote:
> > >
> > >> Interface configuration is hard coded in the config file. This will
> > >> throw an error when this interace (eth0) is not present.
> > >>
> > >> This patch removes the interface to be used from the config and
> > >> appends it
> > >> to the PTP4L_ARGS. With this the custom arguments can be set via
> > >> /etc/defaults/ptp4l.
> > >
> > > Well, you can also just as well provide your custom linuxptp.cfg in a
> > > rootfs overlay, no?
> >
> > You can, but then you'd have to copy the configuration on each and every
> > board which has another interface name than "eth0". That being said, I'd
> > also prefer it to have the normal default config, shipped with linuxptp.
> > Like at least debian does too.
>
> What do you think? Can we change the default configuration to the one
> coming from the package? As Michael mentioned this is also the case
> e.g. for debian.

Since no reply from the package maintainer about changing the default
configuration what do you think? Should we change the used config to
the one provided by the upstream linuxptp?

-- 
Heiko

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Vladimir Oltean]

Hi Heiko,

On Mon, 15 Jun 2020 at 14:36, Heiko Thiery <heiko.thiery@gmail.com> wrote:
>
> Hi Thomas, Hi Michael,
>
> Am Mi., 27. Mai 2020 um 08:51 Uhr schrieb Heiko Thiery <heiko.thiery@gmail.com>:
> >
> > Hi Petr,
> >
> >
> > Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
> > >
> > > Hi all,
> > >
> > > Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> > > > Hello Heiko,
> > > >
> > > > On Wed, 20 May 2020 07:16:48 +0200
> > > > Heiko Thiery <heiko.thiery@gmail.com> wrote:
> > > >
> > > >> Interface configuration is hard coded in the config file. This will
> > > >> throw an error when this interace (eth0) is not present.
> > > >>
> > > >> This patch removes the interface to be used from the config and
> > > >> appends it
> > > >> to the PTP4L_ARGS. With this the custom arguments can be set via
> > > >> /etc/defaults/ptp4l.
> > > >
> > > > Well, you can also just as well provide your custom linuxptp.cfg in a
> > > > rootfs overlay, no?
> > >
> > > You can, but then you'd have to copy the configuration on each and every
> > > board which has another interface name than "eth0". That being said, I'd
> > > also prefer it to have the normal default config, shipped with linuxptp.
> > > Like at least debian does too.
> >
> > What do you think? Can we change the default configuration to the one
> > coming from the package? As Michael mentioned this is also the case
> > e.g. for debian.
>
> Since no reply from the package maintainer about changing the default
> configuration what do you think? Should we change the used config to
> the one provided by the upstream linuxptp?
>
> --
> Heiko
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

I've thought many times about changing the default ptp4l config file
shipped by buildroot (due to that inconvenient eth0), but at the end
of the day, buildroot is meant to be customized for each and every
embedded target (which makes the comparison to debian not so
relevant). So providing a default config doesn't make a lot of sense
in the first place, except for those users who have never used it
before, and have no idea where to start. But apart from that, as you
said, interface names are going to differ, and let me add another one:
PTP profiles are going to differ (automotive, AVB, telecom, etc):
https://github.com/richardcochran/linuxptp/tree/master/configs
So what do you do, ship them all? That's a little bit _not_ in the
spirit of buildroot where you are only supposed to install what the
end appliance will use.
So I guess I'm back to Thomas's question: what do you win if you move
"eth0" from /etc/linuxptp.cfg to /etc/defaults/ptp4l? The default
configuration will remain just as useless for somebody who needs to
customize ptp4l for a particular set of ports and PTP profile.
The furthest I got was to have some advanced Kconfig-based system
where you could specify in the defconfig which profile you want, and
on which ports, and the build system would automatically make things
happen. Sadly it was so complicated that I just tossed it away. Note
that some hardware also needs hardware-specific settings (such as
increasing the tx_timestamp_timeout), which makes Thomas's suggestion
of providing rootfs overlays the only viable path in the general case.

What do you think?

-Vladimir

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Heiko Thiery]

HI Vladimir,

Am Di., 16. Juni 2020 um 21:07 Uhr schrieb Vladimir Oltean <olteanv@gmail.com>:
>
> Hi Heiko,
>
> On Mon, 15 Jun 2020 at 14:36, Heiko Thiery <heiko.thiery@gmail.com> wrote:
> >
> > Hi Thomas, Hi Michael,
> >
> > Am Mi., 27. Mai 2020 um 08:51 Uhr schrieb Heiko Thiery <heiko.thiery@gmail.com>:
> > >
> > > Hi Petr,
> > >
> > >
> > > Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
> > > >
> > > > Hi all,
> > > >
> > > > Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> > > > > Hello Heiko,
> > > > >
> > > > > On Wed, 20 May 2020 07:16:48 +0200
> > > > > Heiko Thiery <heiko.thiery@gmail.com> wrote:
> > > > >
> > > > >> Interface configuration is hard coded in the config file. This will
> > > > >> throw an error when this interace (eth0) is not present.
> > > > >>
> > > > >> This patch removes the interface to be used from the config and
> > > > >> appends it
> > > > >> to the PTP4L_ARGS. With this the custom arguments can be set via
> > > > >> /etc/defaults/ptp4l.
> > > > >
> > > > > Well, you can also just as well provide your custom linuxptp.cfg in a
> > > > > rootfs overlay, no?
> > > >
> > > > You can, but then you'd have to copy the configuration on each and every
> > > > board which has another interface name than "eth0". That being said, I'd
> > > > also prefer it to have the normal default config, shipped with linuxptp.
> > > > Like at least debian does too.
> > >
> > > What do you think? Can we change the default configuration to the one
> > > coming from the package? As Michael mentioned this is also the case
> > > e.g. for debian.
> >
> > Since no reply from the package maintainer about changing the default
> > configuration what do you think? Should we change the used config to
> > the one provided by the upstream linuxptp?
> >
> > --
> > Heiko
> > _______________________________________________
> > buildroot mailing list
> > buildroot at busybox.net
> > http://lists.busybox.net/mailman/listinfo/buildroot
>
> I've thought many times about changing the default ptp4l config file
> shipped by buildroot (due to that inconvenient eth0), but at the end
> of the day, buildroot is meant to be customized for each and every
> embedded target (which makes the comparison to debian not so
> relevant). So providing a default config doesn't make a lot of sense
> in the first place, except for those users who have never used it
> before, and have no idea where to start. But apart from that, as you
> said, interface names are going to differ, and let me add another one:
> PTP profiles are going to differ (automotive, AVB, telecom, etc):
> https://github.com/richardcochran/linuxptp/tree/master/configs
> So what do you do, ship them all? That's a little bit _not_ in the
> spirit of buildroot where you are only supposed to install what the
> end appliance will use.
> So I guess I'm back to Thomas's question: what do you win if you move
> "eth0" from /etc/linuxptp.cfg to /etc/defaults/ptp4l? The default
> configuration will remain just as useless for somebody who needs to
> customize ptp4l for a particular set of ports and PTP profile.
> The furthest I got was to have some advanced Kconfig-based system
> where you could specify in the defconfig which profile you want, and
> on which ports, and the build system would automatically make things
> happen. Sadly it was so complicated that I just tossed it away. Note
> that some hardware also needs hardware-specific settings (such as
> increasing the tx_timestamp_timeout), which makes Thomas's suggestion
> of providing rootfs overlays the only viable path in the general case.
>
> What do you think?

Yes, I think with all these options it makes more sense just to
provide the required changes in an overlay.

Thank you for your comments.

-- 
Heiko

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Michael Walle]

Am 2020-06-16 21:07, schrieb Vladimir Oltean:
> Hi Heiko,
> 
> On Mon, 15 Jun 2020 at 14:36, Heiko Thiery <heiko.thiery@gmail.com> 
> wrote:
>> 
>> Hi Thomas, Hi Michael,
>> 
>> Am Mi., 27. Mai 2020 um 08:51 Uhr schrieb Heiko Thiery 
>> <heiko.thiery@gmail.com>:
>> >
>> > Hi Petr,
>> >
>> >
>> > Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
>> > >
>> > > Hi all,
>> > >
>> > > Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
>> > > > Hello Heiko,
>> > > >
>> > > > On Wed, 20 May 2020 07:16:48 +0200
>> > > > Heiko Thiery <heiko.thiery@gmail.com> wrote:
>> > > >
>> > > >> Interface configuration is hard coded in the config file. This will
>> > > >> throw an error when this interace (eth0) is not present.
>> > > >>
>> > > >> This patch removes the interface to be used from the config and
>> > > >> appends it
>> > > >> to the PTP4L_ARGS. With this the custom arguments can be set via
>> > > >> /etc/defaults/ptp4l.
>> > > >
>> > > > Well, you can also just as well provide your custom linuxptp.cfg in a
>> > > > rootfs overlay, no?
>> > >
>> > > You can, but then you'd have to copy the configuration on each and every
>> > > board which has another interface name than "eth0". That being said, I'd
>> > > also prefer it to have the normal default config, shipped with linuxptp.
>> > > Like at least debian does too.
>> >
>> > What do you think? Can we change the default configuration to the one
>> > coming from the package? As Michael mentioned this is also the case
>> > e.g. for debian.
>> 
>> Since no reply from the package maintainer about changing the default
>> configuration what do you think? Should we change the used config to
>> the one provided by the upstream linuxptp?
>> 
>> --
>> Heiko
>> _______________________________________________
>> buildroot mailing list
>> buildroot at busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
> 
> I've thought many times about changing the default ptp4l config file
> shipped by buildroot (due to that inconvenient eth0), but at the end
> of the day, buildroot is meant to be customized for each and every
> embedded target (which makes the comparison to debian not so
> relevant). So providing a default config doesn't make a lot of sense
> in the first place, except for those users who have never used it
> before, and have no idea where to start.

_BUT_ a customized config file which fits one board doesn't make
sense either. So buildroot should really stick to the default ones
provided by the upstream. Like every other distro does. Esp if it
"just works".

> But apart from that, as you
> said, interface names are going to differ, and let me add another one:
> PTP profiles are going to differ (automotive, AVB, telecom, etc):
> https://github.com/richardcochran/linuxptp/tree/master/configs
> So what do you do, ship them all? That's a little bit _not_ in the
> spirit of buildroot where you are only supposed to install what the
> end appliance will use.
> So I guess I'm back to Thomas's question: what do you win if you move
> "eth0" from /etc/linuxptp.cfg to /etc/defaults/ptp4l?

Having an empty [<interface>] section in linuxptp.cfg is strange. Why
are there the "-i" options for the binary? Because you can then have
one config which can be used for multiple instances on different
interfaces.

> The default
> configuration will remain just as useless for somebody who needs to
> customize ptp4l for a particular set of ports and PTP profile.

Right, but as a starting point it is sufficient.

-michael

> The furthest I got was to have some advanced Kconfig-based system
> where you could specify in the defconfig which profile you want, and
> on which ports, and the build system would automatically make things
> happen. Sadly it was so complicated that I just tossed it away. Note
> that some hardware also needs hardware-specific settings (such as
> increasing the tx_timestamp_timeout), which makes Thomas's suggestion
> of providing rootfs overlays the only viable path in the general case.
> 
> What do you think?
> 
> -Vladimir

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Vladimir Oltean]

On Thu, 18 Jun 2020 at 19:09, Michael Walle <michael@walle.cc> wrote:
>
> Am 2020-06-16 21:07, schrieb Vladimir Oltean:
> > Hi Heiko,
> >
> > On Mon, 15 Jun 2020 at 14:36, Heiko Thiery <heiko.thiery@gmail.com>
> > wrote:
> >>
> >> Hi Thomas, Hi Michael,
> >>
> >> Am Mi., 27. Mai 2020 um 08:51 Uhr schrieb Heiko Thiery
> >> <heiko.thiery@gmail.com>:
> >> >
> >> > Hi Petr,
> >> >
> >> >
> >> > Am Do., 21. Mai 2020 um 23:22 Uhr schrieb Michael Walle <michael@walle.cc>:
> >> > >
> >> > > Hi all,
> >> > >
> >> > > Am 2020-05-21 15:36, schrieb Thomas Petazzoni:
> >> > > > Hello Heiko,
> >> > > >
> >> > > > On Wed, 20 May 2020 07:16:48 +0200
> >> > > > Heiko Thiery <heiko.thiery@gmail.com> wrote:
> >> > > >
> >> > > >> Interface configuration is hard coded in the config file. This will
> >> > > >> throw an error when this interace (eth0) is not present.
> >> > > >>
> >> > > >> This patch removes the interface to be used from the config and
> >> > > >> appends it
> >> > > >> to the PTP4L_ARGS. With this the custom arguments can be set via
> >> > > >> /etc/defaults/ptp4l.
> >> > > >
> >> > > > Well, you can also just as well provide your custom linuxptp.cfg in a
> >> > > > rootfs overlay, no?
> >> > >
> >> > > You can, but then you'd have to copy the configuration on each and every
> >> > > board which has another interface name than "eth0". That being said, I'd
> >> > > also prefer it to have the normal default config, shipped with linuxptp.
> >> > > Like at least debian does too.
> >> >
> >> > What do you think? Can we change the default configuration to the one
> >> > coming from the package? As Michael mentioned this is also the case
> >> > e.g. for debian.
> >>
> >> Since no reply from the package maintainer about changing the default
> >> configuration what do you think? Should we change the used config to
> >> the one provided by the upstream linuxptp?
> >>
> >> --
> >> Heiko
> >> _______________________________________________
> >> buildroot mailing list
> >> buildroot at busybox.net
> >> http://lists.busybox.net/mailman/listinfo/buildroot
> >
> > I've thought many times about changing the default ptp4l config file
> > shipped by buildroot (due to that inconvenient eth0), but at the end
> > of the day, buildroot is meant to be customized for each and every
> > embedded target (which makes the comparison to debian not so
> > relevant). So providing a default config doesn't make a lot of sense
> > in the first place, except for those users who have never used it
> > before, and have no idea where to start.
>
> _BUT_ a customized config file which fits one board doesn't make
> sense either.

For that board, sure it makes sense. For the rest, not so much.

> So buildroot should really stick to the default ones
> provided by the upstream. Like every other distro does. Esp if it
> "just works".
>

And it does. To my knowledge, linuxptp.cfg from buildroot is
default.cfg from upstream, just with this extra [eth0] at the end.

> > But apart from that, as you
> > said, interface names are going to differ, and let me add another one:
> > PTP profiles are going to differ (automotive, AVB, telecom, etc):
> > https://github.com/richardcochran/linuxptp/tree/master/configs
> > So what do you do, ship them all? That's a little bit _not_ in the
> > spirit of buildroot where you are only supposed to install what the
> > end appliance will use.
> > So I guess I'm back to Thomas's question: what do you win if you move
> > "eth0" from /etc/linuxptp.cfg to /etc/defaults/ptp4l?
>
> Having an empty [<interface>] section in linuxptp.cfg is strange.

Why is it strange to have no interfaces defined in linuxptp.cfg?
That's how upstream configurations are shipped.

> Why
> are there the "-i" options for the binary? Because you can then have
> one config which can be used for multiple instances on different
> interfaces.
>

Yes.

> > The default
> > configuration will remain just as useless for somebody who needs to
> > customize ptp4l for a particular set of ports and PTP profile.
>
> Right, but as a starting point it is sufficient.
>

And what do you gain if you move that "eth0" to just some other file?
Isn't the current starting point just as sufficient?

> -michael
>
> > The furthest I got was to have some advanced Kconfig-based system
> > where you could specify in the defconfig which profile you want, and
> > on which ports, and the build system would automatically make things
> > happen. Sadly it was so complicated that I just tossed it away. Note
> > that some hardware also needs hardware-specific settings (such as
> > increasing the tx_timestamp_timeout), which makes Thomas's suggestion
> > of providing rootfs overlays the only viable path in the general case.
> >
> > What do you think?
> >
> > -Vladimir

Thanks,
-Vladimir

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Michael Walle]

Hi Vladimir,

Am 2020-06-18 19:00, schrieb Vladimir Oltean:
> On Thu, 18 Jun 2020 at 19:09, Michael Walle <michael@walle.cc> wrote:
>> 
>> Am 2020-06-16 21:07, schrieb Vladimir Oltean:
[..]
>> > I've thought many times about changing the default ptp4l config file
>> > shipped by buildroot (due to that inconvenient eth0), but at the end
>> > of the day, buildroot is meant to be customized for each and every
>> > embedded target (which makes the comparison to debian not so
>> > relevant). So providing a default config doesn't make a lot of sense
>> > in the first place, except for those users who have never used it
>> > before, and have no idea where to start.
>> 
>> _BUT_ a customized config file which fits one board doesn't make
>> sense either.
> 
> For that board, sure it makes sense. For the rest, not so much.

Then this config should go into the board specific fs overlay.

>> So buildroot should really stick to the default ones
>> provided by the upstream. Like every other distro does. Esp if it
>> "just works".
>> 
> 
> And it does. To my knowledge, linuxptp.cfg from buildroot is
> default.cfg from upstream, just with this extra [eth0] at the end.

It is not even close:
https://github.com/buildroot/buildroot/blob/master/package/linuxptp/linuxptp.cfg
https://github.com/openil/linuxptp/blob/master/configs/default.cfg

>> > But apart from that, as you
>> > said, interface names are going to differ, and let me add another one:
>> > PTP profiles are going to differ (automotive, AVB, telecom, etc):
>> > https://github.com/richardcochran/linuxptp/tree/master/configs
>> > So what do you do, ship them all? That's a little bit _not_ in the
>> > spirit of buildroot where you are only supposed to install what the
>> > end appliance will use.
>> > So I guess I'm back to Thomas's question: what do you win if you move
>> > "eth0" from /etc/linuxptp.cfg to /etc/defaults/ptp4l?
>> 
>> Having an empty [<interface>] section in linuxptp.cfg is strange.
> 
> Why is it strange to have no interfaces defined in linuxptp.cfg?
> That's how upstream configurations are shipped.

I mean an empty [eth0] section, which is the equivalent of having
the "-i" command line option.

>> Why
>> are there the "-i" options for the binary? Because you can then have
>> one config which can be used for multiple instances on different
>> interfaces.
>> 
> 
> Yes.
> 
>> > The default
>> > configuration will remain just as useless for somebody who needs to
>> > customize ptp4l for a particular set of ports and PTP profile.
>> 
>> Right, but as a starting point it is sufficient.
>> 
> 
> And what do you gain if you move that "eth0" to just some other file?

the default.cfg contains all variables which can changed by the user, so
you won't need to look at the manual page to figure out the different
options. the buildroot one, doesn't; I don't know who and why choose
these options.

> Isn't the current starting point just as sufficient?

No, its a board specific configuration, ie. slave-only enabled etc.

I give you another example: lets say I want to provide sensible defaults
to my board supported in buildroot. As you already pointed out, it is
intended behavior to change the configuration for a final board config.
But the board in buildroot should also contain a good template. So right
now, I'd have to make an own copy of the and put it into the overlay.
Which means duplicating the configuration, even worse, I'd have to
track upstream default config. Instead, a package could copy the default
config from the tarball/git repository. This doesn't only apply to
linuxptp, but IMHO for all packages which provide a sane upstream
default config.

If I would put port 2222 into dropbear config installed by buildroot,
I doubt anyone would argue in favor of that.

To go a bit further, I'd like to see that the manual suggests that
any package should install the upstream default config (if there
is a _sensible_ one). Even if that is just because there will never
be an agreement on what the config installed by buildroot should look
like.

-michael

>> > The furthest I got was to have some advanced Kconfig-based system
>> > where you could specify in the defconfig which profile you want, and
>> > on which ports, and the build system would automatically make things
>> > happen. Sadly it was so complicated that I just tossed it away. Note
>> > that some hardware also needs hardware-specific settings (such as
>> > increasing the tx_timestamp_timeout), which makes Thomas's suggestion
>> > of providing rootfs overlays the only viable path in the general case.
>> >
>> > What do you think?
>> >
>> > -Vladimir
> 
> Thanks,
> -Vladimir

[Buildroot] [PATCH 1/1] package/linuxptp: remove hardcoded interface from config to args

[Vladimir Oltean]

Hi Michael,

On Fri, 19 Jun 2020 at 00:20, Michael Walle <michael@walle.cc> wrote:
>
> Hi Vladimir,
>
> Am 2020-06-18 19:00, schrieb Vladimir Oltean:
> > On Thu, 18 Jun 2020 at 19:09, Michael Walle <michael@walle.cc> wrote:
> >>
> >> Am 2020-06-16 21:07, schrieb Vladimir Oltean:
> [..]
> >> > I've thought many times about changing the default ptp4l config file
> >> > shipped by buildroot (due to that inconvenient eth0), but at the end
> >> > of the day, buildroot is meant to be customized for each and every
> >> > embedded target (which makes the comparison to debian not so
> >> > relevant). So providing a default config doesn't make a lot of sense
> >> > in the first place, except for those users who have never used it
> >> > before, and have no idea where to start.
> >>
> >> _BUT_ a customized config file which fits one board doesn't make
> >> sense either.
> >
> > For that board, sure it makes sense. For the rest, not so much.
>
> Then this config should go into the board specific fs overlay.
>
> >> So buildroot should really stick to the default ones
> >> provided by the upstream. Like every other distro does. Esp if it
> >> "just works".
> >>
> >
> > And it does. To my knowledge, linuxptp.cfg from buildroot is
> > default.cfg from upstream, just with this extra [eth0] at the end.
>
> It is not even close:
> https://github.com/buildroot/buildroot/blob/master/package/linuxptp/linuxptp.cfg
> https://github.com/openil/linuxptp/blob/master/configs/default.cfg
>
> >> > But apart from that, as you
> >> > said, interface names are going to differ, and let me add another one:
> >> > PTP profiles are going to differ (automotive, AVB, telecom, etc):
> >> > https://github.com/richardcochran/linuxptp/tree/master/configs
> >> > So what do you do, ship them all? That's a little bit _not_ in the
> >> > spirit of buildroot where you are only supposed to install what the
> >> > end appliance will use.
> >> > So I guess I'm back to Thomas's question: what do you win if you move
> >> > "eth0" from /etc/linuxptp.cfg to /etc/defaults/ptp4l?
> >>
> >> Having an empty [<interface>] section in linuxptp.cfg is strange.
> >
> > Why is it strange to have no interfaces defined in linuxptp.cfg?
> > That's how upstream configurations are shipped.
>
> I mean an empty [eth0] section, which is the equivalent of having
> the "-i" command line option.
>
> >> Why
> >> are there the "-i" options for the binary? Because you can then have
> >> one config which can be used for multiple instances on different
> >> interfaces.
> >>
> >
> > Yes.
> >
> >> > The default
> >> > configuration will remain just as useless for somebody who needs to
> >> > customize ptp4l for a particular set of ports and PTP profile.
> >>
> >> Right, but as a starting point it is sufficient.
> >>
> >
> > And what do you gain if you move that "eth0" to just some other file?
>
> the default.cfg contains all variables which can changed by the user, so
> you won't need to look at the manual page to figure out the different
> options. the buildroot one, doesn't; I don't know who and why choose
> these options.
>
> > Isn't the current starting point just as sufficient?
>
> No, its a board specific configuration, ie. slave-only enabled etc.
>
> I give you another example: lets say I want to provide sensible defaults
> to my board supported in buildroot. As you already pointed out, it is
> intended behavior to change the configuration for a final board config.
> But the board in buildroot should also contain a good template. So right
> now, I'd have to make an own copy of the and put it into the overlay.
> Which means duplicating the configuration, even worse, I'd have to
> track upstream default config. Instead, a package could copy the default
> config from the tarball/git repository. This doesn't only apply to
> linuxptp, but IMHO for all packages which provide a sane upstream
> default config.
>
> If I would put port 2222 into dropbear config installed by buildroot,
> I doubt anyone would argue in favor of that.
>
> To go a bit further, I'd like to see that the manual suggests that
> any package should install the upstream default config (if there
> is a _sensible_ one). Even if that is just because there will never
> be an agreement on what the config installed by buildroot should look
> like.
>
> -michael
>
> >> > The furthest I got was to have some advanced Kconfig-based system
> >> > where you could specify in the defconfig which profile you want, and
> >> > on which ports, and the build system would automatically make things
> >> > happen. Sadly it was so complicated that I just tossed it away. Note
> >> > that some hardware also needs hardware-specific settings (such as
> >> > increasing the tx_timestamp_timeout), which makes Thomas's suggestion
> >> > of providing rootfs overlays the only viable path in the general case.
> >> >
> >> > What do you think?
> >> >
> >> > -Vladimir
> >
> > Thanks,
> > -Vladimir

I agree it would make more sense for buildroot to ship default.cfg or
something equivalent to it (no configuration file).

Thanks,
-Vladimir