Hi Richard,
Any comments for the new version patch?
On Mon, Apr 11, 2022 at 03:33:38PM +0800, Hangbin Liu wrote:
> Latest Linux kernel has supported getting PHC from bond directly. This
> would help topology like VLAN over bond to get PHC natively. To achieve
> this, the new hwtstamp flag is needed when passing hwtstamp_config to
> kernel. Let's supply the flag first, and fall back without flag if user
> run on old kernel.
>
> Split port_change_phc() from function port_link_status() so the event
> functions could call it directly when find the phc changed.
>
> Signed-off-by: Hangbin Liu <liuhang...@gmail.com>
>
> v6: revert changes in clock_create(). Update ts_label by force and do
> interface_get_tsinfo() only once.
> v5: no update
> v4: define HWTSTAMP_FLAG_BONDED_PHC_INDEX in missing.h
> remove unneeded net_tstamp header files
> v3: remove ifdef in clock_create() and FD_RTNL event.
> v2: remove link state PHC_INDEX_CHANGED and use TS_LABEL_CHANGED only.
> ---
> clock.c | 3 ++
> e2e_tc.c | 7 ++++-
> missing.h | 6 ++++
> p2p_tc.c | 7 ++++-
> port.c | 74 +++++++++++++++++++++++++++++++-------------------
> port_private.h | 1 +
> sk.c | 5 ++++
> 7 files changed, 73 insertions(+), 30 deletions(-)
>
> diff --git a/clock.c b/clock.c
> index 7be021f..16ca6ed 100644
> --- a/clock.c
> +++ b/clock.c
> @@ -1767,6 +1767,9 @@ int clock_switch_phc(struct clock *c, int phc_index)
> c->clkid = clkid;
> c->servo = servo;
> c->servo_state = SERVO_UNLOCKED;
> +
> + pr_info("Switched to /dev/ptp%d as PTP clock", phc_index);
> +
> return 0;
> }
>
> diff --git a/e2e_tc.c b/e2e_tc.c
> index 2f8e821..42baea3 100644
> --- a/e2e_tc.c
> +++ b/e2e_tc.c
> @@ -123,7 +123,12 @@ enum fsm_event e2e_event(struct port *p, int fd_index)
>
> case FD_RTNL:
> pr_debug("%s: received link status notification", p->log_name);
> - rtnl_link_status(fd, p->name, port_link_status, p);
> + if (!interface_get_tsinfo(p->iface) &&
> + (p->phc_index != interface_phc_index(p->iface)))
> + port_change_phc(p);
> + else
> + rtnl_link_status(fd, p->name, port_link_status, p);
> +
> if (p->link_status == (LINK_UP|LINK_STATE_CHANGED)) {
> return EV_FAULT_CLEARED;
> } else if ((p->link_status == (LINK_DOWN|LINK_STATE_CHANGED)) ||
> diff --git a/missing.h b/missing.h
> index 20f7193..4c7ac57 100644
> --- a/missing.h
> +++ b/missing.h
> @@ -73,6 +73,12 @@ struct so_timestamping {
> };
> #endif
>
> +#ifndef HWTSTAMP_FLAG_BONDED_PHC_INDEX
> +enum {
> + HWTSTAMP_FLAG_BONDED_PHC_INDEX = (1<<0),
> +};
> +#endif
> +
> #ifdef PTP_EXTTS_REQUEST2
> #define PTP_EXTTS_REQUEST_FAILED "PTP_EXTTS_REQUEST2 failed: %m"
> #else
> diff --git a/p2p_tc.c b/p2p_tc.c
> index 75cb3b9..1164c9a 100644
> --- a/p2p_tc.c
> +++ b/p2p_tc.c
> @@ -126,7 +126,12 @@ enum fsm_event p2p_event(struct port *p, int fd_index)
>
> case FD_RTNL:
> pr_debug("%s: received link status notification", p->log_name);
> - rtnl_link_status(fd, p->name, port_link_status, p);
> + if (!interface_get_tsinfo(p->iface) &&
> + (p->phc_index != interface_phc_index(p->iface)))
> + port_change_phc(p);
> + else
> + rtnl_link_status(fd, p->name, port_link_status, p);
> +
> if (p->link_status == (LINK_UP|LINK_STATE_CHANGED)) {
> return EV_FAULT_CLEARED;
> } else if ((p->link_status == (LINK_DOWN|LINK_STATE_CHANGED)) ||
> diff --git a/port.c b/port.c
> index f2b666c..76e323f 100644
> --- a/port.c
> +++ b/port.c
> @@ -2636,10 +2636,48 @@ static void bc_dispatch(struct port *p, enum
> fsm_event event, int mdiff)
> }
> }
>
> +void port_change_phc(struct port *p)
> +{
> + int required_modes;
> +
> + /* Only switch phc with HW time stamping mode */
> + if (!interface_tsinfo_valid(p->iface) ||
> + interface_phc_index(p->iface) < 0)
> + return;
> +
> + required_modes = clock_required_modes(p->clock);
> + if (!interface_tsmodes_supported(p->iface, required_modes)) {
> + pr_err("interface '%s' does not support requested "
> + "timestamping mode, set link status down by force.",
> + interface_label(p->iface));
> + p->link_status = LINK_DOWN | LINK_STATE_CHANGED;
> + } else if (p->phc_from_cmdline) {
> + pr_warning("%s: taking /dev/ptp%d from the "
> + "command line, not the attached ptp%d",
> + p->log_name, p->phc_index,
> + interface_phc_index(p->iface));
> + } else if (p->phc_index != interface_phc_index(p->iface)) {
> + p->phc_index = interface_phc_index(p->iface);
> +
> + if (clock_switch_phc(p->clock, p->phc_index)) {
> + p->last_fault_type = FT_SWITCH_PHC;
> + port_dispatch(p, EV_FAULT_DETECTED, 0);
> + return;
> + }
> + clock_sync_interval(p->clock, p->log_sync_interval);
> +
> + /* Ensure TS_LABEL_CHANGED is set for failover when
> + * PHC changed as we may not call port_change_phc()
> + * from port_link_status().
> + */
> + p->link_status |= TS_LABEL_CHANGED;
> + }
> +}
> +
> void port_link_status(void *ctx, int linkup, int ts_index)
> {
> char ts_label[MAX_IFNAME_SIZE + 1] = {0};
> - int link_state, required_modes;
> + int link_state;
> const char *old_ts_label;
> struct port *p = ctx;
>
> @@ -2663,32 +2701,7 @@ void port_link_status(void *ctx, int linkup, int
> ts_index)
> if (p->link_status & LINK_UP &&
> (p->link_status & LINK_STATE_CHANGED || p->link_status &
> TS_LABEL_CHANGED)) {
> interface_get_tsinfo(p->iface);
> -
> - /* Only switch phc with HW time stamping mode */
> - if (interface_tsinfo_valid(p->iface) &&
> - interface_phc_index(p->iface) >= 0) {
> - required_modes = clock_required_modes(p->clock);
> - if (!interface_tsmodes_supported(p->iface,
> required_modes)) {
> - pr_err("interface '%s' does not support
> requested "
> - "timestamping mode, set link status down
> by force.",
> - interface_label(p->iface));
> - p->link_status = LINK_DOWN | LINK_STATE_CHANGED;
> - } else if (p->phc_from_cmdline) {
> - pr_warning("%s: taking /dev/ptp%d from the "
> - "command line, not the attached
> ptp%d",
> - p->log_name, p->phc_index,
> - interface_phc_index(p->iface));
> - } else if (p->phc_index !=
> interface_phc_index(p->iface)) {
> - p->phc_index = interface_phc_index(p->iface);
> -
> - if (clock_switch_phc(p->clock, p->phc_index)) {
> - p->last_fault_type = FT_SWITCH_PHC;
> - port_dispatch(p, EV_FAULT_DETECTED, 0);
> - return;
> - }
> - clock_sync_interval(p->clock,
> p->log_sync_interval);
> - }
> - }
> + port_change_phc(p);
> }
>
> /*
> @@ -2802,7 +2815,12 @@ static enum fsm_event bc_event(struct port *p, int
> fd_index)
>
> case FD_RTNL:
> pr_debug("%s: received link status notification", p->log_name);
> - rtnl_link_status(fd, p->name, port_link_status, p);
> + if (!interface_get_tsinfo(p->iface) &&
> + (p->phc_index != interface_phc_index(p->iface)))
> + port_change_phc(p);
> + else
> + rtnl_link_status(fd, p->name, port_link_status, p);
> +
> if (p->link_status == (LINK_UP | LINK_STATE_CHANGED))
> return EV_FAULT_CLEARED;
> else if ((p->link_status == (LINK_DOWN | LINK_STATE_CHANGED)) ||
> diff --git a/port_private.h b/port_private.h
> index d27dceb..2a800a9 100644
> --- a/port_private.h
> +++ b/port_private.h
> @@ -179,6 +179,7 @@ int port_delay_request(struct port *p);
> void port_disable(struct port *p);
> int port_initialize(struct port *p);
> int port_is_enabled(struct port *p);
> +void port_change_phc(struct port *p);
> void port_link_status(void *ctx, int index, int linkup);
> int port_set_announce_tmo(struct port *p);
> int port_set_delay_tmo(struct port *p);
> diff --git a/sk.c b/sk.c
> index b55d6b5..80075be 100644
> --- a/sk.c
> +++ b/sk.c
> @@ -66,6 +66,11 @@ static int hwts_init(int fd, const char *device, int
> rx_filter,
>
> init_ifreq(&ifreq, &cfg, device);
>
> + cfg.flags = HWTSTAMP_FLAG_BONDED_PHC_INDEX;
> + /* Fall back without flag if user run new build on old kernel */
> + if (ioctl(fd, SIOCGHWTSTAMP, &ifreq) == -EINVAL)
> + init_ifreq(&ifreq, &cfg, device);
> +
> switch (sk_hwts_filter_mode) {
> case HWTS_FILTER_CHECK:
> err = ioctl(fd, SIOCGHWTSTAMP, &ifreq);
> --
> 2.35.1
>
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
