HA updates, bug fixes and optimization

- Move initialization of configurable parameters in L1SYNC init hook - Set the L1SYNC state to DISABLE in the startup otherwise the state is not set if the port is not connected - Remove time printing in diag messages - restrict call to servo only when the state is UP - Adjust properly the delay returned by each l1sync state functions - Set REQUEST timeout value dependent of the delay mechanism (E2E/P2P) - Bug fix: PP_P2P_MECH used instead of P2P - Optimize the REQUEST timeout to generate asap a DelayReq message after the first SYNC message received

HA updates, bug fixes and optimization
- Move initialization of configurable parameters in L1SYNC init hook - Set the L1SYNC state to DISABLE in the startup otherwise the state is not set if the port is not connected - Remove time printing in diag messages - restrict call to servo only when the state is UP - Adjust properly the delay returned by each l1sync state functions - Set REQUEST timeout value dependent of the delay mechanism (E2E/P2P) - Bug fix: PP_P2P_MECH used instead of P2P - Optimize the REQUEST timeout to generate asap a DelayReq message after the first SYNC message received
fbe6df4e · Jean-Claude BAU · Adam Wujek · e9f4ec10 · fbe6df4e · fbe6df4e
Commit fbe6df4e authored Sep 14, 2018 by Jean-Claude BAU Committed by Adam Wujek Nov 21, 2018
10 changed files
--- a/arch-wrs/wrs-startup.c
+++ b/arch-wrs/wrs-startup.c
@@ -280,9 +280,11 @@ int main(int argc, char **argv)
 				ppi->asymmetryCorrectionPortDS.scaledDelayCoefficient=
 						(RelativeDifference)(ppi->cfg.delayCoefficient * (double)pow(2.0, REL_DIFF_FRACBITS_AS_FLOAT));
-				/* Set L1SYNC portDS */
+				/* Set L1SYNC state. Must be done here because the init hook is called only if the initializing state. It
-				L1E_DSPOR_BS(ppi)->logL1SyncInterval=ppi->cfg.l1sync_interval;
+				 * the port is not connected, the initializing is then never called so the L1SYNC state is invalid (0)
-				L1E_DSPOR_BS(ppi)->L1SyncReceiptTimeout=ppi->cfg.l1sync_receipt_timeout;
+				 */
+				L1E_DSPOR_BS(ppi)->L1SyncState=L1SYNC_DISABLED;
 				/* Set L1SYNC extension hooks */
 				ppi->ext_hooks=&l1e_ext_hooks;
 			}

--- a/include/ppsi/diag-macros.h
+++ b/include/ppsi/diag-macros.h
@@ -70,7 +70,7 @@ extern void __pp_diag(struct pp_instance *ppi, enum pp_diag_things th,
 #endif
 /* Constant used to enable extra diagnostic information - Inserted at compilation time */
-#define PP_DIAG_EXTRA_PRINT_TIME (1 && PP_HAS_DIAG) /* Print time on each pp_diag calls */
+#define PP_DIAG_EXTRA_PRINT_TIME (0 && PP_HAS_DIAG) /* Print time on each pp_diag calls */
 /* So, this is the function that is to be used by real ppsi code */
 #define pp_diag(ppi_, th_, level_, ...)					\

--- a/proto-ext-l1sync/l1e-hooks.c
+++ b/proto-ext-l1sync/l1e-hooks.c
@@ -84,8 +84,6 @@ static int l1e_init(struct pp_instance *ppi, void *buf, int len)
 	bds->rxCoherentIsRequired     = TRUE;
 	bds->congruentIsRequired      = TRUE;
 	bds->optParamsEnabled         = FALSE;
-	bds->logL1SyncInterval        = 0;
-	bds->L1SyncReceiptTimeout     = L1E_DEFAULT_L1SYNC_RECEIPT_TIMEOUT;
 	// init dynamic data set members with zeros/defaults
 	bds->L1SyncLinkAlive          = FALSE;
 	bds->isTxCoherent             = FALSE;
@@ -98,6 +96,9 @@ static int l1e_init(struct pp_instance *ppi, void *buf, int len)
 	bds->peerIsTxCoherent     	  = FALSE;
 	bds->peerIsRxCoherent     	  = FALSE;
 	bds->peerIsCongruent          = FALSE;
+	/* Init  configurable parameters */
+	bds->logL1SyncInterval=ppi->cfg.l1sync_interval;
+	bds->L1SyncReceiptTimeout=ppi->cfg.l1sync_receipt_timeout;
 	/* Init other specific members */
 	bds->next_state=bds->L1SyncState;
@@ -145,6 +146,9 @@ static int l1e_handle_resp(struct pp_instance *ppi)
 	pp_diag(ppi, ext, 2, "hook: %s\n", __func__);
+	if ( l1epds->basic.L1SyncState != L1SYNC_UP )
+		return 0;
 	/* This correction_field we received is already part of t4 */
 	/*
@@ -161,7 +165,7 @@ static int l1e_handle_resp(struct pp_instance *ppi)
 		pp_servo_got_resp(ppi);
 		/*
 		 * pps always on if offset less than 1 second,
-		 * until ve have a configurable threshold */
+		 * until we have a configurable threshold */
 		WRH_OPER()->enable_timing_output(ppi, ofm->secs==0);
 	}
