Subject: Re: [PATCH net-next 11/12] net: sched: flower: track rtnl lock state

From: Vlad Buslov <[email-masked]>
Date: Mon, 18 Feb 2019 09:35:55 +0000

On Fri 15 Feb 2019 at 22:46, Stefano Brivio <[email-masked]> wrote:
> On Thu, 14 Feb 2019 09:47:11 +0200
> Vlad Buslov <[email-masked]> wrote:
>
>>  static int fl_hw_replace_filter(struct tcf_proto *tp,
>> -				struct cls_fl_filter *f,
>> +				struct cls_fl_filter *f, bool rtnl_held,
>>  				struct netlink_ext_ack *extack)
>>  {
>>  	struct tc_cls_flower_offload cls_flower = {};
>>  	struct tcf_block *block = tp->chain->block;
>>  	bool skip_sw = tc_skip_sw(f->flags);
>> -	int err;
>> +	int err = 0;
>> +
>> +	if (!rtnl_held)
>> +		rtnl_lock();
>>  
>>  	cls_flower.rule = flow_rule_alloc(tcf_exts_num_actions(&f->exts));
>>  	if (!cls_flower.rule)
>
>                 return -ENOMEM;
>
> Don't you need to:
>
> 		err = -ENOMEM;
> 		goto errout;
>
> here?
>
> Same...
>
>         err = tc_setup_flow_action(&cls_flower.rule->action, &f->exts);
>         if (err) {
>                 kfree(cls_flower.rule);
>                 if (skip_sw) {
>                         NL_SET_ERR_MSG_MOD(extack, "Failed to setup flow action");
>                         return err;
>
> here,
>
>                 }
>                 return 0;
>
> and here.

Thanks for catching this!
These error handlers were introduced by recent Pablo's patches and I
missed them during rebase.