On Fri, Jan 07, 2022 at 04:41:02PM +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.
>
> We use ifdef to check if the build kernel support this new flag. And
> fall back without flag if user run new build on old kernel for hwts_init().
>
> Split port_change_phc() from function port_link_status() so the event
> functions could call it directly. Add a new link_state flag PHC_INDEX_CHANGED
> as we may skip TS_LABEL_CHANGED setting if get PHC directly.
The commit message needs to be updated for the v2 change?
> - memset(ts_label, 0, sizeof(ts_label));
> - if (!rtnl_get_ts_device(interface_name(iface), ts_label))
> - interface_set_label(iface, ts_label);
> - interface_get_tsinfo(iface);
> +#ifdef HWTSTAMP_FLAG_BONDED_PHC_INDEX
> + if (interface_get_tsinfo(iface) ||
> + (interface_tsinfo_valid(iface) &&
> + !interface_tsmodes_supported(iface, required_modes))) {
> +#endif
> + memset(ts_label, 0, sizeof(ts_label));
> + if (!rtnl_get_ts_device(interface_name(iface),
> ts_label))
> + interface_set_label(iface, ts_label);
> + interface_get_tsinfo(iface);
> +#ifdef HWTSTAMP_FLAG_BONDED_PHC_INDEX
> + }
> +#endif
If the only reason for these ifdefs is an optimization for older
kernels, I think it's ok to remove them as this runs only once on
start. I'd prefer code that is easier to read, even if it does
something unnecessary.
> @@ -123,7 +124,15 @@ 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);
> +
> +#ifdef HWTSTAMP_FLAG_BONDED_PHC_INDEX
> + if (!interface_get_tsinfo(p->iface) &&
> + (p->phc_index != interface_phc_index(p->iface)))
> + port_change_phc(p);
> + else
> +#endif
> + rtnl_link_status(fd, p->name, port_link_status, p);
> +
I'd prefer these ifdefs to be removed too, but if others agree it
should stay, would it make sense to move it to port_change_phc(),
which would accept the fd and port_link_status, so it could call
rtnl_link_status() if the support is missing?
--
Miroslav Lichvar
_______________________________________________
Linuxptp-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
