On Mon, Mar 21, 2022 at 08:06:33PM -0700, Richard Cochran wrote:
> On Fri, Jan 21, 2022 at 12:42:49PM +0800, Hangbin Liu wrote:
>
> > In clock_create(), try get ts info first, if failed, try the old way.
>
> How does this sentence relate to this...
>
> > @@ -1001,10 +1001,16 @@ struct clock *clock_create(enum clock_type type,
> > struct config *config,
> > c->timestamping = timestamping;
> > required_modes = clock_required_modes(c);
> > STAILQ_FOREACH(iface, &config->interfaces, list) {
> > - 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);
> > + /* try find lowerlay device if we can't get iface tsinfo
> > directly */
> > + if (interface_get_tsinfo(iface) ||
> > + (interface_tsinfo_valid(iface) &&
> > + !interface_tsmodes_supported(iface, required_modes))) {
> > + 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);
> > + }
>
> I really have no idea what this is supposed to do.
>
> Why test for
>
> interface_tsinfo_valid(iface) && !interface_tsmodes_supported(iface,
> required_modes)
>
> twice in a row !?!?!?
If we run the code in old kernel, that bond do not support get_ts_info. The
kernel will still return 0 and set phc_index to -1, as kernel function
__ethtool_get_ts_info() did.
So even interface_get_tsinfo() return 0, we still need to check to make sure
the tsinfo is valid and tsmode is supported. If all the check failed, we
fall back to set ts_label and re-do interface_get_tsinfo().
After this step, we need to re-check the tsinfo_valid and tsmode. Did I
explain it clear?
>
> > +
> > if (interface_tsinfo_valid(iface) &&
> > !interface_tsmodes_supported(iface, required_modes)) {
>
> See that ? ^^^^^^^^^^^^^^^^^^^
We can't only check this once. e.g.
if (interface_get_tsinfo(iface)) {
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);
}
if (interface_tsinfo_valid(iface) &&
!interface_tsmodes_supported(iface, required_modes)) {
pr_err();
return NULL;
}
Thanks
Hangbin
_______________________________________________
Linuxptp-devel mailing list
Linuxptp-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linuxptp-devel
