Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> -Original Message- > From: Maciek Machnikowski > Sent: Monday, November 14, 2022 10:55 PM > To: Keller, Jacob E > Cc: Hal Murray ; Miroslav Lichvar > ; linuxptp-devel@lists.sourceforge.net > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo. > > On Mon, Nov 14, 2022 at 05:06:17PM +, Keller, Jacob E wrote: > > > > > > > -Original Message- > > > From: Hal Murray > > > Sent: Monday, November 14, 2022 4:34 AM > > > To: Miroslav Lichvar > > > Cc: Keller, Jacob E ; linuxptp- > > > de...@lists.sourceforge.net; Hal Murray > > > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo. > > > > > > > > > >> How specific is this to chronyd? > > > > AFAIK no other application implements the server side of the protocol. > > > >> Would it make sense to call this chronysock > > > >> instead of just sock? > > > > Yes, that makes sense. If there are no other issues with the patches, I > > > > can > > > > resend. > > > > > > Calling it chronysock has the disadvantage of sounding like only chrony > > > should > > > use it. > > > > > > > Yea, but I feel that just "sock" is vague. I'm not totally opposed to it > > though. > > What about rcl_sock or refclock_sock? It's used in the file linked by > Miroslav. > > Regards > Maciek Both of those sound good to me. Slight preference to refclock_sock if its not too long. Thanks, Jake ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
On Mon, Nov 14, 2022 at 05:06:17PM +, Keller, Jacob E wrote: > > > > -Original Message- > > From: Hal Murray > > Sent: Monday, November 14, 2022 4:34 AM > > To: Miroslav Lichvar > > Cc: Keller, Jacob E ; linuxptp- > > de...@lists.sourceforge.net; Hal Murray > > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo. > > > > > > >> How specific is this to chronyd? > > > AFAIK no other application implements the server side of the protocol. > > >> Would it make sense to call this chronysock > > >> instead of just sock? > > > Yes, that makes sense. If there are no other issues with the patches, I > > > can > > > resend. > > > > Calling it chronysock has the disadvantage of sounding like only chrony > > should > > use it. > > > > Yea, but I feel that just "sock" is vague. I'm not totally opposed to it > though. What about rcl_sock or refclock_sock? It's used in the file linked by Miroslav. Regards Maciek ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
> -Original Message- > From: Hal Murray > Sent: Monday, November 14, 2022 4:34 AM > To: Miroslav Lichvar > Cc: Keller, Jacob E ; linuxptp- > de...@lists.sourceforge.net; Hal Murray > Subject: Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo. > > > >> How specific is this to chronyd? > > AFAIK no other application implements the server side of the protocol. > >> Would it make sense to call this chronysock > >> instead of just sock? > > Yes, that makes sense. If there are no other issues with the patches, I can > > resend. > > Calling it chronysock has the disadvantage of sounding like only chrony should > use it. > Yea, but I feel that just "sock" is vague. I'm not totally opposed to it though. Thanks, Jake > >> The implementation seems fine but its using an interface that was defined > >> by > >> chrony. I suppose another application could implement the same interface > >> though.. > > > ntpsec might be interested in implementing it. We'll see. > > Is there a URL for the spec? I don't want an RFC. Good comments in a header > file may be enough. A separate document may be better if there are > complications that need explaining. > > is there a version number? > > > > I agree that the current SHM setup is far from wonderful. There is a clean > way to make SHM read-only by receivers so you can have multiple receivers. > That would let you run gpsmon while chronyd/ntpd is running. > > > -- > These are my opinions. I hate spam. > > ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH] bmc: Fix PortId comparison
The PortId is defined as a couple of ClockId (an 8-bytes opaque) and the
PortNumber (UInterger16).
In order to correctly handle the PortId endianess, the comparison has been
split in the ClockIds comparison (using memcmp) and PortNumber comparison.
---
bmc.c | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/bmc.c b/bmc.c
index 6ac7aa0..ebc0789 100644
--- a/bmc.c
+++ b/bmc.c
@@ -21,6 +21,17 @@
#include "bmc.h"
#include "ds.h"
+static int portid_cmp(struct PortIdentity *a, struct PortIdentity *b)
+{
+ int diff = memcmp(&a->clockIdentity, &b->clockIdentity,
sizeof(a->clockIdentity));
+
+ if (diff == 0) {
+ diff = a->portNumber - b->portNumber;
+ }
+
+ return diff;
+}
+
int dscmp2(struct dataset *a, struct dataset *b)
{
int diff;
@@ -35,7 +46,7 @@ int dscmp2(struct dataset *a, struct dataset *b)
* standard, since there is nothing we can do about it anyway.
*/
if (A < B) {
- diff = memcmp(&b->receiver, &b->sender, sizeof(b->receiver));
+ diff = portid_cmp(&b->receiver, &b->sender);
if (diff < 0)
return A_BETTER;
if (diff > 0)
@@ -44,7 +55,7 @@ int dscmp2(struct dataset *a, struct dataset *b)
return 0;
}
if (A > B) {
- diff = memcmp(&a->receiver, &a->sender, sizeof(a->receiver));
+ diff = portid_cmp(&a->receiver, &a->sender);
if (diff < 0)
return B_BETTER;
if (diff > 0)
@@ -53,7 +64,7 @@ int dscmp2(struct dataset *a, struct dataset *b)
return 0;
}
- diff = memcmp(&a->sender, &b->sender, sizeof(a->sender));
+ diff = portid_cmp(&a->sender, &b->sender);
if (diff < 0)
return A_BETTER_TOPO;
if (diff > 0)
--
2.38.1
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] Fix PortId handling in BMCA
During the test I noticed that PortNumber is not correctly handled by BMCA. The ITU-T G.8275.2 is a little bit generic on PortId comparison criteria and the actual LinuxPTP implementation just uses a memcmp() call. This implementation doesn't consider the endianes of PortNumber value (UInteger16) that should be considered instead. The fix consists to split the ClockId comparison part (still using memcmp() call) and the PortNumber comparison when ClockIds are equals. ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
On Mon, Nov 14, 2022 at 04:34:05AM -0800, Hal Murray wrote: > Is there a URL for the spec? I don't want an RFC. Good comments in a header > file may be enough. A separate document may be better if there are > complications that need explaining. Here is the structure of the message with some comments: https://git.tuxfamily.org/chrony/chrony.git/tree/refclock_sock.c#n38 If you started with a copy of the SHM driver in ntpd, it would need to be modified to use a datagram Unix socket instead of the shared memory segment, listen for messages on the socket (io_addclock()) instead of polling in the _timer() function, and interpret the fields in the message like this: - "tv" field in SOCK is the same as receiveTimeStamp in SHM - tv + offset is clockTimestamp - messages with non-zero pulse can be ignored in the initial implementation as nothing really uses it - leap has the same meaning - magic is a protocol sanity+version check, should be always 0x534f434b The missing fields precision and samples can be set to -30 and 1 respectively. That should be it. I can help with the patch if interested. > is there a version number? No. If there was a need to make an incompatible change, we can change the magic number. -- Miroslav Lichvar ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
>> How specific is this to chronyd? > AFAIK no other application implements the server side of the protocol. >> Would it make sense to call this chronysock >> instead of just sock? > Yes, that makes sense. If there are no other issues with the > patches, I can resend. Calling it chronysock has the disadvantage of sounding like only chrony should use it. >> The implementation seems fine but its using an interface that was defined by >> chrony. I suppose another application could implement the same interface >> though.. > ntpsec might be interested in implementing it. We'll see. Is there a URL for the spec? I don't want an RFC. Good comments in a header file may be enough. A separate document may be better if there are complications that need explaining. Is there a version number? (or plan for how to update things) I agree that the current SHM setup is far from wonderful. There is a clean way to make SHM read-only by receivers so you can have multiple receivers. That would let you run gpsmon while chronyd/ntpd is running. -- These are my opinions. I hate spam. ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
[Linuxptp-devel] [PATCH] config: Fix -Wformat-truncation warnings.
Check the snprintf() return value in order to avoid the following
warnings from gcc:
config.c: In function ‘config_create’:
config.c:921:52: warning: ‘%s’ directive output may be truncated writing up to
9679 bytes into a region of size 33 [-Wformat-truncation=]
921 | snprintf(buf, sizeof(buf), "global.%s", ci->label);
|^~
config.c:921:17: note: ‘snprintf’ output between 8 and 9687 bytes into a
destination of size 40
921 | snprintf(buf, sizeof(buf), "global.%s", ci->label);
| ^~
config.c:364:40: warning: ‘%s’ directive output may be truncated writing up to
9679 bytes into a region of size 133 -Wformat-truncation=]
364 | snprintf(buf, sizeof(buf), "%s.%s", section, name);
|^~
In function ‘config_section_item’,
inlined from ‘config_global_item’ at config.c:371:9,
inlined from ‘config_create’ at config.c:931:8:
config.c:364:9: note: ‘snprintf’ output between 8 and 9687 bytes into a
destination of size 140
364 | snprintf(buf, sizeof(buf), "%s.%s", section, name);
| ^~
Signed-off-by: Miroslav Lichvar
---
config.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/config.c b/config.c
index e454c91..e21cde7 100644
--- a/config.c
+++ b/config.c
@@ -361,7 +361,8 @@ static struct config_item *config_section_item(struct
config *cfg,
{
char buf[CONFIG_LABEL_SIZE + MAX_IFNAME_SIZE];
- snprintf(buf, sizeof(buf), "%s.%s", section, name);
+ if (snprintf(buf, sizeof(buf), "%s.%s", section, name) >= sizeof(buf))
+ return NULL;
return hash_lookup(cfg->htab, buf);
}
@@ -918,7 +919,11 @@ struct config *config_create(void)
for (i = 0; i < N_CONFIG_ITEMS; i++) {
ci = &config_tab[i];
ci->flags |= CFG_ITEM_STATIC;
- snprintf(buf, sizeof(buf), "global.%s", ci->label);
+ if (snprintf(buf, sizeof(buf), "global.%s", ci->label) >=
+ sizeof(buf)) {
+ fprintf(stderr, "option %s too long\n", ci->label);
+ goto fail;
+ }
if (hash_insert(cfg->htab, buf, ci)) {
fprintf(stderr, "duplicate item %s\n", ci->label);
goto fail;
--
2.37.3
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
Re: [Linuxptp-devel] [PATCH 2/4] Add sock servo.
On Thu, Nov 10, 2022 at 09:51:52AM -0800, Jacob Keller wrote: > How specific is this to chronyd? AFAIK no other application implements the server side of the protocol. > Would it make sense to call this chronysock > instead of just sock? Yes, that makes sense. If there are no other issues with the patches, I can resend. > The implementation seems fine but its using an interface that was defined by > chrony. I suppose another application could implement the same interface > though.. ntpsec might be interested in implementing it. We'll see. -- Miroslav Lichvar ___ Linuxptp-devel mailing list Linuxptp-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