@@ -191,6 +195,12 @@ static int l1e_handle_followup(struct pp_instance *ppi,
 static __attribute__((used)) int l1e_handle_presp(struct pp_instance *ppi)
 {
 	struct pp_time *ofm = &SRV(ppi)->offsetFromMaster;
+	l1e_ext_portDS_t *pds=L1E_DSPOR(ppi);
+	/* Servo is active only when the state is UP */
+	if ( pds->basic.L1SyncState != L1SYNC_UP )
+		return 0;
 	/*
 	 * If no WR mode is on, run normal code, if T2/T3 are valid.

--- a/proto-ext-l1sync/l1e-servo.c
+++ b/proto-ext-l1sync/l1e-servo.c
@@ -105,10 +105,6 @@ void l1e_servo_reset(struct pp_instance *ppi)
 	struct l1e_servo_state *s=L1E_SRV(ppi);
-	if (!s) {
-		/* Don't clean servo state when is not available */
-		return;
-	}
 	/* shmem lock */
 	wrs_shm_write(ppsi_head, WRS_SHM_WRITE_BEGIN);
 	ppi->flags = 0;

--- a/proto-ext-l1sync/l1e-state-machine.c
+++ b/proto-ext-l1sync/l1e-state-machine.c
@@ -14,8 +14,6 @@ typedef struct {
 	int (*action)(struct pp_instance *ppi, Boolean newState);
 }l1e_state_machine_t;
-#define L1E_DEFAULT_DELAY 500
 extern char *l1e_state_name[];
 #define MAX_STATE_ACTIONS (sizeof(le1_state_actions)/sizeof(l1e_state_machine_t))
@@ -63,7 +61,8 @@ int l1e_run_state_machine(struct pp_instance *ppi) {
 	int delay;
 	if ( nextState>=MAX_STATE_ACTIONS)
-		return L1E_DEFAULT_DELAY; /* return default delay */
+		return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC);
 	/*
 	 *  Update the L1SYNC dynamic data independent of the state machine
@@ -72,9 +71,11 @@ int l1e_run_state_machine(struct pp_instance *ppi) {
 	/* By design, when link is up, it is always tx coherent and congruent*/
 	basicDS->isTxCoherent= ppi->link_up ? 1 : 0;
-	/* Check SYNC reception Time-out */
+	/* Check L1SYNC reception Time-out */
 	if ( pp_timeout(ppi, L1E_TIMEOUT_RX_SYNC) ) {
 		/* Time-out detected */
+		int timeout_tx_sync = (4 << (basicDS->logL1SyncInterval + 8)) * basicDS->L1SyncReceiptTimeout;
+		__pp_timeout_set(ppi, L1E_TIMEOUT_RX_SYNC, timeout_tx_sync);
 		basicDS->L1SyncLinkAlive = FALSE;
 	}
@@ -92,7 +93,7 @@ int l1e_run_state_machine(struct pp_instance *ppi) {
 }
 static int l1e_empty_action(struct pp_instance *ppi, Boolean new_state){
-	return L1E_DEFAULT_DELAY; /* return default delay */
+	return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC); /* Return the shorter timeout */
 }
 /* L1_SYNC_RESET event */
