Re: [Linuxptp-devel] [PATCHv2 1/6] port: Don't assume transport from port number.

2021-02-11 Thread Richard Cochran
On Thu, Jan 28, 2021 at 04:25:06PM +0100, Miroslav Lichvar wrote:

> @@ -3038,24 +3042,28 @@ struct port *port_open(const char *phc_device,
>  
>   p->phc_index = phc_index;
>   p->jbod = config_get_int(cfg, interface_name(interface), 
> "boundary_clock_jbod");
> - transport = config_get_int(cfg, interface_name(interface), 
> "network_transport");
>   p->master_only = config_get_int(cfg, interface_name(interface), 
> "masterOnly");
>   p->bmca = config_get_int(cfg, interface_name(interface), "BMCA");
> + p->trp = transport_create(cfg, config_get_int(cfg,
> +   interface_name(interface), "network_transport"));
> + if (!p->trp) {
> + goto err_port;
> + }

The port_open() method just grew a new 'err_log_name' goto label for
rolling back in the case of errors.  I think that will affect this
patch.  May I ask you to rebase and take a look?

Sorry for the churn.

Thanks,
Richard


___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCHv2 1/6] port: Don't assume transport from port number.

2021-01-28 Thread Miroslav Lichvar
In port_open(), don't assume that UDS ports always have to have a zero
number. Check the transport directly to make the code cleaner. While at
it, switch all checks for the UDS in the port code to use a helper
function.

Signed-off-by: Miroslav Lichvar 
---
 port.c | 40 +++-
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/port.c b/port.c
index 0a93c06..87f2c06 100644
--- a/port.c
+++ b/port.c
@@ -771,6 +771,11 @@ static int port_is_ieee8021as(struct port *p)
return p->follow_up_info ? 1 : 0;
 }
 
+static int port_is_uds(struct port *p)
+{
+   return transport_type(p->trp) == TRANS_UDS;
+}
+
 static void port_management_send_error(struct port *p, struct port *ingress,
   struct ptp_message *msg, int error_id)
 {
@@ -1755,7 +1760,7 @@ int port_initialize(struct port *p)
}
 
/* No need to open rtnl socket on UDS port. */
-   if (transport_type(p->trp) != TRANS_UDS) {
+   if (!port_is_uds(p)) {
/*
 * The delay timer is usually started when the device
 * transitions to PS_LISTENING. But, we are skipping the state
@@ -3008,7 +3013,6 @@ struct port *port_open(const char *phc_device,
enum clock_type type = clock_type(clock);
struct config *cfg = clock_config(clock);
struct port *p = malloc(sizeof(*p));
-   enum transport_type transport;
int i;
 
if (!p) {
@@ -3038,24 +3042,28 @@ struct port *port_open(const char *phc_device,
 
p->phc_index = phc_index;
p->jbod = config_get_int(cfg, interface_name(interface), 
"boundary_clock_jbod");
-   transport = config_get_int(cfg, interface_name(interface), 
"network_transport");
p->master_only = config_get_int(cfg, interface_name(interface), 
"masterOnly");
p->bmca = config_get_int(cfg, interface_name(interface), "BMCA");
+   p->trp = transport_create(cfg, config_get_int(cfg,
+ interface_name(interface), "network_transport"));
+   if (!p->trp) {
+   goto err_port;
+   }
 
-   if (p->bmca == BMCA_NOOP && transport != TRANS_UDS) {
+   if (p->bmca == BMCA_NOOP && !port_is_uds(p)) {
if (p->master_only) {
p->state_machine = designated_master_fsm;
} else if (clock_slave_only(clock)) {
p->state_machine = designated_slave_fsm;
} else {
pr_err("Please enable at least one of masterOnly or 
clientOnly when BMCA == noop.\n");
-   goto err_port;
+   goto err_transport;
}
} else {
p->state_machine = clock_slave_only(clock) ? ptp_slave_fsm : 
ptp_fsm;
}
 
-   if (transport == TRANS_UDS) {
+   if (port_is_uds(p)) {
; /* UDS cannot have a PHC. */
} else if (!interface_tsinfo_valid(interface)) {
pr_warning("port %d: get_ts_info not supported", number);
@@ -3075,7 +3083,7 @@ struct port *port_open(const char *phc_device,
pr_err("port %d: /dev/ptp%d requested, ptp%d attached",
   number, phc_index,
   interface_phc_index(interface));
-   goto err_port;
+   goto err_transport;
}
}
 
@@ -3083,7 +3091,7 @@ struct port *port_open(const char *phc_device,
p->iface = interface;
p->asymmetry = config_get_int(cfg, p->name, "delayAsymmetry");
p->asymmetry <<= 16;
-   p->announce_span = transport == TRANS_UDS ? 0 : ANNOUNCE_SPAN;
+   p->announce_span = port_is_uds(p) ? 0 : ANNOUNCE_SPAN;
p->follow_up_info = config_get_int(cfg, p->name, "follow_up_info");
p->freq_est_interval = config_get_int(cfg, p->name, 
"freq_est_interval");
p->msg_interval_request = config_get_int(cfg, p->name, 
"msg_interval_request");
@@ -3096,10 +3104,6 @@ struct port *port_open(const char *phc_device,
p->tx_timestamp_offset <<= 16;
p->link_status = LINK_UP;
p->clock = clock;
-   p->trp = transport_create(cfg, transport);
-   if (!p->trp) {
-   goto err_port;
-   }
p->timestamping = timestamping;
p->portIdentity.clockIdentity = clock_identity(clock);
p->portIdentity.portNumber = number;
@@ -3108,23 +3112,25 @@ struct port *port_open(const char *phc_device,
p->versionNumber = PTP_VERSION;
p->slave_event_monitor = clock_slave_monitor(clock);
 
-   if (number && unicast_client_initialize(p)) {
+   if (!port_is_uds(p) && unicast_client_initialize(p)) {
goto err_transport;
}
if (unicast_client_enabled(p) &&
config_set_section_int(cfg, p->name, "hybrid_e2e", 1)) {
goto err_uc_client;
}
-   if (number && unicast_service_