Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220815070122.113871-1-shaozhengchao@huawei.com>
In-Reply-To: <20220815070122.113871-1-shaozhengchao@huawei.com>
From:   Jamal Hadi Salim <jhs@mojatatu.com>
Date:   Mon, 15 Aug 2022 07:53:21 -0400
Message-ID: <CAM0EoMnc3Mc1+SNbKeRf0ecJ4g66=1xqFJ4X=Gb=s125TPucPQ@mail.gmail.com>
Subject: Re: [PATCH net-next] net: sched: make tcf_action_dump_1() static
To:     Zhengchao Shao <shaozhengchao@huawei.com>
Cc:     netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        pabeni@redhat.com, xiyou.wangcong@gmail.com, jiri@resnulli.us,
        weiyongjun1@huawei.com, yuehaibing@huawei.com
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

You shouldnt have so many line changes to remove the EXPORT and change
"int" to "static int".
What am i missing?
Unnecessary line changes add extra effort to git archeology

cheers,
jamal

On Mon, Aug 15, 2022 at 2:58 AM Zhengchao Shao <shaozhengchao@huawei.com> wrote:
>
> Function tcf_action_dump_1() is not used outside of act_api.c, so remove
> the superfluous EXPORT_SYMBOL() and marks it static.
>
> Signed-off-by: Zhengchao Shao <shaozhengchao@huawei.com>
> ---
>  include/net/act_api.h |   1 -
>  net/sched/act_api.c   | 100 +++++++++++++++++++++---------------------
>  2 files changed, 49 insertions(+), 52 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index 9cf6870b526e..d51b3f931771 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -215,7 +215,6 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
>  int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[], int bind,
>                     int ref, bool terse);
>  int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
> -int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
>
>  static inline void tcf_action_update_bstats(struct tc_action *a,
>                                             struct sk_buff *skb)
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index b69fcde546ba..9fd98bf5c724 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -510,6 +510,55 @@ tcf_action_dump_terse(struct sk_buff *skb, struct tc_action *a, bool from_act)
>         return -1;
>  }
>
> +int
> +tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
> +{
> +       return a->ops->dump(skb, a, bind, ref);
> +}
> +
> +static int
> +tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
> +{
> +       int err = -EINVAL;
> +       unsigned char *b = skb_tail_pointer(skb);
> +       struct nlattr *nest;
> +       u32 flags;
> +
> +       if (tcf_action_dump_terse(skb, a, false))
> +               goto nla_put_failure;
> +
> +       if (a->hw_stats != TCA_ACT_HW_STATS_ANY &&
> +           nla_put_bitfield32(skb, TCA_ACT_HW_STATS,
> +                              a->hw_stats, TCA_ACT_HW_STATS_ANY))
> +               goto nla_put_failure;
> +
> +       if (a->used_hw_stats_valid &&
> +           nla_put_bitfield32(skb, TCA_ACT_USED_HW_STATS,
> +                              a->used_hw_stats, TCA_ACT_HW_STATS_ANY))
> +               goto nla_put_failure;
> +
> +       flags = a->tcfa_flags & TCA_ACT_FLAGS_USER_MASK;
> +       if (flags &&
> +           nla_put_bitfield32(skb, TCA_ACT_FLAGS,
> +                              flags, flags))
> +               goto nla_put_failure;
> +
> +       if (nla_put_u32(skb, TCA_ACT_IN_HW_COUNT, a->in_hw_count))
> +               goto nla_put_failure;
> +
> +       nest = nla_nest_start_noflag(skb, TCA_OPTIONS);
> +       if (!nest)
> +               goto nla_put_failure;
> +       err = tcf_action_dump_old(skb, a, bind, ref);
> +       if (err > 0) {
> +               nla_nest_end(skb, nest);
> +               return err;
> +       }
> +
> +nla_put_failure:
> +       nlmsg_trim(skb, b);
> +       return -1;
> +}
>  static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
>                            struct netlink_callback *cb)
>  {
> @@ -1132,57 +1181,6 @@ static void tcf_action_put_many(struct tc_action *actions[])
>         }
>  }
>
> -int
> -tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
> -{
> -       return a->ops->dump(skb, a, bind, ref);
> -}
> -
> -int
> -tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
> -{
> -       int err = -EINVAL;
> -       unsigned char *b = skb_tail_pointer(skb);
> -       struct nlattr *nest;
> -       u32 flags;
> -
> -       if (tcf_action_dump_terse(skb, a, false))
> -               goto nla_put_failure;
> -
> -       if (a->hw_stats != TCA_ACT_HW_STATS_ANY &&
> -           nla_put_bitfield32(skb, TCA_ACT_HW_STATS,
> -                              a->hw_stats, TCA_ACT_HW_STATS_ANY))
> -               goto nla_put_failure;
> -
> -       if (a->used_hw_stats_valid &&
> -           nla_put_bitfield32(skb, TCA_ACT_USED_HW_STATS,
> -                              a->used_hw_stats, TCA_ACT_HW_STATS_ANY))
> -               goto nla_put_failure;
> -
> -       flags = a->tcfa_flags & TCA_ACT_FLAGS_USER_MASK;
> -       if (flags &&
> -           nla_put_bitfield32(skb, TCA_ACT_FLAGS,
> -                              flags, flags))
> -               goto nla_put_failure;
> -
> -       if (nla_put_u32(skb, TCA_ACT_IN_HW_COUNT, a->in_hw_count))
> -               goto nla_put_failure;
> -
> -       nest = nla_nest_start_noflag(skb, TCA_OPTIONS);
> -       if (nest == NULL)
> -               goto nla_put_failure;
> -       err = tcf_action_dump_old(skb, a, bind, ref);
> -       if (err > 0) {
> -               nla_nest_end(skb, nest);
> -               return err;
> -       }
> -
> -nla_put_failure:
> -       nlmsg_trim(skb, b);
> -       return -1;
> -}
> -EXPORT_SYMBOL(tcf_action_dump_1);
> -
>  int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[],
>                     int bind, int ref, bool terse)
>  {
> --
> 2.17.1
>