correctionField: simplify code

This removes some needless local variables related to correction field, but relying on a new "cField" entry in pp_instance, near the t1,t2,t3,t4 status stamps. The commit also adds a FIXME note: it is not clear to me how the correction field can be managed in the back message (t3, t4). Since the correctionField is updated by transparent clocks, I think we need the one of the delay request (the one stamped in t3, t3), *not* the delay response that I don't care about. Signed-off-by: Alessandro Rubini <rubini@gnudd.com>

correctionField: simplify code
This removes some needless local variables related to correction field, but relying on a new "cField" entry in pp_instance, near the t1,t2,t3,t4 status stamps. The commit also adds a FIXME note: it is not clear to me how the correction field can be managed in the back message (t3, t4). Since the correctionField is updated by transparent clocks, I think we need the one of the delay request (the one stamped in t3, t3), *not* the delay response that I don't care about. Signed-off-by: Alessandro Rubini <rubini@gnudd.com>
05e8a274 · Alessandro Rubini · a65e4f02 · 05e8a274 · 05e8a274 · 05e8a274
Commit 05e8a274 authored Mar 18, 2013 by Alessandro Rubini
Hide whitespace changes
Inline Side-by-side

Showing with 12 additions and 20 deletions

pp-instance.h include/ppsi/pp-instance.h +1 -0

common-fun.c proto-standard/common-fun.c +5 -16

state-slave.c proto-standard/state-slave.c +6 -4

No files found.
--- a/include/ppsi/pp-instance.h
+++ b/include/ppsi/pp-instance.h
@@ -154,6 +154,7 @@ struct pp_instance {

 	/* Times, for the various offset computations */
 	TimeInternal t1, t2, t3, t4;			/* *the* stamps */
+	TimeInternal cField;				/* transp. clocks */
 	TimeInternal last_rcv_time, last_snt_time;	/* two temporaries */

 	/* Data sets */

--- a/proto-standard/common-fun.c
+++ b/proto-standard/common-fun.c
@@ -135,17 +135,17 @@ int st_com_slave_handle_announce(struct pp_instance *ppi, unsigned char *buf,
 int st_com_slave_handle_sync(struct pp_instance *ppi, unsigned char *buf,
 			     int len)
 {
-	TimeInternal correction_field;
 	MsgHeader *hdr = &ppi->received_ptp_header;
 	MsgSync sync;

 	if (len < PP_SYNC_LENGTH)
 		return -1;
-
 	if (!ppi->is_from_cur_par)
 		return 0;

+	/* t2 may be overriden by follow-up, cField is always valid */
 	ppi->t2 = ppi->last_rcv_time;
+	cField_to_TimeInternal(&ppi->cField, hdr->correctionfield);

 	if ((hdr->flagField[0] & PP_TWO_STEP_FLAG) != 0) {
 		ppi->waiting_for_follow = TRUE;
@@ -153,16 +153,10 @@ int st_com_slave_handle_sync(struct pp_instance *ppi, unsigned char *buf,
 		return 0;
 	}
 	msg_unpack_sync(buf, &sync);
-	cField_to_TimeInternal(&correction_field,
-			      ppi->received_ptp_header.correctionfield);
-
-	display_TimeInternal("Correction field",
-			     &correction_field);
-
 	ppi->waiting_for_follow = FALSE;
 	to_TimeInternal(&ppi->t1,
 			&sync.originTimestamp);
-	pp_update_offset(ppi, &correction_field);
+	pp_update_offset(ppi, &ppi->cField);
 	pp_update_clock(ppi);
 	return 0;
 }
@@ -171,7 +165,6 @@ int st_com_slave_handle_sync(struct pp_instance *ppi, unsigned char *buf,
 int st_com_slave_handle_followup(struct pp_instance *ppi, unsigned char *buf,
 				 int len)
 {
-	TimeInternal correction_field;
 	MsgFollowUp follow;
 	int ret = 0;

@@ -202,19 +195,15 @@ int st_com_slave_handle_followup(struct pp_instance *ppi, unsigned char *buf,
 	ppi->waiting_for_follow = FALSE;
 	to_TimeInternal(&ppi->t1, &follow.preciseOriginTimestamp);

-	cField_to_TimeInternal(&correction_field,
-			      ppi->received_ptp_header.correctionfield);
-
 	/* Call the extension; it may do it all and ask to return */
 	if (pp_hooks.handle_followup)
-		ret = pp_hooks.handle_followup(ppi, &ppi->t1,
-					       &correction_field);
+		ret = pp_hooks.handle_followup(ppi, &ppi->t1, &ppi->cField);
 	if (ret == 1)
 		return 0;
 	if (ret < 0)
 		return ret;

-	pp_update_offset(ppi, &correction_field);
+	pp_update_offset(ppi, &ppi->cField);
 	pp_update_clock(ppi);
 	return 0;
 }

--- a/proto-standard/state-slave.c
+++ b/proto-standard/state-slave.c
@@ -9,7 +9,6 @@
 int pp_slave(struct pp_instance *ppi, unsigned char *pkt, int plen)
 {
 	int e = 0; /* error var, to check errors in msg handling */
-	TimeInternal correction_field;
 	MsgHeader *hdr = &ppi->received_ptp_header;
 	MsgDelayResp resp;
 	int d1, d2;
@@ -71,13 +70,16 @@ int pp_slave(struct pp_instance *ppi, unsigned char *pkt, int plen)

 			to_TimeInternal(&ppi->t4, &resp.receiveTimestamp);

-			cField_to_TimeInternal(&correction_field,
-					      hdr->correctionfield);
+			/*
+			 * FIXME: how is correctionField handled in t3/t4?
+			 * I think the master should consider it when
+			 * generating t4, and report back a modified t4
+			 */

 			if (pp_hooks.update_delay)
 				e = pp_hooks.update_delay(ppi);
 			else
-				pp_update_delay(ppi, &correction_field);
+				pp_update_delay(ppi, &ppi->cField);
 			if (e)
 				goto out;