[Linuxptp-devel] [PATCH 0/8] Add linuxptp bond failover support

[Hangbin Liu]

Hi Richard,

As we know, more and more customers are asking for linuxptp fail over support.

And here is the bond failover patch set. The first two patches add netlink
support to get bond slave info. Then we start track interface info in
port so we can update interface after fail over. And also we start get
ts info from true ts interface.

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

Also we need update clock device and phc_index in phc2sys.

Since it's a large patch set. I may miss some part. Please let me know if
you have any questions, comments, or concerns.

Thanks
Hangbin

---
Testing: I tested bond active-backup with L2 SW/HW and L4 SW/HW. All looks
good so far.

1. set up bond
# ip link add bond0 type bond
# ip link set bond0 type bond mode active-backup
# ip link set bond0 type bond miimon 100
# ip link set p4p1 master bond0
# ip link set p4p2 master bond0
# ip link set bond0 up
# ip addr add 192.168.1.1/24 dev bond0

2. start ptp4l with L2 SW/HW and L4 SW/HW
# ./ptp4l -S -2 -i bond0 -m
# ./ptp4l -2 -i bond0 -m
# ./ptp4l -S -i bond0 -m
# ./ptp4l -i bond0 -m

3. When test HW tampstamping, also start phc2sys
# ./phc2sys -a -r -m

4. trigger fail over
# ip addr show bond0
10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP qlen 1000
link/ether f4:e9:d4:94:62:f0 brd ff:ff:ff:ff:ff:ff
inet 192.168.1.1/24 scope global bond0
valid_lft forever preferred_lft forever
inet6 fe80::f6e9:d4ff:fe94:62f0/64 scope link
valid_lft forever preferred_lft forever
# ip -d link show bond0
10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT qlen 1000
link/ether f4:e9:d4:94:62:f0 brd ff:ff:ff:ff:ff:ff promiscuity 0
bond mode active-backup active_slave p4p1 miimon 100 updelay 0 downdelay 0 use_carrier 1 arp_interval 0 arp_validate none arp_all_targets any primary_reselect always fail_over_mac none xmit_hash_policy layer2 resend_igmp 1 num_grat_arp 1 all_slaves_active 0 min_links 0 lp_interval 1 packets_per_slave 1 lacp_rate slow ad_select stable tlb_dynamic_lb 0 addrgenmode eui64
# ip link set p4p1 down
# ip -d link show bond0
10: bond0: <BROADCAST,MULTICAST,MASTER,UP,LOWER_UP> mtu 1500 qdisc noqueue state UP mode DEFAULT qlen 1000
link/ether f4:e9:d4:94:62:f0 brd ff:ff:ff:ff:ff:ff promiscuity 0
bond mode active-backup active_slave p4p2 miimon 100 updelay 0 downdelay 0 use_carrier 1 arp_interval 0 arp_validate none arp_all_targets any primary_reselect always fail_over_mac none xmit_hash_policy layer2 resend_igmp 1 num_grat_arp 1 all_slaves_active 0 min_links 0 lp_interval 1 packets_per_slave 1 lacp_rate slow ad_select stable tlb_dynamic_lb 0 addrgenmode eui64
# ip link set p4p1 up
# ip link set p4p2 down
# ip link set p4p2 up

5. See the logs from ptp4l and phc2sys
# ./ptp4l -i bond0 -m
ptp4l[43749.389]: selected /dev/ptp0 as PTP clock
ptp4l[43749.408]: port 1: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[43749.408]: port 0: INITIALIZING to LISTENING on INIT_COMPLETE
ptp4l[43755.760]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l[43755.760]: selected best master clock f4e9d4.fffe.9462f0
ptp4l[43755.760]: assuming the grand master role

ptp4l[43767.206]: port 1: link up
ptp4l[43767.225]: port 1: MASTER to LISTENING on INIT_COMPLETE
ptp4l[43767.225]: port 1: link up
ptp4l[43767.225]: port 1: link up
ptp4l[43774.206]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l[43774.206]: selected best master clock f4e9d4.fffe.9462f0
ptp4l[43774.206]: assuming the grand master role

