lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL+tcoAziAJD5b+AMingR4QpTmHLYJCVMCeEsGUeC0TEuRjTHg@mail.gmail.com>
Date: Sun, 27 Apr 2025 08:26:45 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: Kory Maincent <kory.maincent@...tlin.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

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?

The code itself looks good to me, btw.

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
>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