Discussion:
[Linuxptp-devel] [PATCH 0/4] Add rtnl per port support
Hangbin Liu
2017-07-05 09:59:48 UTC
Permalink
As discussed, now we convert single rtnl FD into per-port FD and remove
index2port hash table from clock.c

Hangbin Liu (4):
rtnl: replace obsolete RTMGRP_LINK with RTNLGRP_LINK for nl groups
port: add FD_RTNL event to track per-port status
clock: remove rtnl fd on clock
clock: remove index2port since we don't need it now

clock.c | 86 ++++++++---------------------------------------------------------
clock.h | 7 ++++++
fd.h | 1 +
port.c | 50 ++++++++++++++++++++++++++++++++------
port.h | 7 ------
rtnl.c | 2 +-
6 files changed, 62 insertions(+), 91 deletions(-)
--
2.5.5
Hangbin Liu
2017-07-05 09:59:49 UTC
Permalink
Signed-off-by: Hangbin Liu <***@gmail.com>
---
rtnl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rtnl.c b/rtnl.c
index 5cddc4b..d7a430d 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -151,7 +151,7 @@ int rtnl_open(void)

memset(&sa, 0, sizeof(sa));
sa.nl_family = AF_NETLINK;
- sa.nl_groups = RTMGRP_LINK;
+ sa.nl_groups = RTNLGRP_LINK;

fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (fd < 0) {
--
2.5.5
Hangbin Liu
2017-07-05 09:59:50 UTC
Permalink
With rtnl socket we can track link status per port(except UDS port).

We can make sure we get the correct interface and latest status with function
port_link_status().

At the same time we need to set clock sde after link down. But we return
EV_FAULT_DETECTED in port_event(), which will not set clock sde. So we need
to set it in port_link_status().

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 5 +++++
clock.h | 7 +++++++
fd.h | 1 +
port.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index b6afba9..4c5c4e3 100644
--- a/clock.c
+++ b/clock.c
@@ -1469,6 +1469,11 @@ struct PortIdentity clock_parent_identity(struct clock *c)
return c->dad.pds.parentPortIdentity;
}

+void clock_set_sde(struct clock *c, int sde)
+{
+ c->sde = sde;
+}
+
int clock_poll(struct clock *c)
{
int cnt, i;
diff --git a/clock.h b/clock.h
index fcd9328..49ecb76 100644
--- a/clock.h
+++ b/clock.h
@@ -205,6 +205,13 @@ void clock_peer_delay(struct clock *c, tmv_t ppd, tmv_t req, tmv_t rx,
double nrr);

/**
+ * Set clock sde
+ * @param c A pointer to a clock instance obtained with clock_create().
+ * @param sde Pass one (1) if need a decision event and zero if not.
+ */
+void clock_set_sde(struct clock *c, int sde);
+
+/**
* Poll for events and dispatch them.
* @param c A pointer to a clock instance obtained with clock_create().
* @return Zero on success, non-zero otherwise.
diff --git a/fd.h b/fd.h
index e328e98..23401f4 100644
--- a/fd.h
+++ b/fd.h
@@ -31,6 +31,7 @@ enum {
FD_QUALIFICATION_TIMER,
FD_MANNO_TIMER,
FD_SYNC_TX_TIMER,
+ FD_RTNL,
N_POLLFD,
};

diff --git a/port.c b/port.c
index ec02825..37d9f81 100644
--- a/port.c
+++ b/port.c
@@ -23,6 +23,7 @@
#include <string.h>
#include <unistd.h>
#include <sys/queue.h>
+#include <net/if.h>

#include "bmc.h"
#include "clock.h"
@@ -32,6 +33,7 @@
#include "phc.h"
#include "port.h"
#include "print.h"
+#include "rtnl.h"
#include "sk.h"
#include "tlv.h"
#include "tmv.h"
@@ -1458,7 +1460,9 @@ static void port_disable(struct port *p)
for (i = 0; i < N_TIMER_FDS; i++) {
close(p->fda.fd[FD_ANNOUNCE_TIMER + i]);
}
- port_clear_fda(p, N_POLLFD);
+
+ /* Keep rtnl socket to get link status info. */
+ port_clear_fda(p, FD_RTNL);
clock_fda_changed(p->clock);
}

