[Linuxptp-devel] [PATCHv2 0/9] Add linuxptp bond failover support

[Hangbin Liu]

This patch set is to add linuxptp bond fail over support.

The main idea is get bond's active slave interface via rtnl socket and store
it in struct interface.

When active interface changed, we update the port phc index and switch to
new phc on clock.

After we get true ts interface, we need to use this interface in
sk_timestamping_init() in transport_open().

Also we need update clock and phc_index in phc2sys.

v2:
1. After the rtnl per port update, now we update ts_iface info in
port_link_status().
2. Fix port_dispatch event flood when change ts_iface info. This issue
only happen with bond interface when fail over. Normal ethernet
interface do not have this problem.

Thanks
Hangbin

---
Testing:
1. Run linuxptp-testsuite and all passed.

2. Run ptp4l on non-bond interface

3. Run ptp4l on bond interface with following modes. the result looks good.
# ./ptp4l -S -2 -i bond0 -m
# ./ptp4l -i bond0 -m

4. Any other test scenario recommend?


Hangbin Liu (9):
config: add new element ts_iface in struct interface
port: track interface info in port
rtnl: update rtgenmsg to ifinfomsg when request link info
rtnl: add function rtnl_link_info
clock: use ts interface to get ts info
clock: check required_modes before use new ts info
port.c: fix port_dispatch event flood when change ts_iface info
transport: pass struct interface to transport_open
phc2sys: update clock clkid and phc_index if device changed

clock.c | 52 +++++++++++++-------
clock.h | 7 +++
config.c | 1 -
config.h | 1 +
phc2sys.c | 50 ++++++++++++++++++--
pmc_common.c | 5 +-
port.c | 64 +++++++++++++++++++------
raw.c | 5 +-
rtnl.c | 134 +++++++++++++++++++++++++++++++++++++++++++++++++---
rtnl.h | 17 +++++--
transport.c | 4 +-
transport.h | 3 +-
transport_private.h | 4 +-
udp.c | 7 +--
udp6.c | 7 +--
uds.c | 3 +-
16 files changed, 302 insertions(+), 62 deletions(-)

[Hangbin Liu]

Add new element ts_iface in struct interface to track real ts interface.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
config.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/config.h b/config.h
index 1cc7051..f0ae3fe 100644
--- a/config.h
+++ b/config.h
@@ -36,6 +36,7 @@
struct interface {
STAILQ_ENTRY(interface) list;
char name[MAX_IFNAME_SIZE + 1];
+ char ts_iface[MAX_IFNAME_SIZE + 1];
struct sk_ts_info ts_info;
};

[Hangbin Liu]

Signed-off-by: Hangbin Liu <***@gmail.com>
---
port.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/port.c b/port.c
index 34837cc..849a7c1 100644
--- a/port.c
+++ b/port.c
@@ -68,6 +68,7 @@ struct nrate_estimator {
struct port {
LIST_ENTRY(port) list;
char *name;
+ struct interface *iface;
struct clock *clock;
struct transport *trp;
enum timestamp_type timestamping;
@@ -2619,6 +2620,7 @@ struct port *port_open(int phc_index,
}

p->name = interface->name;
+ p->iface = interface;
p->asymmetry = config_get_int(cfg, p->name, "delayAsymmetry");
p->asymmetry <<= 16;
p->announce_span = transport == TRANS_UDS ? 0 : ANNOUNCE_SPAN;

[Hangbin Liu]

The previous function use general message and will dump all interfaces'
information. Now update with ifinfomsg so we could get specific interface's
information.

We still could get all interfaces' info if set if_index to 0.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
port.c | 2 +-
rtnl.c | 12 +++++++-----
rtnl.h | 7 ++++---
3 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/port.c b/port.c
index 849a7c1..21ab3ea 100644
--- a/port.c
+++ b/port.c
@@ -1512,7 +1512,7 @@ static int port_initialize(struct port *p)
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]);
+ rtnl_link_query(p->fda.fd[FD_RTNL], 0);
}

port_nrate_initialize(p);
diff --git a/rtnl.c b/rtnl.c
index d7a430d..04e1918 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -42,7 +42,7 @@ int rtnl_close(int fd)
return close(fd);
}

