Re: [Linuxptp-devel] [PATCH V3 7/7] clock: Monitor the link status using a RT netlink socket.

2016-10-18 Thread Richard Cochran
On Tue, Oct 18, 2016 at 08:46:46AM +0200, Miroslav Lichvar wrote:
> Would it be possible to make the netlink socket optional?

Ok, I'll make it into a warning if rtnl is missing.

Thanks,
Richard

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH V3 7/7] clock: Monitor the link status using a RT netlink socket.

2016-10-18 Thread Miroslav Lichvar
On Tue, Oct 18, 2016 at 08:35:56AM +0200, Richard Cochran wrote:
> On Mon, Oct 17, 2016 at 05:16:21PM +0200, Miroslav Lichvar wrote:
> > I'm testing this, but I'm not sure if it's working correctly. When I
> > take the interface down I see a "link up" message from ptp4l. When I
> > bring it up, there is another "link up" message.
> 
> I only tested by pulling the cable!  A quick test with ifdown works
> for me.  Does the link actually go down after "ifdown ethX"?  Maybe
> there is a seperate rtnl message for ifup/down...

It looks like ifdown with NetworkManager doesn't really take down the
interface, it just removes its addresses. When I take it down manually
with "ip", it seems to be working as expected. Sorry for the noise.

Would it be possible to make the netlink socket optional? If it can't
be opened (e.g. in a simulated environment), I think it would be nice
if it worked as before. I was thinking something like this:

diff --git a/clock.c b/clock.c
index dd0c631..09c737f 100644
--- a/clock.c
+++ b/clock.c
@@ -272,7 +272,8 @@ void clock_destroy(struct clock *c)
LIST_FOREACH_SAFE(p, >ports, list, tmp) {
clock_remove_port(c, p);
}
-   rtnl_close(c->pollfd[0].fd);
+   if (c->pollfd[0].fd >= 0)
+   rtnl_close(c->pollfd[0].fd);
port_close(c->uds_port);
free(c->pollfd);
hash_destroy(c->index2port, NULL);
@@ -1124,9 +1125,6 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
 
/* Open a RT netlink socket. */
c->pollfd[0].fd = rtnl_open();
-   if (c->pollfd[0].fd < 0) {
-   return NULL;
-   }
c->pollfd[0].events = POLLIN|POLLPRI;
 
/* Create the UDS interface. */
@@ -1156,7 +1154,8 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
port_dispatch(p, EV_INITIALIZE, 0);
}
port_dispatch(c->uds_port, EV_INITIALIZE, 0);
-   rtnl_link_query(c->pollfd[0].fd);
+   if (c->pollfd[0].fd >= 0)
+   rtnl_link_query(c->pollfd[0].fd);
 
return c;
 }

-- 
Miroslav Lichvar

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH V3 7/7] clock: Monitor the link status using a RT netlink socket.

2016-10-18 Thread Richard Cochran
On Mon, Oct 17, 2016 at 05:16:21PM +0200, Miroslav Lichvar wrote:
> I'm testing this, but I'm not sure if it's working correctly. When I
> take the interface down I see a "link up" message from ptp4l. When I
> bring it up, there is another "link up" message.

I only tested by pulling the cable!  A quick test with ifdown works
for me.  Does the link actually go down after "ifdown ethX"?  Maybe
there is a seperate rtnl message for ifup/down...

Thanks,
Richard



--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


Re: [Linuxptp-devel] [PATCH V3 7/7] clock: Monitor the link status using a RT netlink socket.

2016-10-17 Thread Miroslav Lichvar
On Sun, Oct 16, 2016 at 12:55:51PM +0200, Richard Cochran wrote:
> Poll for link up/down events.  When a link goes down, the port becomes
> faulty until the link goes up again.  We keep the fault timer from the
> existing fault detection, but a downed link prevents clear the fault.

I'm testing this, but I'm not sure if it's working correctly. When I
take the interface down I see a "link up" message from ptp4l. When I
bring it up, there is another "link up" message.

ptp4l[2675017.937]: master offset  3 s2 freq  -28502 path delay
-8
ptp4l[2675018.937]: master offset -3 s2 freq  -28507 path delay
-5
ptp4l[2675019.937]: master offset -7 s2 freq  -28512 path delay
-5
ptp4l[2675020.895]: port 1: link up
ptp4l[2675026.440]: port 1: SLAVE to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l[2675026.441]: selected best master clock XXX
ptp4l[2675026.441]: assuming the grand master role
ptp4l[2675457.762]: port 1: link up
ptp4l[2675460.966]: selected best master clock YYY

-- 
Miroslav Lichvar