@@ -1502,11 +1506,22 @@ static int port_initialize(struct port *p)
if (port_set_announce_tmo(p))
goto no_tmo;

+ /* No need to open rtnl socket on UDS port. */
+ if (p->announce_span) {
+ if (p->fda.fd[FD_RTNL] == -1)
+ p->fda.fd[FD_RTNL] = rtnl_open();
+ if (p->fda.fd[FD_RTNL] == -1)
+ goto no_rtnl;
+ } else {
+ p->fda.fd[FD_RTNL] = -1;
+ }
+
port_nrate_initialize(p);

clock_fda_changed(p->clock);
return 0;

+no_rtnl:
no_tmo:
transport_close(p->trp, &p->fda);
no_tropen:
@@ -2025,6 +2040,10 @@ void port_close(struct port *p)
if (port_is_enabled(p)) {
port_disable(p);
}
+
+ if (p->fda.fd[FD_RTNL] >= 0)
+ rtnl_close(p->fda.fd[FD_RTNL]);
+
transport_destroy(p->trp);
tsproc_destroy(p->tsproc);
if (p->fault_fd >= 0)
@@ -2204,6 +2223,24 @@ void port_dispatch(struct port *p, enum fsm_event event, int mdiff)
}
}

+static void port_link_status(void *ctx, int index, int linkup)
+{
+ struct port *p = ctx;
+
+ if (index != if_nametoindex(p->name) || p->link_status == linkup)
+ return;
+
+ p->link_status = linkup;
+ pr_notice("port %hu: link %s", portnum(p), linkup ? "up" : "down");
+
+ /*
+ * A port going down can affect the BMCA result.
+ * Force a state decision event.
+ */
+ if (!p->link_status)
+ clock_set_sde(p->clock, 1);
+}
+
enum fsm_event port_event(struct port *p, int fd_index)
{
enum fsm_event event = EV_NONE;
@@ -2242,6 +2279,11 @@ enum fsm_event port_event(struct port *p, int fd_index)
pr_debug("port %hu: master sync timeout", portnum(p));
port_set_sync_tx_tmo(p);
return port_tx_sync(p) ? EV_FAULT_DETECTED : EV_NONE;
+
+ case FD_RTNL:
+ pr_debug("port %hu: link status changed", portnum(p));
+ rtnl_link_status(fd, port_link_status, p);
+ return port_link_status_get(p) ? EV_FAULT_CLEARED : EV_FAULT_DETECTED;
}

msg = msg_allocate();
--
2.5.5
Richard Cochran
2017-07-08 20:24:05 UTC
Permalink
Post by Hangbin Liu
@@ -1502,11 +1506,22 @@ static int port_initialize(struct port *p)
if (port_set_announce_tmo(p))
goto no_tmo;
+ /* No need to open rtnl socket on UDS port. */
+ if (p->announce_span) {
The type is remembered by the transport instance, so you can and
should test for (transport_type(p->trp) == TRANS_UDS).
Post by Hangbin Liu
+ if (p->fda.fd[FD_RTNL] == -1)
+ p->fda.fd[FD_RTNL] = rtnl_open();
+ if (p->fda.fd[FD_RTNL] == -1)
+ goto no_rtnl;
This test causes almost all of the linuxptp-testsuite (clknetsim)
cases to fail.

Originally, the clock.c code allowed rtnl_open() to fail. I don't
remember why I did that way. In any case, the program will still work
without the rtnl socket.

Maybe you have an argument why rtnl cannot fail?
Post by Hangbin Liu
+ } else {
+ p->fda.fd[FD_RTNL] = -1;
+ }
+
port_nrate_initialize(p);
clock_fda_changed(p->clock);
return 0;
transport_close(p->trp, &p->fda);
@@ -2025,6 +2040,10 @@ void port_close(struct port *p)
if (port_is_enabled(p)) {
port_disable(p);
}
+
+ if (p->fda.fd[FD_RTNL] >= 0)
+ rtnl_close(p->fda.fd[FD_RTNL]);
See? The test allows for the case when rtnl_open() fails.
Post by Hangbin Liu
transport_destroy(p->trp);
tsproc_destroy(p->tsproc);
if (p->fault_fd >= 0)
@@ -2242,6 +2279,11 @@ enum fsm_event port_event(struct port *p, int fd_index)
pr_debug("port %hu: master sync timeout", portnum(p));
port_set_sync_tx_tmo(p);
return port_tx_sync(p) ? EV_FAULT_DETECTED : EV_NONE;
+
+ pr_debug("port %hu: link status changed", portnum(p));
The message isn't always true, is it?