ptp4l[43784.208]: timed out while polling for tx timestamp
ptp4l[43784.208]: increasing tx_timestamp_timeout may correct this issue, but it is likely caused by a driver bug
ptp4l[43784.208]: port 1: send sync failed
ptp4l[43784.208]: port 1: MASTER to FAULTY on FAULT_DETECTED (FT_UNSPECIFIED)
ptp4l[43784.567]: port 1: link up
ptp4l[43784.587]: port 1: FAULTY to LISTENING on INIT_COMPLETE
ptp4l[43784.587]: port 1: link up
ptp4l[43784.587]: port 1: link up
ptp4l[43790.675]: port 1: LISTENING to MASTER on ANNOUNCE_RECEIPT_TIMEOUT_EXPIRES
ptp4l[43790.675]: selected best master clock f4e9d4.fffe.9462f0
ptp4l[43790.675]: assuming the grand master role

# ./phc2sys -a -r -m
phc2sys[43761.356]: reconfiguring after port state change
phc2sys[43761.356]: selecting p4p1 for synchronization
phc2sys[43761.356]: nothing to synchronize

phc2sys[43767.357]: port f4e9d4.fffe.9462f0-1 changed state
phc2sys[43767.357]: reconfiguring after port state change
phc2sys[43767.357]: selecting p4p1 for synchronization
phc2sys[43767.357]: nothing to synchronize
phc2sys[43774.358]: port f4e9d4.fffe.9462f0-1 changed state
phc2sys[43774.358]: reconfiguring after port state change
phc2sys[43774.358]: selecting p4p2 for synchronization
phc2sys[43774.358]: nothing to synchronize

phc2sys[43785.360]: port f4e9d4.fffe.9462f0-1 changed state
phc2sys[43785.360]: reconfiguring after port state change
phc2sys[43785.360]: selecting p4p2 for synchronization
phc2sys[43785.360]: nothing to synchronize
phc2sys[43791.361]: port f4e9d4.fffe.9462f0-1 changed state
phc2sys[43791.361]: reconfiguring after port state change
phc2sys[43791.361]: selecting p4p1 for synchronization
phc2sys[43791.361]: nothing to synchronize

Hangbin Liu (8):
rtnl: use ifinfomsg instead of rtgenmsg to request link info
rtnl: extend struct interface and add new function rtnl_link_info
port: track interface info in port
port: add new function port_set_phc
clock: use ts interface to get ts info
clock: need check required_modes before use new ts info
transport: pass struct interface instread of name to transport_open
phc2sys: update clock clkid and phc_index if device changed

clock.c | 79 +++++++++++++++++++++++----------
config.c | 1 -
config.h | 1 +
phc2sys.c | 50 +++++++++++++++++++--
pmc_common.c | 5 ++-
port.c | 18 ++++++--
port.h | 14 ++++++
raw.c | 5 ++-
rtnl.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++----
rtnl.h | 40 +++++++++++++++--
transport.c | 4 +-
transport.h | 3 +-
transport_private.h | 4 +-
udp.c | 7 +--
udp6.c | 7 +--
uds.c | 3 +-
16 files changed, 308 insertions(+), 56 deletions(-)

[Hangbin Liu]

Also add a new parameter interface index incase we want to query specific
interface information.

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

diff --git a/clock.c b/clock.c
index b6afba9..59b5f0c 100644
--- a/clock.c
+++ b/clock.c
@@ -1165,7 +1165,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
}
port_dispatch(c->uds_port, EV_INITIALIZE, 0);
if (c->pollfd[0].fd >= 0) {
- rtnl_link_query(c->pollfd[0].fd);
+ rtnl_link_query(c->pollfd[0].fd, 0);
}
return c;
}
diff --git a/rtnl.c b/rtnl.c
index 5cddc4b..39faeb7 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..bd74cba 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 A 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.

[Richard Cochran]

Post by Hangbin Liu
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;

What about portability on older kernel versions? We need to run on
kernels starting with v3.0. The rtnetlink(7) man page says:

