On 5/16/23 3:56 AM, Erez wrote:
> On Tue, 16 May 2023 at 00:29, Kishen Maloor <kishen.mal...@intel.com> 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 <andrew.zaborow...@intel.com>
>> Signed-off-by: Kishen Maloor <kishen.mal...@intel.com>
>> ---
>> 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, &target->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 <meanLinkDelay> and <neighborRateRatio>."
>> + */
>> + cmlds->egress_ts =
>> tmv_to_nanoseconds(target->peer_delay_t1);
>> + cmlds->rx_ts = tmv_to_nanoseconds(target->peer_delay_t2);
>> + datalen = sizeof(*cmlds);
>> + break;
>> default:
>> /* The caller should *not* respond to this message. */
>> tlv_extra_recycle(extra);
>> diff --git a/port_private.h b/port_private.h
>> index 3b02d2fe45c4..c9b02bc799f5 100644
>> --- a/port_private.h
>> +++ b/port_private.h
>> @@ -99,6 +99,8 @@ struct port {
>> unsigned int pdr_missing;
>> unsigned int multiple_seq_pdr_count;
>> unsigned int multiple_pdr_detected;
>> + tmv_t peer_delay_t1;
>> + tmv_t peer_delay_t2;
>> enum port_state (*state_machine)(enum port_state state,
>> enum fsm_event event, int mdiff);
>> int bmca;
>> diff --git a/tlv.c b/tlv.c
>> index 79400126cbc4..8b97d2cfc9f5 100644
>> --- a/tlv.c
>> +++ b/tlv.c
>> @@ -176,6 +176,7 @@ static int mgt_post_recv(struct management_tlv *m,
>> uint16_t data_len,
>> struct port_properties_np *ppn;
>> struct port_hwclock_np *phn;
>> struct timePropertiesDS *tp;
>> + struct cmlds_info_np *cmlds;
>> struct time_status_np *tsn;
>> struct port_stats_np *psn;
>> int extra_len = 0, i, len;
>> @@ -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(&cmlds->meanLinkDelay);
>> + NTOHL(cmlds->scaledNeighborRateRatio);
>> + net2host64_unaligned(&cmlds->egress_ts);
>> + net2host64_unaligned(&cmlds->rx_ts);
>> + break;
>> }
>> if (extra_len) {
>> if (extra_len % 2)
>> @@ -514,6 +524,7 @@ static void mgt_pre_send(struct management_tlv *m,
>> struct tlv_extra *extra)
>> struct subscribe_events_np *sen;
>> struct port_properties_np *ppn;
>> struct port_hwclock_np *phn;
>> + struct cmlds_info_np *cmlds;
>> struct timePropertiesDS *tp;
>> struct time_status_np *tsn;
>> struct port_stats_np *psn;
>> @@ -672,6 +683,13 @@ static void mgt_pre_send(struct management_tlv *m,
>> struct tlv_extra *extra)
>> HTONL(pwr->networkTimeInaccuracy);
>> HTONL(pwr->totalTimeInaccuracy);
>> break;
>> + case MID_CMLDS_INFO_NP:
>> + cmlds = (struct cmlds_info_np *)m->data;
>> + host2net64_unaligned(&cmlds->meanLinkDelay);
>> + HTONL(cmlds->scaledNeighborRateRatio);
>> + host2net64_unaligned(&cmlds->egress_ts);
>> + host2net64_unaligned(&cmlds->rx_ts);
>> + break;
>> }
>> }
>>
>> diff --git a/tlv.h b/tlv.h
>> index 8b51ffd88816..6446fc068488 100644
>> --- a/tlv.h
>> +++ b/tlv.h
>> @@ -130,6 +130,9 @@ enum management_action {
>> #define MID_PORT_HWCLOCK_NP 0xC009
>> #define MID_POWER_PROFILE_SETTINGS_NP 0xC00A
>>
>> +/* CMLDS management ID values */
>> +#define MID_CMLDS_INFO_NP 0xC00B
>> +
>> /* Management error ID values */
>> #define MID_RESPONSE_TOO_BIG 0x0001
>> #define MID_NO_SUCH_ID 0x0002
>> @@ -473,6 +476,14 @@ struct msg_interface_rate_tlv {
>> UInteger16 numberOfBitsAfterTimestamp;
>> } PACKED;
>>
>> +struct cmlds_info_np {
>> + Integer8 serviceMeasurementValid;
>> + TimeInterval meanLinkDelay;
>> + Integer32 scaledNeighborRateRatio;
>
> + Integer64 egress_ts;
>> + Integer64 rx_ts;
>>
>
> Not really sure what is better.
> But why not using Timestamp?, the annoying type from IEEE.
> Can you please add a remark here with cons and pros and a decision here?
This TLV (and its conveyance method) is unspecified and is set by the
implementation.
So as such, we're handing these tmv_t timestamps as is to a requesting port,
wherefrom
they're directly passed in a call to clock_peer_delay().
I guess we didn't think to send these values as struct Timestamp instead
because it wasn't
necessary.
>
>
>> +} PACKED;
>> +
>> /**
>> * Allocates a new tlv_extra structure.
>> * @return Pointer to a new structure on success or NULL otherwise.
>> --
>> 2.31.1
>>
>>
>>
>> _______________________________________________
>> Linuxptp-devel mailing list
>> 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