Maybe it should say something like "received link status notification".
Post by Hangbin Liu
+ rtnl_link_status(fd, port_link_status, p);
+ return port_link_status_get(p) ? EV_FAULT_CLEARED : EV_FAULT_DETECTED;
}
Thanks,
Richard
Hangbin Liu
2017-07-05 09:59:52 UTC
Permalink
Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 29 -----------------------------
1 file changed, 29 deletions(-)

diff --git a/clock.c b/clock.c
index 354f038..da15882 100644
--- a/clock.c
+++ b/clock.c
@@ -31,7 +31,6 @@
#include "clockcheck.h"
#include "foreign.h"
#include "filter.h"
-#include "hash.h"
#include "missing.h"
#include "msg.h"
#include "phc.h"
@@ -39,7 +38,6 @@
#include "servo.h"
#include "stats.h"
#include "print.h"
-#include "sk.h"
#include "tlv.h"
#include "tsproc.h"
#include "uds.h"
@@ -96,7 +94,6 @@ struct clock {
int nports; /* does not include the UDS port */
int last_port_number;
int sde;
- struct hash *index2port;
int free_running;
int freq_est_interval;
int grand_master_capable; /* for 802.1AS only */
@@ -275,7 +272,6 @@ void clock_destroy(struct clock *c)
}
port_close(c->uds_port);
free(c->pollfd);
- hash_destroy(c->index2port, NULL);
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
}
@@ -773,8 +769,6 @@ static int clock_add_port(struct clock *c, int phc_index,
struct interface *iface)
{
struct port *p, *piter, *lastp = NULL;
- int fd, index;
- char key[16];

if (clock_resize_pollfd(c, c->nports + 1)) {
return -1;
@@ -795,24 +789,6 @@ static int clock_add_port(struct clock *c, int phc_index,
c->nports++;
clock_fda_changed(c);

- /* Remember the index to port mapping, for link status tracking. */
- fd = sk_interface_fd();
- if (fd < 0) {
- return -1;
- }
- index = sk_interface_index(fd, iface->name);
- if (index < 0) {
- close(fd);
- return -1;
- }
- snprintf(key, sizeof(key), "%d", index);
- if (hash_insert(c->index2port, key, p)) {
- pr_err("failed to add port with index %d twice!", index);
- close(fd);
- return -1;
- }
- close(fd);
-
return 0;
}

@@ -1113,11 +1089,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}
clock_fda_changed(c);

- c->index2port = hash_create();
- if (!c->index2port) {
- pr_err("failed create index-to-port hash table");
- return NULL;
- }
/* Create the ports. */
STAILQ_FOREACH(iface, &config->interfaces, list) {
if (clock_add_port(c, phc_index, timestamping, iface)) {
--
2.5.5
Hangbin Liu
2017-07-05 09:59:51 UTC
Permalink
Remove rtnl fd on clock since we track the link status per port now.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 52 +++++-----------------------------------------------
port.c | 6 ------
port.h | 7 -------
3 files changed, 5 insertions(+), 60 deletions(-)

diff --git a/clock.c b/clock.c
index 4c5c4e3..354f038 100644
--- a/clock.c
+++ b/clock.c
@@ -39,7 +39,6 @@
#include "servo.h"
#include "stats.h"
#include "print.h"
-#include "rtnl.h"
#include "sk.h"
#include "tlv.h"
#include "tsproc.h"
@@ -274,9 +273,6 @@ void clock_destroy(struct clock *c)
LIST_FOREACH_SAFE(p, &c->ports, list, tmp) {
clock_remove_port(c, p);
}
- 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);
@@ -326,30 +322,6 @@ 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);
- /*
- * A port going down can affect the BMCA result.
- * Force a state decision event.
- */
- c->sde = 1;
- }
-}
-
static void clock_management_send_error(struct port *p,
struct ptp_message *msg, int error_id)
{
@@ -1133,10 +1105,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
return NULL;
}