struct ifinfomsg {
unsigned char ifi_family; /* AF_UNSPEC */
unsigned short ifi_type; /* Device type */
int ifi_index; /* Interface index */
unsigned int ifi_flags; /* Device flags */
unsigned int ifi_change; /* change mask */
};

ifi_flags contains the device flags, see netdevice(7); ifi_index
is the unique interface index (since Linux 3.7, it is possible
to feed a nonzero value with the RTM_NEWLINK message, thus cre‐
ating a link with the given ifindex); ifi_change is reserved for
future use and should be always set to 0xFFFFFFFF.

If we could always use a per-port rtnl socket, this would be an
improvement. Back when I introduced rtnl, I wrote this:

From 0b3c045a4236f128ab4c1592e6cc91adc1cd6eb0 Mon Sep 17 00:00:00 2001
From: Richard Cochran <***@gmail.com>
Date: Sun, 31 Jul 2016 12:08:24 +0200
Subject: rtnl: Introduce RT netlink sockets.

This patch adds a source module that implements RT netlink sockets
for the purpose of link monitoring. Unfortunately the netlink API
offers no possibility for per-port notification. Instead it
forces us to use a de-multiplexing pattern.

At that time, I added one extra FD into clock->pollfdp[], and the
clock then farms out the information to the ports. I really did not
to do it that way. A better way would have been to add a new per-port
FD into fd.h, something like this:

enum {
FD_EVENT,
FD_GENERAL,
FD_ANNOUNCE_TIMER,
FD_SYNC_RX_TIMER,
FD_DELAY_TIMER,
FD_QUALIFICATION_TIMER,
FD_MANNO_TIMER,
FD_SYNC_TX_TIMER,
/*here*/ FD_RTNL,
N_POLLFD,
};

struct fdarray {
int fd[N_POLLFD];
};

This would simply your patch set, wouldn't it?

Thanks,
Richard

[Richard Cochran]

Post by Richard Cochran
At that time, I added one extra FD into clock->pollfdp[], and the
clock then farms out the information to the ports. I really did not
to do it that way. A better way would have been to add a new per-port

I meant to say, "I really did not want to do that way."

I can't remember whether I even knew about the possibility of using
ifinfomsg.ifi_index to bind to a particular interface. At the time, I
either overlooked it or I might have rejected this because of lack of
portability to kernels before v3.7.

In any case, kernel v3.7 is getting pretty old, and so I think we can:

1. remove the rtnl socket from clock.c

2. add one rtnl socket per port.

3. use ifinfomsg on kernel 3.7+

4. fall back to rtgenmsg on kernel 3.0 - 3.6 (only if needed).

Q: What is the behavior of pre 3.7 kernels WRT ifinfomsg.ifi_index?
If ifi_index is non-zero, does the kernel return EINVAL, or do
you get information from all interfaces?

The only drawback is that, when running on earlier kernels, ptp4l will
duplicate work, handling all those per-port rtnl messages. But this
is acceptable if we can improve the ptp4l code for the future.


Thanks,
Richard

[Hangbin Liu]

Post by Richard Cochran

Post by Hangbin Liu
request.hdr.nlmsg_type = RTM_GETLINK;
- request.hdr.nlmsg_flags = NLM_F_REQUEST | NLM_F_DUMP;

NLM_F_DUMP flag call function rtnl_dump_ifinfo(), which will dump all
interface info.

Post by Richard Cochran

Post by Hangbin Liu
+ request.hdr.nlmsg_flags = NLM_F_REQUEST;

NLM_F_REQUEST call rtnl_getlink() which get exactally the interface's info.

There is no much change for these two functions since v3.0.

Post by Richard Cochran
What about portability on older kernel versions? We need to run on
ifi_flags contains the device flags, see netdevice(7); ifi_index
is the unique interface index (since Linux 3.7, it is possible
to feed a nonzero value with the RTM_NEWLINK message, thus cre‐
ating a link with the given ifindex); ifi_change is reserved for