@@ -202,7 +203,7 @@ static int l1e_handle_state_disabled(struct pp_instance *ppi, Boolean new_state)
 		 L1E_DSPOR_BS(ppi)->next_state=L1SYNC_IDLE;
 		 return 0; /* no wait to evaluate next state */
 	}
-	return L1E_DEFAULT_DELAY; /* return default delay */
+	return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC); /* Return the shorter timeout */
 }
 /* IDLE state */
@@ -230,15 +231,15 @@ static int l1e_handle_state_idle(struct pp_instance *ppi, Boolean new_state){
 	if ( !le1_evt_L1_SYNC_ENABLED(ppi) || le1_evt_L1_SYNC_RESET(ppi) ) {
 		/* Go to DISABLE state */
 		l1e_portDS->basic.next_state=L1SYNC_DISABLED;
-		return L1E_DEFAULT_DELAY; /* return default delay */
+		return 0; /* Treatment required asap */
 	}
 	if ( le1_evt_LINK_OK(ppi) ) {
 		l1e_portDS->basic.next_state=L1SYNC_LINK_ALIVE;
- 		return L1E_DEFAULT_DELAY; /* return default delay */
+		return 0; /* Treatment required asap */
 	}
 	/* Iterative treatment */
 	l1e_send_sync_msg(ppi,0);
-	return L1E_DEFAULT_DELAY; /* return default delay */
+	return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC); /* Return the shorter timeout */
 }
 /* LINK_ALIVE state */
@@ -256,16 +257,16 @@ static int l1e_handle_state_link_alive(struct pp_instance *ppi, Boolean new_stat
 	if ( !le1_evt_LINK_OK(ppi) ) {
 		/* Go to IDLE state */
 		basic->next_state=L1SYNC_IDLE;
-		return L1E_DEFAULT_DELAY; /* return default delay */
+		return 0; /* Treatment required asap */
 	}
 	/* we will allow to go to CONFIG_MATH only for UNCALIBRATED and MASTER states */
-	if ( le1_evt_CONFIG_OK(ppi) && (ppi->state==PPS_UNCALIBRATED || ppi->state==PPS_MASTER)) {
+	if ( le1_evt_CONFIG_OK(ppi) && (ppi->state==PPS_UNCALIBRATED || ppi->state==PPS_SLAVE || ppi->state==PPS_MASTER)) {
 		basic->next_state=L1SYNC_CONFIG_MATCH;
-		return 0; /* Try to treat next state asap */
+		return 0; /* Treatment required asap */
 	}
 	/* Iterative treatment */
 	l1e_send_sync_msg(ppi,0);
-	return  pp_next_delay_1(ppi, L1E_TIMEOUT_TX_SYNC); /* Return the timeout for next invocation */
+	return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC); /* Return the shorter timeout */
 }
 /* CONFIG_MATCH state */
@@ -295,21 +296,21 @@ static int l1e_handle_state_config_match(struct pp_instance *ppi, Boolean new_st
 	if ( !le1_evt_LINK_OK(ppi) ) {
 		/* Go to IDLE state */
 		L1E_DSPOR_BS(ppi)->next_state=L1SYNC_IDLE;
-		return L1E_DEFAULT_DELAY; /* return default delay */
+		return 0; /* Treatment required asap */
 	}
 	if ( !le1_evt_CONFIG_OK(ppi) ) {
 		/* Return to LINK_ALIVE state */
 		L1E_DSPOR_BS(ppi)->next_state=L1SYNC_LINK_ALIVE;
-		return L1E_DEFAULT_DELAY; /* return default delay */
+		return 0; /* Treatment required asap */
 	}
 	if ( le1_evt_STATE_OK(ppi) ) {
 		/* Return to UP state */
 		L1E_DSPOR_BS(ppi)->next_state=L1SYNC_UP;
-		return 0; /* Treat next state asap */
+		return 0; /* Treatment required asap */
 	}
 	/* Iterative treatment */
 	l1e_send_sync_msg(ppi,0);
-	return  pp_next_delay_1(ppi, L1E_TIMEOUT_TX_SYNC); /* Return the timeout for next invocation */
+	return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC); /* Return the shorter timeout */
 }
 /* UP state */
