On Fri, 21 Jun 2024 10:54:08 +0200 Kory Maincent wrote:
> > > +const struct nla_policy ethnl_tsinfo_get_policy[ETHTOOL_A_TSINFO_MAX + 1]
> > > = { [ETHTOOL_A_TSINFO_HEADER] =
> > > NLA_POLICY_NESTED(ethnl_header_policy_stats),
> > > + [ETHTOOL_A_TSINFO_GHWTSTAMP] =
> > > + NLA_POLICY_MAX(NLA_U8, 1),
> >
> > I think this can be an NLA_FLAG, but TBH I'm also confused about
> > the semantics. Can you explain what it does from user perspective?
>
> As I described it in the documentation it replaces SIOCGHWTSTAMP:
> "Any process can read the actual configuration by requesting tsinfo netlink
> socket ETHTOOL_MSG_TSINFO_GET with ETHTOOL_MSG_TSINFO_GHWTSTAMP netlink
> attribute set.
>
> The legacy usage is to pass this structure to ioctl(SIOCGHWTSTAMP) in the
> same way as the ioctl(SIOCSHWTSTAMP). However, this has not been implemented
> in all drivers."
I did see the words, just didn't get the meaning :> Couple of years
from now hopefully newcomers won't even know ioctls exited, and
therefore what they did. From the user perspective the gist AFAIU is
that instead of *supported* we'll return what's currently *configured*.
This feels a little bit too much like a muxed operation for me :(
Can we create a separate commands for TSCONFIG_GET / _SET ?
Granted it will be higher LOC, but hopefully cleaner ?
Or we can add the configured as completely new attrs, but changing
meaning of existing attrs based on a request flag.. 🙂↔️️
> > > + [ETHTOOL_A_TSINFO_HWTSTAMP_PROVIDER] =
> > > + NLA_POLICY_NESTED(ethnl_tsinfo_hwtstamp_provider_policy),
> > > };
> > >
> > > +static int tsinfo_parse_hwtstamp_provider(const struct nlattr *nest,
> > > + struct hwtst_provider *hwtst,
> > > + struct netlink_ext_ack *extack,
> > > + bool *mod)
> > > +{
> > > + struct nlattr
> > > *tb[ARRAY_SIZE(ethnl_tsinfo_hwtstamp_provider_policy)];
> >
> > Could you find a more sensible name for this policy?
>
> I am not a naming expert but "hwtstamp_provider" is the struct name I have used
> to describe hwtstamp index + qualifier and the prefix of the netlink nested
> attribute, so IMHO it fits well.
> Have you another proposition to clarify what you would expect?
Oh, I just meant that it's way to long. I know y'all youngsters use
IDEs but I have it on good authority that there's still people in
this community who use text editors they wrote themselves, and those
lack auto-completion.. It's good to be more concise.