Before v3.7 we do not support create interface with given ifindex with
RTM_NEWLINK message[1]. Since we only need to get the interface info.
It should be safe to use ifinfomsg on kernel v3.0

But with your remind. I found we start to support bond rtnetlink message
from kernel v3.13. So before v3.13, we could not get slave info and ptp4l
with bond interface will fail with error not support when check ts info.
I think this should be OK with document "bond fail over support start with
kernel v3.13".

What do you think?

Post by Richard Cochran
future use and should be always set to 0xFFFFFFFF.

Post by Hangbin Liu
At that time, I added one extra FD into clock->pollfdp[], and the
clock then farms out the information to the ports. I really did not
to do it that way. A better way would have been to add a new per-port

I meant to say, "I really did not want to do that way."
I can't remember whether I even knew about the possibility of using
ifinfomsg.ifi_index to bind to a particular interface. At the time, I
either overlooked it or I might have rejected this because of lack of
portability to kernels before v3.7.
1. remove the rtnl socket from clock.c
2. add one rtnl socket per port.
3. use ifinfomsg on kernel 3.7+

OK, I think we need a separate patch set to improve this.

Post by Richard Cochran
4. fall back to rtgenmsg on kernel 3.0 - 3.6 (only if needed).

And we do not need to concern about this issue now, right?

Post by Richard Cochran
Q: What is the behavior of pre 3.7 kernels WRT ifinfomsg.ifi_index?
If ifi_index is non-zero, does the kernel return EINVAL, or do
you get information from all interfaces?
The only drawback is that, when running on earlier kernels, ptp4l will
duplicate work, handling all those per-port rtnl messages. But this
is acceptable if we can improve the ptp4l code for the future.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/net/core/rtnetlink.c?h=v3.0#n1728

Thanks
Hangbin

[Richard Cochran]

Post by Hangbin Liu
Before v3.7 we do not support create interface with given ifindex with
RTM_NEWLINK message[1]. Since we only need to get the interface info.
It should be safe to use ifinfomsg on kernel v3.0

Okay, then we need to take two steps.

1. Convert single rtnl FD into one per-port FD. Also remove
index2port hash table from clock.c.

2. Add bonding support.

Post by Hangbin Liu
But with your remind. I found we start to support bond rtnetlink message
from kernel v3.13. So before v3.13, we could not get slave info and ptp4l
with bond interface will fail with error not support when check ts info.
I think this should be OK with document "bond fail over support start with
kernel v3.13".
What do you think?

I think is it okay to let bond interfaces fail on start up when
running on older kernels. We can even call uname(2) to check the
kernel version when bonding is requested in the configuration.

Thanks,
Richard

[Jiri Benc]

We can even call uname(2) to check the kernel version when bonding is
requested in the configuration.

Please don't do that. Always check for the feature being present, never
for a kernel version. It's common for various groups to backport newer
features to older kernels; kernel version alone does not indicate
anything.

Thanks,

Jiri

[Richard Cochran]

Post by Jiri Benc

We can even call uname(2) to check the kernel version when bonding is
requested in the configuration.

Please don't do that. Always check for the feature being present, never
for a kernel version. It's common for various groups to backport newer
features to older kernels; kernel version alone does not indicate
anything.

Ok, then just let ptp4l try to configure the bonding interface, and
when it fails, put a hint about the kernel version into the error
message.

Thanks,
Richard

[Hangbin Liu]

Post by Richard Cochran

Post by Richard Cochran
At that time, I added one extra FD into clock->pollfdp[], and the
clock then farms out the information to the ports. I really did not
to do it that way. A better way would have been to add a new per-port

I meant to say, "I really did not want to do that way."
I can't remember whether I even knew about the possibility of using
ifinfomsg.ifi_index to bind to a particular interface. At the time, I
either overlooked it or I might have rejected this because of lack of
portability to kernels before v3.7.

I guess I know why you didn't use ifinfomsg.ifi_index now. Because you didn't
need it...

After check, I find ifinfomsg.ifi_index only used to get spedific interface's
info, just as we know. But it could not get specific interface's notify.

