On 7/25/2019 6:14 PM, Florian Westphal wrote:
> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
>> On Tue, Jul 23, 2019 at 08:52:40PM +0800, wenxu@xxxxxxxxx wrote:
>>> From: wenxu <wenxu@xxxxxxxxx>
>>>
>>> The nft_setup_cb_call and ndo_setup_tc callback should be under rtnl lock
>>>
>>> or it will report:
>>> kernel: RTNL: assertion failed at
>>> drivers/net/ethernet/mellanox/mlx5/core/en_rep.c (635)
>>>
>>> Signed-off-by: wenxu <wenxu@xxxxxxxxx>
>>> ---
>>> net/netfilter/nf_tables_offload.c | 16 ++++++++++++----
>>> 1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
>>> index 33543f5..3e1a1a8 100644
>>> --- a/net/netfilter/nf_tables_offload.c
>>> +++ b/net/netfilter/nf_tables_offload.c
>>> @@ -115,14 +115,18 @@ static int nft_setup_cb_call(struct nft_base_chain *basechain,
>>> enum tc_setup_type type, void *type_data)
>>> {
>>> struct flow_block_cb *block_cb;
>>> - int err;
>>> + int err = 0;
>>>
>>> + rtnl_lock();
>> Please, have a look at 90d2723c6d4cb2ace50fc3b932a2bcc77710450b and
>> review if this assumption is correct. Probably nfnl_lock() is missing
>> from __nft_release_basechain().
> The mlx driver has a ASSERT_RTNL() in the mlx5e_rep_indr_setup_tc_block()
> callpath. Or are you proposing to remove that assertion? If so, what
> lock should protect the callback lists?
yes, most of the setup_tc callback in mlx driver has a
ASSERT_RTNL() directly or indirectly. Maybe remove this is a good idear
>
>
