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 <michal.swiatkow...@linux.intel.com>
>> Reviewed-by: Marcin Szycik <marcin.szy...@linux.intel.com>
>> Signed-off-by: Wojciech Drewek <wojciech.dre...@intel.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
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,
>> };
>>
>> /**
>
