Re: [Linuxptp-devel] [PATCH v3 3/4] [Interface Rate TLV] organization TLV support for interface rate
>
> IEEE 1588-2019 only define Integer64, but not UInteger64.
> Not sure why, but probably there is no UInteger64 in IEEE standard.
> Are you using UInteger64 for a standard parameter/variables?
>
[Devasish]: Yes this is not part of IEEE 1588. But it is part of ITU-T
G.8275. But this can be used for other profiles as well.
>From ITU-T G.8275.2 (03/2020) Table D.1
" interfaceBitPeriod (Uinteger64)
The period of 1-bit of the transmitting PTP timestamp interface, excluding
line encoding.
The value is encoded as an unsigned integer in units of attoseconds (10–18
s) to accommodate interface bit periods less than 1 ns."
> UInteger8 delay_response_counter;
> > UInteger8 delay_response_timeout;
> > + booliface_rate_tlv;
> Again, using bool in structure is unwise.
> Better use uint8_t and not UInteger8, so we now it is not standard type
> and with a comment saying it holds a boolean value.
>
> > struct PortStatsstats;
> > struct PortServiceStatsservice_stats;
> > /* foreignMasterDS */
>
[Devasish]: Will leave this to Richard for comments.
Thanks,
Devasish
On Fri, 9 Dec 2022 at 17:39, Geva, Erez wrote:
> On Wed, 2022-12-07 at 17:34 +0530, SyncMonk Technologies wrote:
> > adding interface rate TLV as defined by ITU-T G.8275.2 Annex D to
> > enable master to communicate PTP port interface rate to slave.
> >
> > Signed-off-by: Greg Armstrong
> > Signed-off-by: Leon Goldin
> > Signed-off-by: Devasish Dey
> > Signed-off-by: Vipin Sharma
> > ---
> > config.c | 1 +
> > pdt.h | 1 +
> > port.c | 1 +
> > port_private.h | 1 +
> > tlv.c | 1 +
> > tlv.h | 14 ++
> > 6 files changed, 19 insertions(+)
> >
> > diff --git a/config.c b/config.c
> > index 08e3346..2fa95fc 100644
> > --- a/config.c
> > +++ b/config.c
> > @@ -267,6 +267,7 @@ struct config_item config_tab[] = {
> > PORT_ITEM_INT("inhibit_delay_req", 0, 0, 1),
> > PORT_ITEM_INT("inhibit_multicast_service", 0, 0, 1),
> > GLOB_ITEM_INT("initial_delay", 0, 0, INT_MAX),
> > + PORT_ITEM_INT("interface_rate_tlv", 0, 0, 1),
> > GLOB_ITEM_INT("kernel_leap", 1, 0, 1),
> > GLOB_ITEM_STR("leapfile", NULL),
> > PORT_ITEM_INT("logAnnounceInterval", 1, INT8_MIN, INT8_MAX),
> > diff --git a/pdt.h b/pdt.h
> > index e46b218..1ad23d4 100644
> > --- a/pdt.h
> > +++ b/pdt.h
> > @@ -39,6 +39,7 @@ typedef uint16_t UInteger16;
> > typedef int32_t Integer32;
> > typedef uint32_t UInteger32;
> > typedef int64_t Integer64;
> > +typedef uint64_t UInteger64;
> IEEE 1588-2019 only define Integer64, but not UInteger64.
> Not sure why, but probably there is no UInteger64 in IEEE standard.
> Are you using UInteger64 for a standard parameter/variables?
>
> > typedef uint8_t Octet;
> >
> > #endif
> > diff --git a/port.c b/port.c
> > index 7fd50dd..85cfa4a 100644
> > --- a/port.c
> > +++ b/port.c
> > @@ -1854,6 +1854,7 @@ int port_initialize(struct port *p)
> > p->neighborPropDelayThresh = config_get_int(cfg, p->name,
> > "neighborPropDelayThresh");
> > p->min_neighbor_prop_delay = config_get_int(cfg, p->name,
> > "min_neighbor_prop_delay");
> > p->delay_response_timeout = config_get_int(cfg, p->name,
> > "delay_response_timeout");
> > + p->iface_rate_tlv = config_get_int(cfg, p->name,
> > "interface_rate_tlv");
> >
> > if (config_get_int(cfg, p->name, "asCapable") ==
> > AS_CAPABLE_TRUE) {
> > p->asCapable = ALWAYS_CAPABLE;
> > diff --git a/port_private.h b/port_private.h
> > index d27dceb..d6487eb 100644
> > --- a/port_private.h
> > +++ b/port_private.h
> > @@ -145,6 +145,7 @@ struct port {
> > UInteger8 versionNumber; /* UInteger4 */
> > UInteger8 delay_response_counter;
> > UInteger8 delay_response_timeout;
> > + booliface_rate_tlv;
> Again, using bool in structure is unwise.
> Better use uint8_t and not UInteger8, so we now it is not standard type
> and with a comment saying it holds a boolean value.
>
> > struct PortStatsstats;
> > struct PortServiceStatsservice_stats;
> > /* foreignMasterDS */
> > diff --git a/tlv.c b/tlv.c
> > index 1c13460..35bee4f 100644
> > --- a/tlv.c
> > +++ b/tlv.c
> > @@ -35,6 +35,7 @@
> > (tlv->length < sizeof(struct type) - sizeof(struct TLV))
> >
> > uint8_t ieee8021_id[3] = { IEEE_802_1_COMMITTEE };
> > +uint8_t itu_t_id[3] = { ITU_T_COMMITTEE };
> >
> > static TAILQ_HEAD(tlv_pool, tlv_extra) tlv_pool =
> > TAILQ_HEAD_INITIALIZER(tlv_pool);
> > diff --git a/tlv.h b/tlv.h
> > index 8966696..ec22e2f 100644
> > --- a/tlv.h
> > +++ b/tlv.h
> > @@ -395,6 +395,20 @@ struct tlv_extra {
> > };
> > };
> >
> > +/* Organizationally Unique Identifiers */
> > +#define ITU_T_COMMITTEE 0x00, 0x19, 0xA7
> > +extern uint8_t itu_t_id[3];
> > +
> > +struct
Re: [Linuxptp-devel] [PATCH v3 4/4] [Interface Rate TLV] adding delay asymmetry calculation
>
> > + /* Megabits per secon converted to attoseconds per bit. */
> > + return 1ULL/ iface->if_info.speed;
> Performing division in running is not a very good idea.
> It is better to perform the division when updating the speed and store
> it in if_info.
>
> Are you sure we need attoseconds (10^-18), we usually uses nanoseconds
> (10^-9). I would except a better resolution, but that much?
> Please explain your choosing.
>
> > +}
>
[Devasish]: The initial changes were with hardcoded values for the speed
to avoid the calculation.
Miroslav suggested this way to have clean code.
Yes the calculation is based on attoseconds and not nanoseconds This is as
per standard G.8275.2
>From G.8275.2 (06/2021)
"
interfaceBitPeriod (Uinteger64)
The period of 1-bit of the transmitting PTP timestamp interface, excluding
line encoding.
The value is encoded as an unsigned integer in units of attoseconds (10–18
s) to accommodate interface bit periods less than 1 ns."
Thanks,
Devasish.
On Fri, 9 Dec 2022 at 18:08, Geva, Erez wrote:
> On Wed, 2022-12-07 at 17:34 +0530, SyncMonk Technologies wrote:
> > Delay asymmetry calculation based on the PTP port interface speed of
> > master obtained from TLV and the slave interface rate obtained by
> > ethtool.
> >
> > v3: updating network/host byte order handling.
> > v1: initial commit
> >
> > Signed-off-by: Greg Armstrong
> > Signed-off-by: Leon Goldin
> > Signed-off-by: Devasish Dey
> > Signed-off-by: Vipin Sharma
> > ---
> > interface.c | 10 ++
> > interface.h | 7 +++
> > port_private.h| 1 +
> > port_signaling.c | 39 ---
> > ptp4l.8 | 7 +++
> > tlv.c | 29 +
> > unicast_service.c | 32
> > 7 files changed, 122 insertions(+), 3 deletions(-)
> >
> > diff --git a/interface.c b/interface.c
> > index 3157e8c..02d530e 100644
> > --- a/interface.c
> > +++ b/interface.c
> > @@ -94,3 +94,13 @@ int interface_get_vclock(struct interface *iface)
> > {
> > return iface->vclock;
> > }
> > +
> > +uint64_t interface_bitperiod(struct interface *iface)
> > +{
> > + if (!iface->if_info.valid)
> > + return 0;
> > +
> > + /* Megabits per secon converted to attoseconds per bit. */
> > + return 1ULL/ iface->if_info.speed;
> Performing division in running is not a very good idea.
> It is better to perform the division when updating the speed and store
> it in if_info.
>
> Are you sure we need attoseconds (10^-18), we usually uses nanoseconds
> (10^-9). I would except a better resolution, but that much?
> Please explain your choosing.
>
> > +}
> > +
> > diff --git a/interface.h b/interface.h
> > index f4b9545..7c9a6bd 100644
> > --- a/interface.h
> > +++ b/interface.h
> > @@ -113,4 +113,11 @@ void interface_set_vclock(struct interface
> > *iface, int vclock);
> > */
> > int interface_get_vclock(struct interface *iface);
> >
> > +/**
> > + * Obtains the interface bit period based on the speed.
> Perhaps: "Obtains bit period based on interface speed."
>
> > + * @param iface The interface of interest.
> We must be interesting "Pointer to the interface" can be suffiecnt :-)
>
> > + * @return if valid speed return interface bitperiod in atto
> > seconds.
> No need to make the return complicated, make it simple
> "return interface bit period in attoseconds".
> We know functions handle errors.
>
> > + */
> > +uint64_t interface_bitperiod(struct interface *iface);
> > +
> > #endif
> > diff --git a/port_private.h b/port_private.h
> > index d6487eb..6ad4af8 100644
> > --- a/port_private.h
> > +++ b/port_private.h
> > @@ -146,6 +146,7 @@ struct port {
> > UInteger8 delay_response_counter;
> > UInteger8 delay_response_timeout;
> > booliface_rate_tlv;
> > + Integer64 portAsymmetry;
> > struct PortStatsstats;
> > struct PortServiceStatsservice_stats;
> > /* foreignMasterDS */
> > diff --git a/port_signaling.c b/port_signaling.c
> > index ed217c0..75a0689 100644
> > --- a/port_signaling.c
> > +++ b/port_signaling.c
> > @@ -103,10 +103,37 @@ static int process_interval_request(struct port
> > *p,
> > return 0;
> > }
> >
> > +static int process_interface_rate(struct port *p,
> > + struct msg_interface_rate_tlv *r)
> > +{
> > + Integer64 delayAsymmetry;
> > + doublensDelay;
> > + Integer64 slaveBitPeriod;
> > + Integer64 masterBitPeriod;
> > +
> > + if (p->iface_rate_tlv && interface_ifinfo_valid(p->iface)) {
> > + slaveBitPeriod = interface_bitperiod(p->iface);
> > + masterBitPeriod = r->interfaceBitPeriod;
> > +
> > + /* Delay Asymmetry Calculation */
> > + nsDelay = (masterBitPeriod - slaveBitPeriod) / (2 *
> > 1.0e9);
> > +
Re: [Linuxptp-devel] [PATCH v3 1/4] [Interface Rate TLV] function to support get interface speed via ethtool
>
> > + goto failed;
> > + }
>
> I think it is better to merge this ioctl and the socket creation with
> sk_get_ts_info().
> No reason for duplication.
> You can use a static function or inline one.
>
> > +
> > + if (ecmd.req.link_mode_masks_nwords >= 0 ||
> > + ecmd.req.cmd != ETHTOOL_GLINKSETTINGS) {
> > + return 1;
> > + }
>
[Devasish]: This can be done.
> + /* clear data and ensure it is not marked valid */
> + memset(if_info, 0, sizeof(struct sk_if_info));
> + return -1;
You need to close the socket!
[Devasish]: Noted. Will be updated in the next patch.
> +struct sk_if_info {
> + bool valid;
It is better not to use bool in a structure, as the size is usually
int, i.e. 64 bits to hold 1 bit.
[Devasish]: Earlier we had it as an integer. We can update it to uint8_t as
well. But Richard suggested updating it to Boolean.
So, leaving this to Richard.
> + int speed;
What are the units here?
Old systems use 32 bits, if you measure in bits per second, the maximum
is 4 GB.
It is better to use uint64_t and use 1 mbps as units.
[Devasish]: It is the same as the speed in ethtool_link_settings which is a
uint32_t variable and it is in Mbps.
Will change it to uint32_t and update the comments.
Thanks,
Devasish.
On Fri, 9 Dec 2022 at 17:29, Geva, Erez wrote:
> On Wed, 2022-12-07 at 17:34 +0530, SyncMonk Technologies wrote:
> > When master and slave instance interacting with each other operating
> > at different interface speed, delay assymetry needs to be compensated
> > as described in G.8271 appendix V.
> >
> > In this patch we are adding changes to get the interface speed using
> > ethtool.
> >
> > v1: initial commit.
> > v2: updating comments and data types.
> > v3: updating Boolean data type to bool from
> > ---
> > sk.c | 71
> >
> > sk.h | 19
> > 2 files changed, 90 insertions(+)
> >
> > diff --git a/sk.c b/sk.c
> > index d27abff..1d1a656 100644
> > --- a/sk.c
> > +++ b/sk.c
> > @@ -205,6 +205,77 @@ failed:
> > return -1;
> > }
> >
> > +int sk_get_if_info(const char *name, struct sk_if_info *if_info)
> > +{
> > +#ifdef ETHTOOL_GLINKSETTINGS
> > + struct ifreq ifr;
> > + int fd, err;
> > +
> > + struct {
> > + struct ethtool_link_settings req;
> > + /*
> > +* link_mode_data consists of supported[],
> > advertising[],
> > +* lp_advertising[] with size up to 127 each.
> > +* The actual size is provided by the kernel.
> > +*/
> > + __u32 link_mode_data[3 * 127];
> > + } ecmd;
> > +
> > + memset(, 0, sizeof(ifr));
> > + memset(, 0, sizeof(ecmd));
> > +
> > + fd = socket(AF_INET, SOCK_DGRAM, 0);
> > + if (fd < 0) {
> > + goto failed;
> > + }
> > +
> > + ecmd.req.cmd = ETHTOOL_GLINKSETTINGS;
> > +
> > + strncpy(ifr.ifr_name, name, IFNAMSIZ - 1);
> > + ifr.ifr_data = (char *)
> > +
> > + /* Handshake with kernel to determine number of words for
> > link
> > +* mode bitmaps. When requested number of bitmap words is not
> > +* the one expected by kernel, the latter returns the integer
> > +* opposite of what it is expecting. We request length 0
> > below
> > +* (aka. invalid bitmap length) to get this info.
> > +*/
> > + err = ioctl(fd, SIOCETHTOOL, );
> > + if (err < 0) {
> > + pr_err("ioctl SIOCETHTOOL failed: %m");
> > + close(fd);
> > + goto failed;
> > + }
>
> I think it is better to merge this ioctl and the socket creation with
> sk_get_ts_info().
> No reason for duplication.
> You can use a static function or inline one.
>
> > +
> > + if (ecmd.req.link_mode_masks_nwords >= 0 ||
> > + ecmd.req.cmd != ETHTOOL_GLINKSETTINGS) {
> > + return 1;
> > + }
> > + ecmd.req.link_mode_masks_nwords = -
> > ecmd.req.link_mode_masks_nwords;
> > +
> > + err = ioctl(fd, SIOCETHTOOL, );
> > + if (err < 0) {
> > + pr_err("ioctl SIOCETHTOOL failed: %m");
> > + close(fd);
> > + goto failed;
> > + }
> > +
> > + close(fd);
> > +
> > + /* copy the necessary data to sk_info */
> > + memset(if_info, 0, sizeof(struct sk_if_info));
> > + if_info->valid = 1;
> > + if_info->speed = ecmd.req.speed;
> > +
> > + return 0;
> > +failed:
> > +#endif
> > + /* clear data and ensure it is not marked valid */
> > + memset(if_info, 0, sizeof(struct sk_if_info));
> > + return -1;
>
> You need to close the socket!
>
> > +}
> > +
> > +
> > static int sk_interface_guidaddr(const char *name, unsigned char
> > *guid)
> > {
> > char file_name[64], buf[64], addr[8];
> > diff --git a/sk.h b/sk.h
> >
[Linuxptp-devel] [PATCH v2 1/3] lstab: Add LSTAB_EXPIRED result
LSTAB_UNKNOWN is too generic, add LSTAB_EXPIRED result to return the latest
lstab correction and indicate that the tab used to resolve it is expired.
This is useful for more precise reporting of lstab file state and will enable an
override of expired table.
Signed-off-by: Maciek Machnikowski
Reviewed-by: Jacob Keller
---
lstab.c | 9 +
lstab.h | 6 ++
ts2phc_nmea_pps_source.c | 7 ---
3 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/lstab.c b/lstab.c
index 72c02e8..881b5d3 100644
--- a/lstab.c
+++ b/lstab.c
@@ -182,10 +182,6 @@ enum lstab_result lstab_utc2tai(struct lstab *lstab,
uint64_t utctime,
{
int epoch = -1, index, next;
- if (utctime > lstab->expiration_utc) {
- return LSTAB_UNKNOWN;
- }
-
for (index = lstab->length - 1; index > -1; index--) {
if (utctime >= lstab->lstab[index].utc) {
epoch = index;
@@ -203,5 +199,10 @@ enum lstab_result lstab_utc2tai(struct lstab *lstab,
uint64_t utctime,
if (next < lstab->length && utctime == lstab->lstab[next].utc - 1) {
return LSTAB_AMBIGUOUS;
}
+
+ if (utctime > lstab->expiration_utc) {
+ return LSTAB_EXPIRED;
+ }
+
return LSTAB_OK;
}
diff --git a/lstab.h b/lstab.h
index d2393b4..3811aed 100644
--- a/lstab.h
+++ b/lstab.h
@@ -41,6 +41,12 @@ enum lstab_result {
*/
LSTAB_UNKNOWN,
+ /**
+* The given lstab is past its expiry date and the tai_offset return
+* value may not be correct.
+*/
+ LSTAB_EXPIRED,
+
/**
* The given UTC value is ambiguous. The corresponding TAI time is
either
*
diff --git a/ts2phc_nmea_pps_source.c b/ts2phc_nmea_pps_source.c
index db8b5c6..8ea26bf 100644
--- a/ts2phc_nmea_pps_source.c
+++ b/ts2phc_nmea_pps_source.c
@@ -188,7 +188,7 @@ static int ts2phc_nmea_pps_source_getppstime(struct
ts2phc_pps_source *src,
struct ts2phc_nmea_pps_source *m =
container_of(src, struct ts2phc_nmea_pps_source, pps_source);
tmv_t delay_t1, delay_t2, duration_since_rmc, local_t1, local_t2, rmc;
- int lstab_error = 0, tai_offset = 0;
+ int lstab_error = -1, tai_offset = 0;
enum lstab_result result;
struct timespec now;
int64_t utc_time;
@@ -237,11 +237,12 @@ static int ts2phc_nmea_pps_source_getppstime(struct
ts2phc_pps_source *src,
break;
case LSTAB_UNKNOWN:
pr_err("nmea: unable to find utc time in leap second table");
- lstab_error = -1;
+ break;
+ case LSTAB_EXPIRED:
+ pr_err("nmea: utc time is past leap second table expiry date");
break;
case LSTAB_AMBIGUOUS:
pr_err("nmea: utc time stamp is ambiguous");
- lstab_error = -1;
break;
}
ts->tv_sec += tai_offset;
--
2.34.1
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH v2 3/3] ts2phc: Fix generic pps source when tai offset is not set in OS
This patch adds support for the leapfile in the generic pps source.
Such implementations prevents situtation when TAI offset is not set in the
system (which is true when there is no NTP daemon running that sets it) from
setting the UTC time in the disciplined PHC.
Leapfile and lstab will only be used when the tai offset read from system is 0.
Signed-off-by: Maciek Machnikowski
Reviewed-by: Jacob Keller
---
ts2phc_generic_pps_source.c | 83 +
1 file changed, 76 insertions(+), 7 deletions(-)
diff --git a/ts2phc_generic_pps_source.c b/ts2phc_generic_pps_source.c
index 6842d8e..d503aac 100644
--- a/ts2phc_generic_pps_source.c
+++ b/ts2phc_generic_pps_source.c
@@ -6,6 +6,7 @@
#include
#include
+#include "lstab.h"
#include "missing.h"
#include "print.h"
#include "ts2phc_generic_pps_source.h"
@@ -14,15 +15,38 @@
struct ts2phc_generic_pps_source {
struct ts2phc_pps_source pps_source;
+ struct lstab *lstab;
};
static void ts2phc_generic_pps_source_destroy(struct ts2phc_pps_source *src)
{
struct ts2phc_generic_pps_source *s =
container_of(src, struct ts2phc_generic_pps_source, pps_source);
+
+ if (s->lstab) {
+ lstab_destroy(s->lstab);
+ }
+
free(s);
}
+static int get_ntx(struct timex *ntx)
+{
+ int code;
+
+ if (!ntx)
+ return -1;
+
+ memset(ntx, 0, sizeof(*ntx));
+ ntx->modes = ADJ_NANO;
+ code = adjtimex(ntx);
+ if (code == -1) {
+ pr_err("adjtimex failed: %m");
+ return -1;
+ }
+ return 0;
+}
+
/*
* Returns the time on the PPS source device at which the most recent
* PPS event was generated. This implementation assumes that the
@@ -31,17 +55,44 @@ static void ts2phc_generic_pps_source_destroy(struct
ts2phc_pps_source *src)
static int ts2phc_generic_pps_source_getppstime(struct ts2phc_pps_source *src,
struct timespec *ts)
{
+ struct ts2phc_generic_pps_source *s =
+ container_of(src, struct ts2phc_generic_pps_source, pps_source);
+ enum lstab_result result;
+ int64_t utc_time;
struct timex ntx;
- int code;
+ int tai_offset;
- memset(, 0, sizeof(ntx));
- ntx.modes = ADJ_NANO;
- code = adjtimex();
- if (code == -1) {
- pr_err("adjtimex failed: %m");
+ if (get_ntx()) {
return -1;
}
- ts->tv_sec = ntx.time.tv_sec + ntx.tai;
+
+ tai_offset = ntx.tai;
+
+ /* When TAI offset is not set in system - try to get it from leapfile */
+ if (tai_offset == 0) {
+ if (!s->lstab) {
+ return -1;
+ }
+
+ utc_time = ntx.time.tv_sec;
+ result = lstab_utc2tai(s->lstab, utc_time, _offset);
+
+ switch (result) {
+ case LSTAB_OK:
+ break;
+ case LSTAB_UNKNOWN:
+ pr_err("Unable to find utc time in leap second table");
+ return -1;
+ case LSTAB_EXPIRED:
+ pr_err("UTC time is past leap second table expiry
date");
+ return -1;
+ case LSTAB_AMBIGUOUS:
+ pr_err("UTC time stamp is ambiguous");
+ return -1;
+ }
+ }
+
+ ts->tv_sec = ntx.time.tv_sec + tai_offset;
ts->tv_nsec = ntx.time.tv_usec;
return 0;
@@ -51,11 +102,29 @@ struct ts2phc_pps_source
*ts2phc_generic_pps_source_create(struct ts2phc_private
const char *dev)
{
struct ts2phc_generic_pps_source *src;
+ const char *leapfile;
+ struct timex ntx;
+
+ if (get_ntx()) {
+ return NULL;
+ }
src = calloc(1, sizeof(*src));
if (!src) {
return NULL;
}
+
+ if (ntx.tai == 0) {
+ pr_err("UTC-TAI offset not set in system! Trying to revert to
leapfile");
+
+ leapfile = config_get_string(priv->cfg, NULL, "leapfile");
+ src->lstab = lstab_create(leapfile);
+ if (!src->lstab) {
+ free(src);
+ return NULL;
+ }
+ }
+
src->pps_source.destroy = ts2phc_generic_pps_source_destroy;
src->pps_source.getppstime = ts2phc_generic_pps_source_getppstime;
--
2.34.1
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH v2 2/3] lstab: move update_leapsecond_table function to lstab
To enable handling lstab in the same way by different pps sources, move
update_leapsecond_table function from the nmea_pps_source to the generic lstab
file. This also required moving leapfile filename and its modification time to
the struct lstab.
Signed-off-by: Maciek Machnikowski
Reviewed-by: Jacob Keller
---
lstab.c | 52
lstab.h | 8 +++
ts2phc_nmea_pps_source.c | 51 ---
3 files changed, 64 insertions(+), 47 deletions(-)
diff --git a/lstab.c b/lstab.c
index 881b5d3..62429ee 100644
--- a/lstab.c
+++ b/lstab.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include "lstab.h"
@@ -45,6 +46,8 @@ struct epoch_marker {
struct lstab {
struct epoch_marker lstab[N_LEAPS];
uint64_t expiration_utc;
+ const char *leapfile;
+ time_t lsfile_mtime;
int length;
};
@@ -157,6 +160,8 @@ static int lstab_read(struct lstab *lstab, const char *name)
struct lstab *lstab_create(const char *filename)
{
struct lstab *lstab = calloc(1, sizeof(*lstab));
+ struct stat statbuf;
+ int err;
if (!lstab) {
return NULL;
@@ -166,12 +171,54 @@ struct lstab *lstab_create(const char *filename)
free(lstab);
return NULL;
}
+ lstab->leapfile = filename;
+
+ err = stat(lstab->leapfile, );
+ if (err) {
+ fprintf(stderr, "file status failed on %s: %m",
+ lstab->leapfile);
+ free(lstab);
+ return NULL;
+ }
+
+ lstab->lsfile_mtime = statbuf.st_mtim.tv_sec;
+
} else {
lstab_init(lstab);
}
return lstab;
}
+static int update_leapsecond_table(struct lstab *lstab)
+{
+ const char* leapfile;
+ struct stat statbuf;
+ int err;
+
+ if (!lstab->leapfile) {
+ return 0;
+ }
+ err = stat(lstab->leapfile, );
+ if (err) {
+ fprintf(stderr, "file status failed on %s: %m",
+ lstab->leapfile);
+ return -1;
+ }
+ if (lstab->lsfile_mtime == statbuf.st_mtim.tv_sec) {
+ return 0;
+ }
+ printf("updating leap seconds file\n");
+ leapfile = lstab->leapfile;
+ lstab_destroy(lstab);
+
+ lstab = lstab_create(leapfile);
+ if (!lstab) {
+ return -1;
+ }
+
+ return 0;
+}
+
void lstab_destroy(struct lstab *lstab)
{
free(lstab);
@@ -182,6 +229,11 @@ enum lstab_result lstab_utc2tai(struct lstab *lstab,
uint64_t utctime,
{
int epoch = -1, index, next;
+ if (update_leapsecond_table(lstab)) {
+ fprintf(stderr, "Failed to update leap seconds table");
+ return LSTAB_UNKNOWN;
+ }
+
for (index = lstab->length - 1; index > -1; index--) {
if (utctime >= lstab->lstab[index].utc) {
epoch = index;
diff --git a/lstab.h b/lstab.h
index 3811aed..43a5ec0 100644
--- a/lstab.h
+++ b/lstab.h
@@ -19,6 +19,14 @@ struct lstab;
*/
struct lstab *lstab_create(const char *filename);
+/**
+ * Updates an instance of a leap second table from the associated file.
+ * @param lstab Pointer to lstab to be updated.
+ * @return 0 if lstab is up to date, don't use a file or was successfully
updated.
+ * non-zero - on error
+ */
+int update_leapsecond_table(struct lstab *lstab);
+
/**
* Destroys a leap second table instance.
* @param lstab A pointer obtained via lstab_create().
diff --git a/ts2phc_nmea_pps_source.c b/ts2phc_nmea_pps_source.c
index 8ea26bf..3a4267d 100644
--- a/ts2phc_nmea_pps_source.c
+++ b/ts2phc_nmea_pps_source.c
@@ -29,8 +29,6 @@
struct ts2phc_nmea_pps_source {
struct ts2phc_pps_source pps_source;
struct config *config;
- const char *leapfile;
- time_t lsfile_mtime;
struct lstab *lstab;
pthread_t worker;
/* Protects anonymous struct fields, below, from concurrent access. */
@@ -144,34 +142,6 @@ static void *monitor_nmea_status(void *arg)
return NULL;
}
-static int update_leapsecond_table(struct ts2phc_nmea_pps_source *s)
-{
- struct stat statbuf;
- int err;
-
- if (!s->leapfile) {
- return 0;
- }
- err = stat(s->leapfile, );
- if (err) {
- pr_err("nmea: file status failed on %s: %m", s->leapfile);
- return -1;
- }
- if (s->lsfile_mtime == statbuf.st_mtim.tv_sec) {
- return 0;
- }
- pr_info("nmea: updating leap seconds file");
- if (s->lstab) {
- lstab_destroy(s->lstab);
- }
- s->lstab = lstab_create(s->leapfile);
- if (!s->lstab) {
- return -1;
- }
-
[Linuxptp-devel] [PATCH v2 0/3] Fix TAI offset in generic pps source
Current implementation of generic pps source relies on the UTC-TAI offset read from system. Some OSes don't set that offset by default and return 0. In such case ts2phc sets PHC time to UTC timescale without notifying user. This patch series moves leapfile handling to lstab and reuses the logic that was applied to nmea source also to the generic, when the value read from system is incorrect. v2: Fixes after Richard's review Signed-off-by: Maciek Machnikowski Maciek Machnikowski (3): lstab: Add LSTAB_EXPIRED result lstab: move update_leapsecond_table function to lstab ts2phc: Fix generic pps source when tai offset is not set in OS lstab.c | 55 +++- lstab.h | 14 +++ ts2phc_generic_pps_source.c | 83 + ts2phc_nmea_pps_source.c| 58 -- 4 files changed, 152 insertions(+), 58 deletions(-) -- 2.34.1 ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