-int rtnl_link_query(int fd)
+int rtnl_link_query(int fd, unsigned int if_index)
{
struct sockaddr_nl sa;
struct msghdr msg;
@@ -51,19 +51,21 @@ int rtnl_link_query(int fd)

struct {
struct nlmsghdr hdr;
- struct rtgenmsg gen;
+ struct ifinfomsg ifm;
} __attribute__((packed)) request;

memset(&sa, 0, sizeof(sa));
sa.nl_family = AF_NETLINK;

memset(&request, 0, sizeof(request));
- request.hdr.nlmsg_len = NLMSG_LENGTH(sizeof(request.gen));
+ request.hdr.nlmsg_len = NLMSG_LENGTH(sizeof(request.ifm));
request.hdr.nlmsg_type = RTM_GETLINK;
- request.hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;
+ request.hdr.nlmsg_flags = NLM_F_REQUEST;
request.hdr.nlmsg_seq = 1;
request.hdr.nlmsg_pid = 0;
- request.gen.rtgen_family = AF_UNSPEC;
+ request.ifm.ifi_family = AF_UNSPEC;
+ request.ifm.ifi_index = if_index;
+ request.ifm.ifi_change = 0xffffffff;

iov.iov_base = &request;
iov.iov_len = sizeof(request);
diff --git a/rtnl.h b/rtnl.h
index f1871f2..b4db40e 100644
--- a/rtnl.h
+++ b/rtnl.h
@@ -31,10 +31,11 @@ int rtnl_close(int fd);

/**
* Request the link status from the kernel.
- * @param fd A socket obtained via rtnl_open().
- * @return Zero on success, non-zero otherwise.
+ * @param fd A socket obtained via rtnl_open().
+ * @param index An interface index.
+ * @return Zero on success, non-zero otherwise.
*/
-int rtnl_link_query(int fd);
+int rtnl_link_query(int fd, unsigned int index);

/**
* Read kernel messages looking for a link up/down events.

[Hangbin Liu]

Add function rtnl_link_info() to get bond's active interface. If there is
no slave interface, then use our own name as ts interface.

Also add new parameter ts_iface for call back function port_link_status()
to make aware of interface change.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
port.c | 2 +-
rtnl.c | 122 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
rtnl.h | 10 +++++-
3 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/port.c b/port.c
index 21ab3ea..834eb45 100644
--- a/port.c
+++ b/port.c
@@ -2221,7 +2221,7 @@ void port_dispatch(struct port *p, enum fsm_event event, int mdiff)
}
}

-static void port_link_status(void *ctx, int index, int linkup)
+static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
{
struct port *p = ctx;

diff --git a/rtnl.c b/rtnl.c
index 04e1918..b499467 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -31,6 +31,9 @@

static int rtnl_len;
static char *rtnl_buf;
+static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len);
+#define rtnl_nested_rtattr_parse(tb, max, rta) \
+ (rtnl_rtattr_parse((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta)))

int rtnl_close(int fd)
{
@@ -84,6 +87,62 @@ int rtnl_link_query(int fd, unsigned int if_index)
return 0;
}

+static inline __u32 rta_getattr_u32(const struct rtattr *rta)
+{
+ return *(__u32 *)RTA_DATA(rta);
+}
+
+static inline const char *rta_getattr_str(const struct rtattr *rta)
+{
+ return (const char *)RTA_DATA(rta);
+}
+
+static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len)
+{
+ unsigned short type;
+
+ memset(tb, 0, sizeof(struct rtattr *) * (max + 1));
+ while (RTA_OK(rta, len)) {
+ type = rta->rta_type;
+ if ((type <= max) && (!tb[type]))
+ tb[type] = rta;
+ rta = RTA_NEXT(rta, len);
+ }
+ if (len)
+ fprintf(stderr, "!!!Deficit %d, rta_len=%d\n",
+ len, rta->rta_len);
+ return 0;
+}
+
+static int rtnl_linkinfo_parse(struct rtattr *rta, char *device)
+{
+ int index;
+ struct rtattr *linkinfo[IFLA_INFO_MAX + 1];
+ struct rtattr *bond[IFLA_BOND_MAX + 1];
+
+ rtnl_nested_rtattr_parse(linkinfo, IFLA_INFO_MAX, rta);
+
+ if (linkinfo[IFLA_INFO_KIND]) {
+ const char *kind = rta_getattr_str(linkinfo[IFLA_INFO_KIND]);
+
+ if (kind && !strncmp(kind, "bond", 4) &&
+ linkinfo[IFLA_INFO_DATA]) {
+ rtnl_nested_rtattr_parse(bond, IFLA_BOND_MAX,
+ linkinfo[IFLA_INFO_DATA]);
+
+ if (bond[IFLA_BOND_ACTIVE_SLAVE]) {
+ index = rta_getattr_u32(bond[IFLA_BOND_ACTIVE_SLAVE]);
+
+ if (!if_indextoname(index, device)) {
+ pr_err("failed to get device name: %m");
+ return -1;
+ }
+ }
+ }
+ }
+ return 0;
+}
+
int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
{
int index, len;
@@ -92,6 +151,18 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
struct msghdr msg;
struct nlmsghdr *nh;
struct ifinfomsg *info = NULL;
+ char *device;
+ struct rtattr *tb[IFLA_MAX+1];
+
+ if (cb)
+ device = calloc(1, sizeof(MAX_IFNAME_SIZE + 1));
+ else
+ device = (char *)ctx;
+
+ if (!device) {
+ fprintf(stderr, "rtnl: no enought memory for device name\n");
+ return -1;
+ }

if (!rtnl_buf) {
rtnl_len = 4096;
@@ -140,7 +211,18 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
index = info->ifi_index;
pr_debug("interface index %d is %s", index,
info->ifi_flags & IFF_RUNNING ? "up" : "down");
- cb(ctx, index, info->ifi_flags & IFF_RUNNING ? 1 : 0);
+
+ rtnl_rtattr_parse(tb, IFLA_MAX, IFLA_RTA(info),
+ IFLA_PAYLOAD(nh));
+
+ if (tb[IFLA_LINKINFO])
+ rtnl_linkinfo_parse(tb[IFLA_LINKINFO], device);
+
+ if (cb) {
+ cb(ctx, index, info->ifi_flags & IFF_RUNNING ? 1 : 0, device);
+ free(device);
+ }
+
}
}
return 0;
@@ -167,3 +249,41 @@ int rtnl_open(void)
}
return fd;
}
+
+int rtnl_link_info(struct interface *iface)
+{
+ int fd, index;
+
+ index = if_nametoindex(iface->name);
+
+ if (index == 0) {
+ pr_err("failed to get interface %s index: %m", iface->name);
+ goto no_fd;
+ }
+
+ fd = rtnl_open();
+ if (fd < 0)
+ goto no_fd;
+
+ if (rtnl_link_query(fd, index))
+ goto no_info;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto no_info;
+
+ /* If we do not have a slave, then use our own interface name
+ * as ts_iface
+ */
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+
+ rtnl_close(fd);
+ return 0;
+
+no_info:
+ rtnl_close(fd);
+no_fd:
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+
+ return -1;
+}
diff --git a/rtnl.h b/rtnl.h
index b4db40e..c382088 100644
--- a/rtnl.h
+++ b/rtnl.h
@@ -20,7 +20,9 @@
#ifndef HAVE_RTNL_H
#define HAVE_RTNL_H

-typedef void (*rtnl_callback)(void *ctx, int index, int linkup);
+#include "config.h"
+
+typedef void (*rtnl_callback)(void *ctx, int index, int linkup, char *device);

/**
* Close a RT netlink socket.
@@ -52,4 +54,10 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx);
*/
int rtnl_open(void);

+/**
+ * Get interface link status and ts_iface information
+ * @param iface struct interface.
+ * @return Zero on success, or -1 on error.
+ */
+int rtnl_link_info(struct interface *iface);
#endif

