Discussion:
[Linuxptp-devel] [PATCH] Add options to configure TTL/hop limit for UDP/UDP6 transports.
Miroslav Lichvar
2015-08-03 13:49:43 UTC
Permalink
Signed-off-by: Miroslav Lichvar <***@redhat.com>
---
config.c | 12 ++++++++++++
config.h | 2 ++
default.cfg | 2 ++
ptp4l.8 | 10 ++++++++++
ptp4l.c | 3 +++
udp.c | 7 +++++++
udp.h | 5 +++++
udp6.c | 6 ++++++
udp6.h | 5 +++++
9 files changed, 52 insertions(+)

diff --git a/config.c b/config.c
index 934caa2..98e087a 100644
--- a/config.c
+++ b/config.c
@@ -457,6 +457,18 @@ static enum parser_result parse_global_setting(const char *option,
for (i = 0; i < MAC_LEN; i++)
cfg->p2p_dst_mac[i] = mac[i];

+ } else if (!strcmp(option, "udp_ttl")) {
+ r = get_ranged_uint(value, &uval, 0, UINT8_MAX);
+ if (r != PARSED_OK)
+ return r;
+ *cfg->udp_ttl = uval;
+
+ } else if (!strcmp(option, "udp6_hop_limit")) {
+ r = get_ranged_uint(value, &uval, 0, UINT8_MAX);
+ if (r != PARSED_OK)
+ return r;
+ *cfg->udp6_hop_limit = uval;
+
} else if (!strcmp(option, "udp6_scope")) {
r = get_ranged_uint(value, &uval, 0x00, 0x0F);
if (r != PARSED_OK)
diff --git a/config.h b/config.h
index 7bff11f..f5d7c59 100644
--- a/config.h
+++ b/config.h
@@ -88,6 +88,8 @@ struct config {

unsigned char *ptp_dst_mac;
unsigned char *p2p_dst_mac;
+ unsigned char *udp_ttl;
+ unsigned int *udp6_hop_limit;
unsigned char *udp6_scope;
char *uds_address;

diff --git a/default.cfg b/default.cfg
index b46c0f6..b2c77ac 100644
--- a/default.cfg
+++ b/default.cfg
@@ -60,6 +60,8 @@ ntpshm_segment 0
transportSpecific 0x0
ptp_dst_mac 01:1B:19:00:00:00
p2p_dst_mac 01:80:C2:00:00:0E
+udp_ttl 1
+udp6_hop_limit 1
udp6_scope 0x0E
uds_address /var/run/ptp4l
#
diff --git a/ptp4l.8 b/ptp4l.8
index 0bdccb1..516d9cb 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -420,6 +420,16 @@ Relevant only with L2 transport. The default is 01:1B:19:00:00:00.
The MAC address where should be peer delay messages the PTP peer.
Relevant only with L2 transport. The default is 01:80:C2:00:00:0E.
.TP
+.B udp_ttl
+The Time to live (TTL) value set for IPv4 multicast messages. This
+option is only relevant with the IPv4 UDP transport. The default is 1
+to restrict the messages to the same subnet.
+.TP
+.B udp6_hop_limit
+The hop limit set for IPv6 multicast messages. This option is only
+relevant with the IPv6 UDP transport. The default is 1 to restrict
+the messages to the same subnet.
+.TP
.B udp6_scope
Specifies the desired scope for the IPv6 multicast messages. This
will be used as the second byte of the primary address. This option
diff --git a/ptp4l.c b/ptp4l.c
index 61c5854..94984c2 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -32,6 +32,7 @@
#include "raw.h"
#include "sk.h"
#include "transport.h"
+#include "udp.h"
#include "udp6.h"
#include "uds.h"
#include "util.h"
@@ -124,6 +125,8 @@ static struct config cfg_settings = {

.ptp_dst_mac = ptp_dst_mac,
.p2p_dst_mac = p2p_dst_mac,
+ .udp_ttl = &udp_ttl,
+ .udp6_hop_limit = &udp6_hop_limit,
.udp6_scope = &udp6_scope,
.uds_address = uds_path,

diff --git a/udp.c b/udp.c
index b100dbf..c4f6945 100644
--- a/udp.c
+++ b/udp.c
@@ -42,6 +42,8 @@
#define PTP_PRIMARY_MCAST_IPADDR "224.0.1.129"
#define PTP_PDELAY_MCAST_IPADDR "224.0.0.107"

+unsigned char udp_ttl = 1;
+
struct udp {
struct transport t;
struct address ip;
@@ -123,6 +125,11 @@ static int open_socket(const char *name, struct in_addr mc_addr[2], short port)
pr_err("setsockopt SO_BINDTODEVICE failed: %m");
goto no_option;
}
+ if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_TTL, &udp_ttl,
+ sizeof(udp_ttl))) {
+ pr_err("setsockopt IP_MULTICAST_TTL failed: %m");
+ goto no_option;
+ }
addr.sin_addr = mc_addr[0];
if (mcast_join(fd, index, (struct sockaddr *) &addr, sizeof(addr))) {
pr_err("mcast_join failed");
diff --git a/udp.h b/udp.h
index fc0d54d..5c6f2e2 100644
--- a/udp.h
+++ b/udp.h
@@ -24,6 +24,11 @@
#include "transport.h"

/**
+ * The TTL value of sent multicast messages.
+ */
+extern unsigned char udp_ttl;
+
+/**
* Allocate an instance of a UDP/IPv4 transport.
* @return Pointer to a new transport instance on success, NULL otherwise.
*/
diff --git a/udp6.c b/udp6.c
index f098b8c..1836e34 100644
--- a/udp6.c
+++ b/udp6.c
@@ -43,6 +43,7 @@
#define PTP_PDELAY_MCAST_IP6ADDR "FF02:0:0:0:0:0:0:6B"

unsigned char udp6_scope = 0x0E;
+unsigned int udp6_hop_limit = 1;

struct udp6 {
struct transport t;
@@ -133,6 +134,11 @@ static int open_socket_ipv6(const char *name, struct in6_addr mc_addr[2], short
pr_err("setsockopt SO_BINDTODEVICE failed: %m");
goto no_option;
}
+ if (setsockopt(fd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &udp6_hop_limit,
+ sizeof(udp6_hop_limit))) {
+ pr_err("setsockopt IPV6_MULTICAST_HOPS failed: %m");
+ goto no_option;
+ }
addr.sin6_addr = mc_addr[0];
if (mc_join(fd, index, (struct sockaddr *) &addr, sizeof(addr))) {
pr_err("mcast_join failed");
diff --git a/udp6.h b/udp6.h
index e451262..5b7059c 100644
--- a/udp6.h
+++ b/udp6.h
@@ -30,6 +30,11 @@
extern unsigned char udp6_scope;

/**
+ * The hop limit of sent multicast messages.
+ */
+extern unsigned int udp6_hop_limit;
+
+/**
* Allocate an instance of a UDP/IPv6 transport.
* @return Pointer to a new transport instance on success, NULL otherwise.
*/
--
2.1.0


------------------------------------------------------------------------------
Richard Cochran
2015-08-03 20:02:06 UTC
Permalink
I have no problem with adding these options.
Post by Miroslav Lichvar
+unsigned char udp_ttl = 1;
...
Post by Miroslav Lichvar
+ if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_TTL, &udp_ttl,
+ sizeof(udp_ttl))) {
+ pr_err("setsockopt IP_MULTICAST_TTL failed: %m");
+ goto no_option;
+ }
This would be better it it were a per-port option, no?

I want to start reworking the config API to make it easier and cleaner
to add and make use of options. (Jake and I discussed this a bit
off-list.) I'll hold on this patch until we have something. In fact,
I might just use this change as a guinea pig, if you don't mind!

Thanks,
Richard

------------------------------------------------------------------------------
Keller, Jacob E
2015-08-03 20:17:59 UTC
Permalink
Post by Richard Cochran
I have no problem with adding these options.
Post by Miroslav Lichvar
+unsigned char udp_ttl = 1;
...
Post by Miroslav Lichvar
+ if (setsockopt(fd, IPPROTO_IP, IP_MULTICAST_TTL, &udp_ttl,
+ sizeof(udp_ttl))) {
+ pr_err("setsockopt IP_MULTICAST_TTL failed: %m");
+ goto no_option;
+ }
This would be better it it were a per-port option, no?
I want to start reworking the config API to make it easier and
cleaner
to add and make use of options. (Jake and I discussed this a bit
off-list.) I'll hold on this patch until we have something. In fact,
I might just use this change as a guinea pig, if you don't mind!
Thanks,
Richard
Yea. We discussed some ideas, but I have not had any more time to work
on the ideas myself. One option I've recently considered is maybe
porting some library inline to our code that might make it easier than
re-writing all the bits ourselves.. May not be worth it at all but
worth considering ie: static inline a (GPL) library that does what we
need, rather than write our own.. but do so in a way that doesn't
require users to actually install the library themselves.. It's an
option we might want to consider.

Regards,
Jake
------------------------------------------------------------------------------
Miroslav Lichvar
2015-08-04 08:25:37 UTC
Permalink
Post by Richard Cochran
Post by Miroslav Lichvar
+unsigned char udp_ttl = 1;
...
This would be better it it were a per-port option, no?
Definitely. That's actually how I originally wanted it, but when I saw
there was no mechanism to pass port specific options to transports and
didn't see an obvious way to add it, I gave up on the idea :).
Post by Richard Cochran
I want to start reworking the config API to make it easier and cleaner
to add and make use of options. (Jake and I discussed this a bit
off-list.) I'll hold on this patch until we have something. In fact,
I might just use this change as a guinea pig, if you don't mind!
Great!

Thanks,
--
Miroslav Lichvar

------------------------------------------------------------------------------
Jiri Benc
2015-08-10 12:47:01 UTC
Permalink
Post by Miroslav Lichvar
--- a/default.cfg
+++ b/default.cfg
@@ -60,6 +60,8 @@ ntpshm_segment 0
transportSpecific 0x0
ptp_dst_mac 01:1B:19:00:00:00
p2p_dst_mac 01:80:C2:00:00:0E
+udp_ttl 1
+udp6_hop_limit 1
I don't see much reasons for having different options for IPv4 and
IPv6. Just call it udp_ttl (or whatever) and use it for both IPv4 and
IPv6. The fact that IPv6 standard renamed the option sucks (and one can
argue it was not the best decision as it created more confusion than it
solved) but both TTL and hop limit are the exactly same thing in the
current networks and it doesn't make sense to distinguish them.

Jiri
--
Jiri Benc

------------------------------------------------------------------------------
Loading...