[iwl-next] ice: Implement ethtool reset support

Commit Message

Wojciech Drewek July 30, 2024, 10:51 a.m. UTC

Enable ethtool reset support. Each ethtool reset
type is mapped to the CVL reset type:
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 <[e-mail]>
Reviewed-by: Marcin Szycik <[e-mail]>
Signed-off-by: Wojciech Drewek <[e-mail]>
---
 drivers/net/ethernet/intel/ice/ice_ethtool.c | 64 ++++++++++++++++++++
 1 file changed, 64 insertions(+)

Comments

Jakub Kicinski July 30, 2024, 1:58 p.m. UTC | #1

On Tue, 30 Jul 2024 12:51:21 +0200 Wojciech Drewek wrote:
> ETH_RESET_MAC - ICE_RESET_CORER

Core doesn't really sound like MAC, what is it?
And does PF reset reset mostly PCIe side or more?
My knee jerk mapping would be to map Core to dedicated
and PF to DMA.

Przemek Kitszel July 31, 2024, 8:22 a.m. UTC | #2

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 <[e-mail]>
> Reviewed-by: Marcin Szycik <[e-mail]>
> Signed-off-by: Wojciech Drewek <[e-mail]>
> ---
>   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,
>   };
>   
>   /**

Simon Horman July 31, 2024, 9:24 a.m. UTC | #3

On Tue, Jul 30, 2024 at 12:51:21PM +0200, Wojciech Drewek wrote:
> Enable ethtool reset support. Each ethtool reset
> type is mapped to the CVL reset type:
> 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 <[e-mail]>
> Reviewed-by: Marcin Szycik <[e-mail]>
> Signed-off-by: Wojciech Drewek <[e-mail]>
> ---
>  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)
> +{

nit: Please include a "Return:" or "Returns:" section in the Kernel doc
     of new functions that return a value.
     (i.e. also for ice_repr_ethtool_reset)

     Flagged by ./scripts/kernel-doc -none -Wall

...

Wojciech Drewek July 31, 2024, 12:08 p.m. UTC | #4

On 30.07.2024 15:58, Jakub Kicinski wrote:
> On Tue, 30 Jul 2024 12:51:21 +0200 Wojciech Drewek wrote:
>> ETH_RESET_MAC - ICE_RESET_CORER
> 
> Core doesn't really sound like MAC, what is it?
> And does PF reset reset mostly PCIe side or more?
> My knee jerk mapping would be to map Core to dedicated
> and PF to DMA.
> 

Quick summary our reset types:
PF reset reinitialize the resources/data path for PF and its VFs.
It has no impact on other PF/VFs.
Core Reset reinitialize all functions and shared parts of the
device except PHY/MAC units, EMP and PCI Interface.
Global Reset is Core Reset + PHY/MAC units reset (including External PHY)
Because Global Reset is a extended Core it makes sense to map it to all.
PF reset mapping makes sense to me since it is dedicated to a single physical function.

Wojciech Drewek July 31, 2024, 12:11 p.m. UTC | #5

On 31.07.2024 11:24, Simon Horman wrote:
> On Tue, Jul 30, 2024 at 12:51:21PM +0200, Wojciech Drewek wrote:
>> Enable ethtool reset support. Each ethtool reset
>> type is mapped to the CVL reset type:
>> 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 <[e-mail]>
>> Reviewed-by: Marcin Szycik <[e-mail]>
>> Signed-off-by: Wojciech Drewek <[e-mail]>
>> ---
>>  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)
>> +{
> 
> nit: Please include a "Return:" or "Returns:" section in the Kernel doc
>      of new functions that return a value.
>      (i.e. also for ice_repr_ethtool_reset)
> 
>      Flagged by ./scripts/kernel-doc -none -Wall
> 
> ...

sure thing

Wojciech Drewek July 31, 2024, 12:14 p.m. UTC | #6

On 31.07.2024 10:22, Kitszel, Przemyslaw wrote:
> 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"

right

> 
>> 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 <[e-mail]>
>> Reviewed-by: Marcin Szycik <[e-mail]>
>> Signed-off-by: Wojciech Drewek <[e-mail]>
>> ---
>>   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

makes sense

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

Agree, I'll use dedicated here since it is similar case to PF reset

> 
>> + */
>> +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,
>>   };
>>   
>>   /**
>

Jacob Keller July 31, 2024, 4:48 p.m. UTC | #7

On 7/30/2024 6:58 AM, Jakub Kicinski wrote:
> On Tue, 30 Jul 2024 12:51:21 +0200 Wojciech Drewek wrote:
>> ETH_RESET_MAC - ICE_RESET_CORER
> 
> Core doesn't really sound like MAC, what is it?
> And does PF reset reset mostly PCIe side or more?
> My knee jerk mapping would be to map Core to dedicated
> and PF to DMA.

PF reset only affects the single PCI function, and does not affect the
whole adapter. I don't know how it relates to PCIe resets precisely.

CORE reset affects the whole adapter, and the other functions are
notified of the impending reset via their miscellaneous interrupt vector
in combination with some hardware registers.

GLOBAL reset is similar to the CORE reset, (in that it affects the
entire device), but it is more invasive in the hardware. I cannot
remember offhand the differences between CORE and GLOBAL.

There is also an EMP reset, which is the only reset that completely
reloads the EMP firmware. It is currently used by the device flash
update logic, via devlink reload and is only available if the new
firmware image can be reloaded without issue. (Reloading when the new
firmware could impact PCIe config space is likely to produce undesirable
behavior because the PCIe config space is not reloaded except by power
cycling, so you end up with some weird mismatches.)

Jakub Kicinski July 31, 2024, 11:47 p.m. UTC | #8

On Wed, 31 Jul 2024 14:08:20 +0200 Wojciech Drewek wrote:
> Quick summary our reset types:
> PF reset reinitialize the resources/data path for PF and its VFs.
> It has no impact on other PF/VFs.
> Core Reset reinitialize all functions and shared parts of the
> device except PHY/MAC units, EMP and PCI Interface.
> Global Reset is Core Reset + PHY/MAC units reset (including External PHY)
> Because Global Reset is a extended Core it makes sense to map it to all.
> PF reset mapping makes sense to me since it is dedicated to a single physical function.

On Wed, 31 Jul 2024 09:48:07 -0700 Jacob Keller wrote:
> PF reset only affects the single PCI function, and does not affect the
> whole adapter. I don't know how it relates to PCIe resets precisely.
> 
> CORE reset affects the whole adapter, and the other functions are
> notified of the impending reset via their miscellaneous interrupt vector
> in combination with some hardware registers.
> 
> GLOBAL reset is similar to the CORE reset, (in that it affects the
> entire device), but it is more invasive in the hardware. I cannot
> remember offhand the differences between CORE and GLOBAL.
> 
> There is also an EMP reset, which is the only reset that completely
> reloads the EMP firmware. It is currently used by the device flash
> update logic, via devlink reload and is only available if the new
> firmware image can be reloaded without issue. (Reloading when the new
> firmware could impact PCIe config space is likely to produce undesirable
> behavior because the PCIe config space is not reloaded except by power
> cycling, so you end up with some weird mismatches.)

Note that the reset is controlled using individual bits which can be
combined:

	ETH_RESET_MGMT		= 1 << 0,	/* Management processor */
	ETH_RESET_IRQ		= 1 << 1,	/* Interrupt requester */
	ETH_RESET_DMA		= 1 << 2,	/* DMA engine */
	ETH_RESET_FILTER	= 1 << 3,	/* Filtering/flow direction */
	ETH_RESET_OFFLOAD	= 1 << 4,	/* Protocol offload */
	ETH_RESET_MAC		= 1 << 5,	/* Media access controller */
	ETH_RESET_PHY		= 1 << 6,	/* Transceiver/PHY */
	ETH_RESET_RAM		= 1 << 7,	/* RAM shared between
						 * multiple components */
	ETH_RESET_AP		= 1 << 8,	/* Application processor */

	ETH_RESET_DEDICATED	= 0x0000ffff,	/* All components dedicated to
						 * this interface */
	ETH_RESET_ALL		= 0xffffffff,	/* All components used by this
						 * interface, even if shared */