[Keller, Jacob E]

-----Original Message-----
Sent: Saturday, July 15, 2017 6:33 AM
Subject: [Linuxptp-devel] [PATCHv2 4/9] rtnl: add function rtnl_link_info
Add function rtnl_link_info() to get bond's active interface. If there is
no slave interface, then use our own name as ts interface.
Also add new parameter ts_iface for call back function port_link_status()
to make aware of interface change.
---
port.c | 2 +-
rtnl.c | 122
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
++-
rtnl.h | 10 +++++-
3 files changed, 131 insertions(+), 3 deletions(-)
diff --git a/port.c b/port.c
index 21ab3ea..834eb45 100644
--- a/port.c
+++ b/port.c
@@ -2221,7 +2221,7 @@ void port_dispatch(struct port *p, enum fsm_event event, int mdiff)
}
}
-static void port_link_status(void *ctx, int index, int linkup)
+static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
{
struct port *p = ctx;
diff --git a/rtnl.c b/rtnl.c
index 04e1918..b499467 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -31,6 +31,9 @@
static int rtnl_len;
static char *rtnl_buf;
+static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len);
+#define rtnl_nested_rtattr_parse(tb, max, rta) \
+ (rtnl_rtattr_parse((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta)))
int rtnl_close(int fd)
{
@@ -84,6 +87,62 @@ int rtnl_link_query(int fd, unsigned int if_index)
return 0;
}
+static inline __u32 rta_getattr_u32(const struct rtattr *rta)
+{
+ return *(__u32 *)RTA_DATA(rta);
+}
+
+static inline const char *rta_getattr_str(const struct rtattr *rta)
+{
+ return (const char *)RTA_DATA(rta);
+}
+
+static int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len)
+{
+ unsigned short type;
+
+ memset(tb, 0, sizeof(struct rtattr *) * (max + 1));
+ while (RTA_OK(rta, len)) {
+ type = rta->rta_type;
+ if ((type <= max) && (!tb[type]))
+ tb[type] = rta;
+ rta = RTA_NEXT(rta, len);
+ }
+ if (len)
+ fprintf(stderr, "!!!Deficit %d, rta_len=%d\n",
+ len, rta->rta_len);
+ return 0;
+}
+
+static int rtnl_linkinfo_parse(struct rtattr *rta, char *device)
+{
+ int index;
+ struct rtattr *linkinfo[IFLA_INFO_MAX + 1];
+ struct rtattr *bond[IFLA_BOND_MAX + 1];
+
+ rtnl_nested_rtattr_parse(linkinfo, IFLA_INFO_MAX, rta);
+
+ if (linkinfo[IFLA_INFO_KIND]) {
+ const char *kind = rta_getattr_str(linkinfo[IFLA_INFO_KIND]);
+
+ if (kind && !strncmp(kind, "bond", 4) &&
+ linkinfo[IFLA_INFO_DATA]) {
+ rtnl_nested_rtattr_parse(bond, IFLA_BOND_MAX,
+ linkinfo[IFLA_INFO_DATA]);
+
+ if (bond[IFLA_BOND_ACTIVE_SLAVE]) {
+ index =
rta_getattr_u32(bond[IFLA_BOND_ACTIVE_SLAVE]);
+
+ if (!if_indextoname(index, device)) {
+ pr_err("failed to get device name: %m");
+ return -1;
+ }
+ }
+ }
+ }
+ return 0;
+}
+
int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
{
int index, len;
@@ -92,6 +151,18 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
struct msghdr msg;
struct nlmsghdr *nh;
struct ifinfomsg *info = NULL;
+ char *device;
+ struct rtattr *tb[IFLA_MAX+1];
+
+ if (cb)
+ device = calloc(1, sizeof(MAX_IFNAME_SIZE + 1));
+ else
+ device = (char *)ctx;
+
+ if (!device) {
+ fprintf(stderr, "rtnl: no enought memory for device name\n");
+ return -1;
+ }
if (!rtnl_buf) {
rtnl_len = 4096;
@@ -140,7 +211,18 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx)
index = info->ifi_index;
pr_debug("interface index %d is %s", index,
info->ifi_flags & IFF_RUNNING ? "up" : "down");
- cb(ctx, index, info->ifi_flags & IFF_RUNNING ? 1 : 0);
+
+ rtnl_rtattr_parse(tb, IFLA_MAX, IFLA_RTA(info),
+ IFLA_PAYLOAD(nh));
+
+ if (tb[IFLA_LINKINFO])
+ rtnl_linkinfo_parse(tb[IFLA_LINKINFO], device);
+
+ if (cb) {
0, device);
+ free(device);
+ }
+
}
}
return 0;
@@ -167,3 +249,41 @@ int rtnl_open(void)
}
return fd;
}
+
+int rtnl_link_info(struct interface *iface)
+{
+ int fd, index;
+
+ index = if_nametoindex(iface->name);
+
+ if (index == 0) {
+ pr_err("failed to get interface %s index: %m", iface->name);
+ goto no_fd;
+ }
+
+ fd = rtnl_open();
+ if (fd < 0)
+ goto no_fd;
+
+ if (rtnl_link_query(fd, index))
+ goto no_info;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto no_info;
+
+ /* If we do not have a slave, then use our own interface name
+ * as ts_iface
+ */
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+

Seems like we're duplicating code here just to maintain a return value?

+ rtnl_close(fd);
+ return 0;
+
+ rtnl_close(fd);
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+
+ return -1;
+}
diff --git a/rtnl.h b/rtnl.h
index b4db40e..c382088 100644
--- a/rtnl.h
+++ b/rtnl.h
@@ -20,7 +20,9 @@
#ifndef HAVE_RTNL_H
#define HAVE_RTNL_H
-typedef void (*rtnl_callback)(void *ctx, int index, int linkup);
+#include "config.h"
+
+typedef void (*rtnl_callback)(void *ctx, int index, int linkup, char *device);
/**
* Close a RT netlink socket.
@@ -52,4 +54,10 @@ int rtnl_link_status(int fd, rtnl_callback cb, void *ctx);
*/
int rtnl_open(void);
+/**
+ * Get interface link status and ts_iface information
+ */
+int rtnl_link_info(struct interface *iface);
#endif
--
2.5.5
------------------------------------------------------------------------------
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
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel

