> -----Original Message-----
> From: Miroslav Lichvar <mlich...@redhat.com>
> Sent: Wednesday, January 20, 2021 7:15 AM
> To: linuxptp-devel@lists.sourceforge.net
> Subject: [Linuxptp-devel] [RFC PATCH] clock: Add read-only UDS port for
> monitoring.
>
> Add a second UDS port to allow unprivileged applications to monitor
> ptp4l. On this "read-only" port, disable non-GET actions, forwarding,
> and access to subscriptions.
>
It makes sense to remove forwarding, but I am not sure I understand the
justification for removing access to subscriptions.. if the subscription is for
read only data, why doesn't it make sense to allow that over the read only
socket?
Thanks,
Jake
> Ignore non-management messages on both UDS ports to prevent them from
> changing the clock or port state. (This should be a separate patch?)
>
> TODO:
> - add option to configure the UDS address (default /var/run/ptp4ro?)
> - chmod(0666) ?
> - update man page
>
> Signed-off-by: Miroslav Lichvar <mlich...@redhat.com>
> ---
> clock.c | 122 +++++++++++++++++++++++++++++++++++++++-----------------
> port.c | 3 ++
> 2 files changed, 89 insertions(+), 36 deletions(-)
>
> diff --git a/clock.c b/clock.c
> index 08c61eb..475aa3d 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -95,10 +95,11 @@ struct clock {
> struct foreign_clock *best;
> struct ClockIdentity best_id;
> LIST_HEAD(ports_head, port) ports;
> - struct port *uds_port;
> + struct port *uds_rw_port;
> + struct port *uds_ro_port;
> struct pollfd *pollfd;
> int pollfd_valid;
> - int nports; /* does not include the UDS port */
> + int nports; /* does not include the two UDS ports */
> int last_port_number;
> int sde;
> int free_running;
> @@ -129,7 +130,8 @@ struct clock {
> struct clock_stats stats;
> int stats_interval;
> struct clockcheck *sanity_check;
> - struct interface *udsif;
> + struct interface *uds_rw_if;
> + struct interface *uds_ro_if;
> LIST_HEAD(clock_subscribers_head, clock_subscriber) subscribers;
> struct monitor *slave_event_monitor;
> };
> @@ -245,7 +247,7 @@ void clock_send_notification(struct clock *c, struct
> ptp_message *msg,
> {
> unsigned int event_pos = event / 8;
> uint8_t mask = 1 << (event % 8);
> - struct port *uds = c->uds_port;
> + struct port *uds = c->uds_rw_port;
> struct clock_subscriber *s;
>
> LIST_FOREACH(s, &c->subscribers, list) {
> @@ -267,13 +269,15 @@ void clock_destroy(struct clock *c)
> {
> struct port *p, *tmp;
>
> - interface_destroy(c->udsif);
> + interface_destroy(c->uds_rw_if);
> + interface_destroy(c->uds_ro_if);
> clock_flush_subscriptions(c);
> LIST_FOREACH_SAFE(p, &c->ports, list, tmp) {
> clock_remove_port(c, p);
> }
> monitor_destroy(c->slave_event_monitor);
> - port_close(c->uds_port);
> + port_close(c->uds_rw_port);
> + port_close(c->uds_ro_port);
> free(c->pollfd);
> if (c->clkid != CLOCK_REALTIME) {
> phc_close(c->clkid);
> @@ -442,8 +446,8 @@ static int clock_management_fill_response(struct clock
> *c, struct port *p,
> datalen = sizeof(*gsn);
> break;
> case TLV_SUBSCRIBE_EVENTS_NP:
> - if (p != c->uds_port) {
> - /* Only the UDS port allowed. */
> + if (p != c->uds_rw_port) {
> + /* Only the UDS RW port allowed. */
> break;
> }
> sen = (struct subscribe_events_np *)tlv->data;
> @@ -774,6 +778,10 @@ static int clock_utc_correct(struct clock *c, tmv_t
> ingress)
> static int forwarding(struct clock *c, struct port *p)
> {
> enum port_state ps = port_state(p);
> +
> + if (p == c->uds_ro_port)
> + return 0;
> +
> switch (ps) {
> case PS_MASTER:
> case PS_GRAND_MASTER:
> @@ -784,7 +792,7 @@ static int forwarding(struct clock *c, struct port *p)
> default:
> break;
> }
> - if (p == c->uds_port && ps != PS_FAULTY) {
> + if (p == c->uds_rw_port && ps != PS_FAULTY) {
> return 1;
> }
> return 0;
> @@ -818,7 +826,7 @@ static int clock_add_port(struct clock *c, const char
> *phc_device,
> {
> struct port *p, *piter, *lastp = NULL;
>
> - if (clock_resize_pollfd(c, c->nports + 1)) {
> + if (clock_resize_pollfd(c, c->nports + 2)) {
> return -1;
> }
> p = port_open(phc_device, phc_index, timestamping,
> @@ -1044,20 +1052,40 @@ struct clock *clock_create(enum clock_type type,
> struct config *config,
>
> /* Configure the UDS. */
> uds_ifname = config_get_string(config, NULL, "uds_address");
> - c->udsif = interface_create(uds_ifname);
> - if (config_set_section_int(config, interface_name(c->udsif),
> + c->uds_rw_if = interface_create(uds_ifname);
> + if (config_set_section_int(config, interface_name(c->uds_rw_if),
> "announceReceiptTimeout", 0)) {
> return NULL;
> }
> - if (config_set_section_int(config, interface_name(c->udsif),
> + if (config_set_section_int(config, interface_name(c->uds_rw_if),
> "delay_mechanism", DM_AUTO)) {
> return NULL;
> }
> - if (config_set_section_int(config, interface_name(c->udsif),
> + if (config_set_section_int(config, interface_name(c->uds_rw_if),
> "network_transport", TRANS_UDS)) {
> return NULL;
> }
> - if (config_set_section_int(config, interface_name(c->udsif),
> + if (config_set_section_int(config, interface_name(c->uds_rw_if),
> + "delay_filter_length", 1)) {
> + return NULL;
> + }
> +
> + //uds_ifname = config_get_string(config, NULL, "uds_address2");
> + uds_ifname = "/var/run/ptp4lro";
> + c->uds_ro_if = interface_create(uds_ifname);
> + if (config_set_section_int(config, interface_name(c->uds_ro_if),
> + "announceReceiptTimeout", 0)) {
> + return NULL;
> + }
> + if (config_set_section_int(config, interface_name(c->uds_ro_if),
> + "delay_mechanism", DM_AUTO)) {
> + return NULL;
> + }
> + if (config_set_section_int(config, interface_name(c->uds_ro_if),
> + "network_transport", TRANS_UDS)) {
> + return NULL;
> + }
> + if (config_set_section_int(config, interface_name(c->uds_ro_if),
> "delay_filter_length", 1)) {
> return NULL;
> }
> @@ -1173,27 +1201,36 @@ struct clock *clock_create(enum clock_type type,
> struct config *config,
>
> LIST_INIT(&c->subscribers);
> LIST_INIT(&c->ports);
> - c->last_port_number = 0;
> + c->last_port_number = -1;
>
> if (clock_resize_pollfd(c, 0)) {
> pr_err("failed to allocate pollfd");
> return NULL;
> }
>
> - /* Create the UDS interface. */
> - c->uds_port = port_open(phc_device, phc_index, timestamping, 0, c-
> >udsif, c);
> - if (!c->uds_port) {
> - pr_err("failed to open the UDS port");
> + /* Create the UDS interfaces. */
> + c->uds_rw_port = port_open(phc_device, phc_index, timestamping,
> + ++c->last_port_number, c->uds_rw_if, c);
> + if (!c->uds_rw_port) {
> + pr_err("failed to open the UDS RW port");
> return NULL;
> }
> clock_fda_changed(c);
>
> - c->slave_event_monitor = monitor_create(config, c->uds_port);
> + c->slave_event_monitor = monitor_create(config, c->uds_rw_port);
> if (!c->slave_event_monitor) {
> pr_err("failed to create slave event monitor");
> return NULL;
> }
>
> + c->uds_ro_port = port_open(phc_device, phc_index, timestamping,
> + ++c->last_port_number, c->uds_ro_if, c);
> + if (!c->uds_rw_port) {
> + pr_err("failed to open the UDS RO port");
> + return NULL;
> + }
> + clock_fda_changed(c);
> +
> /* Create the ports. */
> STAILQ_FOREACH(iface, &config->interfaces, list) {
> if (clock_add_port(c, phc_device, phc_index, timestamping,
> iface)) {
> @@ -1207,7 +1244,8 @@ struct clock *clock_create(enum clock_type type, struct
> config *config,
> LIST_FOREACH(p, &c->ports, list) {
> port_dispatch(p, EV_INITIALIZE, 0);
> }
> - port_dispatch(c->uds_port, EV_INITIALIZE, 0);
> + port_dispatch(c->uds_rw_port, EV_INITIALIZE, 0);
> + port_dispatch(c->uds_ro_port, EV_INITIALIZE, 0);
>
> return c;
> }
> @@ -1278,9 +1316,9 @@ static int clock_resize_pollfd(struct clock *c, int
> new_nports)
> {
> struct pollfd *new_pollfd;
>
> - /* Need to allocate one whole extra block of fds for UDS. */
> + /* Need to allocate two whole extra blocks of fds for UDS ports. */
> new_pollfd = realloc(c->pollfd,
> - (new_nports + 1) * N_CLOCK_PFD *
> + (new_nports + 2) * N_CLOCK_PFD *
> sizeof(struct pollfd));
> if (!new_pollfd) {
> return -1;
> @@ -1315,7 +1353,9 @@ static void clock_check_pollfd(struct clock *c)
> clock_fill_pollfd(dest, p);
> dest += N_CLOCK_PFD;
> }
> - clock_fill_pollfd(dest, c->uds_port);
> + clock_fill_pollfd(dest, c->uds_rw_port);
> + dest += N_CLOCK_PFD;
> + clock_fill_pollfd(dest, c->uds_ro_port);
> c->pollfd_valid = 1;
> }
>
> @@ -1331,8 +1371,9 @@ static int clock_do_forward_mgmt(struct clock *c,
> if (in == out || !forwarding(c, out))
> return 0;
>
> - /* Don't forward any requests to the UDS port. */
> - if (out == c->uds_port) {
> + /* Don't forward any requests to the UDS RW port
> + (UDS RO port doesn't allow any forwarding). */
> + if (out == c->uds_rw_port) {
> switch (management_action(msg)) {
> case GET:
> case SET:
> @@ -1363,7 +1404,7 @@ static void clock_forward_mgmt_msg(struct clock *c,
> struct port *p, struct ptp_m
> pr_err("port %d: management forward failed",
> port_number(piter));
> }
> - if (clock_do_forward_mgmt(c, p, c->uds_port, msg,
> &msg_ready))
> + if (clock_do_forward_mgmt(c, p, c->uds_rw_port, msg,
> &msg_ready))
> pr_err("uds port: management forward failed");
> if (msg_ready) {
> msg_post_recv(msg, pdulen);
> @@ -1415,8 +1456,8 @@ int clock_manage(struct clock *c, struct port *p, struct
> ptp_message *msg)
> clock_management_send_error(p, msg,
> TLV_WRONG_LENGTH);
> return changed;
> }
> - if (p != c->uds_port) {
> - /* Sorry, only allowed on the UDS port. */
> + if (p != c->uds_rw_port) {
> + /* Sorry, only allowed on the UDS RW port. */
> clock_management_send_error(p, msg,
> TLV_NOT_SUPPORTED);
> return changed;
> }
> @@ -1431,8 +1472,8 @@ int clock_manage(struct clock *c, struct port *p, struct
> ptp_message *msg)
>
> switch (mgt->id) {
> case TLV_PORT_PROPERTIES_NP:
> - if (p != c->uds_port) {
> - /* Only the UDS port allowed. */
> + if (p != c->uds_rw_port) {
> + /* Only the UDS RW port allowed. */
> clock_management_send_error(p, msg,
> TLV_NOT_SUPPORTED);
> return 0;
> }
> @@ -1496,7 +1537,7 @@ int clock_manage(struct clock *c, struct port *p, struct
> ptp_message *msg)
>
> void clock_notify_event(struct clock *c, enum notification event)
> {
> - struct port *uds = c->uds_port;
> + struct port *uds = c->uds_rw_port;
> struct PortIdentity pid = port_identity(uds);
> struct ptp_message *msg;
> int id;
> @@ -1543,7 +1584,7 @@ int clock_poll(struct clock *c)
> struct port *p;
>
> clock_check_pollfd(c);
> - cnt = poll(c->pollfd, (c->nports + 1) * N_CLOCK_PFD, -1);
> + cnt = poll(c->pollfd, (c->nports + 2) * N_CLOCK_PFD, -1);
> if (cnt < 0) {
> if (EINTR == errno) {
> return 0;
> @@ -1597,10 +1638,19 @@ int clock_poll(struct clock *c)
> cur += N_CLOCK_PFD;
> }
>
> - /* Check the UDS port. */
> + /* Check the UDS ports. */
> + for (i = 0; i < N_POLLFD; i++) {
> + if (cur[i].revents & (POLLIN|POLLPRI)) {
> + event = port_event(c->uds_rw_port, i);
> + if (EV_STATE_DECISION_EVENT == event) {
> + c->sde = 1;
> + }
> + }
> + }
> + cur += N_CLOCK_PFD;
> for (i = 0; i < N_POLLFD; i++) {
> if (cur[i].revents & (POLLIN|POLLPRI)) {
> - event = port_event(c->uds_port, i);
> + event = port_event(c->uds_ro_port, i);
> if (EV_STATE_DECISION_EVENT == event) {
> c->sde = 1;
> }
> diff --git a/port.c b/port.c
> index 0a93c06..7f4cd2d 100644
> --- a/port.c
> +++ b/port.c
> @@ -691,6 +691,9 @@ static int port_ignore(struct port *p, struct ptp_message
> *m)
> {
> struct ClockIdentity c1, c2;
>
> + if (transport_type(p->trp) == TRANS_UDS && msg_type(m) !=
> MANAGEMENT) {
> + return 1;
> + }
> if (incapable_ignore(p, m)) {
> return 1;
> }
> --
> 2.26.2
>
>
>
> _______________________________________________
> 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
