Re: [PATCH net-next] net: sched: make tcf_action_dump_1() static

[shaozhengchao]

On 2022/8/15 19:53, Jamal Hadi Salim wrote:
> 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@xxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> >   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

Hi Jamal，
	Thank you for your reply. Maybe just declare function in the file 
without moving the line number?

Zhengchao Shao