On Wed, Jan 20, 2021 at 04:15:21PM +0100, Miroslav Lichvar wrote:
> 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.
>
> Ignore non-management messages on both UDS ports to prevent them from
> changing the clock or port state. (This should be a separate patch?)
Yes, please!
> TODO:
> - add option to configure the UDS address (default /var/run/ptp4ro?)
> - chmod(0666) ?
I think this sort of policy can be left to the admin via scripting.
> - 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;
Part of this series is simply renaming the existing port. It would
ease review to have that in a patch by itself.
> @@ -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);
We'll have to do something about the port numbering. The "real" ports
must start with 1, 2, 3, ... as this is part of the standard.
Also, there is some code in port_open() and maybe elsewhere that
treats zero as a special case (meaning UDS), for example:
p->fault_fd = -1;
if (number) {
p->fault_fd = timerfd_create(CLOCK_MONOTONIC, 0);
...
}
Maybe we can have two [!] ports with number zero?
Thanks,
Richard
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