[Hangbin Liu]

Hi Keller,

Post by Keller, Jacob E

Post by Hangbin Liu
+int rtnl_link_info(struct interface *iface)
+{
+ int fd, index;
+
+ index = if_nametoindex(iface->name);
+
+ if (index == 0) {
+ pr_err("failed to get interface %s index: %m", iface->name);
+ goto no_fd;
+ }
+
+ fd = rtnl_open();
+ if (fd < 0)
+ goto no_fd;
+
+ if (rtnl_link_query(fd, index))
+ goto no_info;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto no_info;
+
+ /* If we do not have a slave, then use our own interface name
+ * as ts_iface
+ */
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+

Seems like we're duplicating code here just to maintain a return value?

Post by Hangbin Liu
+ rtnl_close(fd);
+ return 0;
+
+ rtnl_close(fd);
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+
+ return -1;
+}

hmm, you are right, the code is a little clumsy, how about

fd = rtnl_open();

if (fd > 0 && ! rtnl_link_query(fd, index))
rtnl_link_status(fd, NULL, iface->ts_iface);

/* If we do not have a slave, then use our own interface name
* as ts_iface
*/
if (iface->ts_iface[0] == '\0')
strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);

if (fd > 0) {
rtnl_close(fd);
return 0;
} else {
return -1;
}

Thanks
Hangbin

[Richard Cochran]

You should return early when there are no resources to free:

+int rtnl_link_info(struct interface *iface)
+{
+ int fd, index;
int err, fd, index; (See below for 'err')

+ index = if_nametoindex(iface->name);
+
+ if (index == 0) {
+ pr_err("failed to get interface %s index: %m", iface->name);
return -1;
+ }
+
+ fd = rtnl_open();
+ if (fd < 0)
return fd;

+ if (rtnl_link_query(fd, index))
+ goto no_info;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto no_info;

Here you want to let the error code propagate:

err = rtnl_link_query(fd, index);
if (err) {
goto no_info;
}
err = rtnl_link_status(fd, NULL, iface->ts_iface);
if (err) {
goto no_info;
}

+ /* If we do not have a slave, then use our own interface name
+ * as ts_iface
+ */
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);

Move these two lines to the call site. They could be place in a
helper function in order to make the purpose clear without a comment.
See below...

no_info:
rtnl_close(fd);
return err;
}


clock.c
~~~~~~~

/*
* If we do not have a slave or the rtnl query failed, then use our
* own interface name as the time stamping interface name.
*/
static void ensure_ts_iface(iface)
{
if (iface->ts_iface[0] == '\0')
strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
}

clock_create()
{
STAILQ_FOREACH(iface, &config->interfaces, list) {
rtnl_link_info(iface);
ensure_ts_iface(iface);
...
}
}

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, July 18, 2017 10:52 PM
Subject: Re: [Linuxptp-devel] [PATCHv2 4/9] rtnl: add function rtnl_link_info
+int rtnl_link_info(struct interface *iface)
+{
+ int fd, index;
int err, fd, index; (See below for 'err')
+ index = if_nametoindex(iface->name);
+
+ if (index == 0) {
+ pr_err("failed to get interface %s index: %m", iface->name);
return -1;
+ }
+
+ fd = rtnl_open();
+ if (fd < 0)
return fd;
+ if (rtnl_link_query(fd, index))
+ goto no_info;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto no_info;
err = rtnl_link_query(fd, index);
if (err) {
goto no_info;
}
err = rtnl_link_status(fd, NULL, iface->ts_iface);
if (err) {
goto no_info;
}
+ /* If we do not have a slave, then use our own interface name
+ * as ts_iface
+ */
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
Move these two lines to the call site. They could be place in a
helper function in order to make the purpose clear without a comment.
See below...
rtnl_close(fd);
return err;
}
clock.c
~~~~~~~
/*
* If we do not have a slave or the rtnl query failed, then use our
* own interface name as the time stamping interface name.
*/
static void ensure_ts_iface(iface)
{
if (iface->ts_iface[0] == '\0')
strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
}
clock_create()
{
STAILQ_FOREACH(iface, &config->interfaces, list) {
rtnl_link_info(iface);
ensure_ts_iface(iface);
...
}
}

Richard's solution is even better.

Thanks,
Jake

[Keller, Jacob E]

-----Original Message-----
Sent: Tuesday, July 18, 2017 8:08 PM
Subject: Re: [Linuxptp-devel] [PATCHv2 4/9] rtnl: add function rtnl_link_info
Hi Keller,

Post by Keller, Jacob E

Post by Hangbin Liu
+int rtnl_link_info(struct interface *iface)
+{
+ int fd, index;
+
+ index = if_nametoindex(iface->name);
+
+ if (index == 0) {
+ pr_err("failed to get interface %s index: %m", iface->name);
+ goto no_fd;
+ }
+
+ fd = rtnl_open();
+ if (fd < 0)
+ goto no_fd;
+
+ if (rtnl_link_query(fd, index))
+ goto no_info;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto no_info;
+
+ /* If we do not have a slave, then use our own interface name
+ * as ts_iface
+ */
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+

Seems like we're duplicating code here just to maintain a return value?

Post by Hangbin Liu
+ rtnl_close(fd);
+ return 0;
+
+ rtnl_close(fd);
+ if (iface->ts_iface[0] == '\0')
+ strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
+
+ return -1;
+}

hmm, you are right, the code is a little clumsy, how about
fd = rtnl_open();
if (fd > 0 && ! rtnl_link_query(fd, index))
rtnl_link_status(fd, NULL, iface->ts_iface);
/* If we do not have a slave, then use our own interface name
* as ts_iface
*/
if (iface->ts_iface[0] == '\0')
strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
if (fd > 0) {
rtnl_close(fd);
return 0;
} else {
return -1;
}