The reason we could get interface link up/down notify is we registered sa
group with RTMGRP_LINK/RTNLGRP_LINK. So whenever a interface changed, we
will get the notify.

Before this patch set. We don't need the interface info, and only care about
the link status. So you don't need ifinfomsg when call rtnl_link_query().

Now since we need to know the interface's active slave info. I update
rtnl_link_query() to use ifinfomsg to get the interface slave info.

But for link status monitor, ifinfomsg.ifi_index doesn't do any help. We still
need to register with groupp RTNLGRP_LINK and keep receive all interfaces'
information.

Post by Richard Cochran
1. remove the rtnl socket from clock.c
2. add one rtnl socket per port.

Back to this question, do you still want to move rtnl socket from clock to
per port?

I think we can filter the call back if index to make sure we get correct
interface, something like

+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");
+
+ if (linkup)
+ port_dispatch(p, EV_FAULT_CLEARED, 0);
+ else
+ port_dispatch(p, EV_FAULT_DETECTED, 0);
+}

But this should have no relation with the failover patch set.

Thanks
Hangbin

[Richard Cochran]

Post by Hangbin Liu
But for link status monitor, ifinfomsg.ifi_index doesn't do any help. We still
need to register with groupp RTNLGRP_LINK and keep receive all interfaces'
information.

I see.

Post by Hangbin Liu
Back to this question, do you still want to move rtnl socket from clock to
per port?

Yes, but only if we can use a single rtnl socket for both messages.

Post by Hangbin Liu
I think we can filter the call back if index to make sure we get correct
interface, something like
+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");
+
+ if (linkup)
+ port_dispatch(p, EV_FAULT_CLEARED, 0);
+ else
+ port_dispatch(p, EV_FAULT_DETECTED, 0);
+}

Sounds reasonable.

Post by Hangbin Liu
But this should have no relation with the failover patch set.

Right, and so I would like to see that change first.

Thanks,
Richard

[Hangbin Liu]

Post by Richard Cochran

Post by Hangbin Liu
Back to this question, do you still want to move rtnl socket from clock to
per port?

Yes, but only if we can use a single rtnl socket for both messages.

Hmm.. If we want to use a single rtnl socket on all ports. we need create it
before port_open() and give the fd to the new port.

On the other hand, if we use the same socket,
1) events & POLLIN , we have a notify message with port 2 iface index.
2) on port 1, rtnl_link_status(rtnl_fd, port_link_status, p). Then all notify
message received. But after check, the if index is not match
3) on port 2, call rtnl_link_status(), but the message on this socket already
recieved. Then we will miss the notify.

So I think we need to make each port have their own rtnl socket fd. Then each
fd get their own multicast rtnl message.

What do you think?

Thanks
Hangbin

[Richard Cochran]

Post by Hangbin Liu

Post by Richard Cochran

Post by Hangbin Liu
Back to this question, do you still want to move rtnl socket from clock to
per port?

Yes, but only if we can use a single rtnl socket for both messages.

I meant to say, a single rtnl socket per-port.

Post by Hangbin Liu
So I think we need to make each port have their own rtnl socket fd. Then each
fd get their own multicast rtnl message.
What do you think?

Yes, that is what I meant.

My question was, can we have both RTNLGRP_LINK and ifinfomsg on the
same per-port socket? (I assume that works.)

Thanks,
Richard

[Hangbin Liu]

Post by Richard Cochran

Post by Hangbin Liu

Post by Richard Cochran

Post by Hangbin Liu
Back to this question, do you still want to move rtnl socket from clock to
per port?

Yes, but only if we can use a single rtnl socket for both messages.

I meant to say, a single rtnl socket per-port.

Cool. Sorry for the misunderstanding.

Post by Richard Cochran

Post by Hangbin Liu
So I think we need to make each port have their own rtnl socket fd. Then each
fd get their own multicast rtnl message.
What do you think?

Yes, that is what I meant.
My question was, can we have both RTNLGRP_LINK and ifinfomsg on the
same per-port socket? (I assume that works.)

Yes, it works.

