Re: [PATCH RFC net-next v8 06/13] net: Add struct kernel_ethtool_ts_info

From: Rahul Rameshbabu
Date: Fri Feb 16 2024 - 13:31:00 EST


On Fri, 16 Feb, 2024 16:52:24 +0100 Kory Maincent <kory.maincent@xxxxxxxxxxx> wrote:
> In prevision to add new UAPI for hwtstamp we will be limited to the struct
> ethtool_ts_info that is currently passed in fixed binary format through the
> ETHTOOL_GET_TS_INFO ethtool ioctl. It would be good if new kernel code
> already started operating on an extensible kernel variant of that
> structure, similar in concept to struct kernel_hwtstamp_config vs struct
> hwtstamp_config.
>
> Since struct ethtool_ts_info is in include/uapi/linux/ethtool.h, here
> we introduce the kernel-only structure in include/linux/ethtool.h.
> The manual copy is then made in the function called by ETHTOOL_GET_TS_INFO.
>
> Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx>
> ---
> drivers/net/bonding/bond_main.c | 4 ++--
> drivers/net/can/dev/dev.c | 2 +-
> drivers/net/can/peak_canfd/peak_canfd.c | 2 +-
> drivers/net/can/usb/gs_usb.c | 2 +-
> drivers/net/can/usb/peak_usb/pcan_usb_core.c | 2 +-
> drivers/net/can/usb/peak_usb/pcan_usb_core.h | 2 +-
> drivers/net/dsa/hirschmann/hellcreek_hwtstamp.c | 2 +-
> drivers/net/dsa/hirschmann/hellcreek_hwtstamp.h | 2 +-
> drivers/net/dsa/microchip/ksz_ptp.c | 2 +-
> drivers/net/dsa/microchip/ksz_ptp.h | 2 +-
> drivers/net/dsa/mv88e6xxx/hwtstamp.c | 2 +-
> drivers/net/dsa/mv88e6xxx/hwtstamp.h | 4 ++--
> drivers/net/dsa/ocelot/felix.c | 2 +-
> drivers/net/dsa/sja1105/sja1105_ptp.c | 2 +-
> drivers/net/dsa/sja1105/sja1105_ptp.h | 2 +-
> drivers/net/ethernet/amd/xgbe/xgbe-ethtool.c | 2 +-
> .../net/ethernet/aquantia/atlantic/aq_ethtool.c | 2 +-
> .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 2 +-
> drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 2 +-
> drivers/net/ethernet/broadcom/tg3.c | 2 +-
> drivers/net/ethernet/cadence/macb.h | 2 +-
> drivers/net/ethernet/cadence/macb_main.c | 4 ++--
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c | 2 +-
> .../net/ethernet/cavium/thunder/nicvf_ethtool.c | 2 +-
> drivers/net/ethernet/chelsio/cxgb4/cxgb4_ethtool.c | 2 +-
> drivers/net/ethernet/cisco/enic/enic_ethtool.c | 2 +-
> drivers/net/ethernet/engleder/tsnep_ethtool.c | 2 +-
> drivers/net/ethernet/freescale/dpaa/dpaa_ethtool.c | 2 +-
> .../net/ethernet/freescale/dpaa2/dpaa2-ethtool.c | 2 +-
> .../net/ethernet/freescale/enetc/enetc_ethtool.c | 2 +-
> drivers/net/ethernet/freescale/fec_main.c | 2 +-
> drivers/net/ethernet/freescale/gianfar_ethtool.c | 2 +-
> .../net/ethernet/fungible/funeth/funeth_ethtool.c | 2 +-
> drivers/net/ethernet/hisilicon/hns3/hnae3.h | 2 +-
> drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 2 +-
> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.c | 2 +-
> .../net/ethernet/hisilicon/hns3/hns3pf/hclge_ptp.h | 2 +-
> drivers/net/ethernet/intel/e1000e/ethtool.c | 2 +-
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
> drivers/net/ethernet/intel/ice/ice_ethtool.c | 2 +-
> drivers/net/ethernet/intel/igb/igb_ethtool.c | 2 +-
> drivers/net/ethernet/intel/igc/igc_ethtool.c | 2 +-
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 2 +-
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
> .../ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 2 +-
> drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 2 +-
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 +-
> .../net/ethernet/mellanox/mlx5/core/en_ethtool.c | 4 ++--
> .../ethernet/mellanox/mlx5/core/ipoib/ethtool.c | 2 +-
> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 2 +-
> .../net/ethernet/mellanox/mlxsw/spectrum_ethtool.c | 2 +-
> drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.c | 4 ++--
> drivers/net/ethernet/mellanox/mlxsw/spectrum_ptp.h | 10 ++++-----
> drivers/net/ethernet/microchip/lan743x_ethtool.c | 2 +-
> .../ethernet/microchip/lan966x/lan966x_ethtool.c | 2 +-
> .../net/ethernet/microchip/sparx5/sparx5_ethtool.c | 2 +-
> drivers/net/ethernet/mscc/ocelot_net.c | 2 +-
> drivers/net/ethernet/mscc/ocelot_ptp.c | 2 +-
> .../net/ethernet/pensando/ionic/ionic_ethtool.c | 2 +-
> drivers/net/ethernet/qlogic/qede/qede_ethtool.c | 2 +-
> drivers/net/ethernet/qlogic/qede/qede_ptp.c | 2 +-
> drivers/net/ethernet/qlogic/qede/qede_ptp.h | 2 +-
> drivers/net/ethernet/renesas/ravb_main.c | 2 +-
> drivers/net/ethernet/renesas/rswitch.c | 2 +-
> drivers/net/ethernet/sfc/ethtool.c | 2 +-
> drivers/net/ethernet/sfc/falcon/nic.h | 2 +-
> drivers/net/ethernet/sfc/ptp.c | 2 +-
> drivers/net/ethernet/sfc/ptp.h | 5 +++--
> drivers/net/ethernet/sfc/siena/ethtool.c | 2 +-
> drivers/net/ethernet/sfc/siena/ptp.c | 2 +-
> drivers/net/ethernet/sfc/siena/ptp.h | 4 ++--
> .../net/ethernet/stmicro/stmmac/stmmac_ethtool.c | 2 +-
> drivers/net/ethernet/ti/am65-cpsw-ethtool.c | 2 +-
> drivers/net/ethernet/ti/cpsw_ethtool.c | 4 ++--
> drivers/net/ethernet/ti/cpsw_priv.h | 2 +-
> drivers/net/ethernet/ti/icssg/icssg_ethtool.c | 2 +-
> drivers/net/ethernet/ti/netcp_ethss.c | 4 ++--
> drivers/net/ethernet/xscale/ixp4xx_eth.c | 2 +-
> drivers/net/macvlan.c | 2 +-
> drivers/net/netdevsim/ethtool.c | 2 +-
> drivers/net/phy/bcm-phy-ptp.c | 2 +-
> drivers/net/phy/dp83640.c | 2 +-
> drivers/net/phy/marvell_ptp.c | 2 +-
> drivers/net/phy/micrel.c | 4 ++--
> drivers/net/phy/mscc/mscc_ptp.c | 2 +-
> drivers/net/phy/nxp-c45-tja11xx.c | 2 +-
> drivers/ptp/ptp_ines.c | 2 +-
> drivers/s390/net/qeth_ethtool.c | 2 +-
> include/linux/can/dev.h | 2 +-
> include/linux/ethtool.h | 25 +++++++++++++++++++---
> include/linux/mii_timestamper.h | 2 +-
> include/linux/phy.h | 2 +-
> include/net/dsa.h | 2 +-
> include/soc/mscc/ocelot.h | 2 +-
> net/8021q/vlan_dev.c | 2 +-
> net/dsa/user.c | 2 +-
> net/ethtool/common.c | 6 +++---
> net/ethtool/common.h | 2 +-
> net/ethtool/ioctl.c | 12 +++++++++--
> net/ethtool/tsinfo.c | 6 +++---
> net/sched/sch_taprio.c | 2 +-
> 101 files changed, 150 insertions(+), 122 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index b90c33607594..f74190a8d9ee 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -18,6 +18,7 @@
> #include <linux/if_ether.h>
> #include <linux/netlink.h>
> #include <uapi/linux/ethtool.h>
> +#include <uapi/linux/net_tstamp.h>
>
> struct compat_ethtool_rx_flow_spec {
> u32 flow_type;
> @@ -644,6 +645,22 @@ struct ethtool_rxfh_param {
> u8 input_xfrm;
> };
>
> +/**
> + * struct kernel_ethtool_ts_info - kernel copy of struct ethtool_ts_info
> + * @cmd: command number = %ETHTOOL_GET_TS_INFO
> + * @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
> + * @tx_types: bit mask of the supported hwtstamp_tx_types enumeration values
> + * @rx_filters: bit mask of the supported hwtstamp_rx_filters enumeration values
> + */
> +struct kernel_ethtool_ts_info {
> + u32 cmd;
> + u32 so_timestamping;
> + int phc_index;
> + enum hwtstamp_tx_types tx_types;

I might just be terrible at reading code, but isn't this what
tx_reserved in ethtool_ts_info is for? I feel like using one of the
reserved memspaces for this enum removes the need to make
kernel_ethtool_ts_info? I might be missing something obvious however.

> + enum hwtstamp_rx_filters rx_filters;
> +};

--
Thanks,

Rahul Rameshbabu