Hi forum, Thanks for the comments. Please find our responses below.
From: Erez <erezge...@gmail.com> Sent: 16 November 2021 21:08 To: Vladimir Oltean <olte...@gmail.com> Cc: Karthikkumar Valoor <kval...@altiostar.com>; linuxptp-devel@lists.sourceforge.net Subject: Re: [Linuxptp-devel] [PATCH] Include phcIndex in PORT_PROPERTIES_NP PMC command CAUTION: This email originated from outside of Altiostar. Do not click on links or open attachments unless you recognize the sender and you are sure the content is safe. You will never be asked to reset your Altiostar password via email. 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? (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. > pmc.c | 6 ++++-- > port.c | 1 + > tlv.h | 1 + > 3 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/pmc.c b/pmc.c > index a1ee787..67e33a3 100644 > --- a/pmc.c > +++ b/pmc.c > @@ -443,11 +443,13 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) > IFMT "portIdentity %s" > IFMT "portState %s" > IFMT "timestamping %s" > - IFMT "interface %s", > + IFMT "interface %s" > + IFMT "phcIndex %d", > pid2str(&ppn->portIdentity), > ps_str[ppn->port_state], > ts_str(ppn->timestamping), > - text2str(&ppn->interface)); > + text2str(&ppn->interface), > + ppn->phc_index); > break; > case MID_PORT_STATS_NP: > pcp = (struct port_stats_np *) mgt->data; > diff --git a/port.c b/port.c > index 7e51e77..743b7c7 100644 > --- a/port.c > +++ b/port.c > @@ -956,6 +956,7 @@ static int port_management_fill_response(struct port > *target, > else > ppn->port_state = target->state; > ppn->timestamping = target->timestamping; > + ppn->phc_index = target->phc_index; > ts_label = interface_label(target->iface); > ptp_text_set(&ppn->interface, ts_label); > datalen = sizeof(*ppn) + ppn->interface.length; > diff --git a/tlv.h b/tlv.h > index 97615fd..0f6cf28 100644 > --- a/tlv.h > +++ b/tlv.h > @@ -341,6 +341,7 @@ struct port_properties_np { > struct PortIdentity portIdentity; > uint8_t port_state; > uint8_t timestamping; > + Integer32 phc_index; > struct PTPText interface; > } PACKED; > > -- > 1.8.3.1 > > > > _______________________________________________ > Linuxptp-devel mailing list > Linuxptp-devel@lists.sourceforge.net<mailto:Linuxptp-devel@lists.sourceforge.net> > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel _______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net<mailto:Linuxptp-devel@lists.sourceforge.net> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
_______________________________________________ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