--
Check out the vibrant tech community on one of the world's most 
engaging tech sites, SlashDot.org! http://sdm.link/slashdot
___
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel


[Linuxptp-devel] [PATCH V3 7/7] clock: Monitor the link status using a RT netlink socket.

2016-10-16 Thread Richard Cochran
Poll for link up/down events.  When a link goes down, the port becomes
faulty until the link goes up again.  We keep the fault timer from the
existing fault detection, but a downed link prevents clear the fault.

The new state machine is depicted in this ascii art diagram:

  ++Fault+-+
  ||>| |
  |   UP   | |  FAULT  |
  ||<| |
  +--+--+--+   Timeout   +-+
 A  |   /
 |  |  /
   Link-Up   |  | Link-Down   /
 |  |/
 |  V   /
  +--+--+--+   /  Link-Down
  ||  /
  |  DOWN  |
---
 clock.c | 60 +++-
 1 file changed, 51 insertions(+), 9 deletions(-)

diff --git a/clock.c b/clock.c
index a0124d2..4adb492 100644
--- a/clock.c
+++ b/clock.c
@@ -39,6 +39,7 @@
 #include "servo.h"
 #include "stats.h"
 #include "print.h"
+#include "rtnl.h"
 #include "sk.h"
 #include "tlv.h"
 #include "tsproc.h"
@@ -271,6 +272,7 @@ void clock_destroy(struct clock *c)
LIST_FOREACH_SAFE(p, >ports, list, tmp) {
clock_remove_port(c, p);
}
+   rtnl_close(c->pollfd[0].fd);
port_close(c->uds_port);
free(c->pollfd);
hash_destroy(c->index2port, NULL);
@@ -320,6 +322,25 @@ static void clock_freq_est_reset(struct clock *c)
c->fest.count = 0;
 }
 
+static void clock_link_status(void *ctx, int index, int linkup)
+{
+   struct clock *c = ctx;
+   struct port *p;
+   char key[16];
+
+   snprintf(key, sizeof(key), "%d", index);
+   p = hash_lookup(c->index2port, key);
+   if (!p) {
+   return;
+   }
+   port_link_status_set(p, linkup);
+   if (linkup) {
+   port_dispatch(p, EV_FAULT_CLEARED, 0);
+   } else {
+   port_dispatch(p, EV_FAULT_DETECTED, 0);
+   }
+}
+
 static void clock_management_send_error(struct port *p,
struct ptp_message *msg, int error_id)
 {
@@ -1096,13 +1117,19 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
LIST_INIT(>ports);
c->last_port_number = 0;
 
-   /*
-* Create the UDS interface.
-*/
if (clock_resize_pollfd(c, 0)) {
pr_err("failed to allocate pollfd");
return NULL;
}
+
+   /* Open a RT netlink socket. */
+   c->pollfd[0].fd = rtnl_open();
+   if (c->pollfd[0].fd < 0) {
+   return NULL;
+   }
+   c->pollfd[0].events = POLLIN|POLLPRI;
+
+   /* Create the UDS interface. */
c->uds_port = port_open(phc_index, timestamping, 0, udsif, c);
if (!c->uds_port) {
pr_err("failed to open the UDS port");
@@ -1129,6 +1156,7 @@ struct clock *clock_create(enum clock_type type, struct 
config *config,
port_dispatch(p, EV_INITIALIZE, 0);
}
port_dispatch(c->uds_port, EV_INITIALIZE, 0);
+   rtnl_link_query(c->pollfd[0].fd);
 
return c;
 }
@@ -1194,9 +1222,12 @@ static int clock_resize_pollfd(struct clock *c, int 
new_nports)
 {
struct pollfd *new_pollfd;
 
-   /* Need to allocate one extra block of fds for uds */
+   /*
+* Need to allocate one descriptor for RT netlink and one
+* whole extra block of fds for UDS.
+*/
new_pollfd = realloc(c->pollfd,
-(new_nports + 1) * N_CLOCK_PFD *
+(1 + (new_nports + 1) * N_CLOCK_PFD) *
 sizeof(struct pollfd));
if (!new_pollfd)
return -1;
@@ -1221,7 +1252,7 @@ static void clock_fill_pollfd(struct pollfd *dest, struct 
port *p)
 static void clock_check_pollfd(struct clock *c)
 {
struct port *p;
-   struct pollfd *dest = c->pollfd;
+   struct pollfd *dest = c->pollfd + 1;
 
if (c->pollfd_valid)
return;
@@ -1437,7 +1468,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, 1 + (c->nports + 1) * N_CLOCK_PFD, -1);
if (cnt < 0) {
if (EINTR == errno)