I think the gotos are still fine, if you order things correctly, and track an error value:

int err = 0;

fd = rtnl_open();
if (fd < 0) {
err = -1;
goto no_fd;
}

if (rtnl_link_query(fd, index))
err = -1;
else if (rtnl_link_status(fd, NULL, iface->ts_iface))
err = -1;

rtnl_close(fd);
no_fd:
if (iface->ts_iface[0] == '\0')
strncpy(iface->ts_iface, iface->name, MAX_IFNAME_SIZE);
return err;

IMO this is the most clear and avoids the duplication of the checks.

Thanks,
Jake

[Richard Cochran]

Post by Hangbin Liu
if (rtnl_link_query(fd, index))
err = -1;
else if (rtnl_link_status(fd, NULL, iface->ts_iface))
err = -1;

This is poor practice, because you clobber the code returned from
the sub-routine that experienced the error.

Thanks,
Richard

[Keller, Jacob E]

-----Original Message-----
Sent: Wednesday, July 19, 2017 9:53 PM
Subject: Re: [Linuxptp-devel] [PATCHv2 4/9] rtnl: add function rtnl_link_info

Post by Hangbin Liu
if (rtnl_link_query(fd, index))
err = -1;
else if (rtnl_link_status(fd, NULL, iface->ts_iface))
err = -1;

This is poor practice, because you clobber the code returned from
the sub-routine that experienced the error.
Thanks,
Richard

Correct. I like your proposed solution better. I was merely re-writing to show how the logic flow would work, and preserved the return code the original code had.

Thanks,
Jake

[Richard Cochran]

Post by Keller, Jacob E
Correct. I like your proposed solution better. I was merely
re-writing to show how the logic flow would work, and preserved the
return code the original code had.

Yeah, sorry for the noise. I was so tired, I thought that your mail
was liuhangbin's, and so I replied.

Thanks,
Richard

[Hangbin Liu]

Now the ts interface will be either the active slave or the interface name,
which is the exactly interface we need to get ts info.

