Message ID | 20200520051647.18530-1-heiko.thiery@gmail.com |
---|---|
State | Changes Requested |
Headers | show |
Series | [1/1] package/linuxptp: remove hardcoded interface from config to args | expand |
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>
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
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.
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
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
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).
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.
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.
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?
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@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
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@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.
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@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
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@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
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
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
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]
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(-)