Thread: [Linuxptp-devel] [PATCH] Add support for DELAY_REQ and SYNC packets rx_filter
PTP IEEE 1588 stack for Linux
Brought to you by:
rcochran
Menu
▾
▴
linuxptp-devel
|
From: IlorDash <ilo...@gm...> - 2023-11-13 17:48:11
|
From: Ilya Orazov <ilo...@gm...>
Added adv_rx_filter config that allows to send SIOCSHWTSTAMP ioctl with
HWTSTAMP_FILTER_PTP_XXX_SYNC or HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ
rx filters based on whether the Device is Slave or Master respectively.
This Feature is added for all transport levels.
I’m using Ethernet controller with PTP support, which requires to
determine which PTP packet on Rx it must timestamp: SYNC or DELAY_REQ
packets based on whether the device is Slave or Masterrespectively.
Unfortunately, I’ve found out that ptp4l can’t send SIOCSHWTSTAMP ioctl
with HWTSTAMP_FILTER_PTP_XXX_SYNC or HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ
rx filters, and sends only EVENT rx filters to device driver.
Device driver can only get info about the current PTP port state
from ptp4l, because it doesn’t have direct access to it.
I assumed, if these filters are defined in Kernel, they might be used in
ptp4l utility to fix this. So I added adv_rx_filter config that allows
to send SIOCSHWTSTAMP ioctl with HWTSTAMP_FILTER_PTP_XXX_SYNC or
HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ rx filters for all transport levels.
Signed-off-by: Ilya Orazov <ilo...@gm...>
---
config.c | 1 +
port.c | 17 ++++++++++++++
ptp4l.8 | 8 +++++++
ptp4l.c | 1 +
raw.c | 13 +++++++++++
sk.c | 56 ++++++++++++++++++++++++++++++++++++++++-----
sk.h | 15 ++++++++++++
transport.c | 7 ++++++
transport.h | 5 ++++
transport_private.h | 3 +++
udp.c | 14 ++++++++++++
udp6.c | 14 ++++++++++++
12 files changed, 148 insertions(+), 6 deletions(-)
diff --git a/config.c b/config.c
index b104f1b..642b78c 100644
--- a/config.c
+++ b/config.c
@@ -268,6 +268,7 @@ struct config_item config_tab[] = {
PORT_ITEM_INT("G.8275.portDS.localPriority", 128, 1, UINT8_MAX),
GLOB_ITEM_INT("gmCapable", 1, 0, 1),
GLOB_ITEM_ENU("hwts_filter", HWTS_FILTER_NORMAL, hwts_filter_enu),
+ GLOB_ITEM_INT("adv_rx_filter", 0, 0, 1),
PORT_ITEM_INT("hybrid_e2e", 0, 0, 1),
PORT_ITEM_INT("ignore_source_id", 0, 0, 1),
PORT_ITEM_INT("ignore_transport_specific", 0, 0, 1),
diff --git a/port.c b/port.c
index 5803cd3..2376925 100644
--- a/port.c
+++ b/port.c
@@ -3526,6 +3526,23 @@ int port_state_update(struct port *p, enum fsm_event event, int mdiff)
p->unicast_state_dirty = true;
}
if (next != p->state) {
+ if (sk_adv_rx_filter == 1) {
+ pr_debug("port state update prev %d next %d", p->state,
+ next);
+
+ if ((next == PS_MASTER) || (next == PS_GRAND_MASTER))
+ transport_update_rx_filter(p->trp, p->iface,
+ &p->fda,
+ p->timestamping,
+ true);
+
+ if (next == PS_UNCALIBRATED)
+ transport_update_rx_filter(p->trp, p->iface,
+ &p->fda,
+ p->timestamping,
+ false);
+ }
+
port_show_transition(p, next, event);
p->state = next;
port_notify_event(p, NOTIFY_PORT_STATE);
diff --git a/ptp4l.8 b/ptp4l.8
index 40c66c2..9d838eb 100644
--- a/ptp4l.8
+++ b/ptp4l.8
@@ -142,6 +142,14 @@ See UNICAST DISCOVERY OPTIONS, below.
.SH PORT OPTIONS
+.TP
+.B adv_rx_filter
+Enable support of HWTSTAMP_FILTER_PTP_XX_SYNC and HWTSTAMP_FILTER_PTP_XX_DELAY_REQ.
+Some Ethernet controllers doesn't support PTP event packets,
+thus they must be provided with correct rx_filter for timestamping required packets
+(SYNC or DELAY_REQ) depends on whether it's Slave or Master respectively.
+The default is 0 or disabled.
+
.TP
.B announceReceiptTimeout
The number of missed Announce messages before the last Announce messages
diff --git a/ptp4l.c b/ptp4l.c
index c61175b..8f9531a 100644
--- a/ptp4l.c
+++ b/ptp4l.c
@@ -191,6 +191,7 @@ int main(int argc, char *argv[])
sk_check_fupsync = config_get_int(cfg, NULL, "check_fup_sync");
sk_tx_timeout = config_get_int(cfg, NULL, "tx_timestamp_timeout");
sk_hwts_filter_mode = config_get_int(cfg, NULL, "hwts_filter");
+ sk_adv_rx_filter = config_get_int(cfg, NULL, "adv_rx_filter");
if (config_get_int(cfg, NULL, "clock_servo") == CLOCK_SERVO_NTPSHM) {
config_set_int(cfg, "kernel_leap", 0);
diff --git a/raw.c b/raw.c
index 1b978f0..7af3b2a 100644
--- a/raw.c
+++ b/raw.c
@@ -363,6 +363,18 @@ no_mac:
return -1;
}
+static int raw_update_rx_filter(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master)
+{
+ int err;
+ const char *name;
+
+ name = interface_label(iface);
+ err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
+ TRANS_IEEE_802_3, is_master);
+ return err;
+}
+
static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts)
{
@@ -476,6 +488,7 @@ struct transport *raw_transport_create(void)
return NULL;
raw->t.close = raw_close;
raw->t.open = raw_open;
+ raw->t.update_rx_filter = raw_update_rx_filter;
raw->t.recv = raw_recv;
raw->t.send = raw_send;
raw->t.release = raw_release;
diff --git a/sk.c b/sk.c
index 19395c9..a641e45 100644
--- a/sk.c
+++ b/sk.c
@@ -41,6 +41,8 @@
int sk_tx_timeout = 1;
int sk_check_fupsync;
+int sk_tx_type = -1;
+int sk_adv_rx_filter = -1;
enum hwts_filter_mode sk_hwts_filter_mode = HWTS_FILTER_NORMAL;
/* private methods */
@@ -56,7 +58,7 @@ static void init_ifreq(struct ifreq *ifreq, struct hwtstamp_config *cfg,
ifreq->ifr_data = (void *) cfg;
}
-static int hwts_init(int fd, const char *device, int rx_filter,
+static int hwts_set(int fd, const char *device, int rx_filter,
int rx_filter2, int tx_type)
{
struct ifreq ifreq;
@@ -131,7 +133,7 @@ static int hwts_init(int fd, const char *device, int rx_filter,
pr_err("The current filter does not match the required");
return -1;
}
-
+ sk_tx_type = cfg.tx_type;
return 0;
}
@@ -556,6 +558,36 @@ int sk_set_priority(int fd, int family, uint8_t dscp)
return 0;
}
+int sk_ts_update_rx_filter(int fd, const char *device, enum timestamp_type type,
+ enum transport_type transport, bool is_master)
+{
+ int err, filter1, filter2 = 0;
+
+ if (type == TS_SOFTWARE)
+ return 0;
+
+ filter1 = (is_master) ? HWTSTAMP_FILTER_PTP_V2_DELAY_REQ :
+ HWTSTAMP_FILTER_PTP_V2_SYNC;
+ switch (transport) {
+ case TRANS_UDP_IPV4:
+ case TRANS_UDP_IPV6:
+ filter2 = (is_master) ? HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ :
+ HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
+ break;
+ case TRANS_IEEE_802_3:
+ filter2 = (is_master) ? HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ :
+ HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
+ break;
+ default:
+ return -1;
+ }
+ pr_debug("update rx filter1 %d, filter2 %d, sk_tx_type %d", filter1,
+ filter2, sk_tx_type);
+ err = hwts_set(fd, device, filter1, filter2, sk_tx_type);
+
+ return err;
+}
+
int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
enum transport_type transport, int vclock)
{
@@ -585,7 +617,11 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
}
if (type != TS_SOFTWARE) {
- filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
+ if (sk_adv_rx_filter == 1) {
+ filter1 = HWTSTAMP_FILTER_PTP_V2_SYNC;
+ } else {
+ filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
+ }
switch (type) {
case TS_SOFTWARE:
tx_type = HWTSTAMP_TX_OFF;
@@ -604,10 +640,18 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
switch (transport) {
case TRANS_UDP_IPV4:
case TRANS_UDP_IPV6:
- filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
+ if (sk_adv_rx_filter == 1) {
+ filter2 = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
+ } else {
+ filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
+ }
break;
case TRANS_IEEE_802_3:
- filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
+ if (sk_adv_rx_filter == 1) {
+ filter2 = HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
+ } else {
+ filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
+ }
break;
case TRANS_DEVICENET:
case TRANS_CONTROLNET:
@@ -615,7 +659,7 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
case TRANS_UDS:
return -1;
}
- err = hwts_init(fd, device, filter1, filter2, tx_type);
+ err = hwts_set(fd, device, filter1, filter2, tx_type);
if (err)
return err;
}
diff --git a/sk.h b/sk.h
index 76062d0..88dc878 100644
--- a/sk.h
+++ b/sk.h
@@ -146,6 +146,19 @@ int sk_get_error(int fd);
*/
int sk_set_priority(int fd, int family, uint8_t dscp);
+/**
+ * If sk_adv_rx_filter is set, then handle XXX_SYNC and
+ * XXX_DELAY_REQ hwtstamp_rx_filters.
+ * @param fd An open socket.
+ * @param device The name of the network interface to configure.
+ * @param type The requested flavor of time stamping.
+ * @param transport The type of transport used.
+ * @param is_master Is current port state is Master.
+ * @return Zero on success, non-zero otherwise.
+ */
+int sk_ts_update_rx_filter(int fd, const char *device, enum timestamp_type type,
+ enum transport_type transport, bool is_master);
+
/**
* Enable time stamping on a given network interface.
* @param fd An open socket.
@@ -171,6 +184,8 @@ extern int sk_tx_timeout;
*/
extern int sk_check_fupsync;
+extern int sk_adv_rx_filter;
+
/**
* Hardware time-stamp setting mode
*/
diff --git a/transport.c b/transport.c
index 9366fbf..c27137b 100644
--- a/transport.c
+++ b/transport.c
@@ -37,6 +37,13 @@ int transport_open(struct transport *t, struct interface *iface,
return t->open(t, iface, fda, tt);
}
+int transport_update_rx_filter(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type tt,
+ bool is_master)
+{
+ return t->update_rx_filter(iface, fda, tt, is_master);
+}
+
int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
{
return t->recv(t, fd, msg, sizeof(msg->data), &msg->address, &msg->hwts);
diff --git a/transport.h b/transport.h
index 7a7f87b..37ef0f1 100644
--- a/transport.h
+++ b/transport.h
@@ -22,6 +22,7 @@
#include <time.h>
#include <inttypes.h>
+#include <stdbool.h>
#include "fd.h"
#include "msg.h"
@@ -60,6 +61,10 @@ int transport_close(struct transport *t, struct fdarray *fda);
int transport_open(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type tt);
+int transport_update_rx_filter(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type tt,
+ bool is_master);
+
int transport_recv(struct transport *t, int fd, struct ptp_message *msg);
/**
diff --git a/transport_private.h b/transport_private.h
index ec28e47..61d277f 100644
--- a/transport_private.h
+++ b/transport_private.h
@@ -35,6 +35,9 @@ struct transport {
int (*open)(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type tt);
+ int (*update_rx_filter)(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master);
+
int (*recv)(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts);
diff --git a/udp.c b/udp.c
index 7c9402e..b66ff7e 100644
--- a/udp.c
+++ b/udp.c
@@ -208,6 +208,19 @@ no_event:
return -1;
}
+static int udp_update_rx_filter(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master)
+{
+ int err;
+ const char *name;
+
+ name = interface_label(iface);
+ err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
+ TRANS_UDP_IPV4, is_master);
+
+ return err;
+}
+
static int udp_recv(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts)
{
@@ -302,6 +315,7 @@ struct transport *udp_transport_create(void)
return NULL;
udp->t.close = udp_close;
udp->t.open = udp_open;
+ udp->t.update_rx_filter = udp_update_rx_filter;
udp->t.recv = udp_recv;
udp->t.send = udp_send;
udp->t.release = udp_release;
diff --git a/udp6.c b/udp6.c
index bde1710..6a75962 100644
--- a/udp6.c
+++ b/udp6.c
@@ -225,6 +225,19 @@ no_event:
return -1;
}
+static int udp6_update_rx_filter(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master)
+{
+ int err;
+ const char *name;
+
+ name = interface_label(iface);
+ err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
+ TRANS_UDP_IPV6, is_master);
+
+ return err;
+}
+
static int udp6_recv(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts)
{
@@ -318,6 +331,7 @@ struct transport *udp6_transport_create(void)
return NULL;
udp6->t.close = udp6_close;
udp6->t.open = udp6_open;
+ udp6->t.update_rx_filter = udp6_update_rx_filter;
udp6->t.recv = udp6_recv;
udp6->t.send = udp6_send;
udp6->t.release = udp6_release;
--
2.34.1
|
|
From: Miroslav L. <mli...@re...> - 2023-11-14 12:30:10
|
On Mon, Nov 13, 2023 at 08:47:48PM +0300, IlorDash wrote: > I assumed, if these filters are defined in Kernel, they might be used in > ptp4l utility to fix this. So I added adv_rx_filter config that allows > to send SIOCSHWTSTAMP ioctl with HWTSTAMP_FILTER_PTP_XXX_SYNC or > HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ rx filters for all transport levels. With this dynamic switching of the RX filter on port state changes is there a reason to not do it by default? Or instead of adding a new option add it as a new enum to the existing hwts_filter option? -- Miroslav Lichvar |
|
From: IlorDash <ilo...@gm...> - 2023-11-14 15:50:43
|
Honestly, I don't know, is it necessary to dynamically switch RX filters, If the device supports EVENT RX filters, because with EVENT RX filters it can handle all PTP messages. Maybe you could suggest appropriate behaviour for utility in that case? Yes, I agree that it can be added as a new enum to hwts_filter, but I found that ptp4l can get info about supported RX filters through ethtool, and these RX filters are specified in device driver. So, I think, ptp4l can find out for itself whether the device supports EVENT RX filters or not. Maybe that solution is better? On Tue, 14 Nov 2023 at 15:29, Miroslav Lichvar <mli...@re...> wrote: > On Mon, Nov 13, 2023 at 08:47:48PM +0300, IlorDash wrote: > > I assumed, if these filters are defined in Kernel, they might be used in > > ptp4l utility to fix this. So I added adv_rx_filter config that allows > > to send SIOCSHWTSTAMP ioctl with HWTSTAMP_FILTER_PTP_XXX_SYNC or > > HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ rx filters for all transport levels. > > With this dynamic switching of the RX filter on port state changes is > there a reason to not do it by default? Or instead of adding a new > option add it as a new enum to the existing hwts_filter option? > > -- > Miroslav Lichvar > > -- Best regards, Ilya Orazov MIET Student ilo...@gm... github.com/IlorDash |
|
From: Miroslav L. <mli...@re...> - 2023-11-20 08:19:39
|
On Tue, Nov 14, 2023 at 06:50:26PM +0300, IlorDash wrote: > Honestly, I don't know, is it necessary to dynamically switch RX filters, > If the device supports EVENT RX filters, because with EVENT RX filters it > can handle all PTP messages. Maybe you could suggest appropriate behaviour > for utility in that case? Selectively timestamping only sync messages can be useful in networks with very large numbers of clients sharing the same communication path, so they don't need to timestamp each others delay requests and possibly miss the timestamp of server's sync message. If the hardware supports the type-specific filters, I think ptp4l should be able to set it. -- Miroslav Lichvar |
|
From: Erez <ere...@gm...> - 2023-11-14 20:21:42
|
On Mon, 13 Nov 2023 at 18:49, IlorDash <ilo...@gm...> wrote:
> From: Ilya Orazov <ilo...@gm...>
>
> Added adv_rx_filter config that allows to send SIOCSHWTSTAMP ioctl with
> HWTSTAMP_FILTER_PTP_XXX_SYNC or HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ
> rx filters based on whether the Device is Slave or Master respectively.
> This Feature is added for all transport levels.
> I’m using Ethernet controller with PTP support, which requires to
> determine which PTP packet on Rx it must timestamp: SYNC or DELAY_REQ
> packets based on whether the device is Slave or Masterrespectively.
Unfortunately, I’ve found out that ptp4l can’t send SIOCSHWTSTAMP ioctl
> with HWTSTAMP_FILTER_PTP_XXX_SYNC or HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ
> rx filters, and sends only EVENT rx filters to device driver.
>
This is by design, not a mistake.
Most controllers allow timestamp all RX packets.
Users usually limit, to save resources.
You can set the values using the 'hwstamp_ctl' tool.
The reason to add this functionality to ptp4l is to allow dynamic change
from
master to slave.
Please update the description to explain properly.
> Device driver can only get info about the current PTP port state
> from ptp4l, because it doesn’t have direct access to it.
>
And this helps?
> I assumed, if these filters are defined in Kernel, they might be used in
>
No need to fix the kernel.
> ptp4l utility to fix this. So I added adv_rx_filter config that allows
> to send SIOCSHWTSTAMP ioctl with HWTSTAMP_FILTER_PTP_XXX_SYNC or
> HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ rx filters for all transport levels.
>
This new feature does make sense.
But no need to talk on "Unfortunately" or "fix the kernel".
Just add explanation on why you can *NOT* use RX timestamp on all PTP
packets.
And explanation on the new feature.
>
> Signed-off-by: Ilya Orazov <ilo...@gm...>
> ---
> config.c | 1 +
> port.c | 17 ++++++++++++++
> ptp4l.8 | 8 +++++++
> ptp4l.c | 1 +
> raw.c | 13 +++++++++++
> sk.c | 56 ++++++++++++++++++++++++++++++++++++++++-----
> sk.h | 15 ++++++++++++
> transport.c | 7 ++++++
> transport.h | 5 ++++
> transport_private.h | 3 +++
> udp.c | 14 ++++++++++++
> udp6.c | 14 ++++++++++++
> 12 files changed, 148 insertions(+), 6 deletions(-)
>
> diff --git a/config.c b/config.c
> index b104f1b..642b78c 100644
> --- a/config.c
> +++ b/config.c
> @@ -268,6 +268,7 @@ struct config_item config_tab[] = {
> PORT_ITEM_INT("G.8275.portDS.localPriority", 128, 1, UINT8_MAX),
> GLOB_ITEM_INT("gmCapable", 1, 0, 1),
> GLOB_ITEM_ENU("hwts_filter", HWTS_FILTER_NORMAL, hwts_filter_enu),
> + GLOB_ITEM_INT("adv_rx_filter", 0, 0, 1),
> PORT_ITEM_INT("hybrid_e2e", 0, 0, 1),
> PORT_ITEM_INT("ignore_source_id", 0, 0, 1),
> PORT_ITEM_INT("ignore_transport_specific", 0, 0, 1),
> diff --git a/port.c b/port.c
> index 5803cd3..2376925 100644
> --- a/port.c
> +++ b/port.c
> @@ -3526,6 +3526,23 @@ int port_state_update(struct port *p, enum
> fsm_event event, int mdiff)
> p->unicast_state_dirty = true;
> }
> if (next != p->state) {
> + if (sk_adv_rx_filter == 1) {
> + pr_debug("port state update prev %d next %d",
> p->state,
> + next);
> +
> + if ((next == PS_MASTER) || (next ==
> PS_GRAND_MASTER))
> + transport_update_rx_filter(p->trp,
> p->iface,
> + &p->fda,
> + p->timestamping,
> + true);
> +
> + if (next == PS_UNCALIBRATED)
> + transport_update_rx_filter(p->trp,
> p->iface,
> + &p->fda,
> + p->timestamping,
> + false);
> + }
> +
> port_show_transition(p, next, event);
> p->state = next;
> port_notify_event(p, NOTIFY_PORT_STATE);
> diff --git a/ptp4l.8 b/ptp4l.8
> index 40c66c2..9d838eb 100644
> --- a/ptp4l.8
> +++ b/ptp4l.8
> @@ -142,6 +142,14 @@ See UNICAST DISCOVERY OPTIONS, below.
>
> .SH PORT OPTIONS
>
> +.TP
> +.B adv_rx_filter
> +Enable support of HWTSTAMP_FILTER_PTP_XX_SYNC and
> HWTSTAMP_FILTER_PTP_XX_DELAY_REQ.
> +Some Ethernet controllers doesn't support PTP event packets,
> +thus they must be provided with correct rx_filter for timestamping
> required packets
> +(SYNC or DELAY_REQ) depends on whether it's Slave or Master respectively.
> +The default is 0 or disabled.
>
Do you suggest using HWTSTAMP_FILTER_PTP_V1?
Or only HWTSTAMP_FILTER_PTP_V2?
> +
> .TP
> .B announceReceiptTimeout
> The number of missed Announce messages before the last Announce messages
> diff --git a/ptp4l.c b/ptp4l.c
> index c61175b..8f9531a 100644
> --- a/ptp4l.c
> +++ b/ptp4l.c
> @@ -191,6 +191,7 @@ int main(int argc, char *argv[])
> sk_check_fupsync = config_get_int(cfg, NULL, "check_fup_sync");
> sk_tx_timeout = config_get_int(cfg, NULL, "tx_timestamp_timeout");
> sk_hwts_filter_mode = config_get_int(cfg, NULL, "hwts_filter");
> + sk_adv_rx_filter = config_get_int(cfg, NULL, "adv_rx_filter");
>
> if (config_get_int(cfg, NULL, "clock_servo") ==
> CLOCK_SERVO_NTPSHM) {
> config_set_int(cfg, "kernel_leap", 0);
> diff --git a/raw.c b/raw.c
> index 1b978f0..7af3b2a 100644
> --- a/raw.c
> +++ b/raw.c
> @@ -363,6 +363,18 @@ no_mac:
> return -1;
> }
>
> +static int raw_update_rx_filter(struct interface *iface, struct fdarray
> *fda,
> + enum timestamp_type ts_type, bool
> is_master)
> +{
> + int err;
> + const char *name;
> +
> + name = interface_label(iface);
> + err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
> + TRANS_IEEE_802_3, is_master);
> + return err;
> +}
> +
> static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
> struct address *addr, struct hw_timestamp *hwts)
> {
> @@ -476,6 +488,7 @@ struct transport *raw_transport_create(void)
> return NULL;
> raw->t.close = raw_close;
> raw->t.open = raw_open;
> + raw->t.update_rx_filter = raw_update_rx_filter;
> raw->t.recv = raw_recv;
> raw->t.send = raw_send;
> raw->t.release = raw_release;
> diff --git a/sk.c b/sk.c
> index 19395c9..a641e45 100644
> --- a/sk.c
> +++ b/sk.c
> @@ -41,6 +41,8 @@
>
> int sk_tx_timeout = 1;
> int sk_check_fupsync;
> +int sk_tx_type = -1;
> +int sk_adv_rx_filter = -1;
> enum hwts_filter_mode sk_hwts_filter_mode = HWTS_FILTER_NORMAL;
>
> /* private methods */
> @@ -56,7 +58,7 @@ static void init_ifreq(struct ifreq *ifreq, struct
> hwtstamp_config *cfg,
> ifreq->ifr_data = (void *) cfg;
> }
>
> -static int hwts_init(int fd, const char *device, int rx_filter,
> +static int hwts_set(int fd, const char *device, int rx_filter,
> int rx_filter2, int tx_type)
> {
> struct ifreq ifreq;
> @@ -131,7 +133,7 @@ static int hwts_init(int fd, const char *device, int
> rx_filter,
> pr_err("The current filter does not match the required");
> return -1;
> }
> -
> + sk_tx_type = cfg.tx_type;
> return 0;
> }
>
> @@ -556,6 +558,36 @@ int sk_set_priority(int fd, int family, uint8_t dscp)
> return 0;
> }
>
> +int sk_ts_update_rx_filter(int fd, const char *device, enum
> timestamp_type type,
> + enum transport_type transport, bool is_master)
> +{
> + int err, filter1, filter2 = 0;
> +
> + if (type == TS_SOFTWARE)
> + return 0;
> +
> + filter1 = (is_master) ? HWTSTAMP_FILTER_PTP_V2_DELAY_REQ :
> + HWTSTAMP_FILTER_PTP_V2_SYNC;
> + switch (transport) {
> + case TRANS_UDP_IPV4:
> + case TRANS_UDP_IPV6:
> + filter2 = (is_master) ?
> HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ :
> + HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
> + break;
> + case TRANS_IEEE_802_3:
> + filter2 = (is_master) ?
> HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ :
> + HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
> + break;
> + default:
> + return -1;
> + }
> + pr_debug("update rx filter1 %d, filter2 %d, sk_tx_type %d",
> filter1,
> + filter2, sk_tx_type);
> + err = hwts_set(fd, device, filter1, filter2, sk_tx_type);
> +
> + return err;
> +}
> +
> int sk_timestamping_init(int fd, const char *device, enum timestamp_type
> type,
> enum transport_type transport, int vclock)
> {
> @@ -585,7 +617,11 @@ int sk_timestamping_init(int fd, const char *device,
> enum timestamp_type type,
> }
>
> if (type != TS_SOFTWARE) {
> - filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
> + if (sk_adv_rx_filter == 1) {
> + filter1 = HWTSTAMP_FILTER_PTP_V2_SYNC;
> + } else {
> + filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
> + }
> switch (type) {
> case TS_SOFTWARE:
> tx_type = HWTSTAMP_TX_OFF;
> @@ -604,10 +640,18 @@ int sk_timestamping_init(int fd, const char *device,
> enum timestamp_type type,
> switch (transport) {
> case TRANS_UDP_IPV4:
> case TRANS_UDP_IPV6:
> - filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
> + if (sk_adv_rx_filter == 1) {
> + filter2 = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
> + } else {
> + filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
> + }
> break;
> case TRANS_IEEE_802_3:
> - filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
> + if (sk_adv_rx_filter == 1) {
> + filter2 = HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
> + } else {
> + filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
> + }
> break;
> case TRANS_DEVICENET:
> case TRANS_CONTROLNET:
> @@ -615,7 +659,7 @@ int sk_timestamping_init(int fd, const char *device,
> enum timestamp_type type,
> case TRANS_UDS:
> return -1;
> }
> - err = hwts_init(fd, device, filter1, filter2, tx_type);
> + err = hwts_set(fd, device, filter1, filter2, tx_type);
> if (err)
> return err;
> }
> diff --git a/sk.h b/sk.h
> index 76062d0..88dc878 100644
> --- a/sk.h
> +++ b/sk.h
> @@ -146,6 +146,19 @@ int sk_get_error(int fd);
> */
> int sk_set_priority(int fd, int family, uint8_t dscp);
>
> +/**
> + * If sk_adv_rx_filter is set, then handle XXX_SYNC and
> + * XXX_DELAY_REQ hwtstamp_rx_filters.
> + * @param fd An open socket.
> + * @param device The name of the network interface to configure.
> + * @param type The requested flavor of time stamping.
> + * @param transport The type of transport used.
> + * @param is_master Is current port state is Master.
> + * @return Zero on success, non-zero otherwise.
> + */
> +int sk_ts_update_rx_filter(int fd, const char *device, enum
> timestamp_type type,
> + enum transport_type transport, bool is_master);
> +
> /**
> * Enable time stamping on a given network interface.
> * @param fd An open socket.
> @@ -171,6 +184,8 @@ extern int sk_tx_timeout;
> */
> extern int sk_check_fupsync;
>
> +extern int sk_adv_rx_filter;
> +
> /**
> * Hardware time-stamp setting mode
> */
> diff --git a/transport.c b/transport.c
> index 9366fbf..c27137b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -37,6 +37,13 @@ int transport_open(struct transport *t, struct
> interface *iface,
> return t->open(t, iface, fda, tt);
> }
>
> +int transport_update_rx_filter(struct transport *t, struct interface
> *iface,
> + struct fdarray *fda, enum timestamp_type tt,
> + bool is_master)
> +{
> + return t->update_rx_filter(iface, fda, tt, is_master);
> +}
> +
> int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
> {
> return t->recv(t, fd, msg, sizeof(msg->data), &msg->address,
> &msg->hwts);
> diff --git a/transport.h b/transport.h
> index 7a7f87b..37ef0f1 100644
> --- a/transport.h
> +++ b/transport.h
> @@ -22,6 +22,7 @@
>
> #include <time.h>
> #include <inttypes.h>
> +#include <stdbool.h>
>
> #include "fd.h"
> #include "msg.h"
> @@ -60,6 +61,10 @@ int transport_close(struct transport *t, struct fdarray
> *fda);
> int transport_open(struct transport *t, struct interface *iface,
> struct fdarray *fda, enum timestamp_type tt);
>
> +int transport_update_rx_filter(struct transport *t, struct interface
> *iface,
> + struct fdarray *fda, enum timestamp_type tt,
> + bool is_master);
> +
> int transport_recv(struct transport *t, int fd, struct ptp_message *msg);
>
> /**
> diff --git a/transport_private.h b/transport_private.h
> index ec28e47..61d277f 100644
> --- a/transport_private.h
> +++ b/transport_private.h
> @@ -35,6 +35,9 @@ struct transport {
> int (*open)(struct transport *t, struct interface *iface,
> struct fdarray *fda, enum timestamp_type tt);
>
> + int (*update_rx_filter)(struct interface *iface, struct fdarray
> *fda,
> + enum timestamp_type ts_type, bool
> is_master);
> +
> int (*recv)(struct transport *t, int fd, void *buf, int buflen,
> struct address *addr, struct hw_timestamp *hwts);
>
> diff --git a/udp.c b/udp.c
> index 7c9402e..b66ff7e 100644
> --- a/udp.c
> +++ b/udp.c
> @@ -208,6 +208,19 @@ no_event:
> return -1;
> }
>
> +static int udp_update_rx_filter(struct interface *iface, struct fdarray
> *fda,
> + enum timestamp_type ts_type, bool
> is_master)
> +{
> + int err;
> + const char *name;
> +
> + name = interface_label(iface);
> + err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
> + TRANS_UDP_IPV4, is_master);
> +
> + return err;
> +}
> +
> static int udp_recv(struct transport *t, int fd, void *buf, int buflen,
> struct address *addr, struct hw_timestamp *hwts)
> {
> @@ -302,6 +315,7 @@ struct transport *udp_transport_create(void)
> return NULL;
> udp->t.close = udp_close;
> udp->t.open = udp_open;
> + udp->t.update_rx_filter = udp_update_rx_filter;
> udp->t.recv = udp_recv;
> udp->t.send = udp_send;
> udp->t.release = udp_release;
> diff --git a/udp6.c b/udp6.c
> index bde1710..6a75962 100644
> --- a/udp6.c
> +++ b/udp6.c
> @@ -225,6 +225,19 @@ no_event:
> return -1;
> }
>
> +static int udp6_update_rx_filter(struct interface *iface, struct fdarray
> *fda,
> + enum timestamp_type ts_type, bool
> is_master)
> +{
> + int err;
> + const char *name;
> +
> + name = interface_label(iface);
> + err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
> + TRANS_UDP_IPV6, is_master);
> +
> + return err;
> +}
> +
> static int udp6_recv(struct transport *t, int fd, void *buf, int buflen,
> struct address *addr, struct hw_timestamp *hwts)
> {
> @@ -318,6 +331,7 @@ struct transport *udp6_transport_create(void)
> return NULL;
> udp6->t.close = udp6_close;
> udp6->t.open = udp6_open;
> + udp6->t.update_rx_filter = udp6_update_rx_filter;
> udp6->t.recv = udp6_recv;
> udp6->t.send = udp6_send;
> udp6->t.release = udp6_release;
> --
> 2.34.1
>
>
>
> _______________________________________________
> Linuxptp-devel mailing list
> Lin...@li...
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
|
|
From: IlorDash <ilo...@gm...> - 2023-11-15 08:27:55
|
>
>
>> Device driver can only get info about the current PTP port state
>> from ptp4l, because it doesn’t have direct access to it.
>>
>
> And this helps?
>
Yes, with dynamically switching RX filters I was able to change my Ethernet
controller state from Master to Slave and vice versa.
I meant a little different in my description, but I'll consider your
recommendations and update the description.
Do you suggest using HWTSTAMP_FILTER_PTP_V1?
> Or only HWTSTAMP_FILTER_PTP_V2?
>
I suggest using only HWTSTAMP_FILTER_PTP_V2 filters. I'll also mention it
in the description.
On Tue, 14 Nov 2023 at 23:21, Erez <ere...@gm...> wrote:
>
>
> On Tue, 14 Nov 2023 at 23:21, Erez <ere...@gm...> wrote:
>
>
> On Mon, 13 Nov 2023 at 18:49, IlorDash <ilo...@gm...> wrote:
>
>> From: Ilya Orazov <ilo...@gm...>
>>
>> Added adv_rx_filter config that allows to send SIOCSHWTSTAMP ioctl with
>> HWTSTAMP_FILTER_PTP_XXX_SYNC or HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ
>> rx filters based on whether the Device is Slave or Master respectively.
>> This Feature is added for all transport levels.
>
>
>> I’m using Ethernet controller with PTP support, which requires to
>> determine which PTP packet on Rx it must timestamp: SYNC or DELAY_REQ
>> packets based on whether the device is Slave or Masterrespectively.
>
> Unfortunately, I’ve found out that ptp4l can’t send SIOCSHWTSTAMP ioctl
>> with HWTSTAMP_FILTER_PTP_XXX_SYNC or HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ
>> rx filters, and sends only EVENT rx filters to device driver.
>>
>
> This is by design, not a mistake.
> Most controllers allow timestamp all RX packets.
> Users usually limit, to save resources.
>
> You can set the values using the 'hwstamp_ctl' tool.
>
> The reason to add this functionality to ptp4l is to allow dynamic change
> from
> master to slave.
>
> Please update the description to explain properly.
>
>
>
>> Device driver can only get info about the current PTP port state
>> from ptp4l, because it doesn’t have direct access to it.
>>
>
> And this helps?
>
>
>> I assumed, if these filters are defined in Kernel, they might be used in
>>
>
> No need to fix the kernel.
>
>
>> ptp4l utility to fix this. So I added adv_rx_filter config that allows
>> to send SIOCSHWTSTAMP ioctl with HWTSTAMP_FILTER_PTP_XXX_SYNC or
>> HWTSTAMP_FILTER_PTP_XXX_DELAY_REQ rx filters for all transport levels.
>>
>
> This new feature does make sense.
> But no need to talk on "Unfortunately" or "fix the kernel".
> Just add explanation on why you can *NOT* use RX timestamp on all PTP
> packets.
> And explanation on the new feature.
>
>
>>
>> Signed-off-by: Ilya Orazov <ilo...@gm...>
>> ---
>> config.c | 1 +
>> port.c | 17 ++++++++++++++
>> ptp4l.8 | 8 +++++++
>> ptp4l.c | 1 +
>> raw.c | 13 +++++++++++
>> sk.c | 56 ++++++++++++++++++++++++++++++++++++++++-----
>> sk.h | 15 ++++++++++++
>> transport.c | 7 ++++++
>> transport.h | 5 ++++
>> transport_private.h | 3 +++
>> udp.c | 14 ++++++++++++
>> udp6.c | 14 ++++++++++++
>> 12 files changed, 148 insertions(+), 6 deletions(-)
>>
>> diff --git a/config.c b/config.c
>> index b104f1b..642b78c 100644
>> --- a/config.c
>> +++ b/config.c
>> @@ -268,6 +268,7 @@ struct config_item config_tab[] = {
>> PORT_ITEM_INT("G.8275.portDS.localPriority", 128, 1, UINT8_MAX),
>> GLOB_ITEM_INT("gmCapable", 1, 0, 1),
>> GLOB_ITEM_ENU("hwts_filter", HWTS_FILTER_NORMAL, hwts_filter_enu),
>> + GLOB_ITEM_INT("adv_rx_filter", 0, 0, 1),
>> PORT_ITEM_INT("hybrid_e2e", 0, 0, 1),
>> PORT_ITEM_INT("ignore_source_id", 0, 0, 1),
>> PORT_ITEM_INT("ignore_transport_specific", 0, 0, 1),
>> diff --git a/port.c b/port.c
>> index 5803cd3..2376925 100644
>> --- a/port.c
>> +++ b/port.c
>> @@ -3526,6 +3526,23 @@ int port_state_update(struct port *p, enum
>> fsm_event event, int mdiff)
>> p->unicast_state_dirty = true;
>> }
>> if (next != p->state) {
>> + if (sk_adv_rx_filter == 1) {
>> + pr_debug("port state update prev %d next %d",
>> p->state,
>> + next);
>> +
>> + if ((next == PS_MASTER) || (next ==
>> PS_GRAND_MASTER))
>> + transport_update_rx_filter(p->trp,
>> p->iface,
>> + &p->fda,
>> +
>> p->timestamping,
>> + true);
>> +
>> + if (next == PS_UNCALIBRATED)
>> + transport_update_rx_filter(p->trp,
>> p->iface,
>> + &p->fda,
>> +
>> p->timestamping,
>> + false);
>> + }
>> +
>> port_show_transition(p, next, event);
>> p->state = next;
>> port_notify_event(p, NOTIFY_PORT_STATE);
>> diff --git a/ptp4l.8 b/ptp4l.8
>> index 40c66c2..9d838eb 100644
>> --- a/ptp4l.8
>> +++ b/ptp4l.8
>> @@ -142,6 +142,14 @@ See UNICAST DISCOVERY OPTIONS, below.
>>
>> .SH PORT OPTIONS
>>
>> +.TP
>> +.B adv_rx_filter
>> +Enable support of HWTSTAMP_FILTER_PTP_XX_SYNC and
>> HWTSTAMP_FILTER_PTP_XX_DELAY_REQ.
>> +Some Ethernet controllers doesn't support PTP event packets,
>> +thus they must be provided with correct rx_filter for timestamping
>> required packets
>> +(SYNC or DELAY_REQ) depends on whether it's Slave or Master respectively.
>> +The default is 0 or disabled.
>>
>
> Do you suggest using HWTSTAMP_FILTER_PTP_V1?
> Or only HWTSTAMP_FILTER_PTP_V2?
>
>
>
>> +
>> .TP
>> .B announceReceiptTimeout
>> The number of missed Announce messages before the last Announce messages
>> diff --git a/ptp4l.c b/ptp4l.c
>> index c61175b..8f9531a 100644
>> --- a/ptp4l.c
>> +++ b/ptp4l.c
>> @@ -191,6 +191,7 @@ int main(int argc, char *argv[])
>> sk_check_fupsync = config_get_int(cfg, NULL, "check_fup_sync");
>> sk_tx_timeout = config_get_int(cfg, NULL, "tx_timestamp_timeout");
>> sk_hwts_filter_mode = config_get_int(cfg, NULL, "hwts_filter");
>> + sk_adv_rx_filter = config_get_int(cfg, NULL, "adv_rx_filter");
>>
>> if (config_get_int(cfg, NULL, "clock_servo") ==
>> CLOCK_SERVO_NTPSHM) {
>> config_set_int(cfg, "kernel_leap", 0);
>> diff --git a/raw.c b/raw.c
>> index 1b978f0..7af3b2a 100644
>> --- a/raw.c
>> +++ b/raw.c
>> @@ -363,6 +363,18 @@ no_mac:
>> return -1;
>> }
>>
>> +static int raw_update_rx_filter(struct interface *iface, struct fdarray
>> *fda,
>> + enum timestamp_type ts_type, bool
>> is_master)
>> +{
>> + int err;
>> + const char *name;
>> +
>> + name = interface_label(iface);
>> + err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
>> + TRANS_IEEE_802_3, is_master);
>> + return err;
>> +}
>> +
>> static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
>> struct address *addr, struct hw_timestamp *hwts)
>> {
>> @@ -476,6 +488,7 @@ struct transport *raw_transport_create(void)
>> return NULL;
>> raw->t.close = raw_close;
>> raw->t.open = raw_open;
>> + raw->t.update_rx_filter = raw_update_rx_filter;
>> raw->t.recv = raw_recv;
>> raw->t.send = raw_send;
>> raw->t.release = raw_release;
>> diff --git a/sk.c b/sk.c
>> index 19395c9..a641e45 100644
>> --- a/sk.c
>> +++ b/sk.c
>> @@ -41,6 +41,8 @@
>>
>> int sk_tx_timeout = 1;
>> int sk_check_fupsync;
>> +int sk_tx_type = -1;
>> +int sk_adv_rx_filter = -1;
>> enum hwts_filter_mode sk_hwts_filter_mode = HWTS_FILTER_NORMAL;
>>
>> /* private methods */
>> @@ -56,7 +58,7 @@ static void init_ifreq(struct ifreq *ifreq, struct
>> hwtstamp_config *cfg,
>> ifreq->ifr_data = (void *) cfg;
>> }
>>
>> -static int hwts_init(int fd, const char *device, int rx_filter,
>> +static int hwts_set(int fd, const char *device, int rx_filter,
>> int rx_filter2, int tx_type)
>> {
>> struct ifreq ifreq;
>> @@ -131,7 +133,7 @@ static int hwts_init(int fd, const char *device, int
>> rx_filter,
>> pr_err("The current filter does not match the required");
>> return -1;
>> }
>> -
>> + sk_tx_type = cfg.tx_type;
>> return 0;
>> }
>>
>> @@ -556,6 +558,36 @@ int sk_set_priority(int fd, int family, uint8_t dscp)
>> return 0;
>> }
>>
>> +int sk_ts_update_rx_filter(int fd, const char *device, enum
>> timestamp_type type,
>> + enum transport_type transport, bool is_master)
>> +{
>> + int err, filter1, filter2 = 0;
>> +
>> + if (type == TS_SOFTWARE)
>> + return 0;
>> +
>> + filter1 = (is_master) ? HWTSTAMP_FILTER_PTP_V2_DELAY_REQ :
>> + HWTSTAMP_FILTER_PTP_V2_SYNC;
>> + switch (transport) {
>> + case TRANS_UDP_IPV4:
>> + case TRANS_UDP_IPV6:
>> + filter2 = (is_master) ?
>> HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ :
>> + HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
>> + break;
>> + case TRANS_IEEE_802_3:
>> + filter2 = (is_master) ?
>> HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ :
>> + HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
>> + break;
>> + default:
>> + return -1;
>> + }
>> + pr_debug("update rx filter1 %d, filter2 %d, sk_tx_type %d",
>> filter1,
>> + filter2, sk_tx_type);
>> + err = hwts_set(fd, device, filter1, filter2, sk_tx_type);
>> +
>> + return err;
>> +}
>> +
>> int sk_timestamping_init(int fd, const char *device, enum timestamp_type
>> type,
>> enum transport_type transport, int vclock)
>> {
>> @@ -585,7 +617,11 @@ int sk_timestamping_init(int fd, const char *device,
>> enum timestamp_type type,
>> }
>>
>> if (type != TS_SOFTWARE) {
>> - filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
>> + if (sk_adv_rx_filter == 1) {
>> + filter1 = HWTSTAMP_FILTER_PTP_V2_SYNC;
>> + } else {
>> + filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
>> + }
>> switch (type) {
>> case TS_SOFTWARE:
>> tx_type = HWTSTAMP_TX_OFF;
>> @@ -604,10 +640,18 @@ int sk_timestamping_init(int fd, const char
>> *device, enum timestamp_type type,
>> switch (transport) {
>> case TRANS_UDP_IPV4:
>> case TRANS_UDP_IPV6:
>> - filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
>> + if (sk_adv_rx_filter == 1) {
>> + filter2 = HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
>> + } else {
>> + filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
>> + }
>> break;
>> case TRANS_IEEE_802_3:
>> - filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>> + if (sk_adv_rx_filter == 1) {
>> + filter2 = HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
>> + } else {
>> + filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
>> + }
>> break;
>> case TRANS_DEVICENET:
>> case TRANS_CONTROLNET:
>> @@ -615,7 +659,7 @@ int sk_timestamping_init(int fd, const char *device,
>> enum timestamp_type type,
>> case TRANS_UDS:
>> return -1;
>> }
>> - err = hwts_init(fd, device, filter1, filter2, tx_type);
>> + err = hwts_set(fd, device, filter1, filter2, tx_type);
>> if (err)
>> return err;
>> }
>> diff --git a/sk.h b/sk.h
>> index 76062d0..88dc878 100644
>> --- a/sk.h
>> +++ b/sk.h
>> @@ -146,6 +146,19 @@ int sk_get_error(int fd);
>> */
>> int sk_set_priority(int fd, int family, uint8_t dscp);
>>
>> +/**
>> + * If sk_adv_rx_filter is set, then handle XXX_SYNC and
>> + * XXX_DELAY_REQ hwtstamp_rx_filters.
>> + * @param fd An open socket.
>> + * @param device The name of the network interface to configure.
>> + * @param type The requested flavor of time stamping.
>> + * @param transport The type of transport used.
>> + * @param is_master Is current port state is Master.
>> + * @return Zero on success, non-zero otherwise.
>> + */
>> +int sk_ts_update_rx_filter(int fd, const char *device, enum
>> timestamp_type type,
>> + enum transport_type transport, bool is_master);
>> +
>> /**
>> * Enable time stamping on a given network interface.
>> * @param fd An open socket.
>> @@ -171,6 +184,8 @@ extern int sk_tx_timeout;
>> */
>> extern int sk_check_fupsync;
>>
>> +extern int sk_adv_rx_filter;
>> +
>> /**
>> * Hardware time-stamp setting mode
>> */
>> diff --git a/transport.c b/transport.c
>> index 9366fbf..c27137b 100644
>> --- a/transport.c
>> +++ b/transport.c
>> @@ -37,6 +37,13 @@ int transport_open(struct transport *t, struct
>> interface *iface,
>> return t->open(t, iface, fda, tt);
>> }
>>
>> +int transport_update_rx_filter(struct transport *t, struct interface
>> *iface,
>> + struct fdarray *fda, enum timestamp_type
>> tt,
>> + bool is_master)
>> +{
>> + return t->update_rx_filter(iface, fda, tt, is_master);
>> +}
>> +
>> int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
>> {
>> return t->recv(t, fd, msg, sizeof(msg->data), &msg->address,
>> &msg->hwts);
>> diff --git a/transport.h b/transport.h
>> index 7a7f87b..37ef0f1 100644
>> --- a/transport.h
>> +++ b/transport.h
>> @@ -22,6 +22,7 @@
>>
>> #include <time.h>
>> #include <inttypes.h>
>> +#include <stdbool.h>
>>
>> #include "fd.h"
>> #include "msg.h"
>> @@ -60,6 +61,10 @@ int transport_close(struct transport *t, struct
>> fdarray *fda);
>> int transport_open(struct transport *t, struct interface *iface,
>> struct fdarray *fda, enum timestamp_type tt);
>>
>> +int transport_update_rx_filter(struct transport *t, struct interface
>> *iface,
>> + struct fdarray *fda, enum timestamp_type
>> tt,
>> + bool is_master);
>> +
>> int transport_recv(struct transport *t, int fd, struct ptp_message *msg);
>>
>> /**
>> diff --git a/transport_private.h b/transport_private.h
>> index ec28e47..61d277f 100644
>> --- a/transport_private.h
>> +++ b/transport_private.h
>> @@ -35,6 +35,9 @@ struct transport {
>> int (*open)(struct transport *t, struct interface *iface,
>> struct fdarray *fda, enum timestamp_type tt);
>>
>> + int (*update_rx_filter)(struct interface *iface, struct fdarray
>> *fda,
>> + enum timestamp_type ts_type, bool
>> is_master);
>> +
>> int (*recv)(struct transport *t, int fd, void *buf, int buflen,
>> struct address *addr, struct hw_timestamp *hwts);
>>
>> diff --git a/udp.c b/udp.c
>> index 7c9402e..b66ff7e 100644
>> --- a/udp.c
>> +++ b/udp.c
>> @@ -208,6 +208,19 @@ no_event:
>> return -1;
>> }
>>
>> +static int udp_update_rx_filter(struct interface *iface, struct fdarray
>> *fda,
>> + enum timestamp_type ts_type, bool
>> is_master)
>> +{
>> + int err;
>> + const char *name;
>> +
>> + name = interface_label(iface);
>> + err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
>> + TRANS_UDP_IPV4, is_master);
>> +
>> + return err;
>> +}
>> +
>> static int udp_recv(struct transport *t, int fd, void *buf, int buflen,
>> struct address *addr, struct hw_timestamp *hwts)
>> {
>> @@ -302,6 +315,7 @@ struct transport *udp_transport_create(void)
>> return NULL;
>> udp->t.close = udp_close;
>> udp->t.open = udp_open;
>> + udp->t.update_rx_filter = udp_update_rx_filter;
>> udp->t.recv = udp_recv;
>> udp->t.send = udp_send;
>> udp->t.release = udp_release;
>> diff --git a/udp6.c b/udp6.c
>> index bde1710..6a75962 100644
>> --- a/udp6.c
>> +++ b/udp6.c
>> @@ -225,6 +225,19 @@ no_event:
>> return -1;
>> }
>>
>> +static int udp6_update_rx_filter(struct interface *iface, struct fdarray
>> *fda,
>> + enum timestamp_type ts_type, bool
>> is_master)
>> +{
>> + int err;
>> + const char *name;
>> +
>> + name = interface_label(iface);
>> + err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
>> + TRANS_UDP_IPV6, is_master);
>> +
>> + return err;
>> +}
>> +
>> static int udp6_recv(struct transport *t, int fd, void *buf, int buflen,
>> struct address *addr, struct hw_timestamp *hwts)
>> {
>> @@ -318,6 +331,7 @@ struct transport *udp6_transport_create(void)
>> return NULL;
>> udp6->t.close = udp6_close;
>> udp6->t.open = udp6_open;
>> + udp6->t.update_rx_filter = udp6_update_rx_filter;
>> udp6->t.recv = udp6_recv;
>> udp6->t.send = udp6_send;
>> udp6->t.release = udp6_release;
>> --
>> 2.34.1
>>
>>
>>
>> _______________________________________________
>> Linuxptp-devel mailing list
>> Lin...@li...
>> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>>
>
--
Best regards,
Ilya Orazov
|
|
From: IlorDash <ilo...@gm...> - 2023-11-24 19:30:19
|
From: Ilya Orazov <ilo...@gm...>
I’m using an Ethernet controller with PTP support, which requires
determining which PTP packets on RX must be timestamped: SYNC or
DELAY_REQ, based on whether the device is Slave or Master respectively.
So I can’t use the EVENT RX filter and must dynamically switch RX
filters, when port state changes from Master to Slave and vice versa.
Therefore this feature adds support for DELAY_REQ and SYNC packets
RX filters. If the interface doesn’t support the EVENT RX filter
(getting info about it through ethtool) then RX filters will dynamically
switch between SYNC and DELAY_REQ when port state changes.
Signed-off-by: Ilya Orazov <ilo...@gm...>
---
clock.h | 3 +-
interface.c | 10 +++++
interface.h | 7 ++++
port.c | 10 +++++
raw.c | 17 +++++++-
sk.c | 100 +++++++++++++++++++++++++++++++++-----------
sk.h | 19 ++++++++-
transport.c | 7 ++++
transport.h | 5 +++
transport_private.h | 3 ++
udp.c | 20 ++++++++-
udp6.c | 18 +++++++-
12 files changed, 188 insertions(+), 31 deletions(-)
diff --git a/clock.h b/clock.h
index ce9ae91..43066dd 100644
--- a/clock.h
+++ b/clock.h
@@ -113,8 +113,7 @@ int clock_required_modes(struct clock *c);
* selection based on the network interface.
* @return A pointer to the single global clock instance.
*/
-struct clock *clock_create(enum clock_type type, struct config *config,
- const char *phc_device);
+struct clock *clock_create(enum clock_type type, struct config *config, const char *phc_device);
/**
* Obtains a clock's default data set.
diff --git a/interface.c b/interface.c
index 29229ad..fe031c8 100644
--- a/interface.c
+++ b/interface.c
@@ -7,6 +7,8 @@
#include <stdlib.h>
#include "interface.h"
+#define HWTSTAMP_FILTER_PTP_V2_XX_EVENT 0x1240
+
struct interface {
STAILQ_ENTRY(interface) list;
char name[MAX_IFNAME_SIZE + 1];
@@ -85,6 +87,14 @@ bool interface_tsmodes_supported(struct interface *iface, int modes)
return false;
}
+bool interface_check_rxfilters_event(struct interface *iface)
+{
+ if ((iface->ts_info.rx_filters & HWTSTAMP_FILTER_PTP_V2_XX_EVENT) > 0) {
+ return true;
+ }
+ return false;
+}
+
void interface_set_vclock(struct interface *iface, int vclock)
{
iface->vclock = vclock;
diff --git a/interface.h b/interface.h
index 0873bba..70e887c 100644
--- a/interface.h
+++ b/interface.h
@@ -99,6 +99,13 @@ bool interface_ifinfo_valid(struct interface *iface);
*/
bool interface_tsmodes_supported(struct interface *iface, int modes);
+/**
+ * Tests whether an interface supports a HWTSTAMP_FILTER_PTP_V2_XX_EVENT.
+ * @param iface The interface of interest.
+ * @return True if the HWTSTAMP_FILTER_PTP_V2_XX_EVENT is supported, false otherwise.
+ */
+bool interface_check_rxfilters_event(struct interface *iface);
+
/**
* Set the vclock (virtual PHC) to be used for timestamping on an interface.
* @param iface The interface of interest.
diff --git a/port.c b/port.c
index 5803cd3..98931ab 100644
--- a/port.c
+++ b/port.c
@@ -3526,6 +3526,16 @@ int port_state_update(struct port *p, enum fsm_event event, int mdiff)
p->unicast_state_dirty = true;
}
if (next != p->state) {
+ pr_debug("port state update prev %d next %d", p->state, next);
+
+ if ((next == PS_MASTER) || (next == PS_GRAND_MASTER) ||
+ (next == PS_UNCALIBRATED)) {
+ bool is_state_master = (next == PS_MASTER) ||
+ (next == PS_GRAND_MASTER);
+ transport_update_rx_filter(p->trp, p->iface, &p->fda,
+ p->timestamping,
+ is_state_master);
+ }
port_show_transition(p, next, event);
p->state = next;
port_notify_event(p, NOTIFY_PORT_STATE);
diff --git a/raw.c b/raw.c
index 1b978f0..d5cf56b 100644
--- a/raw.c
+++ b/raw.c
@@ -344,7 +344,8 @@ static int raw_open(struct transport *t, struct interface *iface,
goto no_general;
if (sk_timestamping_init(efd, name, ts_type, TRANS_IEEE_802_3,
- interface_get_vclock(iface)))
+ interface_get_vclock(iface),
+ interface_check_rxfilters_event(iface)))
goto no_timestamping;
if (sk_general_init(gfd))
@@ -363,6 +364,19 @@ no_mac:
return -1;
}
+static int raw_update_rx_filter(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master)
+{
+ int err;
+ const char *name;
+
+ name = interface_label(iface);
+ err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
+ TRANS_IEEE_802_3, is_master,
+ interface_check_rxfilters_event(iface));
+ return err;
+}
+
static int raw_recv(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts)
{
@@ -476,6 +490,7 @@ struct transport *raw_transport_create(void)
return NULL;
raw->t.close = raw_close;
raw->t.open = raw_open;
+ raw->t.update_rx_filter = raw_update_rx_filter;
raw->t.recv = raw_recv;
raw->t.send = raw_send;
raw->t.release = raw_release;
diff --git a/sk.c b/sk.c
index 19395c9..de10698 100644
--- a/sk.c
+++ b/sk.c
@@ -41,6 +41,7 @@
int sk_tx_timeout = 1;
int sk_check_fupsync;
+int sk_tx_type = HWTSTAMP_TX_ON;
enum hwts_filter_mode sk_hwts_filter_mode = HWTS_FILTER_NORMAL;
/* private methods */
@@ -56,7 +57,7 @@ static void init_ifreq(struct ifreq *ifreq, struct hwtstamp_config *cfg,
ifreq->ifr_data = (void *) cfg;
}
-static int hwts_init(int fd, const char *device, int rx_filter,
+static int hwts_set(int fd, const char *device, int rx_filter,
int rx_filter2, int tx_type)
{
struct ifreq ifreq;
@@ -131,7 +132,6 @@ static int hwts_init(int fd, const char *device, int rx_filter,
pr_err("The current filter does not match the required");
return -1;
}
-
return 0;
}
@@ -556,10 +556,70 @@ int sk_set_priority(int fd, int family, uint8_t dscp)
return 0;
}
+int sk_ts_get_rx_filter(enum timestamp_type type, enum transport_type transport,
+ bool is_master, bool filter_event_supported,
+ int *filter1, int *filter2)
+{
+ *filter1 = 0;
+ *filter2 = 0;
+
+ if (type == TS_SOFTWARE)
+ return 0;
+
+ if (filter_event_supported) {
+ *filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
+ } else {
+ *filter1 = (is_master) ? HWTSTAMP_FILTER_PTP_V2_DELAY_REQ :
+ HWTSTAMP_FILTER_PTP_V2_SYNC;
+ }
+
+ switch (transport) {
+ case TRANS_UDP_IPV4:
+ case TRANS_UDP_IPV6:
+ if (filter_event_supported)
+ *filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
+ else
+ *filter2 = (is_master) ?
+ HWTSTAMP_FILTER_PTP_V2_L4_DELAY_REQ :
+ HWTSTAMP_FILTER_PTP_V2_L4_SYNC;
+ break;
+ case TRANS_IEEE_802_3:
+ if (filter_event_supported)
+ *filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
+ else
+ *filter2 = (is_master) ?
+ HWTSTAMP_FILTER_PTP_V2_L2_DELAY_REQ :
+ HWTSTAMP_FILTER_PTP_V2_L2_SYNC;
+ break;
+ default:
+ return -1;
+ }
+
+ return 0;
+}
+
+int sk_ts_update_rx_filter(int fd, const char *device, enum timestamp_type type,
+ enum transport_type transport, bool is_master,
+ bool filter_event_supported)
+{
+ int err, filter1, filter2;
+
+ err = sk_ts_get_rx_filter(type, transport, is_master,
+ filter_event_supported, &filter1, &filter2);
+ if (err)
+ return err;
+
+ pr_debug("update rx filter1 %d, filter2 %d", filter1, filter2);
+
+ err = hwts_set(fd, device, filter1, filter2, sk_tx_type);
+ return err;
+}
+
int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
- enum transport_type transport, int vclock)
+ enum transport_type transport, int vclock,
+ bool filter_event_supported)
{
- int err, filter1, filter2 = 0, flags, tx_type = HWTSTAMP_TX_ON;
+ int err, filter1, filter2 = 0, flags;
struct so_timestamping timestamping;
switch (type) {
@@ -585,37 +645,29 @@ int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
}
if (type != TS_SOFTWARE) {
- filter1 = HWTSTAMP_FILTER_PTP_V2_EVENT;
switch (type) {
case TS_SOFTWARE:
- tx_type = HWTSTAMP_TX_OFF;
+ sk_tx_type = HWTSTAMP_TX_OFF;
break;
case TS_HARDWARE:
case TS_LEGACY_HW:
- tx_type = HWTSTAMP_TX_ON;
+ sk_tx_type = HWTSTAMP_TX_ON;
break;
case TS_ONESTEP:
- tx_type = HWTSTAMP_TX_ONESTEP_SYNC;
+ sk_tx_type = HWTSTAMP_TX_ONESTEP_SYNC;
break;
case TS_P2P1STEP:
- tx_type = HWTSTAMP_TX_ONESTEP_P2P;
+ sk_tx_type = HWTSTAMP_TX_ONESTEP_P2P;
break;
}
- switch (transport) {
- case TRANS_UDP_IPV4:
- case TRANS_UDP_IPV6:
- filter2 = HWTSTAMP_FILTER_PTP_V2_L4_EVENT;
- break;
- case TRANS_IEEE_802_3:
- filter2 = HWTSTAMP_FILTER_PTP_V2_L2_EVENT;
- break;
- case TRANS_DEVICENET:
- case TRANS_CONTROLNET:
- case TRANS_PROFINET:
- case TRANS_UDS:
- return -1;
- }
- err = hwts_init(fd, device, filter1, filter2, tx_type);
+
+ err = sk_ts_get_rx_filter(type, transport, false,
+ filter_event_supported, &filter1,
+ &filter2);
+ if (err)
+ return err;
+
+ err = hwts_set(fd, device, filter1, filter2, sk_tx_type);
if (err)
return err;
}
diff --git a/sk.h b/sk.h
index 76062d0..30e50f0 100644
--- a/sk.h
+++ b/sk.h
@@ -146,6 +146,22 @@ int sk_get_error(int fd);
*/
int sk_set_priority(int fd, int family, uint8_t dscp);
+/**
+ * If interface doesn't support HWTSTAMP_FILTER_PTP_V2_XXX_EVENT RX filters,
+ * and support only HWTSTAMP_FILTER_PTP_V2_XXX_SYNC and
+ * HWTSTAMP_FILTER_PTP_V2_XXX_DELAY_REQ update it's RX filters
+ * when port state is changed.
+ * @param fd An open socket.
+ * @param device The name of the network interface to configure.
+ * @param type The requested flavor of time stamping.
+ * @param transport The type of transport used.
+ * @param is_master Is current port state is Master.
+ * @return Zero on success, non-zero otherwise.
+ */
+int sk_ts_update_rx_filter(int fd, const char *device, enum timestamp_type type,
+ enum transport_type transport, bool is_master,
+ bool filter_all_supported);
+
/**
* Enable time stamping on a given network interface.
* @param fd An open socket.
@@ -156,7 +172,8 @@ int sk_set_priority(int fd, int family, uint8_t dscp);
* @return Zero on success, non-zero otherwise.
*/
int sk_timestamping_init(int fd, const char *device, enum timestamp_type type,
- enum transport_type transport, int vclock);
+ enum transport_type transport, int vclock,
+ bool filter_event_supported);
/**
* Limits the time that RECVMSG(2) will poll while waiting for the tx timestamp
diff --git a/transport.c b/transport.c
index 9366fbf..c27137b 100644
--- a/transport.c
+++ b/transport.c
@@ -37,6 +37,13 @@ int transport_open(struct transport *t, struct interface *iface,
return t->open(t, iface, fda, tt);
}
+int transport_update_rx_filter(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type tt,
+ bool is_master)
+{
+ return t->update_rx_filter(iface, fda, tt, is_master);
+}
+
int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
{
return t->recv(t, fd, msg, sizeof(msg->data), &msg->address, &msg->hwts);
diff --git a/transport.h b/transport.h
index 7a7f87b..37ef0f1 100644
--- a/transport.h
+++ b/transport.h
@@ -22,6 +22,7 @@
#include <time.h>
#include <inttypes.h>
+#include <stdbool.h>
#include "fd.h"
#include "msg.h"
@@ -60,6 +61,10 @@ int transport_close(struct transport *t, struct fdarray *fda);
int transport_open(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type tt);
+int transport_update_rx_filter(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type tt,
+ bool is_master);
+
int transport_recv(struct transport *t, int fd, struct ptp_message *msg);
/**
diff --git a/transport_private.h b/transport_private.h
index ec28e47..61d277f 100644
--- a/transport_private.h
+++ b/transport_private.h
@@ -35,6 +35,9 @@ struct transport {
int (*open)(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type tt);
+ int (*update_rx_filter)(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master);
+
int (*recv)(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts);
diff --git a/udp.c b/udp.c
index 7c9402e..050cda9 100644
--- a/udp.c
+++ b/udp.c
@@ -179,8 +179,9 @@ static int udp_open(struct transport *t, struct interface *iface,
if (gfd < 0)
goto no_general;
- if (sk_timestamping_init(efd, interface_label(iface), ts_type, TRANS_UDP_IPV4,
- interface_get_vclock(iface)))
+ if (sk_timestamping_init(efd, interface_label(iface), ts_type,
+ TRANS_UDP_IPV4, interface_get_vclock(iface),
+ interface_check_rxfilters_event(iface)))
goto no_timestamping;
if (sk_general_init(gfd))
@@ -208,6 +209,20 @@ no_event:
return -1;
}
+static int udp_update_rx_filter(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master)
+{
+ int err;
+ const char *name;
+
+ name = interface_label(iface);
+ err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
+ TRANS_UDP_IPV4, is_master,
+ interface_check_rxfilters_event(iface));
+
+ return err;
+}
+
static int udp_recv(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts)
{
@@ -302,6 +317,7 @@ struct transport *udp_transport_create(void)
return NULL;
udp->t.close = udp_close;
udp->t.open = udp_open;
+ udp->t.update_rx_filter = udp_update_rx_filter;
udp->t.recv = udp_recv;
udp->t.send = udp_send;
udp->t.release = udp_release;
diff --git a/udp6.c b/udp6.c
index bde1710..cd5a479 100644
--- a/udp6.c
+++ b/udp6.c
@@ -197,7 +197,8 @@ static int udp6_open(struct transport *t, struct interface *iface,
goto no_general;
if (sk_timestamping_init(efd, interface_label(iface), ts_type,
- TRANS_UDP_IPV6, interface_get_vclock(iface)))
+ TRANS_UDP_IPV6, interface_get_vclock(iface),
+ interface_check_rxfilters_event(iface)))
goto no_timestamping;
if (sk_general_init(gfd))
@@ -225,6 +226,20 @@ no_event:
return -1;
}
+static int udp6_update_rx_filter(struct interface *iface, struct fdarray *fda,
+ enum timestamp_type ts_type, bool is_master)
+{
+ int err;
+ const char *name;
+
+ name = interface_label(iface);
+ err = sk_ts_update_rx_filter(fda->fd[FD_EVENT], name, ts_type,
+ TRANS_UDP_IPV6, is_master,
+ interface_check_rxfilters_event(iface));
+
+ return err;
+}
+
static int udp6_recv(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts)
{
@@ -318,6 +333,7 @@ struct transport *udp6_transport_create(void)
return NULL;
udp6->t.close = udp6_close;
udp6->t.open = udp6_open;
+ udp6->t.update_rx_filter = udp6_update_rx_filter;
udp6->t.recv = udp6_recv;
udp6->t.send = udp6_send;
udp6->t.release = udp6_release;
--
2.34.1
|
|
From: Vladimir O. <ol...@gm...> - 2023-11-29 17:19:25
|
Hi Ilia, On Fri, Nov 24, 2023 at 10:29:12PM +0300, IlorDash wrote: > From: Ilya Orazov <ilo...@gm...> > > I’m using an Ethernet controller with PTP support, which requires > determining which PTP packets on RX must be timestamped: SYNC or > DELAY_REQ, based on whether the device is Slave or Master respectively. > So I can’t use the EVENT RX filter and must dynamically switch RX > filters, when port state changes from Master to Slave and vice versa. > > Therefore this feature adds support for DELAY_REQ and SYNC packets > RX filters. If the interface doesn’t support the EVENT RX filter > (getting info about it through ethtool) then RX filters will dynamically > switch between SYNC and DELAY_REQ when port state changes. > > Signed-off-by: Ilya Orazov <ilo...@gm...> > --- What hardware are you targeting, specifically? I know the KSZ9477 DSA switch family has this quirk as well. |
|
From: Richard C. <ric...@gm...> - 2023-11-25 19:22:46
|
On Fri, Nov 24, 2023 at 10:29:12PM +0300, IlorDash wrote: > From: Ilya Orazov <ilo...@gm...> > > I’m using an Ethernet controller with PTP support, which requires > determining which PTP packets on RX must be timestamped: SYNC or > DELAY_REQ, based on whether the device is Slave or Master respectively. > So I can’t use the EVENT RX filter and must dynamically switch RX > filters, when port state changes from Master to Slave and vice versa. NAK, we don't support broken hardware. Thanks, Richard |
|
From: IlorDash <ilo...@gm...> - 2023-11-26 10:38:36
|
Could you, please, explain why you think my hardware is broken? I thought, since SYNC and DELAY_REQ filters are defined in Linux, ptp4l can use them to set type-specific hardware filters. On Sat, 25 Nov 2023 at 22:22, Richard Cochran <ric...@gm...> wrote: > On Fri, Nov 24, 2023 at 10:29:12PM +0300, IlorDash wrote: > > From: Ilya Orazov <ilo...@gm...> > > > > I’m using an Ethernet controller with PTP support, which requires > > determining which PTP packets on RX must be timestamped: SYNC or > > DELAY_REQ, based on whether the device is Slave or Master respectively. > > So I can’t use the EVENT RX filter and must dynamically switch RX > > filters, when port state changes from Master to Slave and vice versa. > > NAK, we don't support broken hardware. > > Thanks, > > Richard > -- Best regards, Ilya Orazov |
|
From: Richard C. <ric...@gm...> - 2023-11-26 18:44:22
|
On Sun, Nov 26, 2023 at 01:38:15PM +0300, IlorDash wrote: > Could you, please, explain why you think my hardware is broken? I thought, > since SYNC and DELAY_REQ filters are defined in Linux, ptp4l can use them > to set type-specific hardware filters. The port role can and will suddenly change at run time. Therefore the hardware must time stamp all PTP event messages. Otherwise you have a race condition that cannot be solved. The only hardware I ever saw with this defective design was the IXP 465. That was fifteen years ago. Even at that time, it was clear that the hardware design will not work in real life. I thought that vendors learned from their mistake, because that design quickly went EOL. Thanks, Richard |
|
From: Richard C. <ric...@gm...> - 2023-11-26 19:10:19
|
On Sun, Nov 26, 2023 at 01:38:15PM +0300, IlorDash wrote: > Could you, please, explain why you think my hardware is broken? I thought, > since SYNC and DELAY_REQ filters are defined in Linux, ptp4l can use them > to set type-specific hardware filters. Here is another reason why: The Rx filters are applied globally at the device level, but the PTP operates at the application level. This means that the Rx filter are shared between applications. And so you can see that one application cannot simply change the shared global settings at run time. For example, multiple ptp4l instances running different PTP Domains on the same network interface using virtual PHCs will have different port roles at the same time. Thanks, Richard |
|
From: IlorDash <ilo...@gm...> - 2023-11-27 07:48:05
|
Thanks, Richard, for such a detailed answer! I'll try to find another solution to handle it. On Sun, 26 Nov 2023 at 22:10, Richard Cochran <ric...@gm...> wrote: > On Sun, Nov 26, 2023 at 01:38:15PM +0300, IlorDash wrote: > > Could you, please, explain why you think my hardware is broken? I > thought, > > since SYNC and DELAY_REQ filters are defined in Linux, ptp4l can use them > > to set type-specific hardware filters. > > Here is another reason why: > > The Rx filters are applied globally at the device level, but the PTP > operates at the application level. This means that the Rx filter are > shared between applications. And so you can see that one application > cannot simply change the shared global settings at run time. > > For example, multiple ptp4l instances running different PTP Domains on > the same network interface using virtual PHCs will have different port > roles at the same time. > > Thanks, > Richard > -- Best regards, Ilya Orazov |
|
From: Richard C. <ric...@gm...> - 2023-11-27 23:47:03
|
On Mon, Nov 27, 2023 at 10:47:48AM +0300, IlorDash wrote: > Thanks, Richard, for such a detailed answer! > I'll try to find another solution to handle it. You can always keep your patches for your custom build. After all, it is open source, free for you to adapt. But I don't want to include those changes in the main project code base. Thanks, Richard |
|
From: Erez <ere...@gm...> - 2023-11-28 11:38:28
|
Hi, Debian builds allow using patches during building of Debian packages. Erez On Tue, 28 Nov 2023 at 00:48, Richard Cochran <ric...@gm...> wrote: > On Mon, Nov 27, 2023 at 10:47:48AM +0300, IlorDash wrote: > > Thanks, Richard, for such a detailed answer! > > I'll try to find another solution to handle it. > > You can always keep your patches for your custom build. After all, it > is open source, free for you to adapt. But I don't want to include > those changes in the main project code base. > > Thanks, > Richard > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > |
|
From: Miroslav L. <mli...@re...> - 2023-11-28 11:53:56
|
On Sun, Nov 26, 2023 at 11:10:05AM -0800, Richard Cochran wrote: > The Rx filters are applied globally at the device level, but the PTP > operates at the application level. This means that the Rx filter are > shared between applications. And so you can see that one application > cannot simply change the shared global settings at run time. This problem already exists, e.g. with ptpv2-l4-event vs ptpv2-l2-event. We could say that all hardware that cannot timestamp all packets is broken, but it's so common that it has to be supported. If the hardware which cannot timestamp all event messages is very rare, ok, maybe it's not worth the trouble. However, timestamping only sync messages has an advantage with very large number of clients as they don't have timestamp each other's delay requets and timestamping of sync messages is more reliable. -- Miroslav Lichvar |
|
From: Erez <ere...@gm...> - 2023-11-28 13:40:06
|
On Tue, 28 Nov 2023 at 12:55, Miroslav Lichvar <mli...@re...> wrote: > On Sun, Nov 26, 2023 at 11:10:05AM -0800, Richard Cochran wrote: > > The Rx filters are applied globally at the device level, but the PTP > > operates at the application level. This means that the Rx filter are > > shared between applications. And so you can see that one application > > cannot simply change the shared global settings at run time. > > This problem already exists, e.g. with ptpv2-l4-event vs ptpv2-l2-event. > We could say that all hardware that cannot timestamp all packets is > broken, but it's so common that it has to be supported. If the > hardware which cannot timestamp all event messages is very rare, ok, > maybe it's not worth the trouble. > > However, timestamping only sync messages has an advantage with very > large number of clients as they don't have timestamp each other's > delay requets and timestamping of sync messages is more reliable. > PTP traffic by its nature is very low. I do not see any benefit for a filter that supports only client or only master PTP traffic. Perhaps the kernel should add a HWTSTAMP_FILTER_PTP_V2_ALL_EVENT, that supports multiple PTP services on several layers. Erez > -- > Miroslav Lichvar > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > |
|
From: Maciek M. <ma...@ma...> - 2023-11-28 17:25:49
|
On 28/11/2023 14:39, Erez wrote: > > > On Tue, 28 Nov 2023 at 12:55, Miroslav Lichvar <mli...@re... > <mailto:mli...@re...>> wrote: > > On Sun, Nov 26, 2023 at 11:10:05AM -0800, Richard Cochran wrote: > > The Rx filters are applied globally at the device level, but the PTP > > operates at the application level. This means that the Rx filter are > > shared between applications. And so you can see that one application > > cannot simply change the shared global settings at run time. > > This problem already exists, e.g. with ptpv2-l4-event vs ptpv2-l2-event. > We could say that all hardware that cannot timestamp all packets is > broken, but it's so common that it has to be supported. If the > hardware which cannot timestamp all event messages is very rare, ok, > maybe it's not worth the trouble. > > However, timestamping only sync messages has an advantage with very > large number of clients as they don't have timestamp each other's > delay requets and timestamping of sync messages is more reliable. > > > PTP traffic by its nature is very low. > I do not see any benefit for a filter that supports only client or only > master PTP traffic. That argument does not hold up in the datacenter. Once you scale up to 100000 nodes @128pps trying to synchronize to a GM it's no longer what I'd consider "very low". Thanks, Maciek > > Perhaps the kernel should add a HWTSTAMP_FILTER_PTP_V2_ALL_EVENT, > that supports multiple PTP services on several layers. > > Erez > > > -- > Miroslav Lichvar > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > <mailto:Lin...@li...> > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > <https://lists.sourceforge.net/lists/listinfo/linuxptp-devel> > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel |
|
From: Bilal O. <bil...@ho...> - 2023-11-28 20:03:07
|
>> PTP traffic by its nature is very low. >> I do not see any benefit for a filter that supports only client or only >> master PTP traffic. >That argument does not hold up in the datacenter. Once you scale up to >100000 nodes @128pps trying to synchronize to a GM it's no longer what > I'd consider "very low". Furthermore, considering a TSN context where a low cycle time, for instance 0.5ms, and a lower sync interval of -5(e.g., logSyncInterval=-5) is needed then the PTP traffic is no longer "very low". hence it worth the trouble. If the tx poll time(out) is also considered then it makes sense to reduce this overhead for the critical real time application. Best regards Bilal ________________________________ From: Maciek Machnikowski <ma...@ma...> Sent: 28 November 2023 18:25 To: lin...@li... <lin...@li...> Subject: Re: [Linuxptp-devel] [PATCH v2] Add support for DELAY_REQ and SYNC packets RX filters On 28/11/2023 14:39, Erez wrote: > > > On Tue, 28 Nov 2023 at 12:55, Miroslav Lichvar <mli...@re... > <mailto:mli...@re...>> wrote: > > On Sun, Nov 26, 2023 at 11:10:05AM -0800, Richard Cochran wrote: > > The Rx filters are applied globally at the device level, but the PTP > > operates at the application level. This means that the Rx filter are > > shared between applications. And so you can see that one application > > cannot simply change the shared global settings at run time. > > This problem already exists, e.g. with ptpv2-l4-event vs ptpv2-l2-event. > We could say that all hardware that cannot timestamp all packets is > broken, but it's so common that it has to be supported. If the > hardware which cannot timestamp all event messages is very rare, ok, > maybe it's not worth the trouble. > > However, timestamping only sync messages has an advantage with very > large number of clients as they don't have timestamp each other's > delay requets and timestamping of sync messages is more reliable. > > > PTP traffic by its nature is very low. > I do not see any benefit for a filter that supports only client or only > master PTP traffic. That argument does not hold up in the datacenter. Once you scale up to 100000 nodes @128pps trying to synchronize to a GM it's no longer what I'd consider "very low". Thanks, Maciek > > Perhaps the kernel should add a HWTSTAMP_FILTER_PTP_V2_ALL_EVENT, > that supports multiple PTP services on several layers. > > Erez > > > -- > Miroslav Lichvar > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > <mailto:Lin...@li...> > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > <https://lists.sourceforge.net/lists/listinfo/linuxptp-devel> > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel _______________________________________________ Linuxptp-devel mailing list Lin...@li... https://lists.sourceforge.net/lists/listinfo/linuxptp-devel |
|
From: Richard C. <ric...@gm...> - 2023-11-29 01:26:34
|
On Tue, Nov 28, 2023 at 12:53:38PM +0100, Miroslav Lichvar wrote: > On Sun, Nov 26, 2023 at 11:10:05AM -0800, Richard Cochran wrote: > > The Rx filters are applied globally at the device level, but the PTP > > operates at the application level. This means that the Rx filter are > > shared between applications. And so you can see that one application > > cannot simply change the shared global settings at run time. > > This problem already exists, e.g. with ptpv2-l4-event vs ptpv2-l2-event. That is totally different. As long as the HW supports the given transport, then it works. > If the > hardware which cannot timestamp all event messages is very rare, ok, > maybe it's not worth the trouble. And it simply cannot work in real life. > However, timestamping only sync messages has an advantage with very > large number of clients as they don't have timestamp each other's > delay requets and timestamping of sync messages is more reliable. Actually, in hardware is it much simpler and more efficient just to time stamp every frame. (The reporting can be selectable if the application doesn't care about some frame types) |
|
From: Miroslav L. <mli...@re...> - 2023-11-29 12:07:36
|
On Tue, Nov 28, 2023 at 03:34:54PM -0800, Richard Cochran wrote: > On Tue, Nov 28, 2023 at 12:53:38PM +0100, Miroslav Lichvar wrote: > > However, timestamping only sync messages has an advantage with very > > large number of clients as they don't have timestamp each other's > > delay requets and timestamping of sync messages is more reliable. > > Actually, in hardware is it much simpler and more efficient just to > time stamp every frame. (The reporting can be selectable if the > application doesn't care about some frame types) I think it depends on the hardware design, which impacts its cost. If all hardware was able to accurately and reliably timestamp all packets, we wouldn't need PTP. PTP was specifically designed to make the hardware support as simple/cheap as possible. It separates event messages from the rest and uses multicast messaging in order to reduce the required timestamping rate. Timestamping only sync messages on clients is just another trick to reduce the required timestamping rate. The hardware can keep just one timestamp at a time and it doesn't matter that if it takes 20 milliseconds to read it over MDIO. It's good enough for clients, even if there is a very large number of them on the same communication path. -- Miroslav Lichvar |
|
From: Erez <ere...@gm...> - 2023-11-29 12:42:38
|
On Wed, 29 Nov 2023 at 13:09, Miroslav Lichvar <mli...@re...> wrote: > On Tue, Nov 28, 2023 at 03:34:54PM -0800, Richard Cochran wrote: > > On Tue, Nov 28, 2023 at 12:53:38PM +0100, Miroslav Lichvar wrote: > > > However, timestamping only sync messages has an advantage with very > > > large number of clients as they don't have timestamp each other's > > > delay requets and timestamping of sync messages is more reliable. > > > > Actually, in hardware is it much simpler and more efficient just to > > time stamp every frame. (The reporting can be selectable if the > > application doesn't care about some frame types) > > I think it depends on the hardware design, which impacts its cost. If > all hardware was able to accurately and reliably timestamp all > packets, we wouldn't need PTP. PTP was specifically designed to make > the hardware support as simple/cheap as possible. It separates event > messages from the rest and uses multicast messaging in order to reduce > the required timestamping rate. > Although PTP uses multicast. It is only a multicast MAC address, but it is not a real multicast group. As PTP messages are not forward to all peers as is. So PTP messages are actually unicast messages.using a multicast MAC address. > > Timestamping only sync messages on clients is just another trick to > reduce the required timestamping rate. The hardware can keep just one > timestamp at a time and it doesn't matter that if it takes 20 > milliseconds to read it over MDIO. It's good enough for clients, even > if there is a very large number of them on the same communication > path. > It Makes sense, yet using a PTP events filter is sufficient. The PTP service is a master or client, not both. So the amount of traffic you timestamp does not reduce by using a SYNC/ DELY versa events filter. Erez > > -- > Miroslav Lichvar > > > > _______________________________________________ > Linuxptp-devel mailing list > Lin...@li... > https://lists.sourceforge.net/lists/listinfo/linuxptp-devel > |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X