Thanks
Hangbin

[Hangbin Liu]

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

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

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

Signed-off-by: Hangbin Liu <***@gmail.com>
---
clock.c | 2 +-
config.h | 1 +
rtnl.c | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
rtnl.h | 33 ++++++++++++++++++-
4 files changed, 142 insertions(+), 5 deletions(-)

diff --git a/clock.c b/clock.c
index 59b5f0c..5e9f2cd 100644
--- a/clock.c
+++ b/clock.c
@@ -326,7 +326,7 @@ static void clock_freq_est_reset(struct clock *c)
c->fest.count = 0;
}

-static void clock_link_status(void *ctx, int index, int linkup)
+static void clock_link_status(void *ctx, int index, int linkup, char *ts_iface)
{
struct clock *c = ctx;
struct port *p;
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;
};

diff --git a/rtnl.c b/rtnl.c
index 39faeb7..971f273 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -18,8 +18,6 @@
*/
#include <asm/types.h>
#include <sys/socket.h> /* Must come before linux/netlink.h on some systems. */
-#include <linux/netlink.h>
-#include <linux/rtnetlink.h>
#include <net/if.h>
#include <stdio.h>
#include <stdlib.h>
@@ -84,6 +82,60 @@ 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);
+}
+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;
+}
+
+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 +144,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 +204,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 +242,33 @@ 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 failed;
+ }
+
+ fd = rtnl_open();
+ if (rtnl_link_query(fd, index))
+ goto failed;
+ if (rtnl_link_status(fd, NULL, iface->ts_iface))
+ goto failed;
+
+ /* 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;
+failed:
+ return -1;
+}
diff --git a/rtnl.h b/rtnl.h
index bd74cba..ed46cb7 100644
--- a/rtnl.h
+++ b/rtnl.h
@@ -20,7 +20,12 @@
#ifndef HAVE_RTNL_H
#define HAVE_RTNL_H

-typedef void (*rtnl_callback)(void *ctx, int index, int linkup);
+#include <linux/netlink.h>
+#include <linux/rtnetlink.h>
+
+#include "config.h"
+
+typedef void (*rtnl_callback)(void *ctx, int index, int linkup, char *device);

/**
* Close a RT netlink socket.
@@ -38,6 +43,26 @@ int rtnl_close(int fd);
int rtnl_link_query(int fd, unsigned int index);

/**
+ * Parase the rtattr info
+ * @param tb rtattr array.
+ * @param max max type of ratttr.
+ * @param rta rta header
+ * @param len rta playload length
+ * @return Zero on success, or -1 on error.
+ */
+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)))
+
+/**
+ * Parase the link info
+ * @param rta rta header
+ * @param device interface name.
+ * @return Zero on success, or -1 on error.
+ */
+int rtnl_linkinfo_parse(struct rtattr *rta, char *device);
+
+/**
* Read kernel messages looking for a link up/down events.
* @param fd Readable socket obtained via rtnl_open().
* @param cb Callback function to be invoked on each event.
@@ -52,4 +77,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

[Richard Cochran]

This patch has a number of coding style issues...

Post by Hangbin Liu
diff --git a/rtnl.c b/rtnl.c
index 39faeb7..971f273 100644
--- a/rtnl.c
+++ b/rtnl.c
@@ -18,8 +18,6 @@
*/
#include <asm/types.h>
#include <sys/socket.h> /* Must come before linux/netlink.h on some systems. */
-#include <linux/netlink.h>
-#include <linux/rtnetlink.h>
#include <net/if.h>
#include <stdio.h>
#include <stdlib.h>
@@ -84,6 +82,60 @@ 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);
+}

A function must be followed by one blank line.

Post by Hangbin Liu
+static inline const char *rta_getattr_str(const struct rtattr *rta)
+{
+ return (const char *)RTA_DATA(rta);
+}

Here too.

Post by Hangbin Liu
+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;
+}
+
+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];

array[MAX + 1];
Spaces ^^^