@@ -350,5 +351,5 @@ static int l1e_handle_state_up(struct pp_instance *ppi, Boolean new_state){
 	/* Iterative treatment */
 	l1e_update_correction_values(ppi);
 	l1e_send_sync_msg(ppi,0);
-	return  pp_next_delay_1(ppi, L1E_TIMEOUT_TX_SYNC); /* Return the timeout for next invocation */
+	return pp_next_delay_2(ppi,L1E_TIMEOUT_TX_SYNC, L1E_TIMEOUT_RX_SYNC); /* Return the shorter timeout */
 }
--- a/proto-standard/state-listening.c
+++ b/proto-standard/state-listening.c
@@ -58,7 +58,7 @@ out:
 	if (e != 0)
 		ppi->next_state = PPS_FAULTY;
-	if (CONFIG_HAS_P2P && ppi->delayMechanism == PP_P2P_MECH) {
+	if (CONFIG_HAS_P2P && ppi->delayMechanism ==P2P) {
 		ppi->next_delay = pp_next_delay_2(ppi, 
 			PP_TO_ANN_RECEIPT, PP_TO_REQUEST);
 	} else {

--- a/proto-standard/state-master.c
+++ b/proto-standard/state-master.c
@@ -117,7 +117,7 @@ out:
 	}
 	if (pre) {
-		if (CONFIG_HAS_P2P && ppi->delayMechanism == PP_P2P_MECH) {
+		if (CONFIG_HAS_P2P && ppi->delayMechanism == P2P) {
 			ppi->next_delay = pp_next_delay_2(ppi,
 				PP_TO_QUALIFICATION, PP_TO_REQUEST);
 		} else {
@@ -125,12 +125,12 @@ out:
 				PP_TO_QUALIFICATION);			
 		}		
 	} else {
-		if (CONFIG_HAS_P2P && ppi->delayMechanism == PP_P2P_MECH) {
+		if (CONFIG_HAS_P2P && ppi->delayMechanism == P2P) {
 			ppi->next_delay = pp_next_delay_3(ppi,
 				PP_TO_ANN_SEND, PP_TO_SYNC_SEND, PP_TO_REQUEST);
 		} else {
 			ppi->next_delay = pp_next_delay_2(ppi,
-				PP_TO_ANN_SEND, PP_TO_SYNC_SEND);			
+				PP_TO_ANN_SEND, PP_TO_SYNC_SEND);
 		}		
 	}

--- a/proto-standard/state-passive.c
+++ b/proto-standard/state-passive.c
@@ -79,7 +79,7 @@ int pp_passive(struct pp_instance *ppi, void *buf, int len)
 	if (e != 0)
 		ppi->next_state = PPS_FAULTY;
-	if (CONFIG_HAS_P2P && ppi->delayMechanism == PP_P2P_MECH) {
+	if (CONFIG_HAS_P2P && ppi->delayMechanism == P2P) {
 		ppi->next_delay = pp_next_delay_2(ppi, 
 			PP_TO_ANN_RECEIPT, PP_TO_REQUEST);
 	} else {

--- a/proto-standard/state-slave.c
+++ b/proto-standard/state-slave.c
@@ -42,6 +42,13 @@ static int slave_handle_sync(struct pp_instance *ppi, void *buf,
 	if (!msg_from_current_master(ppi))
 		return 0;
+	if ( ppi->delayMechanism==E2E &&  ppi->t1.scaled_nsecs==0 && ppi->t1.secs==0 ) {
+		/* First time we receive the SYNC message in uncalib/slave state
+		 * We set the REQUEST time-out to the minDelayReqInterval/2 value (500ms)
+		 * in order to provide quickly a DelayReq message
+		 */
+		__pp_timeout_set(ppi, PP_TO_REQUEST, (1000*(1<<PP_MIN_MIN_DELAY_REQ_INTERVAL))/2);
+	}
 	/* t2 may be overriden by follow-up, save it immediately */
 	ppi->t2 = ppi->last_rcv_time;
 	msg_unpack_sync(buf, &sync);
@@ -51,17 +58,17 @@ static int slave_handle_sync(struct pp_instance *ppi, void *buf,
 		ppi->recv_sync_sequence_id = hdr->sequenceId;
 		/* for two-step, the stamp comes later */
 		ppi->t1 = hdr->cField; /* most likely 0 */
-		return 0;
+	} else {
+		/* one-step follows */
+		ppi->flags &= ~PPI_FLAG_WAITING_FOR_F_UP;
+		ppi->t1 = sync.originTimestamp;
+		pp_time_add(&ppi->t1, &hdr->cField);
+		ppi->syncCF = 0;
+		if (CONFIG_HAS_P2P && ppi->delayMechanism == P2P)
+			pp_servo_got_psync(ppi);
+		else
+			pp_servo_got_sync(ppi);
 	}
-	/* one-step folllows */
-	ppi->flags &= ~PPI_FLAG_WAITING_FOR_F_UP;
-	ppi->t1 = sync.originTimestamp;
-	pp_time_add(&ppi->t1, &hdr->cField);
-	ppi->syncCF = 0;
-	if (CONFIG_HAS_P2P && ppi->delayMechanism == P2P)
-		pp_servo_got_psync(ppi);
-	else
-		pp_servo_got_sync(ppi);
 	return 0;
 }
@@ -165,8 +172,11 @@ static int slave_handle_announce(struct pp_instance *ppi, void *buf, int len)
 	ret = st_com_handle_announce(ppi, buf, len);
 	if (ret)
 		return ret;
-	if (!msg_from_current_master(ppi))
+	/* If externalPortConfiguration option is set, we consider that all
+	 * announce messages come from the current master.
+	 */
+	if (!DSDEF(ppi)->externalPortConfigurationEnabled && !msg_from_current_master(ppi))
 		return 0;
 	/* 9.2.6.11 a) reset timeout */

--- a/timeout.c
+++ b/timeout.c
@@ -20,7 +20,7 @@ struct timeout_config {
 /* most timeouts have a static configuration. Save it here */
 static struct timeout_config to_configs[__PP_TO_ARRAY_SIZE] = {
-	[PP_TO_REQUEST] =	{"REQUEST",	RAND_0_200,},
+	[PP_TO_REQUEST] =	{"REQUEST", /* RAND defined at run-time */},
 	[PP_TO_SYNC_SEND] =	{"SYNC_SEND",	RAND_70_130,},
 	[PP_TO_BMC] =		{"BMC",		RAND_NONE,},
 	[PP_TO_ANN_RECEIPT] =	{"ANN_RECEIPT",	RAND_NONE,},
@@ -36,12 +36,17 @@ static struct timeout_config to_configs[__PP_TO_ARRAY_SIZE] = {
 void pp_timeout_init(struct pp_instance *ppi)
 {
 	portDS_t *port = ppi->portDS;
+	Boolean p2p=CONFIG_HAS_P2P && ppi->delayMechanism == P2P;
-	to_configs[PP_TO_REQUEST].value = (CONFIG_HAS_P2P && ppi->delayMechanism == P2P) ?
+	Integer8 logDelayRequest=p2p ?
 			port->logMinPdelayReqInterval : port->logMinDelayReqInterval;
+	to_configs[PP_TO_REQUEST].which_rand = p2p ? RAND_NONE : RAND_0_200;
+	to_configs[PP_TO_REQUEST].value= p2p ?
+			1000*(1<<logDelayRequest) :
+			logDelayRequest;
 	/* fault timeout is 4 avg request intervals, not randomized */
 	to_configs[PP_TO_FAULT].value =
-		1 << (port->logMinDelayReqInterval + 12); /* 0 -> 4096ms */
+		1 << (logDelayRequest + 12); /* 0 -> 4096ms */
 	to_configs[PP_TO_SYNC_SEND].value = port->logSyncInterval;
 	to_configs[PP_TO_BMC].value = 1000 * (1 << port->logAnnounceInterval);
 	to_configs[PP_TO_ANN_RECEIPT].value = 1000 * (