On Tue, Mar 20, 2018 at 11:04:02PM +0100, Anders Selhammer wrote:
> @@ -146,6 +146,7 @@ struct port {
> static int port_capable(struct port *p);
> static int port_is_ieee8021as(struct port *p);
> static void port_nrate_initialize(struct port *p);
> +static void flush_delay_req(struct port *p);
alphabetical order please
> @@ -1317,9 +1315,17 @@ out:
> return -1;
> }
>
> +static int port_outdated_delay_req(struct timespec host)
> +{
> + struct timespec now;
> + clock_gettime(CLOCK_MONOTONIC, &now);
This is backwards. You only need to get 'now' once. Then you compare
it with each (req.host + TMO).
> + now.tv_sec -= 5;
> + return timespec_to_tmv(now).ns > timespec_to_tmv(host).ns ? 1 : 0;
This is an abuse of the tmv_t abstraction. Please use the access
functions.
> +}
> +
> @@ -1367,10 +1373,16 @@ static int port_delay_request(struct port *p)
> goto out;
> }
>
> - if (p->delay_req)
> - msg_put(p->delay_req);
> + TAILQ_INSERT_HEAD(&p->delay_req, msg, list);
> +
> + while ((tmp = TAILQ_LAST(&p->delay_req, delay_req)) != NULL) {
> + if (!port_outdated_delay_req(tmp->ts.host)) {
The word "outdated" sounds funny. We not dealing in fashion
accessories! How about introducing a helper to prune the list.
static void delay_req_prune()
{
clock_gettime(CLOCK_MONOTONIC, &now);
while (!TAILQ_EMPTY(&p->delay_req)) {
tmp = TAILQ_LAST(&p->delay_req, delay_req);
if (delay_req_current(tmp, now))
break;
TAILQ_REMOVE(&p->delay_req, tmp, list);
msg_put(tmp);
}
}
> + break;
> + }
> + TAILQ_REMOVE(&p->delay_req, tmp, list);
> + msg_put(tmp);
> + }
You only prune the list when a new request arrives. Is that
sufficient or should a timer (like the sync timer) also be used?
> @@ -1816,37 +1829,45 @@ out:
>
> static void process_delay_resp(struct port *p, struct ptp_message *m)
> {
> - struct delay_req_msg *req;
> struct delay_resp_msg *rsp = &m->delay_resp;
> + struct ptp_message *req, *tmp;
> struct PortIdentity master;
> tmv_t c3, t3, t4, t4c;
>
> - if (!p->delay_req)
> - return;
> -
> master = clock_parent_identity(p->clock);
> - req = &p->delay_req->delay_req;
>
> if (p->state != PS_UNCALIBRATED && p->state != PS_SLAVE) {
> return;
> }
> - if (!pid_eq(&rsp->requestingPortIdentity,
> &req->hdr.sourcePortIdentity)) {
> + if (!pid_eq(&rsp->requestingPortIdentity, &p->portIdentity)) {
> return;
> }
> - if (rsp->hdr.sequenceId != ntohs(req->hdr.sequenceId)) {
> + if (!pid_eq(&master, &m->header.sourcePortIdentity)) {
> return;
> }
> - if (!pid_eq(&master, &m->header.sourcePortIdentity)) {
> + TAILQ_FOREACH(req, &p->delay_req, list) {
> + if (rsp->hdr.sequenceId ==
> ntohs(req->delay_req.hdr.sequenceId)) {
> + break;
> + }
> + }
> + if (!req) {
> return;
> }
>
> c3 = correction_to_tmv(m->header.correction);
> - t3 = p->delay_req->hwts.ts;
> + t3 = req->hwts.ts;
> t4 = timestamp_to_tmv(m->ts.pdu);
> t4c = tmv_sub(t4, c3);
>
> clock_path_delay(p->clock, t3, t4c);
>
> + while ((tmp = TAILQ_NEXT(req, list)) != NULL) {
> + TAILQ_REMOVE(&p->delay_req, tmp, list);
> + msg_put(tmp);
> + }
> + TAILQ_REMOVE(&p->delay_req, req, list);
> + msg_put(req);
This doesn't handle the case out of order packets. Use the "prune"
helper here, too.
> +
> if (p->logMinDelayReqInterval == rsp->hdr.logMessageInterval) {
> return;
> }
> --
> 1.8.3.1
Thanks,
Richard
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