Post by Hangbin Liu
+ 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 +144,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) {

One space after keywords please.

Post by Hangbin Liu
+ fprintf(stderr, "rtnl: no enought memory for device name\n");
+ return -1;
+ }
if (!rtnl_buf) {
rtnl_len = 4096;
@@ -38,6 +43,26 @@ int rtnl_close(int fd);
int rtnl_link_query(int fd, unsigned int index);
/**
+ * Parase the rtattr info
+ */
+int rtnl_rtattr_parse(struct rtattr *tb[], int max, struct rtattr *rta, int len);

This function should be private in rtnl.c.

Post by Hangbin Liu
+#define rtnl_nested_rtattr_parse(tb, max, rta) \
+ (rtnl_rtattr_parse((tb), (max), RTA_DATA(rta), RTA_PAYLOAD(rta)))

This also.

Post by Hangbin Liu
+/**
+ * Parase the link info
+ */
+int rtnl_linkinfo_parse(struct rtattr *rta, char *device);

This also.

Thanks,
Richard

[Hangbin Liu]

Add interface element in struct port. And update ts iface info after
device fail over.

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

diff --git a/clock.c b/clock.c
index 5e9f2cd..67bdecb 100644
--- a/clock.c
+++ b/clock.c
@@ -331,6 +331,7 @@ static void clock_link_status(void *ctx, int index, int linkup, char *ts_iface)
struct clock *c = ctx;
struct port *p;
char key[16];
+ struct interface *iface;

snprintf(key, sizeof(key), "%d", index);
p = hash_lookup(c->index2port, key);
@@ -338,6 +339,12 @@ static void clock_link_status(void *ctx, int index, int linkup, char *ts_iface)
return;
}
port_link_status_set(p, linkup);
+
+ iface = port_interface_get(p);
+ if (ts_iface[0] != '\0' && strcmp(iface->ts_iface, ts_iface)) {
+ strncpy(iface->ts_iface, ts_iface, MAX_IFNAME_SIZE);
+ }
+
if (linkup) {
port_dispatch(p, EV_FAULT_CLEARED, 0);
} else {
diff --git a/port.c b/port.c
index ec02825..b28849f 100644
--- a/port.c
+++ b/port.c
@@ -66,6 +66,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;
@@ -2378,6 +2379,11 @@ void port_link_status_set(struct port *p, int up)
pr_notice("port %hu: link %s", portnum(p), up ? "up" : "down");
}