If there is a fail over and ts_iface changed. We need to switch phc index and
reset the port status.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 3 +++
config.c | 1 -
port.c | 10 ++++++++++
3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/clock.c b/clock.c
index da15882..26cbd69 100644
--- a/clock.c
+++ b/clock.c
@@ -38,6 +38,7 @@
#include "servo.h"
#include "stats.h"
#include "print.h"
+#include "rtnl.h"
#include "tlv.h"
#include "tsproc.h"
#include "uds.h"
@@ -932,6 +933,8 @@ struct clock *clock_create(enum clock_type type, struct config *config,
break;
}
STAILQ_FOREACH(iface, &config->interfaces, list) {
+ rtnl_link_info(iface);
+ sk_get_ts_info(iface->ts_iface, &iface->ts_info);
if (iface->ts_info.valid &&
((iface->ts_info.so_timestamping & required_modes) != required_modes)) {
pr_err("interface '%s' does not support "
diff --git a/config.c b/config.c
index e6fe676..bbaf36e 100644
--- a/config.c
+++ b/config.c
@@ -633,7 +633,6 @@ struct interface *config_create_interface(char *name, struct config *cfg)
}

strncpy(iface->name, name, MAX_IFNAME_SIZE);
- sk_get_ts_info(iface->name, &iface->ts_info);
STAILQ_INSERT_TAIL(&cfg->interfaces, iface, list);
cfg->n_interfaces++;

diff --git a/port.c b/port.c
index 834eb45..98c76e7 100644
--- a/port.c
+++ b/port.c
@@ -2231,6 +2231,16 @@ static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
p->link_status = linkup;
pr_notice("port %hu: link %s", portnum(p), linkup ? "up" : "down");

+ /* ts_iface changed */
+ if (ts_iface[0] != '\0' && strcmp(p->iface->ts_iface, ts_iface)) {
+ strncpy(p->iface->ts_iface, ts_iface, MAX_IFNAME_SIZE);
+ sk_get_ts_info(p->iface->ts_iface, &p->iface->ts_info);
+ if (p->iface->ts_info.valid) {
+ p->phc_index = p->iface->ts_info.phc_index;
+ clock_switch_phc(p->clock, p->phc_index);
+ }
+ }
+
/*
* A port going down can affect the BMCA result.
* Force a state decision event.

[Hangbin Liu]

When it_iface first used or changed, we need to check the requried
timestamping mode.

If ts_iface changed and does not support requested timestamping mode,
set link status down by force to trigger port EV_FAULT_DETECTED event.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 49 +++++++++++++++++++++++++++++++------------------
clock.h | 7 +++++++
port.c | 14 ++++++++++++--
3 files changed, 50 insertions(+), 20 deletions(-)

diff --git a/clock.c b/clock.c
index 26cbd69..575ed08 100644
--- a/clock.c
+++ b/clock.c
@@ -107,6 +107,7 @@ struct clock {
int time_flags; /* grand master role */
int time_source; /* grand master role */
enum servo_state servo_state;
+ enum timestamp_type timestamping;
tmv_t master_offset;
tmv_t path_delay;
tmv_t ingress_ts;
@@ -806,6 +807,34 @@ static void clock_remove_port(struct clock *c, struct port *p)
port_close(p);
}

+int clock_required_modes(struct clock *c)
+{
+ int required_modes = 0;
+
+ switch (c->timestamping) {
+ case TS_SOFTWARE:
+ required_modes |= SOF_TIMESTAMPING_TX_SOFTWARE |
+ SOF_TIMESTAMPING_RX_SOFTWARE |
+ SOF_TIMESTAMPING_SOFTWARE;
+ break;
+ case TS_LEGACY_HW:
+ required_modes |= SOF_TIMESTAMPING_TX_HARDWARE |
+ SOF_TIMESTAMPING_RX_HARDWARE |
+ SOF_TIMESTAMPING_SYS_HARDWARE;
+ break;
+ case TS_HARDWARE:
+ case TS_ONESTEP:
+ required_modes |= SOF_TIMESTAMPING_TX_HARDWARE |
+ SOF_TIMESTAMPING_RX_HARDWARE |
+ SOF_TIMESTAMPING_RAW_HARDWARE;
+ break;
+ default:
+ break;
+ }
+
+ return required_modes;
+}
+
struct clock *clock_create(enum clock_type type, struct config *config,
const char *phc_device)
{
@@ -914,24 +943,8 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}

/* Check the time stamping mode on each interface. */
- switch (timestamping) {
- case TS_SOFTWARE:
- required_modes |= SOF_TIMESTAMPING_TX_SOFTWARE |
- SOF_TIMESTAMPING_RX_SOFTWARE |
- SOF_TIMESTAMPING_SOFTWARE;
- break;
- case TS_LEGACY_HW:
- required_modes |= SOF_TIMESTAMPING_TX_HARDWARE |
- SOF_TIMESTAMPING_RX_HARDWARE |
- SOF_TIMESTAMPING_SYS_HARDWARE;
- break;
- case TS_HARDWARE:
- case TS_ONESTEP:
- required_modes |= SOF_TIMESTAMPING_TX_HARDWARE |
- SOF_TIMESTAMPING_RX_HARDWARE |
- SOF_TIMESTAMPING_RAW_HARDWARE;
- break;
- }
+ c->timestamping = timestamping;
+ required_modes = clock_required_modes(c);
STAILQ_FOREACH(iface, &config->interfaces, list) {
rtnl_link_info(iface);
sk_get_ts_info(iface->ts_iface, &iface->ts_info);
diff --git a/clock.h b/clock.h
index 49ecb76..18d7250 100644
--- a/clock.h
+++ b/clock.h
@@ -73,6 +73,13 @@ UInteger8 clock_class(struct clock *c);
struct config *clock_config(struct clock *c);

/**
+ * Obtains the required time stamping mode.
+ * @param c The clock instance.
+ * @return The value of required time stamping mode.
+ */
+int clock_required_modes(struct clock *c);
+
+/**
* Create a clock instance. There can only be one clock in any system,
* so subsequent calls will destroy the previous clock instance.
*
diff --git a/port.c b/port.c
index 98c76e7..3c4e39f 100644
--- a/port.c
+++ b/port.c
@@ -2224,6 +2224,7 @@ void port_dispatch(struct port *p, enum fsm_event event, int mdiff)
static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
{
struct port *p = ctx;
+ int required_modes = 0;

if (index != if_nametoindex(p->name) || p->link_status == linkup)
return;
@@ -2235,9 +2236,18 @@ static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
if (ts_iface[0] != '\0' && strcmp(p->iface->ts_iface, ts_iface)) {
strncpy(p->iface->ts_iface, ts_iface, MAX_IFNAME_SIZE);
sk_get_ts_info(p->iface->ts_iface, &p->iface->ts_info);
+
if (p->iface->ts_info.valid) {
- p->phc_index = p->iface->ts_info.phc_index;
- clock_switch_phc(p->clock, p->phc_index);
+ required_modes = clock_required_modes(p->clock);
+ if ((p->iface->ts_info.so_timestamping & required_modes) != required_modes) {
+ pr_err("interface '%s' does not support requested "
+ "timestamping mode, set link status down by force.",
+ p->iface->ts_iface);
+ p->link_status = 0;
+ } else {
+ p->phc_index = p->iface->ts_info.phc_index;
+ clock_switch_phc(p->clock, p->phc_index);
+ }
}
}

[Hangbin Liu]

When bond active slave interface changed. We will receive multi rtnl
messages, i.e. bond and slave interfaces status changing information.

No matter what's the iface index of the rtnl messages, it will trigger
FD_RTNL event. Since the bond interface link status is keeping on. We
will return EV_FAULT_DETECTED every time. At the same time transport_recv
will keep return fail. Then it become port_dispatch message flood. e.g.

ptp4l[77068.125]: recvmsg failed: Network is down
ptp4l[77068.125]: port 1: recv message failed
ptp4l[77068.125]: port 1: LISTENING to FAULTY on FAULT_DETECTED (FT_UNSPECIFIED)
ptp4l[77068.152]: port 1: FAULTY to LISTENING on INIT_COMPLETE
ptp4l[77068.152]: recvmsg failed: Network is down
ptp4l[77068.152]: port 1: recv message failed
ptp4l[77068.152]: port 1: LISTENING to FAULTY on FAULT_DETECTED (FT_UNSPECIFIED)
ptp4l[77068.179]: port 1: FAULTY to LISTENING on INIT_COMPLETE
...

Fix this by moving the port_dispatch event to port_link_status() and return
EV_NONE in port_event().

At the same time, when ts_inface info changed, the port link status will not
change. So remove the port link status check in port_link_status().

Now we will record status_changed with two scenarios. 1) link status changed.
2) ts_iface changed. Others we will do nothing.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
port.c | 32 ++++++++++++++++++++++----------
1 file changed, 22 insertions(+), 10 deletions(-)

diff --git a/port.c b/port.c
index 3c4e39f..05e79e7 100644
--- a/port.c
+++ b/port.c
@@ -2225,12 +2225,16 @@ static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
{
struct port *p = ctx;
int required_modes = 0;
+ int status_changed = 0;

- if (index != if_nametoindex(p->name) || p->link_status == linkup)
+ if (index != if_nametoindex(p->name))
return;

- p->link_status = linkup;
- pr_notice("port %hu: link %s", portnum(p), linkup ? "up" : "down");
+ if (p->link_status != linkup) {
+ p->link_status = linkup;
+ pr_notice("port %hu: link %s", portnum(p), linkup ? "up" : "down");
+ status_changed = 1;
+ }

/* ts_iface changed */
if (ts_iface[0] != '\0' && strcmp(p->iface->ts_iface, ts_iface)) {
@@ -2249,14 +2253,22 @@ static void port_link_status(void *ctx, int index, int linkup, char *ts_iface)
clock_switch_phc(p->clock, p->phc_index);
}
}
+
+ status_changed = 1;
}

