On Thu, 25 Aug 2022 at 07:59, Devasish Dey <devasish....@syncmonk.net>
wrote:
> Hi Erez,
>
> G.8275.2 (section 6.7.11) talks about Signal Fail, which could be because
> of PTSF-lossSync, PTSF-unusable and PTSF-syncUncertain. Since
> PTSF-lossSync could happen because of either loss of sync messages or loss
> of delay response messages the reason cannot be identified from the flag.
> To have a better understanding of failure reasons another management TLV
> has been added which gives more granularity to the failure sub-reason.
>
Great, please add explosions like that in your patches.
It really helps understanding what the patch is about.
If you think we need separate values then go ahead.
Then comes the sequence question.
The 2 TLVs can be one. Management TLV can have 2 values in them.
Why not put them in a single TLV?
Erez
> Thanks,
> Devasish Dey
> SyncMonk Technologies.
>
> On Wed, 24 Aug 2022 at 19:06, Erez <erezge...@gmail.com> wrote:
>
>>
>>
>> On Wed, 24 Aug 2022 at 12:37, SyncMonk Technologies <
>> servi...@syncmonk.net> wrote:
>>
>>> adding TLV support for PORT_PTSF_UNUSABLE
>>>
>>
>> You already add MID_PORT_PTSF_DATA_NP.
>> Why do we need 2 management TLV messages?
>> Can't the user calculate the unstable state from data?
>>
>>
>>> Signed-off-by: Greg Armstrong <greg.armstrong...@renesas.com>
>>> Signed-off-by: Leon Goldin <leon.goldin...@renesas.com>
>>> Signed-off-by: Devasish Dey <devasish....@syncmonk.net>
>>> Signed-off-by: Vipin Sharma <vipin.sha...@syncmonk.net>
>>> ---
>>> pmc.c | 6 ++++++
>>> pmc_common.c | 23 +++++++++++++++++++++++
>>> port.c | 23 +++++++++++++++++++++++
>>> tlv.c | 12 ++++++++++++
>>> tlv.h | 6 ++++++
>>> 5 files changed, 70 insertions(+)
>>>
>>> diff --git a/pmc.c b/pmc.c
>>> index 34c9609..35300b4 100644
>>> --- a/pmc.c
>>> +++ b/pmc.c
>>> @@ -176,6 +176,7 @@ static void pmc_show(struct ptp_message *msg, FILE
>>> *fp)
>>> struct currentDS *cds;
>>> struct parentDS *pds;
>>> struct portDS *p;
>>> + struct port_ptsf_unusable_np *ptsf;
>>> struct port_ptsf_data_np *ptsf_data;
>>> struct TLV *tlv;
>>> uint8_t *buf;
>>> @@ -608,6 +609,11 @@ 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_PORT_PTSF_UNUSABLE_NP:
>>> + ptsf = (struct port_ptsf_unusable_np *) mgt->data;
>>> + fprintf(fp, "PORT_PTSF_UNUSABLE_NP "
>>> + IFMT "unusable %hu", ptsf->ptsf_unusable);
>>> + break;
>>> case MID_PORT_PTSF_DATA_NP:
>>> ptsf_data = (struct port_ptsf_data_np *) mgt->data;
>>> fprintf(fp, "PORT_PTSF_DATA_NP "
>>> diff --git a/pmc_common.c b/pmc_common.c
>>> index 9570841..528bd43 100644
>>> --- a/pmc_common.c
>>> +++ b/pmc_common.c
>>> @@ -154,6 +154,7 @@ struct management_id idtab[] = {
>>> { "PORT_SERVICE_STATS_NP", MID_PORT_SERVICE_STATS_NP,
>>> do_get_action },
>>> { "UNICAST_MASTER_TABLE_NP", MID_UNICAST_MASTER_TABLE_NP,
>>> do_get_action },
>>> { "PORT_HWCLOCK_NP", MID_PORT_HWCLOCK_NP, do_get_action },
>>> + { "PORT_PTSF_UNUSABLE_NP", MID_PORT_PTSF_UNUSABLE_NP,
>>> do_set_action },
>>> { "PORT_PTSF_DATA_NP", MID_PORT_PTSF_DATA_NP, do_get_action },
>>> };
>>>
>>> @@ -173,6 +174,7 @@ static void do_set_action(struct pmc *pmc, int
>>> action, int index, char *str)
>>> struct management_tlv_datum mtd;
>>> struct subscribe_events_np sen;
>>> struct port_ds_np pnp;
>>> + struct port_ptsf_unusable_np ptsf;
>>> char onoff_port_state[4] = "off";
>>> char onoff_time_status[4] = "off";
>>>
>>> @@ -303,6 +305,27 @@ static void do_set_action(struct pmc *pmc, int
>>> action, int index, char *str)
>>> }
>>> pmc_send_set_action(pmc, code, &pnp, sizeof(pnp));
>>> break;
>>> + case MID_PORT_PTSF_UNUSABLE_NP:
>>> + cnt = sscanf(str, " %*s %*s %hu %hu",
>>> + &ptsf.ptsf_unusable,
>>> + &ptsf.portIdentity.portNumber);
>>> + if (cnt != 2) {
>>> + fprintf(stderr, "%s SET needs 2 value\n",
>>> + idtab[index].name);
>>> + break;
>>> + }
>>> + switch (ptsf.ptsf_unusable) {
>>> + case TRUE:
>>> + case FALSE:
>>> + pmc_send_set_action(pmc, code, &ptsf,
>>> sizeof(ptsf));
>>> + break;
>>> + default:
>>> + fprintf(stderr, "\nusage:"
>>> + "set PORT_PTSF_UNUSABLE_NP%hhu (false)
>>> or"
>>> + "%hhu (true) \n\n",
>>> + FALSE, TRUE);
>>> + }
>>> + break;
>>> }
>>> }
>>>
>>> diff --git a/port.c b/port.c
>>> index ff00e46..d0997f8 100644
>>> --- a/port.c
>>> +++ b/port.c
>>> @@ -852,6 +852,7 @@ static int port_management_fill_response(struct port
>>> *target,
>>> struct PortIdentity pid;
>>> const char *ts_label;
>>> struct portDS *pds;
>>> + struct port_ptsf_unusable_np *ptsf;
>>> struct port_ptsf_data_np *ptsf_data;
>>> uint16_t u16;
>>> uint8_t *buf;
>>> @@ -1075,6 +1076,13 @@ static int port_management_fill_response(struct
>>> port *target,
>>> PORT_HWCLOCK_VCLOCK : 0;
>>> datalen = sizeof(*phn);
>>> break;
>>> + case MID_PORT_PTSF_UNUSABLE_NP:
>>> + ptsf = (struct port_ptsf_unusable_np *)tlv->data;
>>> + ptsf->portIdentity = target->portIdentity;
>>> + ptsf->ptsf_unusable =
>>> + (target->signalFail & PDS_PTSF_UNUSABLE) ? 1 : 0;
>>> + datalen = sizeof(*ptsf);
>>> + break;
>>> case MID_PORT_PTSF_DATA_NP:
>>> ptsf_data = (struct port_ptsf_data_np *)tlv->data;
>>> ptsf_data->portIdentity = target->portIdentity;
>>> @@ -1125,6 +1133,8 @@ static int port_management_set(struct port *target,
>>> int respond = 0;
>>> struct management_tlv *tlv;
>>> struct port_ds_np *pdsnp;
>>> + struct port_ptsf_unusable_np *ptsf;
>>> + UInteger16 signalFail;
>>>
>>> tlv = (struct management_tlv *) req->management.suffix;
>>>
>>> @@ -1134,6 +1144,19 @@ static int port_management_set(struct port
>>> *target,
>>> target->neighborPropDelayThresh =
>>> pdsnp->neighborPropDelayThresh;
>>> respond = 1;
>>> break;
>>> + case MID_PORT_PTSF_UNUSABLE_NP:
>>> + ptsf = (struct port_ptsf_unusable_np *) tlv->data;
>>> + signalFail = target->signalFail;
>>> + if (ptsf->portIdentity.portNumber ==
>>> target->portIdentity.portNumber) {
>>> + if (ptsf->ptsf_unusable)
>>> + target->signalFail |= PDS_PTSF_UNUSABLE;
>>> + else
>>> + target->signalFail &= ~PDS_PTSF_UNUSABLE;
>>> + }
>>> + if (signalFail != target->signalFail)
>>> + clock_set_sde(target->clock, 1);
>>> + respond = 1;
>>> + break;
>>> }
>>> if (respond && !port_management_get_response(target, ingress,
>>> id, req))
>>> pr_err("%s: failed to send management set response",
>>> target->log_name);
>>> diff --git a/tlv.c b/tlv.c
>>> index 745c96a..2cfd61b 100644
>>> --- a/tlv.c
>>> +++ b/tlv.c
>>> @@ -130,6 +130,7 @@ static int mgt_post_recv(struct management_tlv *m,
>>> uint16_t data_len,
>>> struct defaultDS *dds;
>>> struct parentDS *pds;
>>> struct portDS *p;
>>> + struct port_ptsf_unusable_np *ptsf;
>>> struct port_ptsf_data_np *ptsf_data;
>>> uint8_t *buf;
>>> uint16_t u16;
>>> @@ -408,6 +409,12 @@ static int mgt_post_recv(struct management_tlv *m,
>>> uint16_t data_len,
>>> if (data_len != 0)
>>> goto bad_length;
>>> break;
>>> + case MID_PORT_PTSF_UNUSABLE_NP:
>>> + if (data_len != sizeof(struct port_ptsf_unusable_np))
>>> + goto bad_length;
>>> + ptsf = (struct port_ptsf_unusable_np *) m->data;
>>> + ptsf->ptsf_unusable = ntohs(ptsf->ptsf_unusable);
>>> + break;
>>> case MID_PORT_PTSF_DATA_NP:
>>> ptsf_data = (struct port_ptsf_data_np *) m->data;
>>> ptsf_data->portIdentity.portNumber =
>>> ntohs(ptsf_data->portIdentity.portNumber);
>>> @@ -442,6 +449,7 @@ static void mgt_pre_send(struct management_tlv *m,
>>> struct tlv_extra *extra)
>>> struct currentDS *cds;
>>> struct parentDS *pds;
>>> struct portDS *p;
>>> + struct port_ptsf_unusable_np *ptsf;
>>> struct port_ptsf_data_np *ptsf_data;
>>> uint8_t *buf;
>>> int i;
>>> @@ -575,6 +583,10 @@ static void mgt_pre_send(struct management_tlv *m,
>>> struct tlv_extra *extra)
>>> phn->portIdentity.portNumber =
>>> htons(phn->portIdentity.portNumber);
>>> phn->phc_index = htonl(phn->phc_index);
>>> break;
>>> + case MID_PORT_PTSF_UNUSABLE_NP:
>>> + ptsf = (struct port_ptsf_unusable_np *) m->data;
>>> + ptsf->ptsf_unusable = htons(ptsf->ptsf_unusable);
>>> + break;
>>> case MID_PORT_PTSF_DATA_NP:
>>> ptsf_data = (struct port_ptsf_data_np *) m->data;
>>> ptsf_data->portIdentity.portNumber =
>>> diff --git a/tlv.h b/tlv.h
>>> index c29f5f2..36e9993 100644
>>> --- a/tlv.h
>>> +++ b/tlv.h
>>> @@ -129,6 +129,7 @@ enum management_action {
>>> #define MID_UNICAST_MASTER_TABLE_NP 0xC008
>>> #define MID_PORT_HWCLOCK_NP 0xC009
>>> #define MID_PORT_PTSF_DATA_NP 0xC00A
>>> +#define MID_PORT_PTSF_UNUSABLE_NP 0xC00B
>>>
>>> /* Management error ID values */
>>> #define MID_RESPONSE_TOO_BIG 0x0001
>>> @@ -368,6 +369,11 @@ struct port_service_stats_np {
>>> struct PortServiceStats stats;
>>> } PACKED;
>>>
>>> +struct port_ptsf_unusable_np {
>>> + struct PortIdentity portIdentity;
>>> + UInteger16 ptsf_unusable;
>>> +} PACKED;
>>> +
>>> struct port_ptsf_data_np {
>>> struct PortIdentity portIdentity;
>>> UInteger8 signalFail;
>>> --
>>> 2.25.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
>>
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