+struct interface *port_interface_get(struct port *p)
+{
+ return p->iface;
+}
+
int port_manage(struct port *p, struct port *ingress, struct ptp_message *msg)
{
struct management_tlv *mgt;
@@ -2586,6 +2592,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;
diff --git a/port.h b/port.h
index b00bc64..7f9be42 100644
--- a/port.h
+++ b/port.h
@@ -139,6 +139,13 @@ int port_link_status_get(struct port *p);
void port_link_status_set(struct port *p, int up);

/**
+ * Obtain the interface pointer of a port.
+ * @param p A port instance.
+ * @return Interface pointer of the port.
+ */
+struct interface *port_interface_get(struct port *p);
+
+/**
* 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.

[Hangbin Liu]

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

diff --git a/port.c b/port.c
index b28849f..d919c3e 100644
--- a/port.c
+++ b/port.c
@@ -2384,6 +2384,11 @@ struct interface *port_interface_get(struct port *p)
return p->iface;
}

+void port_set_phc(struct port *p, int phc_index)
+{
+ p->phc_index = phc_index;
+}
+
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 7f9be42..b0d5db3 100644
--- a/port.h
+++ b/port.h
@@ -146,6 +146,13 @@ void port_link_status_set(struct port *p, int up);
struct interface *port_interface_get(struct port *p);

/**
+ * Sets the phc index for a port.
+ * @param p A port instance.
+ * @param phc_index the new phc index.
+ */
+void port_set_phc(struct port *p, int phc_index);
+
+/**
* 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.

[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 | 14 ++++++++++----
config.c | 1 -
2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/clock.c b/clock.c
index 67bdecb..582438c 100644
--- a/clock.c
+++ b/clock.c
@@ -341,11 +341,15 @@ static void clock_link_status(void *ctx, int index, int linkup, char *ts_iface)
port_link_status_set(p, linkup);

iface = port_interface_get(p);
- if (ts_iface[0] != '\0' && strcmp(iface->ts_iface, ts_iface)) {
+ if (linkup && ts_iface[0] != '\0' && strcmp(iface->ts_iface, ts_iface)) {
strncpy(iface->ts_iface, ts_iface, MAX_IFNAME_SIZE);
- }
-
- if (linkup) {
+ sk_get_ts_info(iface->ts_iface, &iface->ts_info);
+ if (iface->ts_info.valid) {
+ port_set_phc(p, iface->ts_info.phc_index);
+ clock_switch_phc(c, iface->ts_info.phc_index);
+ port_dispatch(p, EV_INITIALIZE, 0);
+ }
+ } else if (linkup) {
port_dispatch(p, EV_FAULT_CLEARED, 0);
} else {
port_dispatch(p, EV_FAULT_DETECTED, 0);
@@ -991,6 +995,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++;

[Hangbin Liu]

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

diff --git a/clock.c b/clock.c
index 582438c..256d52f 100644
--- a/clock.c
+++ b/clock.c
@@ -110,6 +110,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;
@@ -326,12 +327,40 @@ static void clock_freq_est_reset(struct clock *c)
c->fest.count = 0;
}

+static int clock_required_modes(enum timestamp_type timestamping)
+{
+ int required_modes = 0;
+
+ 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;
+ default:
+ break;
+ }
+
+ return required_modes;
+}
static void clock_link_status(void *ctx, int index, int linkup, char *ts_iface)
{
struct clock *c = ctx;
struct port *p;
char key[16];
struct interface *iface;
+ int required_modes = 0;

snprintf(key, sizeof(key), "%d", index);
p = hash_lookup(c->index2port, key);
@@ -344,6 +373,15 @@ static void clock_link_status(void *ctx, int index, int linkup, char *ts_iface)
if (linkup && ts_iface[0] != '\0' && strcmp(iface->ts_iface, ts_iface)) {
strncpy(iface->ts_iface, ts_iface, MAX_IFNAME_SIZE);
sk_get_ts_info(iface->ts_iface, &iface->ts_info);
+
+ required_modes = clock_required_modes(c->timestamping);
+ if (iface->ts_info.valid &&
+ (iface->ts_info.so_timestamping & required_modes) != required_modes) {
+ pr_err("interface '%s' does not support "
+ "requested timestamping mode", iface->ts_iface);
+ return;
+ }
+
if (iface->ts_info.valid) {
port_set_phc(p, iface->ts_info.phc_index);
clock_switch_phc(c, iface->ts_info.phc_index);
@@ -976,31 +1014,14 @@ 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;
- }
+ required_modes = clock_required_modes(timestamping);
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 "
- "requested timestamping mode", iface->name);
+ "requested timestamping mode", iface->ts_iface);
return NULL;
}
}
@@ -1059,6 +1080,7 @@ struct clock *clock_create(enum clock_type type, struct config *config,
c->kernel_leap = config_get_int(config, NULL, "kernel_leap");
c->utc_offset = c->current_utc_offset = config_get_int(config, NULL, "utc_offset");
c->time_source = config_get_int(config, NULL, "timeSource");
+ c->timestamping = timestamping;

if (c->free_running) {
c->clkid = CLOCK_INVALID;

[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 d919c3e..78bf190 100644
--- a/port.c
+++ b/port.c
@@ -1493,7 +1493,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++) {
@@ -1528,7 +1528,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 78bf190..eb8dac2 100644
--- a/port.c
+++ b/port.c
@@ -852,7 +852,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;

[Richard Cochran]

Post by Hangbin Liu
Since it's a large patch set. I may miss some part. Please let me know if
you have any questions, comments, or concerns.

I have only just started to review, but overall the form and structure
of the series are okay. I appreciate that the patches are at a
digestible level of granularity.


Thanks,
Richard