[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250427144621.7265e219@kmaincent-XPS-13-7390>
Date: Sun, 27 Apr 2025 14:46:21 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: Andrew Lunn <andrew@...n.ch>, Jakub Kicinski <kuba@...nel.org>, Donald
Hunter <donald.hunter@...il.com>, "David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>, Simon
Horman <horms@...nel.org>, Willem de Bruijn
<willemdebruijn.kernel@...il.com>, Jason Xing <kernelxing@...cent.com>,
Richard Cochran <richardcochran@...il.com>, Thomas Petazzoni
<thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, Maxime Chevallier
<maxime.chevallier@...tlin.com>, "Russell King (Oracle)"
<linux@...linux.org.uk>
Subject: Re: [PATCH net-next] net: Add support for providing the PTP
hardware source in tsinfo
On Sun, 27 Apr 2025 08:26:45 +0800
Jason Xing <kerneljasonxing@...il.com> wrote:
> Hi Kory,
>
> On Sat, Apr 26, 2025 at 1:45 AM Kory Maincent <kory.maincent@...tlin.com>
> wrote:
> >
> > Multi-PTP source support within a network topology has been merged,
> > but the hardware timestamp source is not yet exposed to users.
> > Currently, users only see the PTP index, which does not indicate
> > whether the timestamp comes from a PHY or a MAC.
>
> Sorry, may I ask what the use case of distinguishing them is? When we
> get the hw timestamp source, I wonder what the next move could be?
Then you can switch between the PTP sources of a network topology, knowing which
PTP index match which PTP source:
$ ethtool --set-hwtimestamp-cfg eth0 index N qualifier precise
Regards,
>
> Thanks,
> Jason
>
> >
> > Add support for reporting the hwtstamp source using a
> > hwtstamp-source field, alongside hwtstamp-phyindex, to describe
> > the origin of the hardware timestamp.
> >
> > Signed-off-by: Kory Maincent <kory.maincent@...tlin.com>
> > ---
> > Not sure moving the hwtstamp_source enum to uapi/linux/net_tstamp.h and
> > adding this header to ynl/Makefile.deps is the best choice. Maybe it is
> > better to move the enum directly to ethtool.h header.
> > ---
> > Documentation/netlink/specs/ethtool.yaml | 16 ++++++++++++++++
> > include/linux/ethtool.h | 4 ++++
> > include/linux/net_tstamp.h | 6 ------
> > include/uapi/linux/ethtool_netlink_generated.h | 2 ++
> > include/uapi/linux/net_tstamp.h | 13 +++++++++++++
> > net/ethtool/common.c | 22 +++++++++++++++++-----
> > net/ethtool/tsinfo.c | 20 ++++++++++++++++++++
> > tools/net/ynl/Makefile.deps | 3 ++-
> > 8 files changed, 74 insertions(+), 12 deletions(-)
> >
> > diff --git a/Documentation/netlink/specs/ethtool.yaml
> > b/Documentation/netlink/specs/ethtool.yaml index
> > c650cd3dcb80bc93c5039dc8ba2c5c18793ff987..4bb44c93e80a83b9520ea297c08a94616f7266aa
> > 100644 --- a/Documentation/netlink/specs/ethtool.yaml +++
> > b/Documentation/netlink/specs/ethtool.yaml @@ -98,6 +98,13 @@ definitions:
> > name: tcp-data-split
> > type: enum
> > entries: [ unknown, disabled, enabled ]
> > + -
> > + name: ts-hwtstamp-source
> > + enum-name: hwtstamp-source
> > + header: linux/net_tstamp.h
> > + type: enum
> > + name-prefix: hwtstamp-source
> > + entries: [ unspec, netdev, phylib ]
> >
> > attribute-sets:
> > -
> > @@ -896,6 +903,13 @@ attribute-sets:
> > name: hwtstamp-provider
> > type: nest
> > nested-attributes: ts-hwtstamp-provider
> > + -
> > + name: hwtstamp-source
> > + type: u32
> > + enum: ts-hwtstamp-source
> > + -
> > + name: hwtstamp-phyindex
> > + type: u32
> > -
> > name: cable-result
> > attr-cnt-name: __ethtool-a-cable-result-cnt
> > @@ -1981,6 +1995,8 @@ operations:
> > - phc-index
> > - stats
> > - hwtstamp-provider
> > + - hwtstamp-source
> > + - hwtstamp-phyindex
> > dump: *tsinfo-get-op
> > -
> > name: cable-test-act
> > diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> > index
> > 117718c2481439d09f60cd596012dfa0feef3ca8..f18fc8269f7066eadd6fa823e0d43b4ae50b8c46
> > 100644 --- a/include/linux/ethtool.h +++ b/include/linux/ethtool.h
> > @@ -830,6 +830,8 @@ struct ethtool_rxfh_param {
> > * @so_timestamping: bit mask of the sum of the supported SO_TIMESTAMPING
> > flags
> > * @phc_index: device index of the associated PHC, or -1 if there is none
> > * @phc_qualifier: qualifier of the associated PHC
> > + * @phc_source: source device of the associated PHC
> > + * @phc_phyindex: index of PHY device source of the associated PHC
> > * @tx_types: bit mask of the supported hwtstamp_tx_types enumeration
> > values
> > * @rx_filters: bit mask of the supported hwtstamp_rx_filters enumeration
> > values */
> > @@ -838,6 +840,8 @@ struct kernel_ethtool_ts_info {
> > u32 so_timestamping;
> > int phc_index;
> > enum hwtstamp_provider_qualifier phc_qualifier;
> > + enum hwtstamp_source phc_source;
> > + int phc_phyindex;
> > enum hwtstamp_tx_types tx_types;
> > enum hwtstamp_rx_filters rx_filters;
> > };
> > diff --git a/include/linux/net_tstamp.h b/include/linux/net_tstamp.h
> > index
> > ff0758e88ea1008efe533cde003b12719bf4fcd3..1414aed0b6adeae15b56e7a99a7d9eeb43ba0b6c
> > 100644 --- a/include/linux/net_tstamp.h +++ b/include/linux/net_tstamp.h
> > @@ -13,12 +13,6 @@
> > SOF_TIMESTAMPING_TX_HARDWARE | \
> > SOF_TIMESTAMPING_RAW_HARDWARE)
> >
> > -enum hwtstamp_source {
> > - HWTSTAMP_SOURCE_UNSPEC,
> > - HWTSTAMP_SOURCE_NETDEV,
> > - HWTSTAMP_SOURCE_PHYLIB,
> > -};
> > -
> > /**
> > * struct hwtstamp_provider_desc - hwtstamp provider description
> > *
> > diff --git a/include/uapi/linux/ethtool_netlink_generated.h
> > b/include/uapi/linux/ethtool_netlink_generated.h index
> > 30c8dad6214e9a882f1707e4835e9efc73c3f92e..7cbcf44d0a3284490006961d3513c58ccda98038
> > 100644 --- a/include/uapi/linux/ethtool_netlink_generated.h +++
> > b/include/uapi/linux/ethtool_netlink_generated.h @@ -401,6 +401,8 @@ enum {
> > ETHTOOL_A_TSINFO_PHC_INDEX,
> > ETHTOOL_A_TSINFO_STATS,
> > ETHTOOL_A_TSINFO_HWTSTAMP_PROVIDER,
> > + ETHTOOL_A_TSINFO_HWTSTAMP_SOURCE,
> > + ETHTOOL_A_TSINFO_HWTSTAMP_PHYINDEX,
> >
> > __ETHTOOL_A_TSINFO_CNT,
> > ETHTOOL_A_TSINFO_MAX = (__ETHTOOL_A_TSINFO_CNT - 1)
> > diff --git a/include/uapi/linux/net_tstamp.h
> > b/include/uapi/linux/net_tstamp.h index
> > a93e6ea37fb3a69f331b1c90851d4e68cb659a83..bf5fb9f7acf5c03aaa121e0cda3c0b1d83e49f71
> > 100644 --- a/include/uapi/linux/net_tstamp.h +++
> > b/include/uapi/linux/net_tstamp.h @@ -13,6 +13,19 @@
> > #include <linux/types.h>
> > #include <linux/socket.h> /* for SO_TIMESTAMPING */
> >
> > +/**
> > + * enum hwtstamp_source - Source of the hardware timestamp
> > + * @HWTSTAMP_SOURCE_UNSPEC: Source not specified or unknown
> > + * @HWTSTAMP_SOURCE_NETDEV: Hardware timestamp comes from the net device
> > + * @HWTSTAMP_SOURCE_PHYLIB: Hardware timestamp comes from one of the PHY
> > + * devices of the network topology
> > + */
> > +enum hwtstamp_source {
> > + HWTSTAMP_SOURCE_UNSPEC,
> > + HWTSTAMP_SOURCE_NETDEV,
> > + HWTSTAMP_SOURCE_PHYLIB,
> > +};
> > +
> > /*
> > * Possible type of hwtstamp provider. Mainly "precise" the default one
> > * is for IEEE 1588 quality and "approx" is for NICs DMA point.
> > diff --git a/net/ethtool/common.c b/net/ethtool/common.c
> > index
> > 49bea6b45bd5c1951ff1a52a9f30791040044d10..43e62885b46b5c0abc484d2661f7cdf8a3e23169
> > 100644 --- a/net/ethtool/common.c +++ b/net/ethtool/common.c
> > @@ -920,12 +920,20 @@ int ethtool_get_ts_info_by_phc(struct net_device *dev,
> > struct phy_device *phy;
> >
> > phy = ethtool_phy_get_ts_info_by_phc(dev, info,
> > hwprov_desc);
> > - if (IS_ERR(phy))
> > + if (IS_ERR(phy)) {
> > err = PTR_ERR(phy);
> > - else
> > - err = 0;
> > + goto out;
> > + }
> > +
> > + info->phc_source = HWTSTAMP_SOURCE_PHYLIB;
> > + info->phc_phyindex = phy->phyindex;
> > + err = 0;
> > + goto out;
> > + } else {
> > + info->phc_source = HWTSTAMP_SOURCE_NETDEV;
> > }
> >
> > +out:
> > info->so_timestamping |= SOF_TIMESTAMPING_RX_SOFTWARE |
> > SOF_TIMESTAMPING_SOFTWARE;
> >
> > @@ -947,10 +955,14 @@ int __ethtool_get_ts_info(struct net_device *dev,
> >
> > ethtool_init_tsinfo(info);
> > if (phy_is_default_hwtstamp(phydev) &&
> > - phy_has_tsinfo(phydev))
> > + phy_has_tsinfo(phydev)) {
> > err = phy_ts_info(phydev, info);
> > - else if (ops->get_ts_info)
> > + info->phc_source = HWTSTAMP_SOURCE_PHYLIB;
> > + info->phc_phyindex = phydev->phyindex;
> > + } else if (ops->get_ts_info) {
> > err = ops->get_ts_info(dev, info);
> > + info->phc_source = HWTSTAMP_SOURCE_NETDEV;
> > + }
> >
> > info->so_timestamping |= SOF_TIMESTAMPING_RX_SOFTWARE |
> > SOF_TIMESTAMPING_SOFTWARE;
> > diff --git a/net/ethtool/tsinfo.c b/net/ethtool/tsinfo.c
> > index
> > 8130b406ef107f7311cba15c5aafba3ba82bb5a3..62e82f43dba998abd840ea15505084e3127b4520
> > 100644 --- a/net/ethtool/tsinfo.c +++ b/net/ethtool/tsinfo.c
> > @@ -160,6 +160,12 @@ static int tsinfo_reply_size(const struct
> > ethnl_req_info *req_base, /* _TSINFO_HWTSTAMP_PROVIDER */
> > len += nla_total_size(0) + 2 * nla_total_size(sizeof(u32));
> > }
> > + if (ts_info->phc_source) {
> > + len += nla_total_size(sizeof(u32)); /*
> > _TSINFO_HWTSTAMP_SOURCE */
> > + if (ts_info->phc_phyindex)
> > + /* _TSINFO_HWTSTAMP_PHYINDEX */
> > + len += nla_total_size(sizeof(u32));
> > + }
> > if (req_base->flags & ETHTOOL_FLAG_STATS)
> > len += nla_total_size(0) + /* _TSINFO_STATS */
> > nla_total_size_64bit(sizeof(u64)) *
> > ETHTOOL_TS_STAT_CNT; @@ -259,6 +265,16 @@ static int
> > tsinfo_fill_reply(struct sk_buff *skb,
> >
> > nla_nest_end(skb, nest);
> > }
> > + if (ts_info->phc_source) {
> > + if (nla_put_u32(skb, ETHTOOL_A_TSINFO_HWTSTAMP_SOURCE,
> > + ts_info->phc_source))
> > + return -EMSGSIZE;
> > +
> > + if (ts_info->phc_phyindex &&
> > + nla_put_u32(skb, ETHTOOL_A_TSINFO_HWTSTAMP_PHYINDEX,
> > + ts_info->phc_phyindex))
> > + return -EMSGSIZE;
> > + }
> > if (req_base->flags & ETHTOOL_FLAG_STATS &&
> > tsinfo_put_stats(skb, &data->stats))
> > return -EMSGSIZE;
> > @@ -346,6 +362,9 @@ static int ethnl_tsinfo_dump_one_phydev(struct sk_buff
> > *skb, if (ret < 0)
> > goto err;
> >
> > + reply_data->ts_info.phc_source = HWTSTAMP_SOURCE_PHYLIB;
> > + reply_data->ts_info.phc_phyindex = phydev->phyindex;
> > +
> > ret = ethnl_tsinfo_end_dump(skb, dev, req_info, reply_data, ehdr);
> > if (ret < 0)
> > goto err;
> > @@ -389,6 +408,7 @@ static int ethnl_tsinfo_dump_one_netdev(struct sk_buff
> > *skb, if (ret < 0)
> > goto err;
> >
> > + reply_data->ts_info.phc_source = HWTSTAMP_SOURCE_NETDEV;
> > ret = ethnl_tsinfo_end_dump(skb, dev, req_info, reply_data,
> > ehdr);
> > if (ret < 0)
> > diff --git a/tools/net/ynl/Makefile.deps b/tools/net/ynl/Makefile.deps
> > index
> > 8b7bf673b686f17db06a3798d23d2350f7cf76c1..6c03b477f672eab80e2c71452c982b9561cb7c4a
> > 100644 --- a/tools/net/ynl/Makefile.deps +++ b/tools/net/ynl/Makefile.deps
> > @@ -18,7 +18,8 @@ CFLAGS_devlink:=$(call
> > get_hdr_inc,_LINUX_DEVLINK_H_,devlink.h) CFLAGS_dpll:=$(call
> > get_hdr_inc,_LINUX_DPLL_H,dpll.h) CFLAGS_ethtool:=$(call
> > get_hdr_inc,_LINUX_ETHTOOL_H,ethtool.h) \ $(call
> > get_hdr_inc,_LINUX_ETHTOOL_NETLINK_H_,ethtool_netlink.h) \
> > - $(call
> > get_hdr_inc,_LINUX_ETHTOOL_NETLINK_GENERATED_H,ethtool_netlink_generated.h)
> > + $(call
> > get_hdr_inc,_LINUX_ETHTOOL_NETLINK_GENERATED_H,ethtool_netlink_generated.h)
> > \
> > + $(call get_hdr_inc,_NET_TIMESTAMPING_H,net_tstamp.h)
> > CFLAGS_handshake:=$(call get_hdr_inc,_LINUX_HANDSHAKE_H,handshake.h)
> > CFLAGS_lockd_netlink:=$(call
> > get_hdr_inc,_LINUX_LOCKD_NETLINK_H,lockd_netlink.h) CFLAGS_mptcp_pm:=$(call
> > get_hdr_inc,_LINUX_MPTCP_PM_H,mptcp_pm.h)
> >
> > ---
> > base-commit: 3a726f8feac35d9b9ee11cf9737d62fe2410d539
> > change-id: 20250418-feature_ptp_source-df4a98c94139
> >
> > Best regards,
> > --
> > Köry Maincent, Bootlin
> > Embedded Linux and kernel engineering
> > https://bootlin.com
> >
> >
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Powered by blists - more mailing lists