linuxptp-devel

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Wed, Sep 26, 2018 at 02:57:36PM -0700, Vedang Patel wrote:
> @@ -2472,6 +2480,7 @@ static enum fsm_event bc_event(struct port *p, int fd_index)
>  		if (p->best)
>  			fc_clear(p->best);
>  		port_set_announce_tmo(p);
> +		port_clr_tmo(p->fda.fd[FD_SYNC_RX_TIMER]);
>  		delay_req_prune(p);
>  		if (clock_slave_only(p->clock) && p->delayMechanism != DM_P2P &&
>  		    port_renew_transport(p)) {
> @@ -2862,10 +2871,24 @@ struct port *port_open(int phc_index,

This hunk needs some kind of justification, especially since you undo
it later in the series.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Sun, Sep 30, 2018 at 08:00:51PM -0700, Richard Cochran wrote:
> On Wed, Sep 26, 2018 at 02:57:36PM -0700, Vedang Patel wrote:
> > @@ -2472,6 +2480,7 @@ static enum fsm_event bc_event(struct port *p, int fd_index)
> >  		if (p->best)
> >  			fc_clear(p->best);
> >  		port_set_announce_tmo(p);
> > +		port_clr_tmo(p->fda.fd[FD_SYNC_RX_TIMER]);
> >  		delay_req_prune(p);
> >  		if (clock_slave_only(p->clock) && p->delayMechanism != DM_P2P &&
> >  		    port_renew_transport(p)) {
> > @@ -2862,10 +2871,24 @@ struct port *port_open(int phc_index,
> 
> This hunk needs some kind of justification, especially since you undo
> it later in the series.

Can you avoid this by putting the inhibit_announce patch first?

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Patel, V. <ved...@in...>]

Hi Richard,

On Sun, 2018-09-30 at 20:02 -0700, Richard Cochran wrote:
> On Sun, Sep 30, 2018 at 08:00:51PM -0700, Richard Cochran wrote:
> > 
> > On Wed, Sep 26, 2018 at 02:57:36PM -0700, Vedang Patel wrote:
> > > 
> > > @@ -2472,6 +2480,7 @@ static enum fsm_event bc_event(struct port
> > > *p, int fd_index)
> > >  		if (p->best)
> > >  			fc_clear(p->best);
> > >  		port_set_announce_tmo(p);
> > > +		port_clr_tmo(p->fda.fd[FD_SYNC_RX_TIMER]);
> > >  		delay_req_prune(p);
> > >  		if (clock_slave_only(p->clock) && p-
> > > >delayMechanism != DM_P2P &&
> > >  		    port_renew_transport(p)) {
> > > @@ -2862,10 +2871,24 @@ struct port *port_open(int phc_index,
> > This hunk needs some kind of justification, especially since you
> > undo
> > it later in the series.
> Can you avoid this by putting the inhibit_announce patch first?
> 
I will add the comment for clearing the SYNC_RX_TIMER. It is basically
to clear out the event returned by poll().

But, I don't understand how moving the inhibit_announce before this
will help. I am not removing the hunk anywhere.

Thanks,
Vedang
> Thanks,
> Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Mon, Oct 01, 2018 at 05:24:48PM +0000, Patel, Vedang wrote:
> I will add the comment for clearing the SYNC_RX_TIMER. It is basically
> to clear out the event returned by poll().

But why?  Does the original code have a bug?  Please explain.

> But, I don't understand how moving the inhibit_announce before this
> will help.

You avoid changing the behavior of the non-noop-BCMA code.

> I am not removing the hunk anywhere.

You are right.  I misread the next patch.

BUT you avoid changing the original behavior by doing this:

	if (p->inhibit_announce) {
		port_clr_tmo(p->fda.fd[FD_ANNOUNCE_TIMER]);
		port_clr_tmo(p->fda.fd[FD_SYNC_RX_TIMER]);
	} else {
		port_set_announce_tmo(p);
	}

See?

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Patel, V. <ved...@in...>]

On Mon, 2018-10-01 at 10:59 -0700, Richard Cochran wrote:
> On Mon, Oct 01, 2018 at 05:24:48PM +0000, Patel, Vedang wrote:
> > 
> > I will add the comment for clearing the SYNC_RX_TIMER. It is
> > basically
> > to clear out the event returned by poll().
> But why?  Does the original code have a bug?  Please explain.
> 
In the original code, whenever we receive a FD_SYNC_RX_TIMER timeout,
it is usually accompanied by a state transition. So,
port_*_transition() will take care of clearing the event for us (by
calling port_clr_tmo() at the beginning). But, when BMCA == 'noop',
there is nothing to do, so we are not clearing out the event and poll()
will report the event in every loop. I am just taking care of clearing
this event here.
> > 
> > But, I don't understand how moving the inhibit_announce before this
> > will help.
> You avoid changing the behavior of the non-noop-BCMA code.
> 
> > 
> > I am not removing the hunk anywhere.
> You are right.  I misread the next patch.
> 
> BUT you avoid changing the original behavior by doing this:
> 
> 	if (p->inhibit_announce) {
> 		port_clr_tmo(p->fda.fd[FD_ANNOUNCE_TIMER]);
> 		port_clr_tmo(p->fda.fd[FD_SYNC_RX_TIMER]);
> 	} else {
> 		port_set_announce_tmo(p);
> 	}
> 
The SYNC_RX_TIMER needs to be cleared out whenever BMCA == 'noop' (or
any other future state machine which does not do any state transition 
in  this case). I will add the condition to clear out the timer only
when BMCA == 'noop'.

Thanks,
Vedang
> See?
> 
> Thanks,
> Richard

Re: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id config option.

[Keller, J. E <jac...@in...>]

> -----Original Message-----
> From: Vedang Patel [mailto:ved...@in...]
> Sent: Wednesday, September 26, 2018 2:58 PM
> To: lin...@li...
> Cc: Gomes, Vinicius <vin...@in...>; Sanchez-Palencia, Jesus
> <jes...@in...>
> Subject: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id config option.
> 
> This config option will skip the source port identity verification in
> the Sync and Follow_up messages. This option is needed when the announce
> messages are disabled because the slave cannot know the identity of
> master without announce messages.
> 
> This is required by Automotive Profile as part of skipping the Best
> Master Clock Algorithm (BMCA).
> 

Would it be better to allow configuring the expected source id instead?

Thanks,
Jake 

Re: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id config option.

[Patel, V. <ved...@in...>]

On Mon, 2018-10-01 at 21:32 +0000, Keller, Jacob E wrote:
> 
> > 
> > -----Original Message-----
> > From: Vedang Patel [mailto:ved...@in...]
> > Sent: Wednesday, September 26, 2018 2:58 PM
> > To: lin...@li...
> > Cc: Gomes, Vinicius <vin...@in...>; Sanchez-Palencia,
> > Jesus
> > <jes...@in...>
> > Subject: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id
> > config option.
> > 
> > This config option will skip the source port identity verification
> > in
> > the Sync and Follow_up messages. This option is needed when the
> > announce
> > messages are disabled because the slave cannot know the identity of
> > master without announce messages.
> > 
> > This is required by Automotive Profile as part of skipping the Best
> > Master Clock Algorithm (BMCA).
> > 
> Would it be better to allow configuring the expected source id
> instead?
> 
Manually configuring source ids for all slave devices is not going to
be ideal in mass production. 

Also, the spec[1] (look at Section 6.3, point #2) specifically asks for
the source id to be ignored.

Thanks,
Vedang

[1] - https://avnu.org/wp-content/uploads/2014/05/Automotive-Ethernet-A
VB-Func-Interop-Spec-v1.5-Public.pdf
> Thanks,
> Jake 
> 

Re: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id config option.

[Keller, J. E <jac...@in...>]

> -----Original Message-----
> From: Patel, Vedang
> Sent: Monday, October 01, 2018 3:29 PM
> To: Keller, Jacob E <jac...@in...>; linuxptp-
> de...@li...
> Cc: Sanchez-Palencia, Jesus <jes...@in...>; Gomes,
> Vinicius <vin...@in...>
> Subject: Re: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id config
> option.
> 
> On Mon, 2018-10-01 at 21:32 +0000, Keller, Jacob E wrote:
> >
> > >
> > > -----Original Message-----
> > > From: Vedang Patel [mailto:ved...@in...]
> > > Sent: Wednesday, September 26, 2018 2:58 PM
> > > To: lin...@li...
> > > Cc: Gomes, Vinicius <vin...@in...>; Sanchez-Palencia,
> > > Jesus
> > > <jes...@in...>
> > > Subject: [Linuxptp-devel] [PATCH v2 6/7] Add ignore_source_id
> > > config option.
> > >
> > > This config option will skip the source port identity verification
> > > in
> > > the Sync and Follow_up messages. This option is needed when the
> > > announce
> > > messages are disabled because the slave cannot know the identity of
> > > master without announce messages.
> > >
> > > This is required by Automotive Profile as part of skipping the Best
> > > Master Clock Algorithm (BMCA).
> > >
> > Would it be better to allow configuring the expected source id
> > instead?
> >
> Manually configuring source ids for all slave devices is not going to
> be ideal in mass production.
> 
> Also, the spec[1] (look at Section 6.3, point #2) specifically asks for
> the source id to be ignored.
> 
> Thanks,
> Vedang

Yep, thanks for the explanation.

Regards,
Jake

> 
> [1] - https://avnu.org/wp-content/uploads/2014/05/Automotive-Ethernet-A
> VB-Func-Interop-Spec-v1.5-Public.pdf
> > Thanks,
> > Jake
> >

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Mon, Oct 01, 2018 at 06:11:16PM +0000, Patel, Vedang wrote:
> On Mon, 2018-10-01 at 10:59 -0700, Richard Cochran wrote:
> > On Mon, Oct 01, 2018 at 05:24:48PM +0000, Patel, Vedang wrote:
> > > 
> > > I will add the comment for clearing the SYNC_RX_TIMER. It is
> > > basically
> > > to clear out the event returned by poll().
> > But why?  Does the original code have a bug?  Please explain.
> > 
> In the original code, whenever we receive a FD_SYNC_RX_TIMER timeout,
> it is usually accompanied by a state transition. So,
> port_*_transition() will take care of clearing the event for us (by
> calling port_clr_tmo() at the beginning). But, when BMCA == 'noop',
> there is nothing to do, so we are not clearing out the event and poll()
> will report the event in every loop. I am just taking care of clearing
> this event here.

Then you should have followed through and removed the redundant clear
in the state transition code.  Or what am I missing?

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Patel, V. <ved...@in...>]

On Mon, 2018-10-01 at 18:26 -0700, Richard Cochran wrote:
> On Mon, Oct 01, 2018 at 06:11:16PM +0000, Patel, Vedang wrote:
> > 
> > On Mon, 2018-10-01 at 10:59 -0700, Richard Cochran wrote:
> > > 
> > > On Mon, Oct 01, 2018 at 05:24:48PM +0000, Patel, Vedang wrote:
> > > > 
> > > > 
> > > > I will add the comment for clearing the SYNC_RX_TIMER. It is
> > > > basically
> > > > to clear out the event returned by poll().
> > > But why?  Does the original code have a bug?  Please explain.
> > > 
> > In the original code, whenever we receive a FD_SYNC_RX_TIMER
> > timeout,
> > it is usually accompanied by a state transition. So,
> > port_*_transition() will take care of clearing the event for us (by
> > calling port_clr_tmo() at the beginning). But, when BMCA == 'noop',
> > there is nothing to do, so we are not clearing out the event and
> > poll()
> > will report the event in every loop. I am just taking care of
> > clearing
> > this event here.
> Then you should have followed through and removed the redundant clear
> in the state transition code.  Or what am I missing?
> 
We still need to clear out the timer when the device transitions out of
slave state for other events like EV_FAULT_DETECTED. So, I think we
still need to keep it there.

Thanks,
Vedang
> Thanks,
> Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Tue, Oct 02, 2018 at 04:25:32PM +0000, Patel, Vedang wrote:
> We still need to clear out the timer when the device transitions out of
> slave state for other events like EV_FAULT_DETECTED. So, I think we
> still need to keep it there.

So, IOW, the reset is a special case for the noop-BCMA, right?

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v2 4/7] Add BMCA config option.

[Patel, V. <ved...@in...>]

On Tue, 2018-10-02 at 16:00 -0700, Richard Cochran wrote:
> On Tue, Oct 02, 2018 at 04:25:32PM +0000, Patel, Vedang wrote:
> > 
> > We still need to clear out the timer when the device transitions
> > out of
> > slave state for other events like EV_FAULT_DETECTED. So, I think we
> > still need to keep it there.
> So, IOW, the reset is a special case for the noop-BCMA, right?
> 
Yes, it is a special case. So, I guess it makes sense to do it only
when we are running noop-BMCA.

Thanks,
Vedang
> Thanks,
> Richard

[Linuxptp-devel] [PATCH v1 3/6] Add BMCA config option.

[Vedang P. <ved...@in...>]

This adds config option to specify static roles for master and slave
in the Best Master Clock Algorithm. This is the case for Automotive
Profile where networks are mostly static and role for each device is
known in advance.

masterOnly and slaveOnly will be used to determine the roles for the
devices. Since masterOnly is a per-port config and slaveOnly is a global
config option, role assignment will be slightly odd in case of bridges.
If slaveOnly is set to 1, all the ports will be in slave roles except
for the ones where masterOnly is set to 1. These ports will assume the
master role.

Two new FSMs which will be used for master and slave roles for this
config option have also been added.

Signed-off-by: Vedang Patel <ved...@in...>
---
 bmc.c               |  11 +++++
 clock.c             |  17 +++++++
 config.c            |   7 +++
 configs/default.cfg |   1 +
 designated_fsm.c    | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 designated_fsm.h    |  43 +++++++++++++++++
 fsm.h               |   6 ++-
 makefile            |  11 +++--
 port.c              |  23 ++++++++-
 port.h              |   8 ++++
 port_private.h      |   1 +
 ptp4l.8             |  17 +++++--
 12 files changed, 266 insertions(+), 12 deletions(-)
 create mode 100644 designated_fsm.c
 create mode 100644 designated_fsm.h

diff --git a/bmc.c b/bmc.c
index 3c3db828764b..b0a80c2198cb 100644
--- a/bmc.c
+++ b/bmc.c
@@ -126,6 +126,17 @@ enum port_state bmc_state_decision(struct clock *c, struct port *r,
 	port_best = port_best_foreign(r);
 	ps = port_state(r);
 
+	/*
+	 * This scenario is particularly important in the designated_slave_fsm
+	 * when it is in PS_UNCALIBRATED state. In this scenario, there is no
+	 * other foriegn master and it will elect itself as master ultimately
+	 * resulting in printing out some unnecessary warnings (see
+	 * port_slave_priority_warning()).
+	 */
+	if (!port_best && port_bmca(r) == BMCA_NOOP) {
+		return ps;
+	}
+
 	if (!port_best && PS_LISTENING == ps)
 		return ps;
 
diff --git a/clock.c b/clock.c
index 0f2e319a19c7..eef534aea718 100644
--- a/clock.c
+++ b/clock.c
@@ -1136,6 +1136,15 @@ struct clock *clock_create(enum clock_type type, struct config *config,
 
 	LIST_FOREACH(p, &c->ports, list) {
 		port_dispatch(p, EV_INITIALIZE, 0);
+
+		/*
+		 * Generate an event to trigger a transition from PS_LISTENING
+		 * to the respective PS_MASTER/PS_SLAVE state for BMCA with
+		 * designated master and slave.
+		 */
+		if (port_bmca(p) == BMCA_NOOP) {
+			port_dispatch(p, EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES, 0);
+		}
 	}
 	port_dispatch(c->uds_port, EV_INITIALIZE, 0);
 
@@ -1502,6 +1511,14 @@ int clock_poll(struct clock *c)
 				if (PS_FAULTY == port_state(p)) {
 					clock_fault_timeout(p, 1);
 					break;
+				} else if (port_state(p) == PS_LISTENING &&
+					   port_bmca(p) == BMCA_NOOP) {
+					/*
+					 * PS_LISTENING is just a transitory
+					 * state in BMCA with designated master
+					 * and slave.
+					 */
+					port_dispatch(p, EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES, 0);
 				}
 			}
 		}
diff --git a/config.c b/config.c
index 7914ba4b5166..47cad7ab6632 100644
--- a/config.c
+++ b/config.c
@@ -188,10 +188,17 @@ static struct config_enum tsproc_enu[] = {
 	{ NULL, 0 },
 };
 
+static struct config_enum bmca_enu[] = {
+	{ "ptp",  BMCA_PTP  },
+	{ "noop", BMCA_NOOP },
+	{ NULL, 0 },
+};
+
 struct config_item config_tab[] = {
 	PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
 	GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
 	PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
+	PORT_ITEM_ENU("BMCA", BMCA_PTP, bmca_enu),
 	GLOB_ITEM_INT("check_fup_sync", 0, 0, 1),
 	GLOB_ITEM_INT("clockAccuracy", 0xfe, 0, UINT8_MAX),
 	GLOB_ITEM_INT("clockClass", 248, 0, UINT8_MAX),
diff --git a/configs/default.cfg b/configs/default.cfg
index c5a8b57c9314..02714150fbab 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -31,6 +31,7 @@ fault_reset_interval	4
 neighborPropDelayThresh	20000000
 masterOnly		0
 G.8275.portDS.localPriority	128
+BMCA                    ptp
 #
 # Run time options
 #
diff --git a/designated_fsm.c b/designated_fsm.c
new file mode 100644
index 000000000000..a56f96283baa
--- /dev/null
+++ b/designated_fsm.c
@@ -0,0 +1,133 @@
+/**
+ * @file designated_fsm.c
+ * @brief Implements designated Finite State Machines.
+ * @note Copyright (C) 2018 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335 USA.
+ */
+#include "fsm.h"
+#include "designated_fsm.h"
+
+enum port_state designated_master_fsm(enum port_state state,
+				      enum fsm_event event,
+				      int mdiff)
+{
+	enum port_state next = state;
+
+	if (EV_INITIALIZE == event || EV_POWERUP == event)
+		return PS_INITIALIZING;
+
+	switch (state) {
+	case PS_INITIALIZING:
+		switch (event) {
+		case EV_FAULT_DETECTED:
+			next = PS_FAULTY;
+			break;
+		case EV_INIT_COMPLETE:
+			next =  PS_LISTENING;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	case PS_LISTENING:
+		if (event == EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES) {
+			next = PS_MASTER;
+		}
+		break;
+
+	case PS_FAULTY:
+		if (event == EV_FAULT_CLEARED) {
+			next = PS_INITIALIZING;
+		}
+		break;
+
+	case PS_MASTER:
+		if (event == EV_FAULT_DETECTED) {
+			next = PS_FAULTY;
+		}
+		break;
+
+	default:
+		break;
+	}
+	return next;
+}
+
+enum port_state designated_slave_fsm(enum port_state state,
+				     enum fsm_event event,
+				     int mdiff)
+{
+	enum port_state next = state;
+
+	if (EV_INITIALIZE == event || EV_POWERUP == event)
+		return PS_INITIALIZING;
+
+	switch (state) {
+	case PS_INITIALIZING:
+		switch (event) {
+		case EV_FAULT_DETECTED:
+			next = PS_FAULTY;
+			break;
+		case EV_INIT_COMPLETE:
+			next =  PS_LISTENING;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	case PS_FAULTY:
+		if (event == EV_FAULT_CLEARED) {
+			next = PS_INITIALIZING;
+		}
+		break;
+
+	case PS_LISTENING:
+		if (event == EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES) {
+			next = PS_SLAVE;
+		}
+		break;
+
+	case PS_UNCALIBRATED:
+		if (event == EV_MASTER_CLOCK_SELECTED) {
+			next = PS_SLAVE;
+		}
+		break;
+
+	case PS_SLAVE:
+		switch (event) {
+		case EV_FAULT_DETECTED:
+			next = PS_FAULTY;
+			break;
+		/*
+		 * Go to PS_UNCALIBRATED when there is a loss of sync messages.
+		 * This will also be useful in changing the Sync Interval (to
+		 * be implemented).
+		 */
+		case EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES:
+			next = PS_UNCALIBRATED;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	default:
+		break;
+	}
+	return next;
+}
diff --git a/designated_fsm.h b/designated_fsm.h
new file mode 100644
index 000000000000..b1c0eae06281
--- /dev/null
+++ b/designated_fsm.h
@@ -0,0 +1,43 @@
+/**
+ * @file designated_fsm.c
+ * @brief Implements designated Finite State Machines.
+ * @note Copyright (C) 2018 Intel Corporation
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1335 USA.
+ */
+#ifndef HAVE_DESIGNATED_FSM_H
+#define HAVE_DESIGNATED_FSM_H
+/**
+ * Run the state machine for a clock which is designated as master port.
+ * @param state  The current state of the port.
+ * @param event  The event to be processed.
+ * @param mdiff  This param is not used by this function.
+ * @return       The new state for the port.
+ */
+enum port_state designated_master_fsm(enum port_state state,
+				      enum fsm_event event,
+				      int mdiff);
+
+/**
+ * Run the state machine for a clock designated as slave port.
+ * @param state  The current state of the port.
+ * @param event  The event to be processed.
+ * @param mdiff  This param is not used by this function.
+ * @return       The new state for the port.
+ */
+enum port_state designated_slave_fsm(enum port_state state,
+				     enum fsm_event event,
+				     int mdiff);
+#endif
diff --git a/fsm.h b/fsm.h
index 0616daa2ab7b..ed89ec6bbcaa 100644
--- a/fsm.h
+++ b/fsm.h
@@ -55,6 +55,11 @@ enum fsm_event {
 	EV_RS_PASSIVE,
 };
 
+enum bmca_select {
+	BMCA_PTP,
+	BMCA_NOOP,
+};
+
 /**
  * Run the state machine for a BC or OC port.
  * @param state  The current state of the port.
@@ -73,5 +78,4 @@ enum port_state ptp_fsm(enum port_state state, enum fsm_event event, int mdiff);
  */
 enum port_state ptp_slave_fsm(enum port_state state, enum fsm_event event,
 			      int mdiff);
-
 #endif
diff --git a/makefile b/makefile
index 693e75d6aa87..6386884a53e3 100644
--- a/makefile
+++ b/makefile
@@ -23,11 +23,12 @@ VER     = -DVER=$(version)
 CFLAGS	= -Wall $(VER) $(incdefs) $(DEBUG) $(EXTRA_CFLAGS)
 LDLIBS	= -lm -lrt $(EXTRA_LDFLAGS)
 PRG	= ptp4l hwstamp_ctl nsm phc2sys phc_ctl pmc timemaster
-OBJ     = bmc.o clock.o clockadj.o clockcheck.o config.o e2e_tc.o fault.o \
- filter.o fsm.o hash.o linreg.o mave.o mmedian.o msg.o ntpshm.o nullf.o phc.o \
- pi.o port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o raw.o rtnl.o \
- servo.o sk.o stats.o tc.o telecom.o tlv.o transport.o tsproc.o udp.o udp6.o \
- uds.o unicast_client.o unicast_fsm.o unicast_service.o util.o version.o
+OBJ     = bmc.o clock.o clockadj.o clockcheck.o config.o designated_fsm.o \
+e2e_tc.o fault.o filter.o fsm.o hash.o linreg.o mave.o mmedian.o msg.o ntpshm.o \
+nullf.o phc.o pi.o port.o port_signaling.o pqueue.o print.o ptp4l.o p2p_tc.o \
+raw.o rtnl.o servo.o sk.o stats.o tc.o telecom.o tlv.o transport.o tsproc.o \
+udp.o udp6.o uds.o unicast_client.o unicast_fsm.o unicast_service.o util.o \
+version.o
 
 OBJECTS	= $(OBJ) hwstamp_ctl.o nsm.o phc2sys.o phc_ctl.o pmc.o pmc_common.o \
  sysoff.o timemaster.o
diff --git a/port.c b/port.c
index 1a0e910a1151..45e870fca69f 100644
--- a/port.c
+++ b/port.c
@@ -27,6 +27,7 @@
 
 #include "bmc.h"
 #include "clock.h"
+#include "designated_fsm.h"
 #include "filter.h"
 #include "missing.h"
 #include "msg.h"
@@ -1593,7 +1594,6 @@ int port_initialize(struct port *p)
 	p->transportSpecific       = config_get_int(cfg, p->name, "transportSpecific");
 	p->transportSpecific     <<= 4;
 	p->match_transport_specific = !config_get_int(cfg, p->name, "ignore_transport_specific");
-	p->master_only             = config_get_int(cfg, p->name, "masterOnly");
 	p->localPriority           = config_get_int(cfg, p->name, "G.8275.portDS.localPriority");
 	p->logSyncInterval         = config_get_int(cfg, p->name, "logSyncInterval");
 	p->logMinPdelayReqInterval = config_get_int(cfg, p->name, "logMinPdelayReqInterval");
@@ -2853,10 +2853,24 @@ struct port *port_open(int phc_index,
 		goto err_port;
 	}
 
-	p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : ptp_fsm;
 	p->phc_index = phc_index;
 	p->jbod = config_get_int(cfg, interface->name, "boundary_clock_jbod");
 	transport = config_get_int(cfg, interface->name, "network_transport");
+	p->master_only = config_get_int(cfg, p->name, "masterOnly");
+	p->bmca = config_get_int(cfg, p->name, "BMCA");
+
+	if (p->bmca == BMCA_NOOP && transport != TRANS_UDS) {
+		if (p->master_only) {
+			p->state_machine = designated_master_fsm;
+		} else if (clock_slave_only(clock)) {
+			p->state_machine = designated_slave_fsm;
+		} else {
+			pr_err("Please enable at least one of masterOnly or slaveOnly when BMCA == noop.\n");
+			goto err_port;
+		}
+	} else {
+		p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : ptp_fsm;
+	}
 
 	if (transport == TRANS_UDS) {
 		; /* UDS cannot have a PHC. */
@@ -3014,3 +3028,8 @@ int port_state_update(struct port *p, enum fsm_event event, int mdiff)
 
 	return 0;
 }
+
+enum bmca_select port_bmca(struct port *p)
+{
+	return p->bmca;
+}
diff --git a/port.h b/port.h
index 9639193d23d1..aa3b1ecf2529 100644
--- a/port.h
+++ b/port.h
@@ -323,6 +323,14 @@ void fault_interval(struct port *port, enum fault_type ft,
 		    struct fault_interval *i);
 
 /**
+ * Obtain the BMCA type of the port.
+ *
+ * @param port        A port instance.
+ * @return            bmca type.
+ */
+enum bmca_select port_bmca(struct port *p);
+
+/**
  * Release all of the memory in the TC transmit descriptor cache.
  */
 void tc_cleanup(void);
diff --git a/port_private.h b/port_private.h
index 19d1d7beaae8..8b5525a2122a 100644
--- a/port_private.h
+++ b/port_private.h
@@ -96,6 +96,7 @@ struct port {
 	unsigned int multiple_pdr_detected;
 	enum port_state (*state_machine)(enum port_state state,
 					 enum fsm_event event, int mdiff);
+	int bmca;
 	/* portDS */
 	struct PortIdentity portIdentity;
 	enum port_state     state; /*portState*/
diff --git a/ptp4l.8 b/ptp4l.8
index 10c5c2f967cb..786fc71ddf06 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -363,10 +363,7 @@ hardware time stamping.
 The default is 1 (enabled).
 .TP
 .B slaveOnly
-The local clock is a slave-only clock if enabled.
-This option is only for use with 1588 clocks and should not be enabled
-for 802.1AS clocks.
-The default is 0 (disabled).
+The local clock is a slave-only clock if enabled. The default is 0 (disabled).
 .TP
 .B gmCapable
 If this option is enabled, then the local clock is able to become grand master.
@@ -661,6 +658,18 @@ The time source is a single byte code that gives an idea of the kind
 of local clock in use. The value is purely informational, having no
 effect on the outcome of the Best Master Clock algorithm, and is
 advertised when the clock becomes grand master.
+.TP
+.B BMCA
+This option enables use of static roles for master and slave devices instead of
+running the best master clock algorithm (BMCA) described in 1588 profile. This
+is useful when you know the roles of the devices in advance. When set to
+\'noop', the traditional BMCA algorithm used by 1588 is skipped. masterOnly and
+slaveOnly will be used to determine master or slave role for the device. In a
+bridge, slaveOnly (which is a global option) can be set to make all ports
+assume the slave role. masterOnly (which is a per-port config option) can then
+be used to set individual ports to take master role. BMCA is used in the
+Automotive profile to speed up the start time for grand master and slaves. The
+default value is 'ptp' which runs the BMCA related state machines.
 
 .SH UNICAST DISCOVERY OPTIONS
 
-- 
2.7.3

Re: [Linuxptp-devel] [PATCH v1 3/6] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Thu, Aug 30, 2018 at 04:54:37PM -0700, Vedang Patel wrote:
> diff --git a/bmc.c b/bmc.c
> index 3c3db828764b..b0a80c2198cb 100644
> --- a/bmc.c
> +++ b/bmc.c
> @@ -126,6 +126,17 @@ enum port_state bmc_state_decision(struct clock *c, struct port *r,
>  	port_best = port_best_foreign(r);
>  	ps = port_state(r);
>  
> +	/*
> +	 * This scenario is particularly important in the designated_slave_fsm
> +	 * when it is in PS_UNCALIBRATED state. In this scenario, there is no
> +	 * other foriegn master and it will elect itself as master ultimately
> +	 * resulting in printing out some unnecessary warnings (see
> +	 * port_slave_priority_warning()).
> +	 */
> +	if (!port_best && port_bmca(r) == BMCA_NOOP) {
> +		return ps;
> +	}

Put this special case into port_slave_priority_warning().

Remember: if you hack the core code, then you are probably doing it wrong.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v1 3/6] Add BMCA config option.

[Richard C. <ric...@gm...>]

On Thu, Aug 30, 2018 at 04:54:37PM -0700, Vedang Patel wrote:
> diff --git a/clock.c b/clock.c
> index 0f2e319a19c7..eef534aea718 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -1136,6 +1136,15 @@ struct clock *clock_create(enum clock_type type, struct config *config,
>  
>  	LIST_FOREACH(p, &c->ports, list) {
>  		port_dispatch(p, EV_INITIALIZE, 0);
> +
> +		/*
> +		 * Generate an event to trigger a transition from PS_LISTENING
> +		 * to the respective PS_MASTER/PS_SLAVE state for BMCA with
> +		 * designated master and slave.
> +		 */
> +		if (port_bmca(p) == BMCA_NOOP) {
> +			port_dispatch(p, EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES, 0);
> +		}
>  	}
>  	port_dispatch(c->uds_port, EV_INITIALIZE, 0);
>  
> @@ -1502,6 +1511,14 @@ int clock_poll(struct clock *c)
>  				if (PS_FAULTY == port_state(p)) {
>  					clock_fault_timeout(p, 1);
>  					break;
> +				} else if (port_state(p) == PS_LISTENING &&
> +					   port_bmca(p) == BMCA_NOOP) {
> +					/*
> +					 * PS_LISTENING is just a transitory
> +					 * state in BMCA with designated master
> +					 * and slave.
> +					 */
> +					port_dispatch(p, EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES, 0);

Remember: if you hack the core code, then you are probably doing it wrong.

^^^ That goes double for clock_poll().

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Patel, V. <ved...@in...>]

Hi Richard,

Is this series somewhere in your list of "things to review"? I just
wanted to check-in and make sure you didnt miss this. 

Thanks,
Vedang

On Thu, 2018-08-30 at 16:54 -0700, Vedang Patel wrote:
> Changes in this version:
> ~~~~~~~~~~~~~~~~~~~~~~~
> 
> Since the series has gone through some feedback and is mature now, it
> is being
> posted as PATCH series instead of RFC.
> 
> This version incorporates feedback from the last version of this
> series. The
> changes include: 
> - Remove AVnu keyword from all commit messages.
> - Instead of adding special checks for BMCA == ‘noop’ in
> port_p2p_transtion, we
>   now transition to PS_LISTENING before going to PS_MASTER/PS_SLAVE
> for
>   designated master or slave.
> - designated_{master/slave}_fsm() functions were moved to their own
> file.
> - Minor change in check_source_identity function.
> - In all the series adds functionality from Sections 6.2.1.1 and
> Section 6.3
>   (pts #1 and #2) of the Automotive Profile[1].
> 
> More information of intention of this and future series at: 
> https://sourceforge.net/p/linuxptp/mailman/message/36369408/
> 
> Thanks,
> Vedang Patel
> 
> [1] Automotive Ethernet AVB Functional and Interoperability
> Specification -
> http://avnu.org/wp-content/uploads/2014/05/
> Automotive-Ethernet-AVB-Func-Interop-Spec-v1.5-Public.pdf
> 
> Vedang Patel (6):
>   port: Add condition to check fc.
>   clock: Add NULL check for best clock in clock_update_slave()
>   Add BMCA config option.
>   Add inhibit_announce config option.
>   Add ignore_source_id config option.
>   Add example configuration for Automotive Profile.
> 
>  bmc.c                         |  11 ++++
>  clock.c                       |  24 +++++++-
>  config.c                      |   9 +++
>  configs/automotive-master.cfg |  27 +++++++++
>  configs/automotive-slave.cfg  |  30 ++++++++++
>  configs/default.cfg           |   3 +
>  designated_fsm.c              | 133
> ++++++++++++++++++++++++++++++++++++++++++
>  designated_fsm.h              |  43 ++++++++++++++
>  fsm.h                         |   6 +-
>  makefile                      |  11 ++--
>  port.c                        |  63 ++++++++++++++++----
>  port.h                        |   8 +++
>  port_private.h                |   3 +
>  ptp4l.8                       |  31 ++++++++--
>  14 files changed, 381 insertions(+), 21 deletions(-)
>  create mode 100644 configs/automotive-master.cfg
>  create mode 100644 configs/automotive-slave.cfg
>  create mode 100644 designated_fsm.c
>  create mode 100644 designated_fsm.h
> 

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Richard C. <ric...@gm...>]

On Thu, Sep 13, 2018 at 04:29:21PM +0000, Patel, Vedang wrote:
> Is this series somewhere in your list of "things to review"? I just
> wanted to check-in and make sure you didnt miss this.

Yes, I'll get to it as soon as I can.  Overall I'm happy with this
series, but I wanted to sit down and think through the automatic
switch to MASTER state that we discussed.  Maybe next week...

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Richard C. <ric...@gm...>]

On Thu, Sep 13, 2018 at 08:35:22PM -0700, Richard Cochran wrote:
> Yes, I'll get to it as soon as I can.  Overall I'm happy with this
> series, but I wanted to sit down and think through the automatic
> switch to MASTER state that we discussed.

Okay, I took your series and hacked the following diff on top of it.
The main difference is that my version doesn't touch clock_poll().

Also, there is no need for the master mode to go into the listening
state.

In addition, you need to implement the asCapable thing.  I suggest
turning it into an enum with something like:

	enum {
		NOT_CAPABLE,
		AS_CAPABLE,
		ALWAYS_CAPABLE
	} asCapable;

Thanks,
Richard

---
diff --git a/clock.c b/clock.c
index eef534a..0f2e319 100644
--- a/clock.c
+++ b/clock.c
@@ -1136,15 +1136,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
 
 	LIST_FOREACH(p, &c->ports, list) {
 		port_dispatch(p, EV_INITIALIZE, 0);
-
-		/*
-		 * Generate an event to trigger a transition from PS_LISTENING
-		 * to the respective PS_MASTER/PS_SLAVE state for BMCA with
-		 * designated master and slave.
-		 */
-		if (port_bmca(p) == BMCA_NOOP) {
-			port_dispatch(p, EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES, 0);
-		}
 	}
 	port_dispatch(c->uds_port, EV_INITIALIZE, 0);
 
@@ -1511,14 +1502,6 @@ int clock_poll(struct clock *c)
 				if (PS_FAULTY == port_state(p)) {
 					clock_fault_timeout(p, 1);
 					break;
-				} else if (port_state(p) == PS_LISTENING &&
-					   port_bmca(p) == BMCA_NOOP) {
-					/*
-					 * PS_LISTENING is just a transitory
-					 * state in BMCA with designated master
-					 * and slave.
-					 */
-					port_dispatch(p, EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES, 0);
 				}
 			}
 		}
diff --git a/configs/automotive-slave.cfg b/configs/automotive-slave.cfg
index 9cabe7a..66b6141 100644
--- a/configs/automotive-slave.cfg
+++ b/configs/automotive-slave.cfg
@@ -28,3 +28,5 @@ BMCA			noop
 slaveOnly		1
 inhibit_announce	1
 ignore_source_id	1
+announceReceiptTimeout	2
+logAnnounceInterval	-20
diff --git a/designated_fsm.c b/designated_fsm.c
index a56f962..9ccf4c9 100644
--- a/designated_fsm.c
+++ b/designated_fsm.c
@@ -36,19 +36,13 @@ enum port_state designated_master_fsm(enum port_state state,
 			next = PS_FAULTY;
 			break;
 		case EV_INIT_COMPLETE:
-			next =  PS_LISTENING;
+			next = PS_MASTER;
 			break;
 		default:
 			break;
 		}
 		break;
 
-	case PS_LISTENING:
-		if (event == EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES) {
-			next = PS_MASTER;
-		}
-		break;
-
 	case PS_FAULTY:
 		if (event == EV_FAULT_CLEARED) {
 			next = PS_INITIALIZING;
@@ -113,14 +107,6 @@ enum port_state designated_slave_fsm(enum port_state state,
 		case EV_FAULT_DETECTED:
 			next = PS_FAULTY;
 			break;
-		/*
-		 * Go to PS_UNCALIBRATED when there is a loss of sync messages.
-		 * This will also be useful in changing the Sync Interval (to
-		 * be implemented).
-		 */
-		case EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES:
-			next = PS_UNCALIBRATED;
-			break;
 		default:
 			break;
 		}
diff --git a/port.c b/port.c
index b326b9c..95c1bf8 100644
--- a/port.c
+++ b/port.c
@@ -743,6 +743,10 @@ static int port_sync_incapable(struct port *p)
 
 static int port_is_ieee8021as(struct port *p)
 {
+	// HACK - actually need an option for asCapable always true!
+	if (p->bmca == BMCA_NOOP) {
+		return 0;
+	}
 	return p->follow_up_info ? 1 : 0;
 }
 
@@ -1036,10 +1040,6 @@ static void port_nrate_initialize(struct port *p)
 
 int port_set_announce_tmo(struct port *p)
 {
-	if (p->inhibit_announce) {
-		return 0;
-	}
-
 	return set_tmo_random(p->fda.fd[FD_ANNOUNCE_TIMER],
 			      p->announceReceiptTimeout,
 			      p->announce_span, p->logAnnounceInterval);
@@ -1632,6 +1632,9 @@ int port_initialize(struct port *p)
 
 	/* No need to open rtnl socket on UDS port. */
 	if (transport_type(p->trp) != TRANS_UDS) {
+		if (p->bmca == BMCA_NOOP) {
+			port_set_delay_tmo(p);
+		}
 		if (p->fda.fd[FD_RTNL] == -1)
 			p->fda.fd[FD_RTNL] = rtnl_open();
 		if (p->fda.fd[FD_RTNL] >= 0)
@@ -2479,9 +2482,14 @@ static enum fsm_event bc_event(struct port *p, int fd_index)
 	case FD_SYNC_RX_TIMER:
 		pr_debug("port %hu: %s timeout", portnum(p),
 			 fd_index == FD_SYNC_RX_TIMER ? "rx sync" : "announce");
-		if (p->best)
+		if (p->best) {
 			fc_clear(p->best);
-		port_set_announce_tmo(p);
+		}
+		if (p->inhibit_announce) {
+			port_clr_tmo(p->fda.fd[FD_ANNOUNCE_TIMER]);
+		} else {
+			port_set_announce_tmo(p);
+		}
 		delay_req_prune(p);
 		if (clock_slave_only(p->clock) && p->delayMechanism != DM_P2P &&
 		    port_renew_transport(p)) {

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Patel, V. <ved...@in...>]

Hi Richard,

On Tue, 2018-09-18 at 06:06 -0700, Richard Cochran wrote:
> On Thu, Sep 13, 2018 at 08:35:22PM -0700, Richard Cochran wrote:
> > 
> > Yes, I'll get to it as soon as I can.  Overall I'm happy with this
> > series, but I wanted to sit down and think through the automatic
> > switch to MASTER state that we discussed.
> Okay, I took your series and hacked the following diff on top of it.
> The main difference is that my version doesn't touch clock_poll().
> 
> Also, there is no need for the master mode to go into the listening
> state.
> 
Is there any specific reason you want to transition to the LISTENING
state first for the slave mode? We are initializing the delay timer in
port_initialize() and the announce timer is also initialized when the
device transitions to PS_SLAVE. So, there is nothing to do in slave
mode.

If we do remove the PS_LISTENING transition, We don't have any need for
the following code as well.
> diff --git a/configs/automotive-slave.cfg b/configs/automotive-
> slave.cfg
> index 9cabe7a..66b6141 100644
> --- a/configs/automotive-slave.cfg
> +++ b/configs/automotive-slave.cfg
> @@ -28,3 +28,5 @@ BMCA			noop
>  slaveOnly		1
>  inhibit_announce	1
>  ignore_source_id	1
> +announceReceiptTimeout	2
> +logAnnounceInterval	-20

Thanks,
Vedang

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Patel, V. <ved...@in...>]

Thanks Richard for the response. I will work on this and send you a
patch soon. Just one question below.

On Tue, 2018-09-18 at 06:06 -0700, Richard Cochran wrote:
> diff --git a/designated_fsm.c b/designated_fsm.c
> index a56f962..9ccf4c9 100644
> --- a/designated_fsm.c
> +++ b/designated_fsm.c
> @@ -113,14 +107,6 @@ enum port_state designated_slave_fsm(enum
> port_state state,
>  		case EV_FAULT_DETECTED:
>  			next = PS_FAULTY;
>  			break;
> -		/*
> -		 * Go to PS_UNCALIBRATED when there is a loss of
> sync messages.
> -		 * This will also be useful in changing the Sync
> Interval (to
> -		 * be implemented).
> -		 */
> -		case EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES:
> -			next = PS_UNCALIBRATED;
> -			break;
Is there any particular reason you removed this? This is needed to
process the FD_SYNC_RX_TIMER expiration when the master disconnects
unexpectedly.

Thanks,
Vedang
>  		default:
>  			break;
>  		}

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Richard C. <ric...@gm...>]

On Tue, Sep 18, 2018 at 11:21:55PM +0000, Patel, Vedang wrote:
> Is there any particular reason you removed this? This is needed to
> process the FD_SYNC_RX_TIMER expiration when the master disconnects
> unexpectedly.

AFAICT there is no point in handling that timer in any special way.
After all, there is nothing for the slave to do.

Thanks,
Richard

Re: [Linuxptp-devel] [PATCH v1 0/6] Adding support for Automotive Profile

[Patel, V. <ved...@in...>]

On Tue, 2018-09-18 at 18:07 -0700, Richard Cochran wrote:
> On Tue, Sep 18, 2018 at 11:21:55PM +0000, Patel, Vedang wrote:
> > 
> > Is there any particular reason you removed this? This is needed to
> > process the FD_SYNC_RX_TIMER expiration when the master disconnects
> > unexpectedly.
> AFAICT there is no point in handling that timer in any special way.
> After all, there is nothing for the slave to do.
> 
Yeah there is no action required. 

Since, we are not processing this event, poll() always returns this
event resulting in following being continuously printed:

ptp4l[182621.542]: selected local clock 001b21.fffe.decf15 as best
master

An easy way to reproduce this is to start ptp4l with automotive profile
on 2 systems and then killing off the master.

Following diff fixes the issue. The issue here is, we will be clearing
it out twice when we are other profiles. Thoughts?

Thanks,
Vedang
--
diff --git a/port.c b/port.c
index 2ee463ddbf7e..a88b0d26f363 100644
--- a/port.c
+++ b/port.c
@@ -2485,6 +2485,7 @@ static enum fsm_event bc_event(struct port *p,
int fd_index)
                if (p->best) {
                        fc_clear(p->best);
                }
+               port_clr_tmo(p->fda.fd[FD_SYNC_RX_TIMER]);
                if (p->inhibit_announce) {
                        port_clr_tmo(p->fda.fd[FD_ANNOUNCE_TIMER]);
                } else {
> Thanks,
> Richard

[Linuxptp-devel] [RFC v2 2/5] Add BMCA config option.

[Vedang P. <ved...@in...>]

This adds a config option to specify static roles for master and slave
in the Best Master Clock Algorithm. This is the case for AVnu
Automotive Profile [1] where networks are mostly static and role for
each device is known in advance.

masterOnly and slaveOnly will be used to determine the roles for the
devices. Since masterOnly is a per-port config and slaveOnly is a global
config option, role assignment will be slightly odd in case of bridges.
If slaveOnly is set to 1, all the ports will be in slave roles except
for the ones where masterOnly is set to 1. These ports will assume the
master role.

Two new FSMs which will be used for master and slave roles for this
config option have also been added.

This commit adds functionality equivalent to the 'isGM' variable defined
in Section 6.2.1.1 of the AVnu Automotive Profile[1].

[1] - http://avnu.org/wp-content/uploads/2014/05/
      Automotive-Ethernet-AVB-Func-Interop-Spec-v1.5-Public.pdf

Signed-off-by: Vedang Patel <ved...@in...>
Change-Id: I85ed0484ffed2f3136f333c03611f1983099ce7b
---
 bmc.c               |  11 ++++++
 config.c            |   7 ++++
 configs/default.cfg |   1 +
 fsm.c               | 100 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 fsm.h               |  26 ++++++++++++++
 port.c              |  38 ++++++++++++++++++--
 port.h              |   8 +++++
 port_private.h      |   1 +
 ptp4l.8             |  17 ++++++---
 9 files changed, 203 insertions(+), 6 deletions(-)

diff --git a/bmc.c b/bmc.c
index 3c3db828764b..b0a80c2198cb 100644
--- a/bmc.c
+++ b/bmc.c
@@ -126,6 +126,17 @@ enum port_state bmc_state_decision(struct clock *c, struct port *r,
 	port_best = port_best_foreign(r);
 	ps = port_state(r);
 
+	/*
+	 * This scenario is particularly important in the designated_slave_fsm
+	 * when it is in PS_UNCALIBRATED state. In this scenario, there is no
+	 * other foriegn master and it will elect itself as master ultimately
+	 * resulting in printing out some unnecessary warnings (see
+	 * port_slave_priority_warning()).
+	 */
+	if (!port_best && port_bmca(r) == BMCA_NOOP) {
+		return ps;
+	}
+
 	if (!port_best && PS_LISTENING == ps)
 		return ps;
 
diff --git a/config.c b/config.c
index 7914ba4b5166..47cad7ab6632 100644
--- a/config.c
+++ b/config.c
@@ -188,10 +188,17 @@ static struct config_enum tsproc_enu[] = {
 	{ NULL, 0 },
 };
 
+static struct config_enum bmca_enu[] = {
+	{ "ptp",  BMCA_PTP  },
+	{ "noop", BMCA_NOOP },
+	{ NULL, 0 },
+};
+
 struct config_item config_tab[] = {
 	PORT_ITEM_INT("announceReceiptTimeout", 3, 2, UINT8_MAX),
 	GLOB_ITEM_INT("assume_two_step", 0, 0, 1),
 	PORT_ITEM_INT("boundary_clock_jbod", 0, 0, 1),
+	PORT_ITEM_ENU("BMCA", BMCA_PTP, bmca_enu),
 	GLOB_ITEM_INT("check_fup_sync", 0, 0, 1),
 	GLOB_ITEM_INT("clockAccuracy", 0xfe, 0, UINT8_MAX),
 	GLOB_ITEM_INT("clockClass", 248, 0, UINT8_MAX),
diff --git a/configs/default.cfg b/configs/default.cfg
index c5a8b57c9314..02714150fbab 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -31,6 +31,7 @@ fault_reset_interval	4
 neighborPropDelayThresh	20000000
 masterOnly		0
 G.8275.portDS.localPriority	128
+BMCA                    ptp
 #
 # Run time options
 #
diff --git a/fsm.c b/fsm.c
index ce6efad30937..a0c98891e39f 100644
--- a/fsm.c
+++ b/fsm.c
@@ -335,3 +335,103 @@ enum port_state ptp_slave_fsm(enum port_state state, enum fsm_event event,
 
 	return next;
 }
+
+enum port_state designated_master_fsm(enum port_state state,
+				      enum fsm_event event,
+				      int mdiff)
+{
+	enum port_state next = state;
+
+	if (EV_INITIALIZE == event || EV_POWERUP == event)
+		return PS_INITIALIZING;
+
+	switch (state) {
+	case PS_INITIALIZING:
+		switch (event) {
+		case EV_FAULT_DETECTED:
+			next = PS_FAULTY;
+			break;
+		case EV_INIT_COMPLETE:
+			next =  PS_MASTER;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	case PS_FAULTY:
+		if (event == EV_FAULT_CLEARED) {
+			next = PS_INITIALIZING;
+		}
+		break;
+
+	case PS_MASTER:
+		if (event == EV_FAULT_DETECTED) {
+			next = PS_FAULTY;
+		}
+		break;
+
+	default:
+		break;
+	}
+	return next;
+}
+
+enum port_state designated_slave_fsm(enum port_state state,
+				     enum fsm_event event,
+				     int mdiff)
+{
+	enum port_state next = state;
+
+	if (EV_INITIALIZE == event || EV_POWERUP == event)
+		return PS_INITIALIZING;
+
+	switch (state) {
+	case PS_INITIALIZING:
+		switch (event) {
+		case EV_FAULT_DETECTED:
+			next = PS_FAULTY;
+			break;
+		case EV_INIT_COMPLETE:
+			next =  PS_SLAVE;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	case PS_FAULTY:
+		if (event == EV_FAULT_CLEARED) {
+			next = PS_INITIALIZING;
+		}
+		break;
+
+	case PS_UNCALIBRATED:
+		if (event == EV_MASTER_CLOCK_SELECTED) {
+			next = PS_SLAVE;
+		}
+		break;
+
+	case PS_SLAVE:
+		switch (event) {
+		case EV_FAULT_DETECTED:
+			next = PS_FAULTY;
+			break;
+		/*
+		 * Go to PS_UNCALIBRATED when there is a loss of sync messages.
+		 * This will also be useful in changing the Sync Interval (to
+		 * be implemented).
+		 */
+		case EV_ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES:
+			next = PS_UNCALIBRATED;
+			break;
+		default:
+			break;
+		}
+		break;
+
+	default:
+		break;
+	}
+	return next;
+}
diff --git a/fsm.h b/fsm.h
index 0616daa2ab7b..f8a1cbf2d2bf 100644
--- a/fsm.h
+++ b/fsm.h
@@ -55,6 +55,11 @@ enum fsm_event {
 	EV_RS_PASSIVE,
 };
 
+enum bmca_select {
+	BMCA_PTP,
+	BMCA_NOOP,
+};
+
 /**
  * Run the state machine for a BC or OC port.
  * @param state  The current state of the port.
@@ -74,4 +79,25 @@ enum port_state ptp_fsm(enum port_state state, enum fsm_event event, int mdiff);
 enum port_state ptp_slave_fsm(enum port_state state, enum fsm_event event,
 			      int mdiff);
 
+/**
+ * Run the state machine for a clock which is designated as master port.
+ * @param state  The current state of the port.
+ * @param event  The event to be processed.
+ * @param mdiff  This param is not used by this function.
+ * @return       The new state for the port.
+ */
+enum port_state designated_master_fsm(enum port_state state,
+				      enum fsm_event event,
+				      int mdiff);
+
+/**
+ * Run the state machine for a clock designated as slave port.
+ * @param state  The current state of the port.
+ * @param event  The event to be processed.
+ * @param mdiff  This param is not used by this function.
+ * @return       The new state for the port.
+ */
+enum port_state designated_slave_fsm(enum port_state state,
+				     enum fsm_event event,
+				     int mdiff);
 #endif
diff --git a/port.c b/port.c
index 1a0e910a1151..6144b1a05de5 100644
--- a/port.c
+++ b/port.c
@@ -1593,7 +1593,6 @@ int port_initialize(struct port *p)
 	p->transportSpecific       = config_get_int(cfg, p->name, "transportSpecific");
 	p->transportSpecific     <<= 4;
 	p->match_transport_specific = !config_get_int(cfg, p->name, "ignore_transport_specific");
-	p->master_only             = config_get_int(cfg, p->name, "masterOnly");
 	p->localPriority           = config_get_int(cfg, p->name, "G.8275.portDS.localPriority");
 	p->logSyncInterval         = config_get_int(cfg, p->name, "logSyncInterval");
 	p->logMinPdelayReqInterval = config_get_int(cfg, p->name, "logMinPdelayReqInterval");
@@ -2339,6 +2338,14 @@ static void port_p2p_transition(struct port *p, enum port_state next)
 		break;
 	case PS_MASTER:
 	case PS_GRAND_MASTER:
+		/*
+		 * If BMCA is set to 'noop', we are skipping the PS_LISTENING
+		 * state and directly going to PS_MASTER. So, set the delay
+		 * timeout here.
+		 */
+		if (p->bmca == BMCA_NOOP) {
+			port_set_delay_tmo(p);
+		}
 		set_tmo_log(p->fda.fd[FD_MANNO_TIMER], 1, -10); /*~1ms*/
 		port_set_sync_tx_tmo(p);
 		break;
@@ -2350,6 +2357,14 @@ static void port_p2p_transition(struct port *p, enum port_state next)
 		flush_peer_delay(p);
 		/* fall through */
 	case PS_SLAVE:
+		/*
+		 * If BMCA is set to 'noop', we are skipping the PS_LISTENING
+		 * state and directly going to PS_SLAVE. So, set the delay
+		 * timeout here.
+		 */
+		if (p->bmca == BMCA_NOOP) {
+			port_set_delay_tmo(p);
+		}
 		port_set_announce_tmo(p);
 		break;
 	};
@@ -2853,10 +2868,24 @@ struct port *port_open(int phc_index,
 		goto err_port;
 	}
 
-	p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : ptp_fsm;
 	p->phc_index = phc_index;
 	p->jbod = config_get_int(cfg, interface->name, "boundary_clock_jbod");
 	transport = config_get_int(cfg, interface->name, "network_transport");
+	p->master_only = config_get_int(cfg, p->name, "masterOnly");
+	p->bmca = config_get_int(cfg, p->name, "BMCA");
+
+	if (p->bmca == BMCA_NOOP && transport != TRANS_UDS) {
+		if (p->master_only) {
+			p->state_machine = designated_master_fsm;
+		} else if (clock_slave_only(clock)) {
+			p->state_machine = designated_slave_fsm;
+		} else {
+			pr_err("Please enable atleast one of masterOnly or slaveOnly when BMCA == noop.\n");
+			goto err_port;
+		}
+	} else {
+		p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : ptp_fsm;
+	}
 
 	if (transport == TRANS_UDS) {
 		; /* UDS cannot have a PHC. */
@@ -3014,3 +3043,8 @@ int port_state_update(struct port *p, enum fsm_event event, int mdiff)
 
 	return 0;
 }
+
+enum bmca_select port_bmca(struct port *p)
+{
+	return p->bmca;
+}
diff --git a/port.h b/port.h
index 9639193d23d1..aa3b1ecf2529 100644
--- a/port.h
+++ b/port.h
@@ -323,6 +323,14 @@ void fault_interval(struct port *port, enum fault_type ft,
 		    struct fault_interval *i);
 
 /**
+ * Obtain the BMCA type of the port.
+ *
+ * @param port        A port instance.
+ * @return            bmca type.
+ */
+enum bmca_select port_bmca(struct port *p);
+
+/**
  * Release all of the memory in the TC transmit descriptor cache.
  */
 void tc_cleanup(void);
diff --git a/port_private.h b/port_private.h
index 19d1d7beaae8..8b5525a2122a 100644
--- a/port_private.h
+++ b/port_private.h
@@ -96,6 +96,7 @@ struct port {
 	unsigned int multiple_pdr_detected;
 	enum port_state (*state_machine)(enum port_state state,
 					 enum fsm_event event, int mdiff);
+	int bmca;
 	/* portDS */
 	struct PortIdentity portIdentity;
 	enum port_state     state; /*portState*/
diff --git a/ptp4l.8 b/ptp4l.8
index 10c5c2f967cb..8c19d1d276ee 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -363,10 +363,7 @@ hardware time stamping.
 The default is 1 (enabled).
 .TP
 .B slaveOnly
-The local clock is a slave-only clock if enabled.
-This option is only for use with 1588 clocks and should not be enabled
-for 802.1AS clocks.
-The default is 0 (disabled).
+The local clock is a slave-only clock if enabled. The default is 0 (disabled).
 .TP
 .B gmCapable
 If this option is enabled, then the local clock is able to become grand master.
@@ -661,6 +658,18 @@ The time source is a single byte code that gives an idea of the kind
 of local clock in use. The value is purely informational, having no
 effect on the outcome of the Best Master Clock algorithm, and is
 advertised when the clock becomes grand master.
+.TP
+.B BMCA
+This option enables use of static roles for master and slave devices instead of
+running the best master clock algorithm (BMCA) described in 1588 profile. This
+is useful when you know the roles of the devices in advance. When set to
+\'noop', the traditional BMCA algorithm used by 1588 is skipped. masterOnly and
+slaveOnly will be used to determine master or slave role for the device. In a
+bridge, slaveOnly (which is a global option) can be set to make all ports
+assume the slave role. masterOnly (which is a per-port config option) can then
+be used to set individual ports to take master role. BMCA is used in the AVnu
+Automotive profile to speed up the start time for grand master and slaves. The
+default value is 'ptp' which runs the BMCA related state machines.
 
 .SH UNICAST DISCOVERY OPTIONS
 
-- 
2.7.3

[Linuxptp-devel] [RFC v2 4/5] Add ignore_source_id config option.

[Vedang P. <ved...@in...>]

This config option will skip the source port identity verification in
the Sync and Follow_up messages. This option is needed when the announce
messages are disabled because the slave cannot know the identity of
master without announce messages.

This is required by AVnu Automotive Profile[1] as described in Section
6.3 pt #2 (lines 196 to 199).

[1] - http://avnu.org/wp-content/uploads/2014/05/
      Automotive-Ethernet-AVB-Func-Interop-Spec-v1.5-Public.pdf

Signed-off-by: Vedang Patel <ved...@in...>
Change-Id: Iae5e0c1d8946499e05c498792ece1bd40b33cee0
---
 config.c            |  1 +
 configs/default.cfg |  1 +
 port.c              | 24 ++++++++++++++++++------
 port_private.h      |  1 +
 ptp4l.8             |  6 ++++++
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/config.c b/config.c
index 702ee1b4e52a..1e7734bc8607 100644
--- a/config.c
+++ b/config.c
@@ -223,6 +223,7 @@ struct config_item config_tab[] = {
 	PORT_ITEM_INT("G.8275.portDS.localPriority", 128, 1, UINT8_MAX),
 	GLOB_ITEM_INT("gmCapable", 1, 0, 1),
 	PORT_ITEM_INT("hybrid_e2e", 0, 0, 1),
+	PORT_ITEM_INT("ignore_source_id", 0, 0, 1),
 	PORT_ITEM_INT("ignore_transport_specific", 0, 0, 1),
 	PORT_ITEM_INT("ingressLatency", 0, INT_MIN, INT_MAX),
 	PORT_ITEM_INT("inhibit_announce", 0, 0, 1),
diff --git a/configs/default.cfg b/configs/default.cfg
index 56dfbe4aace1..3f18830f8d9f 100644
--- a/configs/default.cfg
+++ b/configs/default.cfg
@@ -33,6 +33,7 @@ masterOnly		0
 G.8275.portDS.localPriority	128
 BMCA                    ptp
 inhibit_announce        0
+ignore_source_id        0
 #
 # Run time options
 #
diff --git a/port.c b/port.c
index dacc8d383607..897da704aad4 100644
--- a/port.c
+++ b/port.c
@@ -1593,6 +1593,7 @@ int port_initialize(struct port *p)
 	p->peerMeanPathDelay       = 0;
 	p->logAnnounceInterval     = config_get_int(cfg, p->name, "logAnnounceInterval");
 	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");
 	p->syncReceiptTimeout      = config_get_int(cfg, p->name, "syncReceiptTimeout");
 	p->transportSpecific       = config_get_int(cfg, p->name, "transportSpecific");
@@ -1866,10 +1867,22 @@ void process_delay_resp(struct port *p, struct ptp_message *m)
 	port_set_delay_tmo(p);
 }
 
+static int check_source_identity(struct port *p, struct ptp_message *m)
+{
+	struct PortIdentity master;
+
+	if (!p->ignore_source_id) {
+		master = clock_parent_identity(p->clock);
+		if (!pid_eq(&master, &m->header.sourcePortIdentity)) {
+			return -1;
+		}
+	}
+	return 0;
+}
+
 void process_follow_up(struct port *p, struct ptp_message *m)
 {
 	enum syfu_event event;
-	struct PortIdentity master;
 	switch (p->state) {
 	case PS_INITIALIZING:
 	case PS_FAULTY:
@@ -1884,8 +1897,8 @@ void process_follow_up(struct port *p, struct ptp_message *m)
 	case PS_SLAVE:
 		break;
 	}
-	master = clock_parent_identity(p->clock);
-	if (!pid_eq(&master, &m->header.sourcePortIdentity)) {
+
+	if (check_source_identity(p, m)) {
 		return;
 	}
 
@@ -2174,7 +2187,6 @@ void process_pdelay_resp_fup(struct port *p, struct ptp_message *m)
 void process_sync(struct port *p, struct ptp_message *m)
 {
 	enum syfu_event event;
-	struct PortIdentity master;
 	switch (p->state) {
 	case PS_INITIALIZING:
 	case PS_FAULTY:
@@ -2189,8 +2201,8 @@ void process_sync(struct port *p, struct ptp_message *m)
 	case PS_SLAVE:
 		break;
 	}
-	master = clock_parent_identity(p->clock);
-	if (!pid_eq(&master, &m->header.sourcePortIdentity)) {
+
+	if (check_source_identity(p, m)) {
 		return;
 	}
 
diff --git a/port_private.h b/port_private.h
index b454901f208d..55ce791f8494 100644
--- a/port_private.h
+++ b/port_private.h
@@ -98,6 +98,7 @@ struct port {
 					 enum fsm_event event, int mdiff);
 	int bmca;
 	int inhibit_announce;
+	int ignore_source_id;
 	/* portDS */
 	struct PortIdentity portIdentity;
 	enum port_state     state; /*portState*/
diff --git a/ptp4l.8 b/ptp4l.8
index 24ba45342460..2adcbca1ac7d 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -678,6 +678,12 @@ by the AVnu Automotive profile as part of switching over to a static BMCA. if
 this option is enabled, ignore_source_id has to be enabled in the slave because
 it has no way to identify master identity in Sync and Follow_Up messages. The
 default is 0 (disabled).
+.TP
+.B ignore_source_port_id
+This will disable source port identity checking for Sync and Follow_Up
+messages. This is useful when the announce messages are disabled in the master
+and the slave does not have any way to know it's identity. The default is 0
+(disabled).
 
 .SH UNICAST DISCOVERY OPTIONS
 
-- 
2.7.3

[Linuxptp-devel] [RFC v2 5/5] Add example configuration for AVnu Automotive Profile.

[Vedang P. <ved...@in...>]

This commit adds example for configuring master and slave devices that
support features from the AVnu Automotive Profile[1].

[1] - http://avnu.org/wp-content/uploads/2014/05/
      Automotive-Ethernet-AVB-Func-Interop-Spec-v1.5-Public.pdf

Signed-off-by: Vedang Patel <ved...@in...>
Change-Id: I2e0deb3518108c5b3188ae1d3ddd0099ff42666a
---
 configs/AVnu-master.cfg | 27 +++++++++++++++++++++++++++
 configs/AVnu-slave.cfg  | 30 ++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)
 create mode 100644 configs/AVnu-master.cfg
 create mode 100644 configs/AVnu-slave.cfg

diff --git a/configs/AVnu-master.cfg b/configs/AVnu-master.cfg
new file mode 100644
index 000000000000..8b0c4fe16331
--- /dev/null
+++ b/configs/AVnu-master.cfg
@@ -0,0 +1,27 @@
+#
+# AVnu Automotive Profile example configuration for master containing those
+# attributes which differ from the defaults.  See the file, default.cfg, for
+# the complete list of available options.
+#
+[global]
+# Options carried over from gPTP.
+gmCapable		1
+priority1		248
+priority2		248
+logSyncInterval		-3
+syncReceiptTimeout	3
+neighborPropDelayThresh	800
+min_neighbor_prop_delay	-20000000
+assume_two_step		1
+path_trace_enabled	1
+follow_up_info		1
+transportSpecific	0x1
+ptp_dst_mac		01:80:C2:00:00:0E
+network_transport	L2
+delay_mechanism		P2P
+#
+# AVnu AP specific options
+#
+BMCA			noop
+masterOnly		1
+inhibit_announce	1
diff --git a/configs/AVnu-slave.cfg b/configs/AVnu-slave.cfg
new file mode 100644
index 000000000000..7790c73915f1
--- /dev/null
+++ b/configs/AVnu-slave.cfg
@@ -0,0 +1,30 @@
+#
+# AVnu Automotive Profile example configuration for slaves containing those
+# attributes which differ from the defaults.  See the file, default.cfg, for
+# the complete list of available options.
+#
+[global]
+#
+# Options carried over from gPTP.
+#
+gmCapable		1
+priority1		248
+priority2		248
+logSyncInterval		-3
+syncReceiptTimeout	3
+neighborPropDelayThresh	800
+min_neighbor_prop_delay	-20000000
+assume_two_step		1
+path_trace_enabled	1
+follow_up_info		1
+transportSpecific	0x1
+ptp_dst_mac		01:80:C2:00:00:0E
+network_transport	L2
+delay_mechanism		P2P
+#
+# AVnu AP specific options
+#
+BMCA			noop
+slaveOnly		1
+inhibit_announce	1
+ignore_source_id	1
-- 
2.7.3