Re: [Linuxptp-devel] Adding peer-to-peer transparent clock

[Keller, Jacob E]

> -Original Message-
> From: Richard Cochran [mailto:richardcoch...@gmail.com]
> Sent: Friday, August 04, 2017 12:19 PM
> To: Keller, Jacob E 
> Cc: Rodney Greenstreet ; linuxptp-
> de...@lists.sourceforge.net
> Subject: Re: [Linuxptp-devel] Adding peer-to-peer transparent clock
> 
> On Fri, Aug 04, 2017 at 04:20:15PM +, Keller, Jacob E wrote:
> > I haven't had time to review the actual implementation in detail, but I did 
> > have
> a few nits, and the commit subjects should probably be reworded a bit. Thanks
> for the effort in bringing these patches forward though!
> 
> Let's hold off reviewing these until I can do a proper rebase of the
> original series.
> 
> Thanks,
> Richard

Sounds good!

Thanks,
Jake

--
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

Re: [Linuxptp-devel] Adding peer-to-peer transparent clock

[Richard Cochran]

On Fri, Aug 04, 2017 at 04:20:15PM +, Keller, Jacob E wrote:
> I haven't had time to review the actual implementation in detail, but I did 
> have a few nits, and the commit subjects should probably be reworded a bit. 
> Thanks for the effort in bringing these patches forward though!

Let's hold off reviewing these until I can do a proper rebase of the
original series.

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

Re: [Linuxptp-devel] [PATCH 0/6] Adding peer-to-peer transparent clock

[Richard Cochran]

On Thu, Aug 03, 2017 at 09:44:13PM +, Rodney Greenstreet wrote:
> The following patches originated from patches authored by Richard Cocharan.
> Unfortunately, I was unsuccessful in applying the original set of patches to
> the latest master branch. As a result, I generated a new set of patches after
> manually applying Richard’s patches.

I don't like how the original patches got mangled here.

I am going to make this a bit easier for you and rebase the original
series onto the current git head.  Then, you can concentrate on
expanding on them.

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

Re: [Linuxptp-devel] Adding peer-to-peer transparent clock

[Keller, Jacob E]

> -Original Message-
> From: Rodney Greenstreet [mailto:rodney.greenstr...@ni.com]
> Sent: Thursday, August 03, 2017 2:22 PM
> To: linuxptp-devel@lists.sourceforge.net
> Subject: [Linuxptp-devel] Adding peer-to-peer transparent clock
> 
> The following set of patches originated from patches authored by Richard
> Cocharan. Unfortunately, I was unsuccessful in applying the original set of
> patches to the latest master branch. As a result, I generated a new set of
> patches after manually applied Richards patches.
> 

I haven't had time to review the actual implementation in detail, but I did 
have a few nits, and the commit subjects should probably be reworded a bit. 
Thanks for the effort in bringing these patches forward though!

Thanks,
Jake

> 
> --
> 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

--
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

Re: [Linuxptp-devel] [PATCH 3/6] This patch places the internal port data structure into a common header for use by the original BC and the new TC code.

[Keller, Jacob E]

> -Original Message-
> From: Rodney Greenstreet [mailto:rodney.greenstr...@ni.com]
> Sent: Thursday, August 03, 2017 2:22 PM
> To: linuxptp-devel@lists.sourceforge.net
> Cc: Rodney Greenstreet 
> Subject: [Linuxptp-devel] [PATCH 3/6] This patch places the internal port data
> structure into a common header for use by the original BC and the new TC code.
> 
> Signed-off-by: Richard Cochran 
> 
> port_private: Updated return type of variable dispatch routine to be void.
> 

I'd change the title, since the subject is quite long.

