On Mon, 13 Nov 2023 at 18:49, IlorDash <ilordas...@gmail.com> wrote:
> From: Ilya Orazov <ilordas...@gmail.com>
>
> 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 <ilordas...@gmail.com>
> ---
> 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
> Linuxptp-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
