>
>
>> 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 <erezge...@gmail.com> wrote:
>
>
> On Tue, 14 Nov 2023 at 23:21, Erez <erezge...@gmail.com> wrote:
>
>
> 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
>>
>
--
Best regards,
Ilya Orazov
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
