Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Fri, Nov 17, 2023 at 08:27:43PM -0800, Richard Cochran wrote: > How does this requirement improve synchronization? > > What benefit does it bring to users of the PTP? rhetorical questions :^( ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Fri, Nov 17, 2023 at 01:51:18PM +0100, Andrew Zaborowski wrote: > It is exposed on the wire in the Pdelay messages. Compliance tests > look at this. They also simulate a few hypothetical scenarios like a > domain 0 PTP port trying to communicate with a CMLDS link port since > 1588 talks about this. How does this requirement improve synchronization? What benefit does it bring to users of the PTP? Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Fri, 17 Nov 2023 at 05:50, Richard Cochran wrote: > On Fri, Nov 17, 2023 at 01:41:19AM +0100, Andrew Zaborowski wrote: > > Do you want to require the user to enforce that the port numbering is > > the same between the ptp4l processes? > > No. (I meant: do you want to require that the user takes care to maintain the order of interfaces on the command line / in the config file, which I think you do -- Ok) > > > In our use case spec compliance > > is the priority, including the unique clockIdentity for CMLDS > > requirement, so we'd definitely need to be running a separate ptp4l > > process for CMLDS in this schema. > > The clockIdentity is not exposed, so what do you mean by "compliance"? It is exposed on the wire in the Pdelay messages. Compliance tests look at this. They also simulate a few hypothetical scenarios like a domain 0 PTP port trying to communicate with a CMLDS link port since 1588 talks about this. Best regards ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Fri, Nov 17, 2023 at 01:41:19AM +0100, Andrew Zaborowski wrote: > On Thu, 16 Nov 2023 at 23:46, Richard Cochran > wrote: > > No, I mean the PTP port number. These are taken from the order of the > > interfaces on the command line and in the configuration file. > > Won't this be the same as the UDS message's sourcePortIdentity.portNumber? Ah, yes, you are right. Even simpler! > Do you want to require the user to enforce that the port numbering is > the same between the ptp4l processes? No. > In our use case spec compliance > is the priority, including the unique clockIdentity for CMLDS > requirement, so we'd definitely need to be running a separate ptp4l > process for CMLDS in this schema. The clockIdentity is not exposed, so what do you mean by "compliance"? > I'm asking because the port > numbering thing is unobvious and the user effort to configure a > compliant setup with ptp4l is already very high. That is just par for the course. I'm not the one making up tons of crazy new options. The strength of linuxptp is modularity and fine grained, configurable options. This means that a) ptp4l works even with equipment that fails to follow the standards and/or profiles, and b) ptp4l can be really hard to configure. > So one solution would be to let the user configure a custom portNumber > under port settings. Port numbers start with 1 and increase in the order of the configured interfaces. If it really is required for CMLDS ports to have *different* numbers than the normal ports, we can add a "port index offset" option that would start the numbering at some given value. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Thu, 16 Nov 2023 at 23:46, Richard Cochran wrote: > On Thu, Nov 16, 2023 at 11:11:50PM +0100, Andrew Zaborowski wrote: > > The two timestamps are passed to clock_peer_delay() by the receiving > > port and stored in c->tsproc. Then they're accessed by > > get_raw_delay() which is used in the filter logic. I'm not sure how > > much value that has, we can possibly pass 0s to clock_peer_delay(). > > If the client uses CMLDS, then it doesn't need tsproc logic at all. > It simply take the delay value from the CMLDS server. Make sense, the CMLDS would already be filtering the timestamps once. > > > Regarding "source_port_index", I assume that would contain the ifindex > > of the interface? With virtual clocks I believe the PHC indices may > > differ between the PTP ports on one physical port ("link port"). > > No, I mean the PTP port number. These are taken from the order of the > interfaces on the command line and in the configuration file. Won't this be the same as the UDS message's sourcePortIdentity.portNumber? Do you want to require the user to enforce that the port numbering is the same between the ptp4l processes? In our use case spec compliance is the priority, including the unique clockIdentity for CMLDS requirement, so we'd definitely need to be running a separate ptp4l process for CMLDS in this schema. I'm asking because the port numbering thing is unobvious and the user effort to configure a compliant setup with ptp4l is already very high. So one solution would be to let the user configure a custom portNumber under port settings. My colleague even had a local change to allow passing -i ,. This would also remove the (possibly unobvious) requirement to run all ports sharing the clockIdentity as one ptp4l process. Another option would be to pass the ifindex as I thought you were suggesting. Best regards ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Thu, Nov 16, 2023 at 11:11:50PM +0100, Andrew Zaborowski wrote: > The two timestamps are passed to clock_peer_delay() by the receiving > port and stored in c->tsproc. Then they're accessed by > get_raw_delay() which is used in the filter logic. I'm not sure how > much value that has, we can possibly pass 0s to clock_peer_delay(). If the client uses CMLDS, then it doesn't need tsproc logic at all. It simply take the delay value from the CMLDS server. > Regarding "source_port_index", I assume that would contain the ifindex > of the interface? With virtual clocks I believe the PHC indices may > differ between the PTP ports on one physical port ("link port"). No, I mean the PTP port number. These are taken from the order of the interfaces on the command line and in the configuration file. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Thu, 16 Nov 2023 at 05:27, Richard Cochran wrote: > On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > > @@ -1129,6 +1130,27 @@ static int port_management_fill_response(struct port > > *target, > > memcpy(pwr, >pwr, sizeof(*pwr)); > > datalen = sizeof(*pwr); > > break; > > + case MID_CMLDS_INFO_NP: > > + cmlds = (struct cmlds_info_np *)tlv->data; > > + /* IEEE1588-2019 16.6.3.2 h) 1) && nrate.ratio_valid because > > + * we have no extra field to convey that separately. > > + */ > > + cmlds->serviceMeasurementValid = > > + target->peer_portid_valid && !target->pdr_missing && > > + !target->multiple_pdr_detected && > > + target->nrate.ratio_valid; > > + cmlds->meanLinkDelay = target->peerMeanPathDelay; > > + cmlds->scaledNeighborRateRatio = > > + (Integer32) (target->nrate.ratio * POW2_41 - POW2_41); > > > + /* 16.6.3.2: "Upon receipt of a request for information, the > > + * Common Mean Link Delay Service may in addition return the > > + * raw measurement data gathered by the service for use in > > + * estimating the and ." > > + */ > > + cmlds->egress_ts = tmv_to_nanoseconds(target->peer_delay_t1); > > + cmlds->rx_ts = tmv_to_nanoseconds(target->peer_delay_t2); > > Please drop these two fields. They don't provide any benefit. If the > clients don't trust the CMLDS values, then they are free to measure > the p2p delay themselves! The two timestamps are passed to clock_peer_delay() by the receiving port and stored in c->tsproc. Then they're accessed by get_raw_delay() which is used in the filter logic. I'm not sure how much value that has, we can possibly pass 0s to clock_peer_delay(). Regarding "source_port_index", I assume that would contain the ifindex of the interface? With virtual clocks I believe the PHC indices may differ between the PTP ports on one physical port ("link port"). Best regards ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Wed, Nov 15, 2023 at 09:45:52PM -0800, Richard Cochran wrote: > On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > > > @@ -473,6 +476,14 @@ struct msg_interface_rate_tlv { > > UInteger16numberOfBitsAfterTimestamp; > > } PACKED; > > > > +struct cmlds_info_np { > > + Integer8 serviceMeasurementValid; > > + TimeInterval meanLinkDelay; > > + Integer32scaledNeighborRateRatio; > > I think you need one more field, like "source_port_index" > > If a client subscribes to TLVs from multiple ports, then it needs a > way to tell which is which. Or maybe the subscription will cause the CMLDS server to forward all p2p delay measurements from all p2p ports. Still the TLVs needs to include the port index. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > @@ -473,6 +476,14 @@ struct msg_interface_rate_tlv { > UInteger16numberOfBitsAfterTimestamp; > } PACKED; > > +struct cmlds_info_np { > + Integer8 serviceMeasurementValid; > + TimeInterval meanLinkDelay; > + Integer32scaledNeighborRateRatio; I think you need one more field, like "source_port_index" If a client subscribes to TLVs from multiple ports, then it needs a way to tell which is which. > + Integer64egress_ts; > + Integer64rx_ts; > +} PACKED; Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > @@ -490,6 +491,15 @@ static int mgt_post_recv(struct management_tlv *m, > uint16_t data_len, > if (data_len != 0) > goto bad_length; > break; > + case MID_CMLDS_INFO_NP: > + if (data_len < sizeof(struct cmlds_info_np)) > + goto bad_length; > + cmlds = (struct cmlds_info_np *)m->data; > + net2host64_unaligned(>meanLinkDelay); > + NTOHL(cmlds->scaledNeighborRateRatio); > + net2host64_unaligned(>egress_ts); > + net2host64_unaligned(>rx_ts); > + break; Place after MID_POWER_PROFILE_SETTINGS_NP please. > } > if (extra_len) { > if (extra_len % 2) Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > @@ -651,6 +652,18 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) > fprintf(fp, "LOG_MIN_PDELAY_REQ_INTERVAL " > IFMT "logMinPdelayReqInterval %hhd", mtd->val); > break; > + case MID_CMLDS_INFO_NP: > + cmlds = (struct cmlds_info_np *) mgt->data; > + fprintf(fp, "CMLDS INFO " > + IFMT "serviceMeasurementValid %i" > + IFMT "meanLinkDelay %" PRId64 > + IFMT "scaledNeighborRateRatio %" PRId32 > + IFMT "egress_ts %" PRId64 > + IFMT "rx_ts %" PRId64, > + cmlds->serviceMeasurementValid, cmlds->meanLinkDelay, > + cmlds->scaledNeighborRateRatio, > + cmlds->egress_ts, cmlds->rx_ts); > + break; Nit: please place this after MID_POWER_PROFILE_SETTINGS_NP rather than tacking on the end of the switch/case. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > @@ -473,6 +476,14 @@ struct msg_interface_rate_tlv { > UInteger16numberOfBitsAfterTimestamp; > } PACKED; > > +struct cmlds_info_np { > + Integer8 serviceMeasurementValid; > + TimeInterval meanLinkDelay; > + Integer32scaledNeighborRateRatio; > + Integer64egress_ts; > + Integer64rx_ts; > +} PACKED; This layout is very poor for packed formats -- think about alignment! Please order the fields according to size, from largest to smallest. Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Mon, May 15, 2023 at 06:26:04PM -0400, Kishen Maloor wrote: > @@ -1129,6 +1130,27 @@ static int port_management_fill_response(struct port > *target, > memcpy(pwr, >pwr, sizeof(*pwr)); > datalen = sizeof(*pwr); > break; > + case MID_CMLDS_INFO_NP: > + cmlds = (struct cmlds_info_np *)tlv->data; > + /* IEEE1588-2019 16.6.3.2 h) 1) && nrate.ratio_valid because > + * we have no extra field to convey that separately. > + */ > + cmlds->serviceMeasurementValid = > + target->peer_portid_valid && !target->pdr_missing && > + !target->multiple_pdr_detected && > + target->nrate.ratio_valid; > + cmlds->meanLinkDelay = target->peerMeanPathDelay; > + cmlds->scaledNeighborRateRatio = > + (Integer32) (target->nrate.ratio * POW2_41 - POW2_41); > + /* 16.6.3.2: "Upon receipt of a request for information, the > + * Common Mean Link Delay Service may in addition return the > + * raw measurement data gathered by the service for use in > + * estimating the and ." > + */ > + cmlds->egress_ts = tmv_to_nanoseconds(target->peer_delay_t1); > + cmlds->rx_ts = tmv_to_nanoseconds(target->peer_delay_t2); Please drop these two fields. They don't provide any benefit. If the clients don't trust the CMLDS values, then they are free to measure the p2p delay themselves! Thanks, Richard ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
Thanks for the reply. Please add the explanation to the commit and to the structure. Personally, I do not have an opinion, yet I did not participate in the IEEE 1558 committee. Erez On Tue, 16 May 2023 at 23:43, Kishen Maloor wrote: > On 5/16/23 3:56 AM, Erez wrote: > > On Tue, 16 May 2023 at 00:29, Kishen Maloor > wrote: > > > >> In a setup with multiple gPTP domains, the Common Mean Link Delay > Service > >> (CMLDS) (IEEE 1588/16.6.3) performs link delay measurements in a single > >> domain and must (somehow) convey those to other domains. IEEE 1588 does > not > >> specify this interface and flags it as an implementation > >> detail (IEEE 1588/16.6.1). Accordningly, this change introduces a new > >> TLV to convey link delay measurements by the CMLDS over the management > >> interface. > >> > >> In addition to the parameters suggested in IEEE 1588/16.6.3.2, > >> the TLV also conveys the latest 'up measurements' (req and rx > timestamps) > >> recorded in the CMLDS. These values collectively aid other gPTP domains > to > >> complete their delay/offset computations via the COMMON_P2P > >> delay mechanism. > >> > >> Updated 'pmc' to support the new MID, MID_CMLDS_INFO_NP. > >> > >> Co-authored-by: Andrew Zaborowski > >> Signed-off-by: Kishen Maloor > >> --- > >> clock.c| 1 - > >> msg.h | 2 ++ > >> pmc.c | 13 + > >> pmc_common.c | 1 + > >> port.c | 22 ++ > >> port_private.h | 2 ++ > >> tlv.c | 18 ++ > >> tlv.h | 11 +++ > >> 8 files changed, 69 insertions(+), 1 deletion(-) > >> > >> diff --git a/clock.c b/clock.c > >> index fe08d48ed8f8..c74a6baa9f61 100644 > >> --- a/clock.c > >> +++ b/clock.c > >> @@ -47,7 +47,6 @@ > >> #include "util.h" > >> > >> #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault > >> timer */ > >> -#define POW2_41 ((double)(1ULL << 41)) > >> > >> struct interface { > >> STAILQ_ENTRY(interface) list; > >> diff --git a/msg.h b/msg.h > >> index cbd09e75a2aa..db12e249f89f 100644 > >> --- a/msg.h > >> +++ b/msg.h > >> @@ -69,6 +69,8 @@ > >> #define SIGNAL_NO_CHANGE -128 > >> #define SIGNAL_SET_INITIAL 126 > >> > >> +#define POW2_41 ((double)(1ULL << 41)) > >> + > >> enum timestamp_type { > >> TS_SOFTWARE, > >> TS_HARDWARE, > >> diff --git a/pmc.c b/pmc.c > >> index 00e691f0c244..cac06fb5b41d 100644 > >> --- a/pmc.c > >> +++ b/pmc.c > >> @@ -169,6 +169,7 @@ static void pmc_show(struct ptp_message *msg, FILE > *fp) > >> struct subscribe_events_np *sen; > >> struct port_properties_np *ppn; > >> struct port_hwclock_np *phn; > >> + struct cmlds_info_np *cmlds; > >> struct timePropertiesDS *tp; > >> struct management_tlv *mgt; > >> struct time_status_np *tsn; > >> @@ -651,6 +652,18 @@ static void pmc_show(struct ptp_message *msg, FILE > >> *fp) > >> fprintf(fp, "LOG_MIN_PDELAY_REQ_INTERVAL " > >> IFMT "logMinPdelayReqInterval %hhd", mtd->val); > >> break; > >> + case MID_CMLDS_INFO_NP: > >> + cmlds = (struct cmlds_info_np *) mgt->data; > >> + fprintf(fp, "CMLDS INFO " > >> + IFMT "serviceMeasurementValid %i" > >> + IFMT "meanLinkDelay %" PRId64 > >> + IFMT "scaledNeighborRateRatio %" PRId32 > >> + IFMT "egress_ts %" PRId64 > >> + IFMT "rx_ts %" PRId64, > >> + cmlds->serviceMeasurementValid, > >> cmlds->meanLinkDelay, > >> + cmlds->scaledNeighborRateRatio, > >> + cmlds->egress_ts, cmlds->rx_ts); > >> + break; > >> } > >> out: > >> fprintf(fp, "\n"); > >> diff --git a/pmc_common.c b/pmc_common.c > >> index 9e251c43e95b..d7a6114dcd62 100644 > >> --- a/pmc_common.c > >> +++ b/pmc_common.c > >> @@ -156,6 +156,7 @@ struct management_id idtab[] = { > >> { "UNICAST_MASTER_TABLE_NP", MID_UNICAST_MASTER_TABLE_NP, > >> do_get_action }, > >> { "PORT_HWCLOCK_NP", MID_PORT_HWCLOCK_NP, do_get_action }, > >> { "POWER_PROFILE_SETTINGS_NP", MID_POWER_PROFILE_SETTINGS_NP, > >> do_set_action }, > >> + { "CMLDS_INFO_NP", MID_CMLDS_INFO_NP, do_get_action }, > >> }; > >> > >> static void do_get_action(struct pmc *pmc, int action, int index, char > >> *str) > >> diff --git a/port.c b/port.c > >> index 8b2eb04a855a..d467a69e519a 100644 > >> --- a/port.c > >> +++ b/port.c > >> @@ -887,6 +887,7 @@ static int port_management_fill_response(struct port > >> *target, > >> struct clock_description *desc; > >> struct port_properties_np *ppn; > >> struct port_hwclock_np *phn; > >> + struct cmlds_info_np *cmlds; > >> struct management_tlv *tlv; > >> struct port_stats_np *psn; > >> struct foreign_clock *fc; > >> @@ -1129,6
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On 5/16/23 3:56 AM, Erez wrote: > On Tue, 16 May 2023 at 00:29, Kishen Maloor wrote: > >> In a setup with multiple gPTP domains, the Common Mean Link Delay Service >> (CMLDS) (IEEE 1588/16.6.3) performs link delay measurements in a single >> domain and must (somehow) convey those to other domains. IEEE 1588 does not >> specify this interface and flags it as an implementation >> detail (IEEE 1588/16.6.1). Accordningly, this change introduces a new >> TLV to convey link delay measurements by the CMLDS over the management >> interface. >> >> In addition to the parameters suggested in IEEE 1588/16.6.3.2, >> the TLV also conveys the latest 'up measurements' (req and rx timestamps) >> recorded in the CMLDS. These values collectively aid other gPTP domains to >> complete their delay/offset computations via the COMMON_P2P >> delay mechanism. >> >> Updated 'pmc' to support the new MID, MID_CMLDS_INFO_NP. >> >> Co-authored-by: Andrew Zaborowski >> Signed-off-by: Kishen Maloor >> --- >> clock.c| 1 - >> msg.h | 2 ++ >> pmc.c | 13 + >> pmc_common.c | 1 + >> port.c | 22 ++ >> port_private.h | 2 ++ >> tlv.c | 18 ++ >> tlv.h | 11 +++ >> 8 files changed, 69 insertions(+), 1 deletion(-) >> >> diff --git a/clock.c b/clock.c >> index fe08d48ed8f8..c74a6baa9f61 100644 >> --- a/clock.c >> +++ b/clock.c >> @@ -47,7 +47,6 @@ >> #include "util.h" >> >> #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault >> timer */ >> -#define POW2_41 ((double)(1ULL << 41)) >> >> struct interface { >> STAILQ_ENTRY(interface) list; >> diff --git a/msg.h b/msg.h >> index cbd09e75a2aa..db12e249f89f 100644 >> --- a/msg.h >> +++ b/msg.h >> @@ -69,6 +69,8 @@ >> #define SIGNAL_NO_CHANGE -128 >> #define SIGNAL_SET_INITIAL 126 >> >> +#define POW2_41 ((double)(1ULL << 41)) >> + >> enum timestamp_type { >> TS_SOFTWARE, >> TS_HARDWARE, >> diff --git a/pmc.c b/pmc.c >> index 00e691f0c244..cac06fb5b41d 100644 >> --- a/pmc.c >> +++ b/pmc.c >> @@ -169,6 +169,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) >> struct subscribe_events_np *sen; >> struct port_properties_np *ppn; >> struct port_hwclock_np *phn; >> + struct cmlds_info_np *cmlds; >> struct timePropertiesDS *tp; >> struct management_tlv *mgt; >> struct time_status_np *tsn; >> @@ -651,6 +652,18 @@ static void pmc_show(struct ptp_message *msg, FILE >> *fp) >> fprintf(fp, "LOG_MIN_PDELAY_REQ_INTERVAL " >> IFMT "logMinPdelayReqInterval %hhd", mtd->val); >> break; >> + case MID_CMLDS_INFO_NP: >> + cmlds = (struct cmlds_info_np *) mgt->data; >> + fprintf(fp, "CMLDS INFO " >> + IFMT "serviceMeasurementValid %i" >> + IFMT "meanLinkDelay %" PRId64 >> + IFMT "scaledNeighborRateRatio %" PRId32 >> + IFMT "egress_ts %" PRId64 >> + IFMT "rx_ts %" PRId64, >> + cmlds->serviceMeasurementValid, >> cmlds->meanLinkDelay, >> + cmlds->scaledNeighborRateRatio, >> + cmlds->egress_ts, cmlds->rx_ts); >> + break; >> } >> out: >> fprintf(fp, "\n"); >> diff --git a/pmc_common.c b/pmc_common.c >> index 9e251c43e95b..d7a6114dcd62 100644 >> --- a/pmc_common.c >> +++ b/pmc_common.c >> @@ -156,6 +156,7 @@ struct management_id idtab[] = { >> { "UNICAST_MASTER_TABLE_NP", MID_UNICAST_MASTER_TABLE_NP, >> do_get_action }, >> { "PORT_HWCLOCK_NP", MID_PORT_HWCLOCK_NP, do_get_action }, >> { "POWER_PROFILE_SETTINGS_NP", MID_POWER_PROFILE_SETTINGS_NP, >> do_set_action }, >> + { "CMLDS_INFO_NP", MID_CMLDS_INFO_NP, do_get_action }, >> }; >> >> static void do_get_action(struct pmc *pmc, int action, int index, char >> *str) >> diff --git a/port.c b/port.c >> index 8b2eb04a855a..d467a69e519a 100644 >> --- a/port.c >> +++ b/port.c >> @@ -887,6 +887,7 @@ static int port_management_fill_response(struct port >> *target, >> struct clock_description *desc; >> struct port_properties_np *ppn; >> struct port_hwclock_np *phn; >> + struct cmlds_info_np *cmlds; >> struct management_tlv *tlv; >> struct port_stats_np *psn; >> struct foreign_clock *fc; >> @@ -1129,6 +1130,27 @@ static int port_management_fill_response(struct >> port *target, >> memcpy(pwr, >pwr, sizeof(*pwr)); >> datalen = sizeof(*pwr); >> break; >> + case MID_CMLDS_INFO_NP: >> + cmlds = (struct cmlds_info_np *)tlv->data; >> + /* IEEE1588-2019 16.6.3.2 h) 1) && nrate.ratio_valid >> because >> +* we have no extra field to convey that separately. >> +*/ >> +
Re: [Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
On Tue, 16 May 2023 at 00:29, Kishen Maloor wrote: > In a setup with multiple gPTP domains, the Common Mean Link Delay Service > (CMLDS) (IEEE 1588/16.6.3) performs link delay measurements in a single > domain and must (somehow) convey those to other domains. IEEE 1588 does not > specify this interface and flags it as an implementation > detail (IEEE 1588/16.6.1). Accordningly, this change introduces a new > TLV to convey link delay measurements by the CMLDS over the management > interface. > > In addition to the parameters suggested in IEEE 1588/16.6.3.2, > the TLV also conveys the latest 'up measurements' (req and rx timestamps) > recorded in the CMLDS. These values collectively aid other gPTP domains to > complete their delay/offset computations via the COMMON_P2P > delay mechanism. > > Updated 'pmc' to support the new MID, MID_CMLDS_INFO_NP. > > Co-authored-by: Andrew Zaborowski > Signed-off-by: Kishen Maloor > --- > clock.c| 1 - > msg.h | 2 ++ > pmc.c | 13 + > pmc_common.c | 1 + > port.c | 22 ++ > port_private.h | 2 ++ > tlv.c | 18 ++ > tlv.h | 11 +++ > 8 files changed, 69 insertions(+), 1 deletion(-) > > diff --git a/clock.c b/clock.c > index fe08d48ed8f8..c74a6baa9f61 100644 > --- a/clock.c > +++ b/clock.c > @@ -47,7 +47,6 @@ > #include "util.h" > > #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault > timer */ > -#define POW2_41 ((double)(1ULL << 41)) > > struct interface { > STAILQ_ENTRY(interface) list; > diff --git a/msg.h b/msg.h > index cbd09e75a2aa..db12e249f89f 100644 > --- a/msg.h > +++ b/msg.h > @@ -69,6 +69,8 @@ > #define SIGNAL_NO_CHANGE -128 > #define SIGNAL_SET_INITIAL 126 > > +#define POW2_41 ((double)(1ULL << 41)) > + > enum timestamp_type { > TS_SOFTWARE, > TS_HARDWARE, > diff --git a/pmc.c b/pmc.c > index 00e691f0c244..cac06fb5b41d 100644 > --- a/pmc.c > +++ b/pmc.c > @@ -169,6 +169,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) > struct subscribe_events_np *sen; > struct port_properties_np *ppn; > struct port_hwclock_np *phn; > + struct cmlds_info_np *cmlds; > struct timePropertiesDS *tp; > struct management_tlv *mgt; > struct time_status_np *tsn; > @@ -651,6 +652,18 @@ static void pmc_show(struct ptp_message *msg, FILE > *fp) > fprintf(fp, "LOG_MIN_PDELAY_REQ_INTERVAL " > IFMT "logMinPdelayReqInterval %hhd", mtd->val); > break; > + case MID_CMLDS_INFO_NP: > + cmlds = (struct cmlds_info_np *) mgt->data; > + fprintf(fp, "CMLDS INFO " > + IFMT "serviceMeasurementValid %i" > + IFMT "meanLinkDelay %" PRId64 > + IFMT "scaledNeighborRateRatio %" PRId32 > + IFMT "egress_ts %" PRId64 > + IFMT "rx_ts %" PRId64, > + cmlds->serviceMeasurementValid, > cmlds->meanLinkDelay, > + cmlds->scaledNeighborRateRatio, > + cmlds->egress_ts, cmlds->rx_ts); > + break; > } > out: > fprintf(fp, "\n"); > diff --git a/pmc_common.c b/pmc_common.c > index 9e251c43e95b..d7a6114dcd62 100644 > --- a/pmc_common.c > +++ b/pmc_common.c > @@ -156,6 +156,7 @@ struct management_id idtab[] = { > { "UNICAST_MASTER_TABLE_NP", MID_UNICAST_MASTER_TABLE_NP, > do_get_action }, > { "PORT_HWCLOCK_NP", MID_PORT_HWCLOCK_NP, do_get_action }, > { "POWER_PROFILE_SETTINGS_NP", MID_POWER_PROFILE_SETTINGS_NP, > do_set_action }, > + { "CMLDS_INFO_NP", MID_CMLDS_INFO_NP, do_get_action }, > }; > > static void do_get_action(struct pmc *pmc, int action, int index, char > *str) > diff --git a/port.c b/port.c > index 8b2eb04a855a..d467a69e519a 100644 > --- a/port.c > +++ b/port.c > @@ -887,6 +887,7 @@ static int port_management_fill_response(struct port > *target, > struct clock_description *desc; > struct port_properties_np *ppn; > struct port_hwclock_np *phn; > + struct cmlds_info_np *cmlds; > struct management_tlv *tlv; > struct port_stats_np *psn; > struct foreign_clock *fc; > @@ -1129,6 +1130,27 @@ static int port_management_fill_response(struct > port *target, > memcpy(pwr, >pwr, sizeof(*pwr)); > datalen = sizeof(*pwr); > break; > + case MID_CMLDS_INFO_NP: > + cmlds = (struct cmlds_info_np *)tlv->data; > + /* IEEE1588-2019 16.6.3.2 h) 1) && nrate.ratio_valid > because > +* we have no extra field to convey that separately. > +*/ > + cmlds->serviceMeasurementValid = > + target->peer_portid_valid && !target->pdr_missing > && > +
[Linuxptp-devel] [RFC PATCH v2 1/9] Add new TLV for CommonMeanLinkDelayInformation
In a setup with multiple gPTP domains, the Common Mean Link Delay Service (CMLDS) (IEEE 1588/16.6.3) performs link delay measurements in a single domain and must (somehow) convey those to other domains. IEEE 1588 does not specify this interface and flags it as an implementation detail (IEEE 1588/16.6.1). Accordningly, this change introduces a new TLV to convey link delay measurements by the CMLDS over the management interface. In addition to the parameters suggested in IEEE 1588/16.6.3.2, the TLV also conveys the latest 'up measurements' (req and rx timestamps) recorded in the CMLDS. These values collectively aid other gPTP domains to complete their delay/offset computations via the COMMON_P2P delay mechanism. Updated 'pmc' to support the new MID, MID_CMLDS_INFO_NP. Co-authored-by: Andrew Zaborowski Signed-off-by: Kishen Maloor --- clock.c| 1 - msg.h | 2 ++ pmc.c | 13 + pmc_common.c | 1 + port.c | 22 ++ port_private.h | 2 ++ tlv.c | 18 ++ tlv.h | 11 +++ 8 files changed, 69 insertions(+), 1 deletion(-) diff --git a/clock.c b/clock.c index fe08d48ed8f8..c74a6baa9f61 100644 --- a/clock.c +++ b/clock.c @@ -47,7 +47,6 @@ #include "util.h" #define N_CLOCK_PFD (N_POLLFD + 1) /* one extra per port, for the fault timer */ -#define POW2_41 ((double)(1ULL << 41)) struct interface { STAILQ_ENTRY(interface) list; diff --git a/msg.h b/msg.h index cbd09e75a2aa..db12e249f89f 100644 --- a/msg.h +++ b/msg.h @@ -69,6 +69,8 @@ #define SIGNAL_NO_CHANGE -128 #define SIGNAL_SET_INITIAL 126 +#define POW2_41 ((double)(1ULL << 41)) + enum timestamp_type { TS_SOFTWARE, TS_HARDWARE, diff --git a/pmc.c b/pmc.c index 00e691f0c244..cac06fb5b41d 100644 --- a/pmc.c +++ b/pmc.c @@ -169,6 +169,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) struct subscribe_events_np *sen; struct port_properties_np *ppn; struct port_hwclock_np *phn; + struct cmlds_info_np *cmlds; struct timePropertiesDS *tp; struct management_tlv *mgt; struct time_status_np *tsn; @@ -651,6 +652,18 @@ static void pmc_show(struct ptp_message *msg, FILE *fp) fprintf(fp, "LOG_MIN_PDELAY_REQ_INTERVAL " IFMT "logMinPdelayReqInterval %hhd", mtd->val); break; + case MID_CMLDS_INFO_NP: + cmlds = (struct cmlds_info_np *) mgt->data; + fprintf(fp, "CMLDS INFO " + IFMT "serviceMeasurementValid %i" + IFMT "meanLinkDelay %" PRId64 + IFMT "scaledNeighborRateRatio %" PRId32 + IFMT "egress_ts %" PRId64 + IFMT "rx_ts %" PRId64, + cmlds->serviceMeasurementValid, cmlds->meanLinkDelay, + cmlds->scaledNeighborRateRatio, + cmlds->egress_ts, cmlds->rx_ts); + break; } out: fprintf(fp, "\n"); diff --git a/pmc_common.c b/pmc_common.c index 9e251c43e95b..d7a6114dcd62 100644 --- a/pmc_common.c +++ b/pmc_common.c @@ -156,6 +156,7 @@ struct management_id idtab[] = { { "UNICAST_MASTER_TABLE_NP", MID_UNICAST_MASTER_TABLE_NP, do_get_action }, { "PORT_HWCLOCK_NP", MID_PORT_HWCLOCK_NP, do_get_action }, { "POWER_PROFILE_SETTINGS_NP", MID_POWER_PROFILE_SETTINGS_NP, do_set_action }, + { "CMLDS_INFO_NP", MID_CMLDS_INFO_NP, do_get_action }, }; static void do_get_action(struct pmc *pmc, int action, int index, char *str) diff --git a/port.c b/port.c index 8b2eb04a855a..d467a69e519a 100644 --- a/port.c +++ b/port.c @@ -887,6 +887,7 @@ static int port_management_fill_response(struct port *target, struct clock_description *desc; struct port_properties_np *ppn; struct port_hwclock_np *phn; + struct cmlds_info_np *cmlds; struct management_tlv *tlv; struct port_stats_np *psn; struct foreign_clock *fc; @@ -1129,6 +1130,27 @@ static int port_management_fill_response(struct port *target, memcpy(pwr, >pwr, sizeof(*pwr)); datalen = sizeof(*pwr); break; + case MID_CMLDS_INFO_NP: + cmlds = (struct cmlds_info_np *)tlv->data; + /* IEEE1588-2019 16.6.3.2 h) 1) && nrate.ratio_valid because +* we have no extra field to convey that separately. +*/ + cmlds->serviceMeasurementValid = + target->peer_portid_valid && !target->pdr_missing && + !target->multiple_pdr_detected && + target->nrate.ratio_valid; + cmlds->meanLinkDelay = target->peerMeanPathDelay; + cmlds->scaledNeighborRateRatio = + (Integer32) (target->nrate.ratio * POW2_41 - POW2_41); + /* 16.6.3.2: "Upon receipt