Re: [PATCH net-next RFC v4 2/5] net: Expose available time stamping layers to user space.

Jakub Kicinski
2023-04-07T01:46:53Z

On Thu, 6 Apr 2023 19:33:05 +0200 Köry Maincent wrote:
> +Gets transceiver module parameters.
> +
> +Request contents:
> +
> + ================================= ====== ==========================
> + ``ETHTOOL_A_TS_HEADER`` nested request header
> + ================================= ====== ==========================
> +
> +Kernel response contents:
> +
> + ======================= ====== ====================================
> + ``ETHTOOL_A_TS_HEADER`` nested reply header
> + ``ETHTOOL_A_TS_LAYER`` u32 current hardware timestamping
> + ======================= ====== ====================================
> +
> +TSLIST_GET
> +==========
> +
> +Gets transceiver module parameters.
> +
> +Request contents:
> +
> + ================================= ====== ==========================
> + ``ETHTOOL_A_TS_HEADER`` nested request header
> + ================================= ====== ==========================
> +
> +Kernel response contents:
> +
> + ======================= ====== ===================================
> + ``ETHTOOL_A_TS_HEADER`` nested reply header
> + ``ETHTOOL_A_TS_LAYER`` u32 available hardware timestamping
> + ======================= ====== ===================================

Please put some words in here :) Documentation matters to the users.

Also let me advertise tools/net/ynl/cli.py to you:
https://docs.kernel.org/next/userspace-api/netlink/intro-specs.html

Please fill in the new commands in the ethtool spec. Feel free to ask
if you have any questions, it's still a bit of a rough.. clay?

> +/* Hardware layer of the SO_TIMESTAMPING provider */
> +enum timestamping_layer {
> + SOF_MAC_TIMESTAMPING = (1<<0),
> + SOF_PHY_TIMESTAMPING = (1<<1),

What does SOF_ stand for? 🤔️

We need a value for DMA timestamps here.

> +/* TSLIST_GET */
> +static int tslist_prepare_data(const struct ethnl_req_info *req_base,
> + struct ethnl_reply_data *reply_base,
> + struct genl_info *info)
> +{
> + struct ts_reply_data *data = TS_REPDATA(reply_base);
> + struct net_device *dev = reply_base->dev;
> + const struct ethtool_ops *ops = dev->ethtool_ops;
> + int ret;
> +
> + ret = ethnl_ops_begin(dev);
> + if (ret < 0)
> + return ret;
> +
> + data->ts = 0;
> + if (phy_has_tsinfo(dev->phydev))
> + data->ts = SOF_PHY_TIMESTAMPING;
> + if (ops->get_ts_info)
> + data->ts |= SOF_MAC_TIMESTAMPING;

We can't make that assumption, that info must come from the driver.

Also don't we need some way to identify the device / phc from which
the timestamp at the given layer will come?