Received-SPF: pass (google.com: domain of linux-kernel+bounces-137095-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Tue, 09 Apr 2024 10:29:55 -0400
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
To: John Fraker <jfraker@google.com>, 
 netdev@vger.kernel.org
Cc: John Fraker <jfraker@google.com>, 
 Praveen Kaligineedi <pkaligineedi@google.com>, 
 Harshitha Ramamurthy <hramamurthy@google.com>, 
 Shailend Chand <shailend@google.com>, 
 Willem de Bruijn <willemb@google.com>, 
 "David S. Miller" <davem@davemloft.net>, 
 Jakub Kicinski <kuba@kernel.org>, 
 Junfeng Guo <junfeng.guo@intel.com>, 
 Ziwei Xiao <ziweixiao@google.com>, 
 Jeroen de Borst <jeroendb@google.com>, 
 linux-kernel@vger.kernel.org, 
 kory.maincent@bootlin.com, 
 andrew@lunn.ch, 
 richardcochran@gmail.com
Message-ID: <661550e348224_23a2b2294f7@willemb.c.googlers.com.notmuch>
In-Reply-To: <20240408180918.2773238-1-jfraker@google.com>
References: <20240408180918.2773238-1-jfraker@google.com>
Subject: Re: [PATCH net-next] gve: Correctly report software timestamping
 capabilities
Precedence: bulk
Mime-Version: 1.0
Content-Type: text/plain;
 charset=utf-8
Content-Transfer-Encoding: 7bit

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@google.com>
> Reviewed-by: Harshitha Ramamurthy <hramamurthy@google.com>
> Signed-off-by: John Fraker <jfraker@google.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

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