From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <netdev-owner@vger.kernel.org>
Received: from mail-io0-f194.google.com ([209.85.223.194]:41573 "EHLO
        mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1424880AbeCBPwK (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 2 Mar 2018 10:52:10 -0500
Received: by mail-io0-f194.google.com with SMTP id q24so11039148ioh.8
        for <netdev@vger.kernel.org>; Fri, 02 Mar 2018 07:52:10 -0800 (PST)
From: Roman Mashak <mrv@mojatatu.com>
To: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Cc: davem@davemloft.net, netdev@vger.kernel.org, jhs@mojatatu.com,
        jiri@resnulli.us, xiyou.wangcong@gmail.com
Subject: Re: [PATCH 4/5] net sched actions: implement get_fill_size routine in act_gact
References: <1519929931-1119-1-git-send-email-mrv@mojatatu.com>
        <1519929931-1119-5-git-send-email-mrv@mojatatu.com>
        <20180301190329.GD3887@localhost.localdomain>
Date: Fri, 02 Mar 2018 10:52:08 -0500
In-Reply-To: <20180301190329.GD3887@localhost.localdomain> (Marcelo Ricardo
        Leitner's message of "Thu, 1 Mar 2018 16:03:29 -0300")
Message-ID: <85d10mqxuf.fsf@sevai.TCC.com>
MIME-Version: 1.0
Content-Type: text/plain
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> writes:

> On Thu, Mar 01, 2018 at 01:45:30PM -0500, Roman Mashak wrote:
>> Signed-off-by: Roman Mashak <mrv@mojatatu.com>
>> ---
>>  net/sched/act_gact.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>> 
>> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
>> index 7456325..ee775ac 100644
>> --- a/net/sched/act_gact.c
>> +++ b/net/sched/act_gact.c
>> @@ -217,6 +217,22 @@ static int tcf_gact_search(struct net *net, struct tc_action **a, u32 index,
>>  	return tcf_idr_search(tn, a, index);
>>  }
>>  
>> +static size_t tcf_gact_get_fill_size(const struct tc_action *act)
>> +{
>> +#ifdef CONFIG_GACT_PROB
>> +	struct tcf_gact *gact = to_gact(act);
>> +	int prob_len = 0;
>> +
>> +	if (gact->tcfg_ptype)
>> +		prob_len = nla_total_size(sizeof(struct tc_gact_p));
>> +#endif
>> +	return	nla_total_size(sizeof(struct tc_gact)) /* TCA_GACT_PARMS */
>> +#ifdef CONFIG_GACT_PROB
>> +		+ prob_len /* TCA_GACT_PROB */
>> +#endif
>> +		;
>> +}
>
> This is quite bad to the eye. What about:
>
> static size_t tcf_gact_get_fill_size(const struct tc_action *act)
> {
> 	size_t sz = nla_total_size(sizeof(struct tc_gact)); /* TCA_GACT_PARMS */
>
> #ifdef CONFIG_GACT_PROB
> 	if (to_gact(act)->tcfg_ptype)
> 		sz += nla_total_size(sizeof(struct tc_gact_p)); /* TCA_GACT_PROB */
> #endif
>
> 	return sz;
> }
>
> Should be the same, bug easier to read.

Agree, I will send v2.