Hi Richard,
I haven't tested this but here are some comments.
On Thu, 30 Nov 2023 at 08:58, Richard Cochran <richardcoch...@gmail.com> wrote:
> From: Kishen Maloor <kishen.mal...@intel.com>
>
> Signed-off-by: Richard Cochran <richardcoch...@gmail.com>
> ---
> config.c | 4 ++
> dm.h | 3 +
> fd.h | 1 +
> makefile | 4 +-
> port.c | 166 +++++++++++++++++++++++++++++++++++++++++++++++--
> port_private.h | 3 +
> 6 files changed, 174 insertions(+), 7 deletions(-)
>
> diff --git a/config.c b/config.c
> index fe65b76..05ffd3c 100644
> --- a/config.c
> +++ b/config.c
> @@ -171,6 +171,7 @@ static struct config_enum delay_filter_enu[] = {
>
> static struct config_enum delay_mech_enu[] = {
> { "Auto", DM_AUTO },
> + { "COMMON_P2P", DM_COMMON_P2P },
> { "E2E", DM_E2E },
> { "P2P", DM_P2P },
> { "NONE", DM_NO_MECHANISM },
> @@ -249,6 +250,9 @@ struct config_item config_tab[] = {
> GLOB_ITEM_INT("clock_class_threshold", CLOCK_CLASS_THRESHOLD_DEFAULT,
> 6, CLOCK_CLASS_THRESHOLD_DEFAULT),
> GLOB_ITEM_ENU("clock_servo", CLOCK_SERVO_PI, clock_servo_enu),
> GLOB_ITEM_ENU("clock_type", CLOCK_TYPE_ORDINARY, clock_type_enu),
> + PORT_ITEM_STR("cmlds_client_address", "/var/run/cmlds_cleint"),
I assume the use of this is simply to set unique names for per-port
sockets. Maybe something like tempnam() could be used instead.
> + PORT_ITEM_INT("cmlds_port", 1, 1, UINT16_MAX),
> + PORT_ITEM_STR("cmlds_server_address", "/var/run/cmlds_server"),
> GLOB_ITEM_ENU("dataset_comparison", DS_CMP_IEEE1588,
> dataset_comp_enu),
> PORT_ITEM_INT("delayAsymmetry", 0, INT_MIN, INT_MAX),
> PORT_ITEM_ENU("delay_filter", FILTER_MOVING_MEDIAN, delay_filter_enu),
> diff --git a/dm.h b/dm.h
> index 47bd847..80d1ce5 100644
> --- a/dm.h
> +++ b/dm.h
> @@ -34,6 +34,9 @@ enum delay_mechanism {
> /** Peer delay mechanism. */
> DM_P2P,
>
> + /** Peer delay as measured by CMLDS. */
> + DM_COMMON_P2P,
> +
> /** No Delay Mechanism. */
> DM_NO_MECHANISM = 0xFE,
> };
> diff --git a/fd.h b/fd.h
> index 16420d7..9a072ab 100644
> --- a/fd.h
> +++ b/fd.h
> @@ -39,6 +39,7 @@ enum {
> FD_SYNC_TX_TIMER,
> FD_UNICAST_REQ_TIMER,
> FD_UNICAST_SRV_TIMER,
> + FD_CMLDS,
> FD_RTNL,
> N_POLLFD,
> };
> diff --git a/makefile b/makefile
> index 7fc5f6f..e9c1ccc 100644
> --- a/makefile
> +++ b/makefile
> @@ -30,8 +30,8 @@ TS2PHC = ts2phc.o lstab.o nmea.o serial.o sock.o
> ts2phc_generic_pps_source.o \
> ts2phc_nmea_pps_source.o ts2phc_phc_pps_source.o ts2phc_pps_sink.o
> ts2phc_pps_source.o
> OBJ = bmc.o clock.o clockadj.o clockcheck.o config.o designated_fsm.o \
> e2e_tc.o fault.o $(FILTERS) fsm.o hash.o interface.o monitor.o msg.o phc.o \
> - port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o rtnl.o $(SERVOS) \
> - sk.o stats.o tc.o $(TRANSP) telecom.o tlv.o tsproc.o unicast_client.o \
> + pmc_common.o port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o
> rtnl.o \
> + $(SERVOS) sk.o stats.o tc.o $(TRANSP) telecom.o tlv.o tsproc.o
> unicast_client.o \
> unicast_fsm.o unicast_service.o util.o version.o
>
> OBJECTS = $(OBJ) hwstamp_ctl.o nsm.o phc2sys.o phc_ctl.o pmc.o
> pmc_agent.o \
> diff --git a/port.c b/port.c
> index 8afe1b2..75b4dd8 100644
> --- a/port.c
> +++ b/port.c
> @@ -963,7 +963,8 @@ static int port_management_fill_response(struct port
> *target,
> ptp_text_copy(cd->userDescription, &desc->userDescription);
> buf += sizeof(struct PTPText) + cd->userDescription->length;
>
> - if (target->delayMechanism == DM_P2P) {
> + if (target->delayMechanism == DM_P2P ||
> + target->delayMechanism == DM_COMMON_P2P) {
> memcpy(buf, profile_id_p2p, PROFILE_ID_LEN);
> } else {
> struct config *cfg = clock_config(target->clock);
> @@ -1868,6 +1869,33 @@ static void port_clear_fda(struct port *p, int count)
> p->fda.fd[i] = -1;
> }
>
> +static int port_cmlds_initialize(struct port *p)
> +{
> + struct config *cfg = clock_config(p->clock);
> + struct subscribe_events_np sen = {0};
> + const int zero_datalen = 1;
> + const UInteger8 hops = 0;
> +
> + p->cmlds_port = config_get_int(cfg, p->name, "cmlds_port");
Should this fall back to port_number(p)?
> + p->cmlds_pmc = pmc_create(cfg, TRANS_UDS,
> + config_get_string(cfg, p->name,
> "cmlds_client_address"),
> + config_get_string(cfg, p->name,
> "cmlds_server_address"),
> + hops,
> + config_get_int(cfg, NULL, "domainNumber"),
> + config_get_int(cfg, p->name,
> "transportSpecific") << 4,
The sdoId is (by either IEEE 1588 or 802.1AS) different for the CMLDS
than for a P2P_COMMON port, I think a check in port_ignore will
discard the messages as a result. Maybe hardcode TS_CMLDS here? (with
TS_CMLDS defined in msg.h next to TS_IEEE_8021AS)
Using pmc here surely works but I wonder if it's not actually more
work than reusing c->uds_port.
> + zero_datalen);
> + if (!p->cmlds_pmc) {
> + return -1;
> + }
> +
> + event_bitmask_set(sen.bitmask, NOTIFY_CMLDS, TRUE);
> + sen.duration = 3600;
> + p->fda.fd[FD_CMLDS] = pmc_get_transport_fd(p->cmlds_pmc);
> + pmc_send_set_action(p->cmlds_pmc, MID_SUBSCRIBE_EVENTS_NP, &sen,
> sizeof(sen));
> +
> + return 0;
> +}
> +
> void port_disable(struct port *p)
> {
> int i;
> @@ -1885,6 +1913,12 @@ void port_disable(struct port *p)
> close(p->fda.fd[FD_FIRST_TIMER + i]);
> }
>
> + if (p->cmlds_pmc) {
> + pmc_destroy(p->cmlds_pmc);
> + p->fda.fd[FD_CMLDS] = -1;
> + p->cmlds_pmc = NULL;
> + }
> +
> /* Keep rtnl socket to get link status info. */
> port_clear_fda(p, FD_RTNL);
> clock_fda_changed(p->clock);
> @@ -1932,7 +1966,8 @@ int port_initialize(struct port *p)
> pr_err("inhibit_delay_req can only be set when asCapable ==
> 'true'.");
> return -1;
> }
> - if (port_delay_mechanism(p) == DM_NO_MECHANISM) {
> + if (port_delay_mechanism(p) == DM_COMMON_P2P ||
> + port_delay_mechanism(p) == DM_NO_MECHANISM) {
> p->inhibit_delay_req = 1;
> }
>
> @@ -1960,6 +1995,10 @@ int port_initialize(struct port *p)
> goto no_tmo;
> }
>
> + if (port_delay_mechanism(p) == DM_COMMON_P2P &&
> port_cmlds_initialize(p)) {
> + goto no_tmo;
> + }
> +
> /* No need to open rtnl socket on UDS port. */
> if (!port_is_uds(p)) {
> /*
> @@ -2101,6 +2140,65 @@ int process_announce(struct port *p, struct
> ptp_message *m)
> return result;
> }
>
> +static int process_cmlds(struct port *p)
> +{
> + struct cmlds_info_np *cmlds;
> + struct management_tlv *mgt;
> + struct ptp_message *msg;
> + struct TLV *tlv;
> + int err = 0;
> +
> + msg = pmc_recv(p->cmlds_pmc);
> + if (!msg) {
> + pr_err("%s: pmc_recv failed", p->log_name);
> + return -1;
> + }
> + if (msg_type(msg) != MANAGEMENT) {
> + pr_err("%s: pmc_recv bad message", p->log_name);
> + err = -1;
> + goto out;
> + }
> + tlv = (struct TLV *) msg->management.suffix;
> + if (tlv->type != TLV_MANAGEMENT) {
> + pr_err("%s: pmc_recv bad message", p->log_name);
> + err = -1;
> + goto out;
> + }
> + mgt = (struct management_tlv *) msg->management.suffix;
> + if (mgt->length == 2 && mgt->id != MID_NULL_MANAGEMENT) {
> + pr_err("%s: pmc_recv bad length", p->log_name);
> + goto out;
> + }
> +
> + switch (mgt->id) {
> + case MID_CMLDS_INFO_NP:
> + if (msg->header.sourcePortIdentity.portNumber !=
> p->cmlds_port) {
> + break;
> + }
> + cmlds = (struct cmlds_info_np *) mgt->data;
> + p->peer_delay = nanoseconds_to_tmv(cmlds->meanLinkDelay >>
> 16);
> + p->peerMeanPathDelay = tmv_to_TimeInterval(p->peer_delay);
cmlds->meanLinkDelay is already a TimeInterval in case you want to use that.
> +
> + if (p->state == PS_UNCALIBRATED || p->state == PS_SLAVE) {
> + const struct timestamp dummy = {0};
> + const tmv_t tx = timestamp_to_tmv(dummy);
tmv_zero() could also be used.
> + clock_peer_delay(p->clock, p->peer_delay, tx, tx,
> + p->nrate.ratio);
p->nrate.ratio hasn't been set yet, I think a line similar to this is missing:
p->nrate.ratio = 1.0 + (double) cmlds->scaledNeighborRateRatio / POW2_41;
The CMLDS port will also need to ensure that the NRR is calculated
because now it's done conditionally.
In Kishen's patch series struct cmlds_info_np also had a
.serviceMeasurementValid which enabled the port_capable() logic to
work and IIRC that is important at least for 802.1AS compliance. If
that was to be re-added, the event would need to be emitted in an
extra place in the CMLDS, perhaps where pdr_missing is incremented.
Best regards
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
