On Fri, Nov 19, 2021 at 05:22:16AM +0000, Karthikkumar Valoor wrote: > On Tue, 16 Nov 2021 at 15:22, Vladimir Oltean > <olte...@gmail.com<mailto:olte...@gmail.com>> wrote: > On Tue, Nov 16, 2021 at 12:47:09PM +0000, Karthikkumar V via Linuxptp-devel > wrote: > > PORT_PROPERTIES_NP PMC Command will now display the phcIndex along with > > other interface details. A typical use case will be Users can fetch the > > phcIndex to use the clock APIs of the kernel (clock_gettime and > > clock_settime) > > to get the PHC time from user applications. > > > > Signed-off-by: Karthikkumar V > > <kval...@altiostar.com<mailto:kval...@altiostar.com>> > > Signed-off-by: Ramana Reddy > > <rre...@altiostar.com<mailto:rre...@altiostar.com>> > > --- > > May I suggest that > (a) you should not modify the binary format of management TLVs in > incompatible ways, as that would require everybody else to adapt and > somehow know which format ptp4l expects, and > Totally agree. > You should add new fields at the end of the port_properties_np structure. > Make sure a new pmc tool prints the message from an old ptp4l properly. > And verify an old pmc tool works with a new ptp4l. > > [Karthikkumar Valoor] Valid comment. Seems we have issue whether we add at > the end or middle of the structure (error thrown as “pmc[153304.857]: bad > message” > from pmc_recv() function due to size mismatch). > There will always be compatibility issues if one of the binaries (ptp4l/pmc) > are > old versions. Not sure how do we address these issues? What is the way > forward?
I'm not sure that there is a precedent for this in ptp4l, but generally speaking your options are either to create a separate management TLV which contains just the additional info (phc_index), or to create a v2 of the PORT_PROPERTIES_NP which has a different management ID and has the size you need. None of those is ideal, and still appear unnecessary to me at this stage. > (b) you can deduce the PHC index already from the struct PTPText interface, > see posix_clock_open() -> sk_get_ts_info(). > > Changing management TLV should be done carefully. > Only when we must. > > [Karthikkumar Valoor] sk_get_ts_info() needs access to the host interface, > which > will not be visible in cloud environment for applications (for ex: > POD/container > trying to fetch the info).The change was added so that Client applications > can directly > access the /dev/ptpX device without calling the sk_get_ts_info() call. Since PTP management messages can also be sent over the network, not just over AF_UNIX, the PHC index would be equally irrelevant outside this host. You just gave one example where the PHC in /dev is visible across multiple containers running on the same host, but individual interfaces are in network namespaces. Personally, I think it is a non-goal for a non-portable management TLV created by ptp4l for phc2sys use to be netns-aware. What I'd do is I'd simply put the consumer of ptp4l management messages right into the same container as ptp4l itself. Then you can also do whatever with the interface name. You can create your own protocol over a separate socket through which your application forwards the information it gets from the local ptp4l to whomever else needs it. _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
