On Wed, Jan 27, 2021 at 3:58 AM Saeed Mahameed <sae...@nvidia.com> wrote:
>
> From: Maxim Mikityanskiy <maxi...@mellanox.com>
>
> Sometimes, channel params are changed without recreating the channels.
> It happens in two basic cases: when the channels are closed, and when
> the parameter being changed doesn't affect how channels are configured.
> Such changes invoke a hardware command that might fail. The whole
> operation should be reverted in such cases, but the code that restores
> the parameters' values in the driver was missing. This commit adds this
> handling.
>
> Fixes: 2e20a151205b ("net/mlx5e: Fail safe mtu and lro setting")
> Signed-off-by: Maxim Mikityanskiy <maxi...@mellanox.com>
> Reviewed-by: Tariq Toukan <tar...@nvidia.com>
> Signed-off-by: Saeed Mahameed <sae...@nvidia.com>
> ---
> .../net/ethernet/mellanox/mlx5/core/en_main.c | 30 +++++++++++++------
> 1 file changed, 21 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> index ac76d32bad7d..a9d824a9cb05 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> @@ -3764,7 +3764,7 @@ static int set_feature_lro(struct net_device *netdev,
> bool enable)
> struct mlx5e_priv *priv = netdev_priv(netdev);
> struct mlx5_core_dev *mdev = priv->mdev;
> struct mlx5e_channels new_channels = {};
> - struct mlx5e_params *old_params;
> + struct mlx5e_params *cur_params;
> int err = 0;
> bool reset;
>
> @@ -3777,8 +3777,8 @@ static int set_feature_lro(struct net_device *netdev,
> bool enable)
> goto out;
> }
>
> - old_params = &priv->channels.params;
> - if (enable && !MLX5E_GET_PFLAG(old_params,
> MLX5E_PFLAG_RX_STRIDING_RQ)) {
> + cur_params = &priv->channels.params;
> + if (enable && !MLX5E_GET_PFLAG(cur_params,
> MLX5E_PFLAG_RX_STRIDING_RQ)) {
> netdev_warn(netdev, "can't set LRO with legacy RQ\n");
> err = -EINVAL;
> goto out;
> @@ -3786,18 +3786,23 @@ static int set_feature_lro(struct net_device *netdev,
> bool enable)
>
> reset = test_bit(MLX5E_STATE_OPENED, &priv->state);
>
> - new_channels.params = *old_params;
> + new_channels.params = *cur_params;
> new_channels.params.lro_en = enable;
>
> - if (old_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
> - if (mlx5e_rx_mpwqe_is_linear_skb(mdev, old_params, NULL) ==
> + if (cur_params->rq_wq_type != MLX5_WQ_TYPE_CYCLIC) {
> + if (mlx5e_rx_mpwqe_is_linear_skb(mdev, cur_params, NULL) ==
> mlx5e_rx_mpwqe_is_linear_skb(mdev, &new_channels.params,
> NULL))
> reset = false;
> }
>
> if (!reset) {
> - *old_params = new_channels.params;
> + struct mlx5e_params old_params;
> +
> + old_params = *cur_params;
> + *cur_params = new_channels.params;
> err = mlx5e_modify_tirs_lro(priv);
> + if (err)
> + *cur_params = old_params;
No need to explicitly save and restore all params if the only one
changed is lro_en?