- /* Open a RT netlink socket. */
- c->pollfd[0].fd = rtnl_open();
- 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) {
@@ -1164,9 +1132,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);
- if (c->pollfd[0].fd >= 0) {
- rtnl_link_query(c->pollfd[0].fd);
- }
+
return c;
}

@@ -1231,12 +1197,9 @@ static int clock_resize_pollfd(struct clock *c, int new_nports)
{
struct pollfd *new_pollfd;

- /*
- * Need to allocate one descriptor for RT netlink and one
- * whole extra block of fds for UDS.
- */
+ /* Need to allocate one whole extra block of fds for UDS. */
new_pollfd = realloc(c->pollfd,
- (1 + (new_nports + 1) * N_CLOCK_PFD) *
+ (new_nports + 1) * N_CLOCK_PFD *
sizeof(struct pollfd));
if (!new_pollfd)
return -1;
@@ -1261,7 +1224,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 + 1;
+ struct pollfd *dest = c->pollfd;

if (c->pollfd_valid)
return;
@@ -1482,7 +1445,7 @@ int clock_poll(struct clock *c)
struct port *p;

clock_check_pollfd(c);
- cnt = poll(c->pollfd, 1 + (c->nports + 1) * N_CLOCK_PFD, -1);
+ cnt = poll(c->pollfd, (c->nports + 1) * N_CLOCK_PFD, -1);
if (cnt < 0) {
if (EINTR == errno) {
return 0;
@@ -1494,13 +1457,8 @@ int clock_poll(struct clock *c)
return 0;
}

- /* Check the RT netlink. */
cur = c->pollfd;
- if (cur->revents & (POLLIN|POLLPRI)) {
- rtnl_link_status(cur->fd, clock_link_status, c);
- }

- cur++;
LIST_FOREACH(p, &c->ports, list) {
/* Let the ports handle their events. */
for (i = 0; i < N_POLLFD; i++) {
diff --git a/port.c b/port.c
index 37d9f81..a801ad9 100644
--- a/port.c
+++ b/port.c
@@ -2414,12 +2414,6 @@ int port_link_status_get(struct port *p)
return p->link_status;
}

-void port_link_status_set(struct port *p, int up)
-{
- p->link_status = up ? 1 : 0;
- pr_notice("port %hu: link %s", portnum(p), up ? "up" : "down");
-}
-
int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
{
struct management_tlv *mgt;
diff --git a/port.h b/port.h
index b00bc64..60fd0a4 100644
--- a/port.h
+++ b/port.h
@@ -132,13 +132,6 @@ int port_number(struct port *p);
int port_link_status_get(struct port *p);

/**
- * Sets the link status for a port.
- * @param p A port instance.
- * @param up Pass one (1) if the link is up and zero if down.
- */
-void port_link_status_set(struct port *p, int up);
-
-/**
* Manage a port according to a given message.
* @param p A pointer previously obtained via port_open().
* @param ingress The port on which 'msg' was received.
--
2.5.5
Richard Cochran
2017-07-08 20:55:19 UTC
Permalink
Post by Hangbin Liu
@@ -1164,9 +1132,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);
- if (c->pollfd[0].fd >= 0) {
- rtnl_link_query(c->pollfd[0].fd);
- }
This call should also be called initially by each port in order to find
out the link status from the very start.

If the link is down at start, then the port goes faulty when it first
transmits an event message.

Thanks,
Richard
Richard Cochran
2017-07-08 20:44:19 UTC
Permalink
Post by Hangbin Liu
As discussed, now we convert single rtnl FD into per-port FD and remove
index2port hash table from clock.c
Apart from a small issue in #2, the series looks good to go.

Thanks,
Richard
Richard Cochran
2017-07-08 20:50:48 UTC
Permalink
Post by Richard Cochran
Apart from a small issue in #2, the series looks good to go.
Also found an issue in #3 by testing...

Thanks,
Richard
Hangbin Liu
2017-07-10 07:39:27 UTC
Permalink
As discussed, now we convert single rtnl FD into per-port FD and remove
index2port hash table from clock.c

v2:
1. Let program still work even rtnl_open() failed.
2. Start rtnl_link_query() to check link status if rtnl_open() succeed.

Run linuxptp-testsuite and all passed.
SUMMARY:
TOTAL 22
PASSED 22
FAILED 0 ()

Hangbin Liu (4):
rtnl: replace obsolete RTMGRP_LINK with RTNLGRP_LINK for nl groups
port: add FD_RTNL event to track per-port status
clock: remove rtnl fd on clock
clock: remove hash table index2port

clock.c | 86 ++++++++---------------------------------------------------------
clock.h | 7 ++++++
fd.h | 1 +
port.c | 47 +++++++++++++++++++++++++++++------
port.h | 7 ------
rtnl.c | 2 +-
6 files changed, 59 insertions(+), 91 deletions(-)
--
2.5.5
Hangbin Liu
2017-07-10 07:39:28 UTC
Permalink
Signed-off-by: Hangbin Liu <***@gmail.com>
---
rtnl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/rtnl.c b/rtnl.c
index 5cddc4b..d7a430d 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -151,7 +151,7 @@ int rtnl_open(void)

memset(&sa, 0, sizeof(sa));
sa.nl_family = AF_NETLINK;
- sa.nl_groups = RTMGRP_LINK;
+ sa.nl_groups = RTNLGRP_LINK;

fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
if (fd < 0) {
--
2.5.5
Hangbin Liu
2017-07-10 07:39:29 UTC
Permalink
With rtnl socket we can track link status per port(except UDS port).

We can make sure we get the correct interface and latest status with function
port_link_status().

At the same time we need to set clock sde after link down. But we return
EV_FAULT_DETECTED in port_event(), which will not set clock sde. So we need
to set it in port_link_status().

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 5 +++++
clock.h | 7 +++++++
fd.h | 1 +
port.c | 41 ++++++++++++++++++++++++++++++++++++++++-
4 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index b6afba9..4c5c4e3 100644
--- a/clock.c
+++ b/clock.c
@@ -1469,6 +1469,11 @@ struct PortIdentity clock_parent_identity(struct clock *c)
return c->dad.pds.parentPortIdentity;
}

+void clock_set_sde(struct clock *c, int sde)
+{
+ c->sde = sde;
+}
+
int clock_poll(struct clock *c)
{
int cnt, i;
diff --git a/clock.h b/clock.h
index fcd9328..49ecb76 100644
--- a/clock.h
+++ b/clock.h
@@ -205,6 +205,13 @@ void clock_peer_delay(struct clock *c, tmv_t ppd, tmv_t req, tmv_t rx,
double nrr);

/**
+ * Set clock sde
+ * @param c A pointer to a clock instance obtained with clock_create().
+ * @param sde Pass one (1) if need a decision event and zero if not.
+ */
+void clock_set_sde(struct clock *c, int sde);
+
+/**
* Poll for events and dispatch them.
* @param c A pointer to a clock instance obtained with clock_create().
* @return Zero on success, non-zero otherwise.
diff --git a/fd.h b/fd.h
index e328e98..23401f4 100644
--- a/fd.h
+++ b/fd.h
@@ -31,6 +31,7 @@ enum {
FD_QUALIFICATION_TIMER,
FD_MANNO_TIMER,
FD_SYNC_TX_TIMER,
+ FD_RTNL,
N_POLLFD,
};

diff --git a/port.c b/port.c
index ec02825..2896d1a 100644
--- a/port.c
+++ b/port.c
@@ -23,6 +23,7 @@
#include <string.h>
#include <unistd.h>
#include <sys/queue.h>
+#include <net/if.h>

#include "bmc.h"
#include "clock.h"
@@ -32,6 +33,7 @@
#include "phc.h"
#include "port.h"
#include "print.h"
+#include "rtnl.h"
#include "sk.h"
#include "tlv.h"
#include "tmv.h"
@@ -1458,7 +1460,9 @@ static void port_disable(struct port *p)
for (i = 0; i < N_TIMER_FDS; i++) {
close(p->fda.fd[FD_ANNOUNCE_TIMER + i]);
}
- port_clear_fda(p, N_POLLFD);
+
+ /* Keep rtnl socket to get link status info. */
+ port_clear_fda(p, FD_RTNL);
clock_fda_changed(p->clock);
}

@@ -1502,6 +1506,14 @@ static int port_initialize(struct port *p)
if (port_set_announce_tmo(p))
goto no_tmo;

+ /* No need to open rtnl socket on UDS port. */
+ if (transport_type(p->trp) != TRANS_UDS) {
+ if (p->fda.fd[FD_RTNL] == -1)
+ p->fda.fd[FD_RTNL] = rtnl_open();
+ if (p->fda.fd[FD_RTNL] >= 0)
+ rtnl_link_query(p->fda.fd[FD_RTNL]);
+ }
+
port_nrate_initialize(p);

clock_fda_changed(p->clock);
@@ -2025,6 +2037,10 @@ void port_close(struct port *p)
if (port_is_enabled(p)) {
port_disable(p);
}
+
+ if (p->fda.fd[FD_RTNL] >= 0)
+ rtnl_close(p->fda.fd[FD_RTNL]);
+
transport_destroy(p->trp);
tsproc_destroy(p->tsproc);
if (p->fault_fd >= 0)
@@ -2204,6 +2220,24 @@ void port_dispatch(struct port *p, enum fsm_event event, int mdiff)
}
}

+static void port_link_status(void *ctx, int index, int linkup)
+{
+ struct port *p = ctx;
+
+ if (index != if_nametoindex(p->name) || p->link_status == linkup)
+ return;
+
+ p->link_status = linkup;
+ pr_notice("port %hu: link %s", portnum(p), linkup ? "up" : "down");
+
+ /*
+ * A port going down can affect the BMCA result.
+ * Force a state decision event.
+ */
+ if (!p->link_status)
+ clock_set_sde(p->clock, 1);
+}
+
enum fsm_event port_event(struct port *p, int fd_index)
{
enum fsm_event event = EV_NONE;
@@ -2242,6 +2276,11 @@ enum fsm_event port_event(struct port *p, int fd_index)
pr_debug("port %hu: master sync timeout", portnum(p));
port_set_sync_tx_tmo(p);
return port_tx_sync(p) ? EV_FAULT_DETECTED : EV_NONE;
+
+ case FD_RTNL:
+ pr_debug("port %hu: received link status notification", portnum(p));
+ rtnl_link_status(fd, port_link_status, p);
+ return port_link_status_get(p) ? EV_FAULT_CLEARED : EV_FAULT_DETECTED;
}

msg = msg_allocate();
--
2.5.5
Hangbin Liu
2017-07-10 07:39:30 UTC
Permalink
Remove rtnl fd on clock since we track the link status per port now.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 52 +++++-----------------------------------------------
port.c | 6 ------
port.h | 7 -------
3 files changed, 5 insertions(+), 60 deletions(-)

diff --git a/clock.c b/clock.c
index 4c5c4e3..354f038 100644
--- a/clock.c
+++ b/clock.c
@@ -39,7 +39,6 @@
#include "servo.h"
#include "stats.h"
#include "print.h"
-#include "rtnl.h"
#include "sk.h"
#include "tlv.h"
#include "tsproc.h"
@@ -274,9 +273,6 @@ void clock_destroy(struct clock *c)
LIST_FOREACH_SAFE(p, &c->ports, list, tmp) {
clock_remove_port(c, p);
}
- 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);
@@ -326,30 +322,6 @@ 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);
- /*
- * A port going down can affect the BMCA result.
- * Force a state decision event.
- */
- c->sde = 1;
- }
-}
-
static void clock_management_send_error(struct port *p,
struct ptp_message *msg, int error_id)
{
@@ -1133,10 +1105,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
return NULL;
}