Note that ethtool CLI defines "shared" version of all bits as bits
shifted up by 16. And it is forward compatible (accepts raw "flags")
if we need to define new bits.

I guess in your case EMP == MGMT? So if these resets don't reset EMP
I presume we shouldn't use any option that includes MGMT..

Could you express your resets in the correct combination of these bits
instead of picking a single one?

Wojciech Drewek Aug. 1, 2024, 11:01 a.m. UTC | #9

On 01.08.2024 01:47, Jakub Kicinski wrote:
> On Wed, 31 Jul 2024 14:08:20 +0200 Wojciech Drewek wrote:
>> Quick summary our reset types:
>> PF reset reinitialize the resources/data path for PF and its VFs.
>> It has no impact on other PF/VFs.
>> Core Reset reinitialize all functions and shared parts of the
>> device except PHY/MAC units, EMP and PCI Interface.
>> Global Reset is Core Reset + PHY/MAC units reset (including External PHY)
>> Because Global Reset is a extended Core it makes sense to map it to all.
>> PF reset mapping makes sense to me since it is dedicated to a single physical function.
> 
> On Wed, 31 Jul 2024 09:48:07 -0700 Jacob Keller wrote:
>> PF reset only affects the single PCI function, and does not affect the
>> whole adapter. I don't know how it relates to PCIe resets precisely.
>>
>> CORE reset affects the whole adapter, and the other functions are
>> notified of the impending reset via their miscellaneous interrupt vector
>> in combination with some hardware registers.
>>
>> GLOBAL reset is similar to the CORE reset, (in that it affects the
>> entire device), but it is more invasive in the hardware. I cannot
>> remember offhand the differences between CORE and GLOBAL.
>>
>> There is also an EMP reset, which is the only reset that completely
>> reloads the EMP firmware. It is currently used by the device flash
>> update logic, via devlink reload and is only available if the new
>> firmware image can be reloaded without issue. (Reloading when the new
>> firmware could impact PCIe config space is likely to produce undesirable
>> behavior because the PCIe config space is not reloaded except by power
>> cycling, so you end up with some weird mismatches.)
> 
> Note that the reset is controlled using individual bits which can be
> combined:
> 
> 	ETH_RESET_MGMT		= 1 << 0,	/* Management processor */
> 	ETH_RESET_IRQ		= 1 << 1,	/* Interrupt requester */
> 	ETH_RESET_DMA		= 1 << 2,	/* DMA engine */
> 	ETH_RESET_FILTER	= 1 << 3,	/* Filtering/flow direction */
> 	ETH_RESET_OFFLOAD	= 1 << 4,	/* Protocol offload */
> 	ETH_RESET_MAC		= 1 << 5,	/* Media access controller */
> 	ETH_RESET_PHY		= 1 << 6,	/* Transceiver/PHY */
> 	ETH_RESET_RAM		= 1 << 7,	/* RAM shared between
> 						 * multiple components */
> 	ETH_RESET_AP		= 1 << 8,	/* Application processor */
> 
> 	ETH_RESET_DEDICATED	= 0x0000ffff,	/* All components dedicated to
> 						 * this interface */
> 	ETH_RESET_ALL		= 0xffffffff,	/* All components used by this
> 						 * interface, even if shared */
> 
> Note that ethtool CLI defines "shared" version of all bits as bits
> shifted up by 16. And it is forward compatible (accepts raw "flags")
> if we need to define new bits.
> 
> I guess in your case EMP == MGMT? So if these resets don't reset EMP
> I presume we shouldn't use any option that includes MGMT..
> 
> Could you express your resets in the correct combination of these bits
> instead of picking a single one?
> 

