Re: [PATCH net-next v3 05/10] net: sched: add API for registering unlocked offload block callbacks

[Vlad Buslov <vladbu@mellanox.com>]

On Wed 28 Aug 2019 at 14:53, tanhuazhong <tanhuazhong@huawei.com> wrote:
> On 2019/8/26 21:45, Vlad Buslov wrote:
>> Extend struct flow_block_offload with "unlocked_driver_cb" flag to allow
>> registering and unregistering block hardware offload callbacks that do not
>> require caller to hold rtnl lock. Extend tcf_block with additional
>> lockeddevcnt counter that is incremented for each non-unlocked driver
>> callback attached to device. This counter is necessary to conditionally
>> obtain rtnl lock before calling hardware callbacks in following patches.
>>
>> Register mlx5 tc block offload callbacks as "unlocked".
>>
>> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
>> Acked-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>>   drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 2 ++
>>   drivers/net/ethernet/mellanox/mlx5/core/en_rep.c  | 3 +++
>>   include/net/flow_offload.h                        | 1 +
>>   include/net/sch_generic.h                         | 1 +
>>   net/sched/cls_api.c                               | 6 ++++++
>>   5 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> index fa4bf2d4bcd4..8592b98d0e70 100644
>> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
>> @@ -3470,10 +3470,12 @@ static int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type,
>>   			  void *type_data)
>>   {
>>   	struct mlx5e_priv *priv = netdev_priv(dev);
>> +	struct flow_block_offload *f = type_data;
>>     	switch (type) {
>>   #ifdef CONFIG_MLX5_ESWITCH
>>   	case TC_SETUP_BLOCK:
>> +		f->unlocked_driver_cb = true;
>>   		return flow_block_cb_setup_simple(type_data,
>>   						  &mlx5e_block_cb_list,
>>   						  mlx5e_setup_tc_block_cb,
> Hi,
>
> I have got below warning when compiling the latest net-next:
> drivers/net/ethernet/mellanox//mlx5/core/en_main.c:3473:29: warning: unused
> variable ‘f’ [-Wunused-variable]
>   struct flow_block_offload *f = type_data;
>
> Could this variable be defined within "#ifdef CONFIG_MLX5_ESWITCH"?
> BTW, it seems varible f has not been used in any place in addition to assigning
> true to its member unlocked_driver_cb in "case TC_SETUP_BLOCK:". Maybe I have
> miss something about it:).
>
> Huazhong.
> Thanks.
>

Hi Huazhong,

Thanks for reporting!

Yes, it looks like f declaration needs to be moved into "#ifdef
CONFIG_MLX5_ESWITCH" block. I'll send the patch.

Regards,
Vlad