- /* Open a RT netlink socket. */
- c->pollfd[0].fd = rtnl_open();
- 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) {
@@ -1164,9 +1132,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);
- if (c->pollfd[0].fd >= 0) {
- rtnl_link_query(c->pollfd[0].fd);
- }
+
return c;
}

@@ -1231,12 +1197,9 @@ static int clock_resize_pollfd(struct clock *c, int new_nports)
{
struct pollfd *new_pollfd;

- /*
- * Need to allocate one descriptor for RT netlink and one
- * whole extra block of fds for UDS.
- */
+ /* Need to allocate one whole extra block of fds for UDS. */
new_pollfd = realloc(c->pollfd,
- (1 + (new_nports + 1) * N_CLOCK_PFD) *
+ (new_nports + 1) * N_CLOCK_PFD *
sizeof(struct pollfd));
if (!new_pollfd)
return -1;
@@ -1261,7 +1224,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 + 1;
+ struct pollfd *dest = c->pollfd;

if (c->pollfd_valid)
return;
@@ -1482,7 +1445,7 @@ int clock_poll(struct clock *c)
struct port *p;

clock_check_pollfd(c);
- cnt = poll(c->pollfd, 1 + (c->nports + 1) * N_CLOCK_PFD, -1);
+ cnt = poll(c->pollfd, (c->nports + 1) * N_CLOCK_PFD, -1);
if (cnt < 0) {
if (EINTR == errno) {
return 0;
@@ -1494,13 +1457,8 @@ int clock_poll(struct clock *c)
return 0;
}

- /* Check the RT netlink. */
cur = c->pollfd;
- if (cur->revents & (POLLIN|POLLPRI)) {
- rtnl_link_status(cur->fd, clock_link_status, c);
- }

- cur++;
LIST_FOREACH(p, &c->ports, list) {
/* Let the ports handle their events. */
for (i = 0; i < N_POLLFD; i++) {
diff --git a/port.c b/port.c
index 2896d1a..34837cc 100644
--- a/port.c
+++ b/port.c
@@ -2411,12 +2411,6 @@ int port_link_status_get(struct port *p)
return p->link_status;
}

-void port_link_status_set(struct port *p, int up)
-{
- p->link_status = up ? 1 : 0;
- pr_notice("port %hu: link %s", portnum(p), up ? "up" : "down");
-}
-
int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
{
struct management_tlv *mgt;
diff --git a/port.h b/port.h
index b00bc64..60fd0a4 100644
--- a/port.h
+++ b/port.h
@@ -132,13 +132,6 @@ int port_number(struct port *p);
int port_link_status_get(struct port *p);

/**
- * Sets the link status for a port.
- * @param p A port instance.
- * @param up Pass one (1) if the link is up and zero if down.
- */
-void port_link_status_set(struct port *p, int up);
-
-/**
* Manage a port according to a given message.
* @param p A pointer previously obtained via port_open().
* @param ingress The port on which 'msg' was received.
--
2.5.5
Hangbin Liu
2017-07-10 07:39:31 UTC
Permalink
Remove index2port since we don't need it now.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 29 -----------------------------
1 file changed, 29 deletions(-)

diff --git a/clock.c b/clock.c
index 354f038..da15882 100644
--- a/clock.c
+++ b/clock.c
@@ -31,7 +31,6 @@
#include "clockcheck.h"
#include "foreign.h"
#include "filter.h"
-#include "hash.h"
#include "missing.h"
#include "msg.h"
#include "phc.h"
@@ -39,7 +38,6 @@
#include "servo.h"
#include "stats.h"
#include "print.h"
-#include "sk.h"
#include "tlv.h"
#include "tsproc.h"
#include "uds.h"
@@ -96,7 +94,6 @@ struct clock {
int nports; /* does not include the UDS port */
int last_port_number;
int sde;
- struct hash *index2port;
int free_running;
int freq_est_interval;
int grand_master_capable; /* for 802.1AS only */
@@ -275,7 +272,6 @@ void clock_destroy(struct clock *c)
}
port_close(c->uds_port);
free(c->pollfd);
- hash_destroy(c->index2port, NULL);
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
}
@@ -773,8 +769,6 @@ static int clock_add_port(struct clock *c, int phc_index,
struct interface *iface)
{
struct port *p, *piter, *lastp = NULL;
- int fd, index;
- char key[16];

if (clock_resize_pollfd(c, c->nports + 1)) {
return -1;
@@ -795,24 +789,6 @@ static int clock_add_port(struct clock *c, int phc_index,
c->nports++;
clock_fda_changed(c);

- /* Remember the index to port mapping, for link status tracking. */
- fd = sk_interface_fd();
- if (fd < 0) {
- return -1;
- }
- index = sk_interface_index(fd, iface->name);
- if (index < 0) {
- close(fd);
- return -1;
- }
- snprintf(key, sizeof(key), "%d", index);
- if (hash_insert(c->index2port, key, p)) {
- pr_err("failed to add port with index %d twice!", index);
- close(fd);
- return -1;
- }
- close(fd);
-
return 0;
}

@@ -1113,11 +1089,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}
clock_fda_changed(c);

- c->index2port = hash_create();
- if (!c->index2port) {
- pr_err("failed create index-to-port hash table");
- return NULL;
- }
/* Create the ports. */
STAILQ_FOREACH(iface, &config->interfaces, list) {
if (clock_add_port(c, phc_index, timestamping, iface)) {
--
2.5.5
Hangbin Liu
2017-07-10 07:39:32 UTC
Permalink
Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 29 -----------------------------
1 file changed, 29 deletions(-)

diff --git a/clock.c b/clock.c
index 354f038..da15882 100644
--- a/clock.c
+++ b/clock.c
@@ -31,7 +31,6 @@
#include "clockcheck.h"
#include "foreign.h"
#include "filter.h"
-#include "hash.h"
#include "missing.h"
#include "msg.h"
#include "phc.h"
@@ -39,7 +38,6 @@
#include "servo.h"
#include "stats.h"
#include "print.h"
-#include "sk.h"
#include "tlv.h"
#include "tsproc.h"
#include "uds.h"
@@ -96,7 +94,6 @@ struct clock {
int nports; /* does not include the UDS port */
int last_port_number;
int sde;
- struct hash *index2port;
int free_running;
int freq_est_interval;
int grand_master_capable; /* for 802.1AS only */
@@ -275,7 +272,6 @@ void clock_destroy(struct clock *c)
}
port_close(c->uds_port);
free(c->pollfd);
- hash_destroy(c->index2port, NULL);
if (c->clkid != CLOCK_REALTIME) {
phc_close(c->clkid);
}
@@ -773,8 +769,6 @@ static int clock_add_port(struct clock *c, int phc_index,
struct interface *iface)
{
struct port *p, *piter, *lastp = NULL;
- int fd, index;
- char key[16];

if (clock_resize_pollfd(c, c->nports + 1)) {
return -1;
@@ -795,24 +789,6 @@ static int clock_add_port(struct clock *c, int phc_index,
c->nports++;
clock_fda_changed(c);

- /* Remember the index to port mapping, for link status tracking. */
- fd = sk_interface_fd();
- if (fd < 0) {
- return -1;
- }
- index = sk_interface_index(fd, iface->name);
- if (index < 0) {
- close(fd);
- return -1;
- }
- snprintf(key, sizeof(key), "%d", index);
- if (hash_insert(c->index2port, key, p)) {
- pr_err("failed to add port with index %d twice!", index);
- close(fd);
- return -1;
- }
- close(fd);
-
return 0;
}

@@ -1113,11 +1089,6 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}
clock_fda_changed(c);

- c->index2port = hash_create();
- if (!c->index2port) {
- pr_err("failed create index-to-port hash table");
- return NULL;
- }
/* Create the ports. */
STAILQ_FOREACH(iface, &config->interfaces, list) {
if (clock_add_port(c, phc_index, timestamping, iface)) {
--
2.5.5
Hangbin Liu
2017-07-10 08:00:48 UTC
Permalink
Sorry, this patch was send by accident. Please ignore this one.

Thanks
Hangbin
Post by Hangbin Liu
---
clock.c | 29 -----------------------------
1 file changed, 29 deletions(-)
Richard Cochran
2017-07-13 05:37:58 UTC
Permalink
Post by Hangbin Liu
As discussed, now we convert single rtnl FD into per-port FD and remove
index2port hash table from clock.c
1. Let program still work even rtnl_open() failed.
2. Start rtnl_link_query() to check link status if rtnl_open() succeed.
Series applied.

Thanks,
Richard

Loading...