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

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: <661550e348224_23a2b2294f7@willemb.c.googlers.com.notmuch>
Date: Tue, 09 Apr 2024 10:29:55 -0400
From: Willem de Bruijn <willemdebruijn.kernel@...il.com>
To: John Fraker <jfraker@...gle.com>, 
 netdev@...r.kernel.org
Cc: John Fraker <jfraker@...gle.com>, 
 Praveen Kaligineedi <pkaligineedi@...gle.com>, 
 Harshitha Ramamurthy <hramamurthy@...gle.com>, 
 Shailend Chand <shailend@...gle.com>, 
 Willem de Bruijn <willemb@...gle.com>, 
 "David S. Miller" <davem@...emloft.net>, 
 Jakub Kicinski <kuba@...nel.org>, 
 Junfeng Guo <junfeng.guo@...el.com>, 
 Ziwei Xiao <ziweixiao@...gle.com>, 
 Jeroen de Borst <jeroendb@...gle.com>, 
 linux-kernel@...r.kernel.org, 
 kory.maincent@...tlin.com, 
 andrew@...n.ch, 
 richardcochran@...il.com
Subject: Re: [PATCH net-next] gve: Correctly report software timestamping
 capabilities

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

Reviewed-by: Willem de Bruijn <willemb@...gle.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
>