netdev - Re: [Intel-wired-lan] [PATCH iwl-next] ice: Implement ethtool reset support

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: <a3682c39-054e-44f7-80c9-b181264c2413@intel.com>
Date: Wed, 31 Jul 2024 10:22:53 +0200
From: Przemek Kitszel <przemyslaw.kitszel@...el.com>
To: Wojciech Drewek <wojciech.drewek@...el.com>
CC: <simon.horman@...igine.com>, <edumazet@...gle.com>,
	<anthony.l.nguyen@...el.com>, <kuba@...nel.org>,
	<intel-wired-lan@...ts.osuosl.org>, <pabeni@...hat.com>,
	<netdev@...r.kernel.org>
Subject: Re: [Intel-wired-lan] [PATCH iwl-next] ice: Implement ethtool reset
 support

On 7/30/24 12:51, Wojciech Drewek wrote:
> Enable ethtool reset support. Each ethtool reset
> type is mapped to the CVL reset type:

not CVL, perhaps "device" or "E810"

> ETH_RESET_MAC - ICE_RESET_CORER
> ETH_RESET_ALL - ICE_RESET_GLOBR
> ETH_RESET_DEDICATED - ICE_RESET_PFR
> 
> Multiple reset flags are not supported.
> Calling any reset type on port representor triggers VF reset.
> 
> Command example:
> GLOBR:
> $ ethtool --reset enp1s0f0np0 all
> CORER:
> $ ethtool --reset enp1s0f0np0 mac
> PFR:
> $ ethtool --reset enp1s0f0np0 dedicated
> VF reset:
> $ ethtool --reset $port_representor mac
> 
> Reviewed-by: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
> Reviewed-by: Marcin Szycik <marcin.szycik@...ux.intel.com>
> Signed-off-by: Wojciech Drewek <wojciech.drewek@...el.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_ethtool.c | 64 ++++++++++++++++++++
>   1 file changed, 64 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ethtool.c b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> index 39d2652c3ee1..00b8ac3f1dff 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ethtool.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ethtool.c
> @@ -4794,6 +4794,68 @@ static void ice_get_ts_stats(struct net_device *netdev,
>   	ts_stats->lost = ptp->tx_hwtstamp_timeouts;
>   }
>   
> +/**
> + * ice_ethtool_reset - triggers a given type of reset
> + * @dev: network interface device structure
> + * @flags: set of reset flags
> + *
> + * Note that multiple reset flags are not supported
> + */
> +static int ice_ethtool_reset(struct net_device *dev, u32 *flags)
> +{
> +	struct ice_netdev_priv *np = netdev_priv(dev);
> +	struct ice_pf *pf = np->vsi->back;
> +	enum ice_reset_req reset;
> +
> +	switch (*flags) {
> +	case ETH_RESET_MAC:
> +		*flags &= ~ETH_RESET_MAC;

this line is equivalent to:
*flags = 0;

> +		reset = ICE_RESET_CORER;
> +		break;
> +	case ETH_RESET_ALL:
> +		*flags &= ~ETH_RESET_ALL;

ditto

> +		reset = ICE_RESET_GLOBR;
> +		break;
> +	case ETH_RESET_DEDICATED:
> +		*flags &= ~ETH_RESET_DEDICATED;

ditto
you could just move *flags = 0; after the switch statement

> +		reset = ICE_RESET_PFR;
> +		break;
> +	default:
> +		netdev_info(dev, "Unsupported set of ethtool flags, multiple flags are not supported");
> +		return -EOPNOTSUPP;
> +	}
> +
> +	ice_schedule_reset(pf, reset);
> +
> +	return 0;
> +}
> +
> +/**
> + * ice_repr_ethtool_reset - triggers a VF reset
> + * @dev: network interface device structure
> + * @flags: set of reset flags
> + *
> + * VF associated with the given port representor will be reset
> + * Any type of reset will trigger VF reset

why not to support just one type of reset here?
(that would left us with future option of different behavior on
different reset type requested)

> + */
> +static int ice_repr_ethtool_reset(struct net_device *dev, u32 *flags)
> +{
> +	struct ice_repr *repr = ice_netdev_to_repr(dev);
> +	struct ice_vf *vf;
> +
> +	if (repr->type != ICE_REPR_TYPE_VF)
> +		return -EOPNOTSUPP;
> +
> +	vf = repr->vf;
> +
> +	if (ice_check_vf_ready_for_cfg(vf))
> +		return -EBUSY;
> +
> +	*flags = 0;
> +
> +	return ice_reset_vf(vf, ICE_VF_RESET_VFLR | ICE_VF_RESET_LOCK);
> +}
> +
>   static const struct ethtool_ops ice_ethtool_ops = {
>   	.cap_rss_ctx_supported  = true,
>   	.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
> @@ -4829,6 +4891,7 @@ static const struct ethtool_ops ice_ethtool_ops = {
>   	.nway_reset		= ice_nway_reset,
>   	.get_pauseparam		= ice_get_pauseparam,
>   	.set_pauseparam		= ice_set_pauseparam,
> +	.reset			= ice_ethtool_reset,
>   	.get_rxfh_key_size	= ice_get_rxfh_key_size,
>   	.get_rxfh_indir_size	= ice_get_rxfh_indir_size,
>   	.get_rxfh		= ice_get_rxfh,
> @@ -4885,6 +4948,7 @@ static const struct ethtool_ops ice_ethtool_repr_ops = {
>   	.get_strings		= ice_repr_get_strings,
>   	.get_ethtool_stats      = ice_repr_get_ethtool_stats,
>   	.get_sset_count		= ice_repr_get_sset_count,
> +	.reset			= ice_repr_ethtool_reset,
>   };
>   
>   /**