Re: [PATCH net-next] gve: Correctly report software timestamping capabilities

[Willem de Bruijn]

John Fraker wrote:
> gve has supported software timestamp generation since its inception,
> but has not advertised that support via ethtool. This patch correctly
> advertises that support.
> 
> Reviewed-by: Praveen Kaligineedi <pkaligineedi@xxxxxxxxxx>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@xxxxxxxxxx>
> Signed-off-by: John Fraker <jfraker@xxxxxxxxxx>

Reviewed-by: Willem de Bruijn <willemb@xxxxxxxxxx>

> 
> ---
>  drivers/net/ethernet/google/gve/gve_ethtool.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/google/gve/gve_ethtool.c b/drivers/net/ethernet/google/gve/gve_ethtool.c
> index 815dead..99f5aeb 100644
> --- a/drivers/net/ethernet/google/gve/gve_ethtool.c
> +++ b/drivers/net/ethernet/google/gve/gve_ethtool.c
> @@ -4,6 +4,8 @@
>   * Copyright (C) 2015-2021 Google, Inc.
>   */
>  
> +#include <linux/ethtool.h>
> +#include <linux/net_tstamp.h>
>  #include <linux/rtnetlink.h>
>  #include "gve.h"
>  #include "gve_adminq.h"
> @@ -763,6 +765,15 @@ static int gve_set_coalesce(struct net_device *netdev,
>  	return 0;
>  }
>  
> +static int gve_get_ts_info(struct net_device *netdev, struct ethtool_ts_info *info)
> +{
> +	info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
> +				SOF_TIMESTAMPING_TX_SOFTWARE |
> +				SOF_TIMESTAMPING_SOFTWARE;
> +
> +	return 0;

This device calls skb_tx_timestamp in its ndo_start_xmit: the
prerequisite for SOF_TIMESTAMPING_TX_SOFTWARE.

All devices support SOF_TIMESTAMPING_RX_SOFTWARE by virtue of
net_timestamp_check being called in the device independent code.

To ethtool timestamping maintainers: It's quite unnecessary to have
each device advertise SOF_TIMESTAMPING_RX_SOFTWARE |
SOF_TIMESTAMPING_SOFTWARE. In __ethtool_get_ts_info we could just
always add those flags to the result from the callees.

        if (phy_has_tsinfo(phydev))
                return phy_ts_info(phydev, info);
        if (ops->get_ts_info)
                return ops->get_ts_info(dev, info);

        info->so_timestamping = SOF_TIMESTAMPING_RX_SOFTWARE |
                                SOF_TIMESTAMPING_SOFTWARE;

> +}
> +
>  const struct ethtool_ops gve_ethtool_ops = {
>  	.supported_coalesce_params = ETHTOOL_COALESCE_USECS,
>  	.supported_ring_params = ETHTOOL_RING_USE_TCP_DATA_SPLIT,
> @@ -784,5 +795,6 @@ const struct ethtool_ops gve_ethtool_ops = {
>  	.set_tunable = gve_set_tunable,
>  	.get_priv_flags = gve_get_priv_flags,
>  	.set_priv_flags = gve_set_priv_flags,
> -	.get_link_ksettings = gve_get_link_ksettings
> +	.get_link_ksettings = gve_get_link_ksettings,
> +	.get_ts_info = gve_get_ts_info
>  };
> -- 
> 2.44.0.478.gd926399ef9-goog
>