On Wed, Nov 14, 2018 at 02:10:10PM -0800, Vedang Patel wrote:
> diff --git a/port.c b/port.c
> index 4c845358f612..015766103e05 100644
> --- a/port.c
> +++ b/port.c
> @@ -1591,6 +1591,7 @@ int port_initialize(struct port *p)
> p->logMinDelayReqInterval = config_get_int(cfg, p->name,
> "logMinDelayReqInterval");
> p->peerMeanPathDelay = 0;
> p->logAnnounceInterval = config_get_int(cfg, p->name,
> "logAnnounceInterval");
> + p->initialLogAnnounceInterval = p->logAnnounceInterval;
This is backwards. You want:
p->initialLogAnnounceInterval = config_get_int(cfg, p->name,
"logAnnounceInterval");
p->logAnnounceInterval = p->initialLogAnnounceInterval;
> p->inhibit_announce = config_get_int(cfg, p->name,
> "inhibit_announce");
> p->ignore_source_id = config_get_int(cfg, p->name,
> "ignore_source_id");
> p->announceReceiptTimeout = config_get_int(cfg, p->name,
> "announceReceiptTimeout");
> @@ -1600,6 +1601,7 @@ int port_initialize(struct port *p)
> p->match_transport_specific = !config_get_int(cfg, p->name,
> "ignore_transport_specific");
> p->localPriority = config_get_int(cfg, p->name,
> "G.8275.portDS.localPriority");
> p->logSyncInterval = config_get_int(cfg, p->name,
> "logSyncInterval");
> + p->initialLogSyncInterval = p->logSyncInterval;
also backwards.
> p->logMinPdelayReqInterval = config_get_int(cfg, p->name,
> "logMinPdelayReqInterval");
> p->logPdelayReqInterval = p->logMinPdelayReqInterval;
> p->neighborPropDelayThresh = config_get_int(cfg, p->name,
> "neighborPropDelayThresh");
> @@ -185,5 +187,10 @@ enum fsm_event process_sync(struct port *p, struct
> ptp_message *m);
> int source_pid_eq(struct ptp_message *m1, struct ptp_message *m2);
> void ts_add(tmv_t *ts, Integer64 correction);
> int port_capable(struct port *p);
> +int port_tx_signaling(struct port *p,
> + Integer8 announceInterval,
> + Integer8 timeSyncInterval,
> + Integer8 linkDelayInterval);
This is poorly named. You make it sound like it is a generic transmit
function for signaling message, but it isn't.
Also, please keep the prototypes in alphabetical order.
> @@ -61,6 +65,48 @@ struct ptp_message *port_unicast_construct(struct port *p,
> return msg;
> }
>
> +static void set_interval(int8_t *current_interval,
> + int8_t new_interval,
> + int8_t initial_interval)
> +{
> + switch (new_interval) {
> + case SIGNAL_NO_CHANGE:
> + break;
> + case SIGNAL_SET_INITIAL:
> + *current_interval = initial_interval;
> + break;
> + default:
> + *current_interval = new_interval;
> + break;
> + }
Ugh. Just return current_interval:
static int8_t set_interval(int8_t new_interval, int8_t initial_interval);
> +}
> +
> +static int process_interval_request(struct port *p,
> + struct tlv_extra *extra)
> +{
> + struct msg_interval_req_tlv *r;
> +
> + r = (struct msg_interval_req_tlv *) extra->tlv;
You need to check the length of the TLV, in tlv.c:org_post_recv().
> +
> + if (r->subtype[2] != 2) {
But you didn't even bother to check the ID!
> + return -EBADMSG;
> + }
> +
> + set_interval(&p->logAnnounceInterval,
> + r->announceInterval,
> + p->initialLogAnnounceInterval);
> +
> + set_interval(&p->logSyncInterval,
> + r->timeSyncInterval,
> + p->initialLogSyncInterval);
> +
> + set_interval(&p->logPdelayReqInterval,
> + r->linkDelayInterval,
> + p->logMinPdelayReqInterval);
> +
> + return 0;
> +}
> +
> int process_signaling(struct port *p, struct ptp_message *m)
> {
> struct tlv_extra *extra;
> @@ -109,7 +155,59 @@ int process_signaling(struct port *p, struct ptp_message
> *m)
>
> case TLV_ACKNOWLEDGE_CANCEL_UNICAST_TRANSMISSION:
> break;
> +
> + case TLV_ORGANIZATION_EXTENSION:
You need to check .id and .subtype, and then dispatch your helper
function. It isn't an error if those two fields are unknown.
> + err = process_interval_request(p, extra);
> + break;
> }
> }
> return err;
> }
> +
> +int port_tx_signaling(struct port *p,
> + Integer8 announceInterval,
> + Integer8 timeSyncInterval,
> + Integer8 linkDelayInterval)
> +{
> + int err;
> + struct ptp_message *msg;
> + struct tlv_extra *extra;
> + struct msg_interval_req_tlv *mir;
Please use reverse Christmas tree style.
> + struct PortIdentity tpid = {0};
This won't compile on older gcc.
> + tpid.portNumber = 0xFF;
This is silly to set after clearing. Instead you want:
struct PortIdentity tpid;
if (!port_capable(p)) {
return 0;
}
memset(&tpid.clockIdentity, 0, sizeof(tpid.clockIdentity));
tpid.portNumber = 0xFF;
> + msg = port_signaling_construct(p, &tpid);
> + if (!msg) {
> + return -1;
> + }
> +
Drop newline please here
> + extra = msg_tlv_append(msg, sizeof(*mir));
> + if (!extra) {
> + err = -1;
> + goto out;
> + }
> +
and here.
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