> Signed-off-by: Rodney Greenstreet 
> ---
>  port.c |  86 +--
>  port_private.h | 113
> +
>  2 files changed, 114 insertions(+), 85 deletions(-)
>  create mode 100644 port_private.h
> 
> diff --git a/port.c b/port.c
> index c8c4424..c1d2f04 100644
> --- a/port.c
> +++ b/port.c
> @@ -32,6 +32,7 @@
>  #include "msg.h"
>  #include "phc.h"
>  #include "port.h"
> +#include "port_private.h"
>  #include "print.h"
>  #include "rtnl.h"
>  #include "sk.h"
> @@ -43,12 +44,6 @@
>  #define ALLOWED_LOST_RESPONSES 3
>  #define ANNOUNCE_SPAN 1
> 
> -enum syfu_state {
> - SF_EMPTY,
> - SF_HAVE_SYNC,
> - SF_HAVE_FUP,
> -};
> -
>  enum syfu_event {
>   SYNC_MISMATCH,
>   SYNC_MATCH,
> @@ -56,87 +51,8 @@ enum syfu_event {
>   FUP_MATCH,
>  };
> 
> -struct nrate_estimator {
> - double ratio;
> - tmv_t origin1;
> - tmv_t ingress1;
> - unsigned int max_count;
> - unsigned int count;
> - int ratio_valid;
> -};
> -
> -struct port {
> - LIST_ENTRY(port) list;
> - char *name;
> - struct clock *clock;
> - struct transport *trp;
> - enum timestamp_type timestamping;
> - struct fdarray fda;
> - int fault_fd;
> - int phc_index;
> -
> - void (*dispatch)(struct port *p, enum fsm_event event, int mdiff);
> - enum fsm_event (*event)(struct port *p, int fd_index);
> -
> - int jbod;
> - struct foreign_clock *best;
> - enum syfu_state syfu;
> - struct ptp_message *last_syncfup;
> - struct ptp_message *delay_req;
> - struct ptp_message *peer_delay_req;
> - struct ptp_message *peer_delay_resp;
> - struct ptp_message *peer_delay_fup;
> - int peer_portid_valid;
> - struct PortIdentity peer_portid;
> - struct {
> - UInteger16 announce;
> - UInteger16 delayreq;
> - UInteger16 sync;
> - } seqnum;
> - tmv_t peer_delay;
> - struct tsproc *tsproc;
> - int log_sync_interval;
> - struct nrate_estimator nrate;
> - unsigned int pdr_missing;
> - unsigned int multiple_seq_pdr_count;
> - unsigned int multiple_pdr_detected;
> - enum port_state (*state_machine)(enum port_state state,
> -  enum fsm_event event, int mdiff);
> - /* portDS */
> - struct PortIdentity portIdentity;
> - enum port_state state; /*portState*/
> - Integer64   asymmetry;
> - int asCapable;
> - Integer8logMinDelayReqInterval;
> - TimeIntervalpeerMeanPathDelay;
> - Integer8logAnnounceInterval;
> - UInteger8   announceReceiptTimeout;
> - int announce_span;
> - UInteger8   syncReceiptTimeout;
> - UInteger8   transportSpecific;
> - Integer8logSyncInterval;
> - Enumeration8delayMechanism;
> - Integer8logMinPdelayReqInterval;
> - UInteger32  neighborPropDelayThresh;
> - int follow_up_info;
> - int freq_est_interval;
> - int hybrid_e2e;
> - int min_neighbor_prop_delay;
> - int path_trace_enabled;
> - int rx_timestamp_offset;
> - int tx_timestamp_offset;
> - int link_status;
> - struct fault_interval flt_interval_pertype[FT_CNT];
> - enum fault_type last_fault_type;
> - unsigned intversionNumber; /*UInteger4*/
> - /* foreignMasterDS */
> - LIST_HEAD(fm, foreign_clock) foreign_masters;
> -};
> -
>  #define portnum(p) (p->portIdentity.portNumber)
> 
> -#define NSEC2SEC 10LL
> -
>  static int port_capable(struct port *p);
>  static int port_is_ieee8021as(struct port *p);
>  static void port_nrate_initialize(struct port *p);
> diff --git a/port_private.h b/port_private.h
> new file mode 100644
> index 000..8815bd4
> --- /dev/null
> +++ b/port_private.h
> @@ -0,0 +1,113 @@
> +/**
> + * @file port_private.h
> + * @note Copyright (C) 2015 Richard Cochran 
> + *
> + * 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 

Re: [Linuxptp-devel] [PATCH 1/6] This patch adds a new flag '-t' that enables transparent clock mode. When active, the clock will be created as an E2E or P2P TC, depending on the configured delay mech

[Keller, Jacob E]

> -Original Message-
> From: Rodney Greenstreet [mailto:rodney.greenstr...@ni.com]
> Sent: Thursday, August 03, 2017 2:22 PM
> To: linuxptp-devel@lists.sourceforge.net
> Cc: Rodney Greenstreet 
> Subject: [Linuxptp-devel] [PATCH 1/6] This patch adds a new flag '-t' that 
> enables
> transparent clock mode. When active, the clock will be created as an E2E or 
> P2P
> TC, depending on the configured delay mechanism.
> 
> Signed-off-by: Richard Cochran 
> ---
>  ptp4l.c | 39 +++
>  1 file changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/ptp4l.c b/ptp4l.c
> index f01ff6f..8a1dd4b 100644
> --- a/ptp4l.c
> +++ b/ptp4l.c
> @@ -1,6 +1,6 @@
>  /**
>   * @file ptp4l.c
> - * @brief PTP Boundary Clock main program
> + * @brief PTP Boundary Clock or Transparent Clock main program
>   * @note Copyright (C) 2011 Richard Cochran 
>   *
>   * This program is free software; you can redistribute it and/or modify
> @@ -61,6 +61,7 @@ static void usage(char *progname)
>   " -p [dev]  PTP hardware clock device to use, default auto\n"
>   "   (ignored for SOFTWARE/LEGACY HW time stamping)\n"
>   " -sslave only mode (overrides configuration file)\n"
> + " -ttransparent clock\n"

This should have a configuration option. I'm not convinced it deserves a lower 
case letter, but it definitely should be a configure option.

>   " -l [num]  set the logging level to 'num'\n"
>   " -mprint messages to stdout\n"
>   " -qdo not print messages to the syslog\n"
> @@ -73,6 +74,7 @@ static void usage(char *progname)
>  int main(int argc, char *argv[])
>  {
>   char *config = NULL, *req_phc = NULL, *progname;
> + enum clock_type type = CLOCK_TYPE_ORDINARY;
>   int c, err = -1, index, print_level;
>   struct clock *clock = NULL;
>   struct option *opts;
> @@ -90,7 +92,7 @@ int main(int argc, char *argv[])
>   /* Process the command line arguments. */
>   progname = strrchr(argv[0], '/');
>   progname = progname ? 1+progname : argv[0];
> - while (EOF != (c = getopt_long(argc, argv, "AEP246HSLf:i:p:sl:mqvh",
> + while (EOF != (c = getopt_long(argc, argv, "AEP246HSLf:i:p:stl:mqvh",
>  opts, ))) {
>   switch (c) {
>   case 0:
> @@ -151,6 +153,9 @@ int main(int argc, char *argv[])
>   goto out;
>   }
>   break;
> + case 't':
> + type = CLOCK_TYPE_E2E;

This should probably be called: CLOCK_TYPE_TRANSPARENT.

> + break;
>   case 'l':
>   if (get_arg_val_i(c, optarg, _level,
> PRINT_LEVEL_MIN, PRINT_LEVEL_MAX))
> @@ -203,8 +208,34 @@ int main(int argc, char *argv[])
>   goto out;
>   }
> 
> - clock = clock_create(cfg->n_interfaces > 1 ? CLOCK_TYPE_BOUNDARY :
> -  CLOCK_TYPE_ORDINARY, cfg, req_phc);
> + switch (type) {
> + case CLOCK_TYPE_ORDINARY:
> + case CLOCK_TYPE_BOUNDARY:
> + if (cfg->n_interfaces > 1) {
> + type = CLOCK_TYPE_BOUNDARY;
> + }
> + break;
> + case CLOCK_TYPE_P2P:

I don't think CLOCK_TYPE_P2P and CLOCK_TYPE_E2E need to exist. Just use 
CLOCK_TYPE_TRANSPARENT. Anywhere that you check the CLOCK_TYPE_E2E, check for 
CLOCK_TYPE_TRANSPARENT and then check for whether it's DM_E2E or DM_P2P?

Hmm. If you must separate them this early, call them CLOCK_TYPE_TRANSPARENT_P2P 
and CLOCK_TYPE_TRANSPARENT_E2E

> + case CLOCK_TYPE_E2E:
> + if (cfg->n_interfaces < 2) {
> + fprintf(stderr, "TC needs at least two interfaces\n");
> + goto out;
> + }
> + switch (config_get_int(cfg, NULL, "delay_mechanism")) {
> + case DM_AUTO:
> + case DM_E2E:
> + type = CLOCK_TYPE_E2E;
> + break;
> + case DM_P2P:
> + type = CLOCK_TYPE_P2P;
> + break;
> + }
> + break;
> + case CLOCK_TYPE_MANAGEMENT:
> + goto out;
> + }
> +
> + clock = clock_create(type, cfg, req_phc);
>   if (!clock) {
>   fprintf(stderr, "failed to create a clock\n");
>   goto out;
> --
> 2.13.0
> 
> 
> --
> 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