Hey folks, any feedback for this patch I've sent over a month ago?
I realize it could have slipped through the cracks because of the Xmas time.

We have more changes lined up, but first would be nice to get any response on 
this, simple one.

Regards,
Alexander Bulimov

-----Original Message-----
From: Alexander Bulimov <abuli...@fb.com> 
Sent: Monday, December 20, 2021 5:03 PM
To: alexander.buli...@gmail.com
Cc: Alexander Bulimov <abuli...@fb.com>
Subject: [PATCH] Add PORT_SERVICE_STATS_NP management TLV

We'd like to monitor the service quality of unicast PTPv2 in the DCs.
This change adds counters for various timeouts and events happening in the 
client code, and exposes them as a new management TLV.

Signed-off-by: Alexander Bulimov <abuli...@fb.com>
---
 ddt.h          | 13 ++++++++++++
 pmc.c          | 27 +++++++++++++++++++++++++
 pmc_common.c   |  1 +
 port.c         | 23 +++++++++++++++++++++
 port_private.h |  1 +
 tlv.c          | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++
 tlv.h          |  6 ++++++
 7 files changed, 126 insertions(+)

diff --git a/ddt.h b/ddt.h
index 56449a3..ef30f73 100644
--- a/ddt.h
+++ b/ddt.h
@@ -107,4 +107,17 @@ struct PortStats {
        uint64_t txMsgType[MAX_MESSAGE_TYPES];  };
 
+struct PortServiceStats {
+       uint64_t announce_timeout;
+       uint64_t sync_timeout;
+       uint64_t delay_timeout;
+       uint64_t unicast_service_timeout;
+       uint64_t unicast_request_timeout;
+       uint64_t master_announce_timeout;
+       uint64_t master_sync_timeout;
+       uint64_t qualification_timeout;
+       uint64_t sync_mismatch;
+       uint64_t followup_mismatch;
+};
+
 #endif
diff --git a/pmc.c b/pmc.c
index a1ee787..2e5e93e 100644
--- a/pmc.c
+++ b/pmc.c
@@ -148,6 +148,7 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
        struct management_tlv *mgt;
        struct time_status_np *tsn;
        struct port_stats_np *pcp;
+       struct port_service_stats_np *pssp;
        struct tlv_extra *extra;
        struct port_ds_np *pnp;
        struct defaultDS *dds;
@@ -495,6 +496,32 @@ static void pmc_show(struct ptp_message *msg, FILE *fp)
                        pcp->stats.txMsgType[SIGNALING],
                        pcp->stats.txMsgType[MANAGEMENT]);
                break;
