On Thu, Jun 8, 2017 at 12:35 PM, Or Gerlitz <gerlitz...@gmail.com> wrote:
> On Thu, Jun 8, 2017 at 2:42 AM, Saeed Mahameed <sae...@mellanox.com> wrote:
>
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -2961,9 +2961,8 @@ static int mlx5e_modify_channels_vsd(struct
>> mlx5e_channels *chs, bool vsd)
>> return 0;
>> }
>>
>> -static int mlx5e_setup_tc(struct net_device *netdev, u8 tc)
>> +static int mlx5e_setup_tc(struct mlx5e_priv *priv, u8 tc)
>> {
>> - struct mlx5e_priv *priv = netdev_priv(netdev);
>> struct mlx5e_channels new_channels = {};
>> int err = 0;
>>
>> @@ -2995,6 +2994,7 @@ static int mlx5e_ndo_setup_tc(struct net_device *dev,
>> u32 handle,
>> {
>> struct mlx5e_priv *priv = netdev_priv(dev);
>>
>> +#ifdef CONFIG_MLX5_ESWITCH
>> if (TC_H_MAJ(handle) != TC_H_MAJ(TC_H_INGRESS))
>> goto mqprio;
>>
>> @@ -3013,12 +3013,13 @@ static int mlx5e_ndo_setup_tc(struct net_device
>> *dev, u32 handle,
>> }
>>
>> mqprio:
>> +#endif
>> if (tc->type != TC_SETUP_MQPRIO)
>> - return -EINVAL;
>> + return -EOPNOTSUPP;
>
> why change this corner in this patch? we're doing enough changes
>
>>
>> tc->mqprio->hw = TC_MQPRIO_HW_OFFLOAD_TCS;
>>
>> - return mlx5e_setup_tc(dev, tc->mqprio->num_tc);
>> + return mlx5e_setup_tc(priv, tc->mqprio->num_tc);
>> }
>
> same comment
>
Ok will change both.
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
>
>> @@ -948,13 +946,11 @@ static int mlx5_init_once(struct mlx5_core_dev *dev,
>> struct mlx5_priv *priv)
>> goto err_rl_cleanup;
>> }
>>
>> -#ifdef CONFIG_MLX5_CORE_EN
>> err = mlx5_eswitch_init(dev);
>> if (err) {
>> dev_err(&pdev->dev, "Failed to init eswitch %d\n", err);
>> goto err_mpfs_cleanup;
>> }
>> -#endif
>
> why? before this patch we were doing it only if Ethernet
> (MLX5_CORE_EN) was defined into the build,
> and now we are returning blindly zero if another config isn't there
> (CONFIG_MLX5_ESWITCH) - is that
> fully equivalent? do we want to be fully equiv?
>
Fully equivalent and more correct behavior and design.
eswitch is now separate from MPFS and EN and fully independent and has
its own CONFIG_MLX5_ESWITCH,
you can choose to have it or not (CONFIG_MLX5_ESWITCH) it won't
affects other components.
>> @@ -965,10 +961,8 @@ static int mlx5_init_once(struct mlx5_core_dev *dev,
>> struct mlx5_priv *priv)
>> return 0;
>>
>> err_eswitch_cleanup:
>> -#ifdef CONFIG_MLX5_CORE_EN
>> mlx5_eswitch_cleanup(dev->priv.eswitch);
>> err_mpfs_cleanup:
>> -#endif
>
> same comment/question
Same answer.