- /*
- * A port going down can affect the BMCA result.
- * Force a state decision event.
- */
- if (!p->link_status)
- clock_set_sde(p->clock, 1);
+ if (status_changed == 1) {
+ if (p->link_status) {
+ 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.
+ */
+ clock_set_sde(p->clock, 1);
+ }
+ }
}

enum fsm_event port_event(struct port *p, int fd_index)
@@ -2301,7 +2313,7 @@ enum fsm_event port_event(struct port *p, int fd_index)
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;
+ return EV_NONE;
}

msg = msg_allocate();

[Hangbin Liu]

Pass struct interface so we can use ts_iface in HW filter.

Signed-off-by: Hangbin Liu <***@gmail.com>
---
pmc_common.c | 5 ++++-
port.c | 4 ++--
raw.c | 5 +++--
transport.c | 4 ++--
transport.h | 3 ++-
transport_private.h | 4 ++--
udp.c | 7 ++++---
udp6.c | 7 ++++---
uds.c | 3 ++-
9 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/pmc_common.c b/pmc_common.c
index d92b0cd..447cf99 100644
--- a/pmc_common.c
+++ b/pmc_common.c
@@ -67,6 +67,7 @@ struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
int zero_datalen)
{
struct pmc *pmc;
+ struct interface iface;

pmc = calloc(1, sizeof *pmc);
if (!pmc)
@@ -90,7 +91,9 @@ struct pmc *pmc_create(struct config *cfg, enum transport_type transport_type,
pr_err("failed to create transport");
goto failed;
}
- if (transport_open(pmc->transport, iface_name,
+
+ strncpy(iface.name, iface_name, MAX_IFNAME_SIZE);
+ if (transport_open(pmc->transport, &iface,
&pmc->fdarray, TS_SOFTWARE)) {
pr_err("failed to open transport");
goto failed;
diff --git a/port.c b/port.c
index 05e79e7..b5fec20 100644
--- a/port.c
+++ b/port.c
@@ -1497,7 +1497,7 @@ static int port_initialize(struct port *p)
goto no_timers;
}
}
- if (transport_open(p->trp, p->name, &p->fda, p->timestamping))
+ if (transport_open(p->trp, p->iface, &p->fda, p->timestamping))
goto no_tropen;

for (i = 0; i < N_TIMER_FDS; i++) {
@@ -1540,7 +1540,7 @@ static int port_renew_transport(struct port *p)
}
transport_close(p->trp, &p->fda);
port_clear_fda(p, FD_ANNOUNCE_TIMER);
- res = transport_open(p->trp, p->name, &p->fda, p->timestamping);
+ res = transport_open(p->trp, p->iface, &p->fda, p->timestamping);
/* Need to call clock_fda_changed even if transport_open failed in
* order to update clock to the now closed descriptors. */
clock_fda_changed(p->clock);
diff --git a/raw.c b/raw.c
index 73e45b4..abee8f6 100644
--- a/raw.c
+++ b/raw.c
@@ -198,15 +198,16 @@ static void addr_to_mac(void *mac, struct address *addr)
memcpy(mac, &addr->sll.sll_addr, MAC_LEN);
}

-static int raw_open(struct transport *t, const char *name,
+static int raw_open(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type ts_type)
{
struct raw *raw = container_of(t, struct raw, t);
unsigned char ptp_dst_mac[MAC_LEN];
unsigned char p2p_dst_mac[MAC_LEN];
int efd, gfd;
- char *str;
+ char *str, *name;

+ name = iface->ts_iface;
str = config_get_string(t->cfg, name, "ptp_dst_mac");
if (str2mac(str, ptp_dst_mac)) {
pr_err("invalid ptp_dst_mac %s", str);
diff --git a/transport.c b/transport.c
index d24c05b..3541394 100644
--- a/transport.c
+++ b/transport.c
@@ -31,10 +31,10 @@ int transport_close(struct transport *t, struct fdarray *fda)
return t->close(t, fda);
}

-int transport_open(struct transport *t, const char *name,
+int transport_open(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type tt)
{
- return t->open(t, name, fda, tt);
+ return t->open(t, iface, fda, tt);
}

int transport_recv(struct transport *t, int fd, struct ptp_message *msg)
diff --git a/transport.h b/transport.h
index 5d6ba98..15616bb 100644
--- a/transport.h
+++ b/transport.h
@@ -27,6 +27,7 @@
#include "msg.h"

struct config;
+struct interface;

/* Values from networkProtocol enumeration 7.4.1 Table 3 */
enum transport_type {
@@ -54,7 +55,7 @@ struct transport;

int transport_close(struct transport *t, struct fdarray *fda);

-int transport_open(struct transport *t, const char *name,
+int transport_open(struct transport *t, struct interface *iface,
struct fdarray *fda, enum timestamp_type tt);

int transport_recv(struct transport *t, int fd, struct ptp_message *msg);
diff --git a/transport_private.h b/transport_private.h
index b54f32a..7530896 100644
--- a/transport_private.h
+++ b/transport_private.h
@@ -32,8 +32,8 @@ struct transport {

int (*close)(struct transport *t, struct fdarray *fda);

- int (*open)(struct transport *t, const char *name, struct fdarray *fda,
- enum timestamp_type tt);
+ int (*open)(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type tt);

int (*recv)(struct transport *t, int fd, void *buf, int buflen,
struct address *addr, struct hw_timestamp *hwts);
diff --git a/udp.c b/udp.c
index 530a2ee..d68c8b3 100644
--- a/udp.c
+++ b/udp.c
@@ -152,12 +152,13 @@ enum { MC_PRIMARY, MC_PDELAY };

static struct in_addr mcast_addr[2];

-static int udp_open(struct transport *t, const char *name, struct fdarray *fda,
- enum timestamp_type ts_type)
+static int udp_open(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type ts_type)
{
struct udp *udp = container_of(t, struct udp, t);
uint8_t event_dscp, general_dscp;
int efd, gfd, ttl;
+ char *name = iface->name;

ttl = config_get_int(t->cfg, name, "udp_ttl");
udp->mac.len = 0;
@@ -180,7 +181,7 @@ static int udp_open(struct transport *t, const char *name, struct fdarray *fda,
if (gfd < 0)
goto no_general;

- if (sk_timestamping_init(efd, name, ts_type, TRANS_UDP_IPV4))
+ if (sk_timestamping_init(efd, iface->ts_iface, ts_type, TRANS_UDP_IPV4))
goto no_timestamping;

if (sk_general_init(gfd))
diff --git a/udp6.c b/udp6.c
index 89e27bf..cbc483f 100644
--- a/udp6.c
+++ b/udp6.c
@@ -160,12 +160,13 @@ enum { MC_PRIMARY, MC_PDELAY };

static struct in6_addr mc6_addr[2];

-static int udp6_open(struct transport *t, const char *name, struct fdarray *fda,
- enum timestamp_type ts_type)
+static int udp6_open(struct transport *t, struct interface *iface,
+ struct fdarray *fda, enum timestamp_type ts_type)
{
struct udp6 *udp6 = container_of(t, struct udp6, t);
uint8_t event_dscp, general_dscp;
int efd, gfd, hop_limit;
+ char *name = iface->name;

hop_limit = config_get_int(t->cfg, name, "udp_ttl");
udp6->mac.len = 0;
@@ -190,7 +191,7 @@ static int udp6_open(struct transport *t, const char *name, struct fdarray *fda,
if (gfd < 0)
goto no_general;

- if (sk_timestamping_init(efd, name, ts_type, TRANS_UDP_IPV6))
+ if (sk_timestamping_init(efd, iface->ts_iface, ts_type, TRANS_UDP_IPV6))
goto no_timestamping;

if (sk_general_init(gfd))
diff --git a/uds.c b/uds.c
index d5e8f50..7e11f63 100644
--- a/uds.c
+++ b/uds.c
@@ -52,13 +52,14 @@ static int uds_close(struct transport *t, struct fdarray *fda)
return 0;
}

-static int uds_open(struct transport *t, const char *name, struct fdarray *fda,
+static int uds_open(struct transport *t, struct interface *iface, struct fdarray *fda,
enum timestamp_type tt)
{
int fd, err;
struct sockaddr_un sa;
struct uds *uds = container_of(t, struct uds, t);
char *uds_path = config_get_string(t->cfg, NULL, "uds_address");
+ char *name = iface->name;

fd = socket(AF_LOCAL, SOCK_DGRAM, 0);
if (fd < 0) {

[Hangbin Liu]

Signed-off-by: Hangbin Liu <***@gmail.com>
---
phc2sys.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++---
port.c | 2 +-
2 files changed, 48 insertions(+), 4 deletions(-)

diff --git a/phc2sys.c b/phc2sys.c
index b6f6719..35df731 100644
--- a/phc2sys.c
+++ b/phc2sys.c
@@ -128,6 +128,11 @@ static int clock_handle_leap(struct node *node, struct clock *clock,
static int run_pmc_get_utc_offset(struct node *node, int timeout);
static void run_pmc_events(struct node *node);

+static int normalize_state(int state);
+static int run_pmc_port_properties(struct node *node, int timeout,
+ unsigned int port,
+ int *state, int *tstamping, char *iface);
+
static clockid_t clock_open(char *device, int *phc_index)
{
struct sk_ts_info ts_info;
@@ -294,8 +299,47 @@ static struct port *port_add(struct node *node, unsigned int number,
return p;
}

-static void clock_reinit(struct clock *clock)
+static void clock_reinit(struct node *node, struct clock *clock)
{
+ struct port *p;
+ int state, timestamping, ret;
+ int phc_index = -1;
+ char iface[IFNAMSIZ];
+ clockid_t clkid = CLOCK_INVALID;
+
+ LIST_FOREACH(p, &node->ports, list) {
+ if (p->clock == clock) {
+ ret = run_pmc_port_properties(node, 1000, p->number,
+ &state, &timestamping,
+ iface);
+ if (ret == -1) {
+ /* port does not exist, ignore the port */
+ continue;
+ }
+ if (ret <= 0) {
+ pr_err("failed to get port properties");
+ return;
+ }
+ if (timestamping == TS_SOFTWARE) {
+ /* ignore ports with software time stamping */
+ continue;
+ }
+
+ p->state = normalize_state(state);
+ }
+ }
+
+ if (strcmp(clock->device, iface)) {
+ free(clock->device);
+ clock->device = strdup(iface);
+ clkid = clock_open(clock->device, &phc_index);
+ if (clkid == CLOCK_INVALID)
+ return;
+ phc_close(clock->clkid);
+ clock->clkid = clkid;
+ clock->phc_index = phc_index;
+ }
+
servo_reset(clock->servo);
clock->servo_state = SERVO_UNLOCKED;

@@ -322,7 +366,7 @@ static void reconfigure(struct node *node)

if (c->new_state) {
if (c->new_state == PS_MASTER)
- clock_reinit(c);
+ clock_reinit(node, c);

c->state = c->new_state;
c->new_state = 0;
@@ -388,7 +432,7 @@ static void reconfigure(struct node *node)
} else if (rt) {
if (rt->state != PS_MASTER) {
rt->state = PS_MASTER;
- clock_reinit(rt);
+ clock_reinit(node, rt);
}
pr_info("selecting %s for synchronization", rt->device);
}
diff --git a/port.c b/port.c
index b5fec20..a22d031 100644
--- a/port.c
+++ b/port.c
@@ -854,7 +854,7 @@ static int port_management_fill_response(struct port *target,
else
ppn->port_state = target->state;
ppn->timestamping = target->timestamping;
- ptp_text_set(&ppn->interface, target->name);
+ ptp_text_set(&ppn->interface, target->iface->ts_iface);
datalen = sizeof(*ppn) + ppn->interface.length;
respond = 1;
break;