On Sun, 2015-01-11 at 12:01 +0100, Richard Cochran wrote:
> Jacob,
>
> I am glad to see this patch series. With all of its command line
> options, having a configuration file for phc2sys would be a usability
> improvement.
>
> On Fri, Jan 09, 2015 at 04:16:28PM -0800, Jacob Keller wrote:
> > I didn't think the effort necessary to re-write the configuration such
> > that it was more generic was worth it. Thus, there is some level of code
> > duplication as phc2sys does not need the same configuration values as
> > ptp4l.
>
> If you have the time, I would welcome an overhaul of the config code.
> My feeling is that the config code is growing a bit ugly, and it
> certainly is smarter to clean it up before trying to expand it.
>
I can look at this though it will take a lot more real time since I have
less to dedicate to it. But I am interested in solving this.
> Some ideas for the config code:
>
> - Model the configuration file in memory explicitly.
>
> - Each program (ptp4l, phc2sys, and pmc, too) reads the file early on,
> and then queries the model for the parameters it needs.
>
> - Get rid of all of the pointers in 'struct config'. They are a bit
> hacky, what with points to random globals in the various modules.
>
> - Instead, each configuration parameter could look like this.
>
> struct config_item {
> char label[N];
> int flags; /* bit field of allowed sections */
> /* maybe ignore bits says, "was set on command line" */
> union {
> int i;
> double d;
> unsigned char *s;
> } value;
> };
>
What about section settings for things like the per-adapter stuff? This
is where it got really complicated.
Other question I have regarding this, is should we share options between
programs? I think this is ok, because in reality if you *need* differing
options for each program you can just use a separate file.
> - The modules which now have globals can query the configuration model
> directly during initialization. I am thinking of things like:
>
> $ grep extern *.h
> msg.h:extern int assume_two_step;
> ntpshm.h:extern int ntpshm_segment;
> pi.h:extern double configured_pi_kp;
> pi.h:extern double configured_pi_ki;
> pi.h:extern double configured_pi_kp_scale;
> pi.h:extern double configured_pi_kp_exponent;
> pi.h:extern double configured_pi_kp_norm_max;
> pi.h:extern double configured_pi_ki_scale;
> pi.h:extern double configured_pi_ki_exponent;
> pi.h:extern double configured_pi_ki_norm_max;
> raw.h:extern unsigned char ptp_dst_mac[];
> raw.h:extern unsigned char p2p_dst_mac[];
> servo.h:extern double servo_step_threshold;
> servo.h:extern double servo_first_step_threshold;
> servo.h:extern int servo_max_frequency;
> sk.h:extern int sk_tx_timeout;
> sk.h:extern int sk_check_fupsync;
> tlv.h:extern uint8_t ieee8021_id[3];
> udp6.h:extern unsigned char udp6_scope;
> uds.h:extern char uds_path[MAX_IFNAME_SIZE + 1];
>
> > The intention is to enable additional non-command line options for
> > phc2sys. We could even deprecate several current phc2sys options which
> > are no longer necessary (similar to ptp4l which has many config options
> > but few command line parameters).
>
> Yes.
>
> > In addition this opens the door to bring several settings from ptp4l
> > into phc2sys (like the PI servo settings).
>
> Yes, yes.
>
> Thanks,
> Richard
------------------------------------------------------------------------------
New Year. New Location. New Benefits. New Data Center in Ashburn, VA.
GigeNET is offering a free month of service with a new server in Ashburn.
Choose from 2 high performing configs, both with 100TB of bandwidth.
Higher redundancy.Lower latency.Increased capacity.Completely compliant.
www.gigenet.com
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