+       case MID_PORT_SERVICE_STATS_NP:
+               pssp = (struct port_service_stats_np *) mgt->data;
+               fprintf(fp, "PORT_SERVICE_STATS_NP "
+               IFMT "portIdentity              %s"
+               IFMT "announce_timeout          %" PRIu64
+               IFMT "sync_timeout              %" PRIu64
+               IFMT "delay_timeout             %" PRIu64
+               IFMT "unicast_service_timeout   %" PRIu64
+               IFMT "unicast_request_timeout   %" PRIu64
+               IFMT "master_announce_timeout   %" PRIu64
+               IFMT "master_sync_timeout       %" PRIu64
+               IFMT "qualification_timeout     %" PRIu64
+               IFMT "sync_mismatch             %" PRIu64
+               IFMT "followup_mismatch         %" PRIu64,
+               pid2str(&pssp->portIdentity),
+               pssp->stats.announce_timeout,
+               pssp->stats.sync_timeout,
+               pssp->stats.delay_timeout,
+               pssp->stats.unicast_service_timeout,
+               pssp->stats.unicast_request_timeout,
+               pssp->stats.master_announce_timeout,
+               pssp->stats.master_sync_timeout,
+               pssp->stats.qualification_timeout,
+               pssp->stats.sync_mismatch,
+               pssp->stats.followup_mismatch);
+               break;
        case MID_LOG_ANNOUNCE_INTERVAL:
                mtd = (struct management_tlv_datum *) mgt->data;
                fprintf(fp, "LOG_ANNOUNCE_INTERVAL "
diff --git a/pmc_common.c b/pmc_common.c index 7a1dbb4..b691bbb 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -131,6 +131,7 @@ struct management_id idtab[] = {
        { "LOG_MIN_PDELAY_REQ_INTERVAL", MID_LOG_MIN_PDELAY_REQ_INTERVAL, 
do_get_action },
        { "PORT_DATA_SET_NP", MID_PORT_DATA_SET_NP, do_set_action },
        { "PORT_STATS_NP", MID_PORT_STATS_NP, do_get_action },
+       { "PORT_SERVICE_STATS_NP", MID_PORT_SERVICE_STATS_NP, do_get_action },
        { "PORT_PROPERTIES_NP", MID_PORT_PROPERTIES_NP, do_get_action },  };
 
diff --git a/port.c b/port.c
index 7e51e77..c1d0ac8 100644
--- a/port.c
+++ b/port.c
@@ -798,6 +798,7 @@ static int port_management_fill_response(struct port 
*target,
        struct clock_description *desc;
        struct port_properties_np *ppn;
        struct port_stats_np *psn;
+       struct port_service_stats_np *pssn;
        struct management_tlv *tlv;
        struct port_ds_np *pdsnp;
        struct tlv_extra *extra;
@@ -966,6 +967,12 @@ static int port_management_fill_response(struct port 
*target,
                psn->stats = target->stats;
                datalen = sizeof(*psn);
                break;
+       case MID_PORT_SERVICE_STATS_NP:
+               pssn = (struct port_service_stats_np *)tlv->data;
+               pssn->portIdentity = target->portIdentity;
+               pssn->stats = target->service_stats;
+               datalen = sizeof(*pssn);
+               break;
        default:
                /* The caller should *not* respond to this message. */
                tlv_extra_recycle(extra);
@@ -1285,6 +1292,7 @@ static void port_syfufsm(struct port *p, enum syfu_event 
event,
                        msg_put(p->last_syncfup);
                        msg_get(m);
                        p->last_syncfup = m;
+                       p->service_stats.sync_mismatch++;
                        break;
                case SYNC_MATCH:
                        break;
@@ -1294,6 +1302,7 @@ static void port_syfufsm(struct port *p, enum syfu_event 
event,
                        msg_get(m);
                        p->last_syncfup = m;
                        p->syfu = SF_HAVE_FUP;
+                       p->service_stats.followup_mismatch++;
                        break;
                case FUP_MATCH:
                        syn = p->last_syncfup;
@@ -1316,6 +1325,7 @@ static void port_syfufsm(struct port *p, enum syfu_event 
event,
                        msg_get(m);
                        p->last_syncfup = m;
                        p->syfu = SF_HAVE_SYNC;
+                       p->service_stats.sync_mismatch++;
                        break;
                case SYNC_MATCH:
                        fup = p->last_syncfup;
@@ -1332,6 +1342,7 @@ static void port_syfufsm(struct port *p, enum syfu_event 
event,
                        msg_put(p->last_syncfup);
                        msg_get(m);
                        p->last_syncfup = m;
+                       p->service_stats.followup_mismatch++;
                        break;
                case FUP_MATCH:
                        break;
@@ -2651,6 +2662,12 @@ static enum fsm_event bc_event(struct port *p, int 
fd_index)
                        fc_clear(p->best);
                }
 
+               if (fd_index == FD_SYNC_RX_TIMER) {
+                       p->service_stats.sync_timeout++;
+               } else {
+                       p->service_stats.announce_timeout++;
+               }
+
                /*
                 * Clear out the event returned by poll(). It is only cleared
                 * in port_*_transition(). But, when BMCA == 'noop', there is 
no @@ -2681,6 +2698,7 @@ static enum fsm_event bc_event(struct port *p, int 
fd_index)
                pr_debug("%s: delay timeout", p->log_name);
                port_set_delay_tmo(p);
                delay_req_prune(p);
+               p->service_stats.delay_timeout++;
                if (port_delay_request(p)) {
                        return EV_FAULT_DETECTED;
                }
@@ -2697,11 +2715,13 @@ static enum fsm_event bc_event(struct port *p, int 
fd_index)
 
        case FD_QUALIFICATION_TIMER:
                pr_debug("%s: qualification timeout", p->log_name);
+               p->service_stats.qualification_timeout++;
                return EV_QUALIFICATION_TIMEOUT_EXPIRES;
 
        case FD_MANNO_TIMER:
                pr_debug("%s: master tx announce timeout", p->log_name);
                port_set_manno_tmo(p);
+               p->service_stats.master_announce_timeout++;
                clock_update_leap_status(p->clock);
                return port_tx_announce(p, NULL, p->seqnum.announce++) ?
                        EV_FAULT_DETECTED : EV_NONE;
@@ -2709,15 +2729,18 @@ static enum fsm_event bc_event(struct port *p, int 
fd_index)
        case FD_SYNC_TX_TIMER:
                pr_debug("%s: master sync timeout", p->log_name);
                port_set_sync_tx_tmo(p);
+               p->service_stats.master_sync_timeout++;
                return port_tx_sync(p, NULL, p->seqnum.sync++) ?
                        EV_FAULT_DETECTED : EV_NONE;
 
        case FD_UNICAST_SRV_TIMER:
                pr_debug("%s: unicast service timeout", p->log_name);
+               p->service_stats.unicast_service_timeout++;
                return unicast_service_timer(p) ? EV_FAULT_DETECTED : EV_NONE;
 
        case FD_UNICAST_REQ_TIMER:
                pr_debug("%s: unicast request timeout", p->log_name);
+               p->service_stats.unicast_request_timeout++;
                return unicast_client_timer(p) ? EV_FAULT_DETECTED : EV_NONE;
 
        case FD_RTNL:
diff --git a/port_private.h b/port_private.h index 19eb944..d27dceb 100644
--- a/port_private.h
+++ b/port_private.h
@@ -146,6 +146,7 @@ struct port {
        UInteger8           delay_response_counter;
        UInteger8           delay_response_timeout;
        struct PortStats    stats;
+       struct PortServiceStats    service_stats;
        /* foreignMasterDS */
        LIST_HEAD(fm, foreign_clock) foreign_masters;
        /* TC book keeping */
diff --git a/tlv.c b/tlv.c
index cc8d507..df516be 100644
--- a/tlv.c
+++ b/tlv.c
@@ -120,6 +120,7 @@ static int mgt_post_recv(struct management_tlv *m, uint16_t 
data_len,
        struct timePropertiesDS *tp;
        struct time_status_np *tsn;
        struct port_stats_np *psn;
+       struct port_service_stats_np *pssn;
        int extra_len = 0, i, len;
        struct port_ds_np *pdsnp;
        struct currentDS *cds;
@@ -331,6 +332,34 @@ static int mgt_post_recv(struct management_tlv *m, 
uint16_t data_len,
                }
                extra_len = sizeof(struct port_stats_np);
                break;
+       case MID_PORT_SERVICE_STATS_NP:
+               if (data_len < sizeof(struct port_service_stats_np))
+                       goto bad_length;
+               pssn = (struct port_service_stats_np *)m->data;
+               pssn->portIdentity.portNumber =
+                       htons(pssn->portIdentity.portNumber);
+               pssn->stats.announce_timeout =
+                       __le64_to_cpu(pssn->stats.announce_timeout);
+               pssn->stats.sync_timeout =
+                       __le64_to_cpu(pssn->stats.sync_timeout);
+               pssn->stats.delay_timeout =
+                       __le64_to_cpu(pssn->stats.delay_timeout);
+               pssn->stats.unicast_service_timeout =
+                       __le64_to_cpu(pssn->stats.unicast_service_timeout);
+               pssn->stats.unicast_request_timeout =
+                       __le64_to_cpu(pssn->stats.unicast_request_timeout);
+               pssn->stats.master_announce_timeout =
+                       __le64_to_cpu(pssn->stats.master_announce_timeout);
+               pssn->stats.master_sync_timeout =
+                       __le64_to_cpu(pssn->stats.master_sync_timeout);
+               pssn->stats.qualification_timeout =
+                       __le64_to_cpu(pssn->stats.qualification_timeout);
+               pssn->stats.sync_mismatch =
+                       __le64_to_cpu(pssn->stats.sync_mismatch);
+               pssn->stats.followup_mismatch =
+                       __le64_to_cpu(pssn->stats.followup_mismatch);
+               extra_len = sizeof(struct port_service_stats_np);
+               break;
        case MID_SAVE_IN_NON_VOLATILE_STORAGE:
        case MID_RESET_NON_VOLATILE_STORAGE:
        case MID_INITIALIZE:
@@ -361,6 +390,7 @@ static void mgt_pre_send(struct management_tlv *m, struct 
tlv_extra *extra)
        struct timePropertiesDS *tp;
        struct time_status_np *tsn;
        struct port_stats_np *psn;
+       struct port_service_stats_np *pssn;
        struct port_ds_np *pdsnp;
        struct defaultDS *dds;
        struct currentDS *cds;
@@ -448,6 +478,31 @@ static void mgt_pre_send(struct management_tlv *m, struct 
tlv_extra *extra)
                        psn->stats.txMsgType[i] = 
__cpu_to_le64(psn->stats.txMsgType[i]);
                }
                break;
+       case MID_PORT_SERVICE_STATS_NP:
+               pssn = (struct port_service_stats_np *)m->data;
+               pssn->portIdentity.portNumber =
+                       htons(pssn->portIdentity.portNumber);
+               pssn->stats.announce_timeout =
+                       __cpu_to_le64(pssn->stats.announce_timeout);
+               pssn->stats.sync_timeout =
+                       __cpu_to_le64(pssn->stats.sync_timeout);
+               pssn->stats.delay_timeout =
+                       __cpu_to_le64(pssn->stats.delay_timeout);
+               pssn->stats.unicast_service_timeout =
+                       __cpu_to_le64(pssn->stats.unicast_service_timeout);
+               pssn->stats.unicast_request_timeout =
+                       __cpu_to_le64(pssn->stats.unicast_request_timeout);
+               pssn->stats.master_announce_timeout =
+                       __cpu_to_le64(pssn->stats.master_announce_timeout);
+               pssn->stats.master_sync_timeout =
+                       __cpu_to_le64(pssn->stats.master_sync_timeout);
+               pssn->stats.qualification_timeout =
+                       __cpu_to_le64(pssn->stats.qualification_timeout);
+               pssn->stats.sync_mismatch =
+                       __cpu_to_le64(pssn->stats.sync_mismatch);
+               pssn->stats.followup_mismatch =
+                       __cpu_to_le64(pssn->stats.followup_mismatch);
+               break;
        }
 }
 
diff --git a/tlv.h b/tlv.h
index 97615fd..408c22c 100644
--- a/tlv.h
+++ b/tlv.h
@@ -125,6 +125,7 @@ enum management_action {
 #define MID_PORT_DATA_SET_NP                           0xC002
 #define MID_PORT_PROPERTIES_NP                         0xC004
 #define MID_PORT_STATS_NP                              0xC005
+#define MID_PORT_SERVICE_STATS_NP                      0xC007
 
 /* Management error ID values */
 #define MID_RESPONSE_TOO_BIG                           0x0001
@@ -349,6 +350,11 @@ struct port_stats_np {
        struct PortStats stats;
 } PACKED;
 
+struct port_service_stats_np {
+       struct PortIdentity portIdentity;
+       struct PortServiceStats stats;
+} PACKED;
+
 #define PROFILE_ID_LEN 6
 
 struct mgmt_clock_description {
--
2.30.2



_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

Reply via email to