We've came up with below mapping:

PF reset:
ethtool --reset eth0 irq dma filter offload
(we reset all those components but only for the given PF)

CORE reset:
ethtool --reset eth0 irq-shared dma-shared filter-shared offload-shared ram-shared
(whole adapter is affected so we use shared versions + ram)

GLOBAL reset:
ethtool --reset eth0 irq-shared dma-shared filter-shared offload-shared mac-shared phy-shared ram-shared
(GLOBALR is CORER plus mac and phy components are also reinitialized)

Jakub Kicinski Aug. 1, 2024, 2:13 p.m. UTC | #10

On Thu, 1 Aug 2024 13:01:52 +0200 Wojciech Drewek wrote:
> We've came up with below mapping:
> 
> PF reset:
> ethtool --reset eth0 irq dma filter offload
> (we reset all those components but only for the given PF)
> 
> CORE reset:
> ethtool --reset eth0 irq-shared dma-shared filter-shared offload-shared ram-shared
> (whole adapter is affected so we use shared versions + ram)
> 
> GLOBAL reset:
> ethtool --reset eth0 irq-shared dma-shared filter-shared offload-shared mac-shared phy-shared ram-shared
> (GLOBALR is CORER plus mac and phy components are also reinitialized)

SG!

Patch

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;
+		reset = ICE_RESET_CORER;
+		break;
+	case ETH_RESET_ALL:
+		*flags &= ~ETH_RESET_ALL;
+		reset = ICE_RESET_GLOBR;
+		break;
+	case ETH_RESET_DEDICATED:
+		*flags &= ~ETH_RESET_DEDICATED;
+		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
+ */
+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,
 };
 
 /**