mbox series

[net-next,v2,00/22] refactor the walk and lookup hook functions in tc_action_ops

Message ID 20220906121346.71578-1-shaozhengchao@huawei.com (mailing list archive)
Headers show
Series refactor the walk and lookup hook functions in tc_action_ops | expand

Message

shaozhengchao Sept. 6, 2022, 12:13 p.m. UTC 
The implementation logic of the walk/lookup hook function in each action
module is the same. Therefore, the two functions can be reconstructed.
When registering tc_action_ops of each action module, the corresponding
net_id is saved to tc_action_ops. In this way, the net_id of the
corresponding module can be directly obtained in act_api without executing
the specific walk and lookup hook functions. Then, generic functions can
be added to replace the walk and lookup hook functions of each action
module. Last, modify each action module in alphabetical order. 

Reserve the walk and lookup interfaces and delete them when they are no 
longer used.

This patchset has been tested by using TDC, and I will add selftest in
other patchset.

---
v1: save the net_id of each TC action module to the tc_action_ops structure
---

Zhengchao Shao (22):
  net: sched: act: move global static variable net_id to tc_action_ops
  net: sched: act_api: implement generic walker and search for tc action
  net: sched: act_bpf: get rid of tcf_bpf_walker and tcf_bpf_search
  net: sched: act_connmark: get rid of tcf_connmark_walker and
    tcf_connmark_search
  net: sched: act_csum: get rid of tcf_csum_walker and tcf_csum_search
  net: sched: act_ct: get rid of tcf_ct_walker and tcf_ct_search
  net: sched: act_ctinfo: get rid of tcf_ctinfo_walker and
    tcf_ctinfo_search
  net: sched: act_gact: get rid of tcf_gact_walker and tcf_gact_search
  net: sched: act_gate: get rid of tcf_gate_walker and tcf_gate_search
  net: sched: act_ife: get rid of tcf_ife_walker and tcf_ife_search
  net: sched: act_ipt: get rid of tcf_ipt_walker/tcf_xt_walker and
    tcf_ipt_search/tcf_xt_search
  net: sched: act_mirred: get rid of tcf_mirred_walker and
    tcf_mirred_search
  net: sched: act_mpls: get rid of tcf_mpls_walker and tcf_mpls_search
  net: sched: act_nat: get rid of tcf_nat_walker and tcf_nat_search
  net: sched: act_pedit: get rid of tcf_pedit_walker and
    tcf_pedit_search
  net: sched: act_police: get rid of tcf_police_walker and
    tcf_police_search
  net: sched: act_sample: get rid of tcf_sample_walker and
    tcf_sample_search
  net: sched: act_simple: get rid of tcf_simp_walker and tcf_simp_search
  net: sched: act_skbedit: get rid of tcf_skbedit_walker and
    tcf_skbedit_search
  net: sched: act_skbmod: get rid of tcf_skbmod_walker and
    tcf_skbmod_search
  net: sched: act_tunnel_key: get rid of tunnel_key_walker and
    tunnel_key_search
  net: sched: act_vlan: get rid of tcf_vlan_walker and tcf_vlan_search

 include/net/act_api.h      |  1 +
 net/sched/act_api.c        | 33 ++++++++++++++++++---
 net/sched/act_bpf.c        | 28 +++--------------
 net/sched/act_connmark.c   | 28 +++--------------
 net/sched/act_csum.c       | 28 +++--------------
 net/sched/act_ct.c         | 32 ++++----------------
 net/sched/act_ctinfo.c     | 28 +++--------------
 net/sched/act_gact.c       | 28 +++--------------
 net/sched/act_gate.c       | 28 +++--------------
 net/sched/act_ife.c        | 28 +++--------------
 net/sched/act_ipt.c        | 61 +++++++-------------------------------
 net/sched/act_mirred.c     | 28 +++--------------
 net/sched/act_mpls.c       | 28 +++--------------
 net/sched/act_nat.c        | 28 +++--------------
 net/sched/act_pedit.c      | 28 +++--------------
 net/sched/act_police.c     | 28 +++--------------
 net/sched/act_sample.c     | 28 +++--------------
 net/sched/act_simple.c     | 28 +++--------------
 net/sched/act_skbedit.c    | 28 +++--------------
 net/sched/act_skbmod.c     | 28 +++--------------
 net/sched/act_tunnel_key.c | 28 +++--------------
 net/sched/act_vlan.c       | 28 +++--------------
 22 files changed, 118 insertions(+), 513 deletions(-)

Comments

Jamal Hadi Salim Sept. 7, 2022, 3:03 p.m. UTC | #1 
On Tue, Sep 6, 2022 at 8:11 AM Zhengchao Shao <shaozhengchao@huawei.com> wrote:
>
> The implementation logic of the walk/lookup hook function in each action
> module is the same. Therefore, the two functions can be reconstructed.
> When registering tc_action_ops of each action module, the corresponding
> net_id is saved to tc_action_ops. In this way, the net_id of the
> corresponding module can be directly obtained in act_api without executing
> the specific walk and lookup hook functions. Then, generic functions can
> be added to replace the walk and lookup hook functions of each action
> module. Last, modify each action module in alphabetical order.
>
> Reserve the walk and lookup interfaces and delete them when they are no
> longer used.
>
> This patchset has been tested by using TDC, and I will add selftest in
> other patchset.
>

Thank you for running the tdc tests! Please for any future changes to tc try
to do the same. And you are going to be a hero if you add more tests!
This is a  better patchset and overall a nice cleanup. For the series:

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal
Cong Wang Sept. 7, 2022, 5:25 p.m. UTC | #2 
On Tue, Sep 06, 2022 at 08:13:24PM +0800, Zhengchao Shao wrote:
> The implementation logic of the walk/lookup hook function in each action
> module is the same. Therefore, the two functions can be reconstructed.
> When registering tc_action_ops of each action module, the corresponding
> net_id is saved to tc_action_ops. In this way, the net_id of the
> corresponding module can be directly obtained in act_api without executing
> the specific walk and lookup hook functions. Then, generic functions can
> be added to replace the walk and lookup hook functions of each action
> module. Last, modify each action module in alphabetical order. 
> 
> Reserve the walk and lookup interfaces and delete them when they are no 
> longer used.
> 
> This patchset has been tested by using TDC, and I will add selftest in
> other patchset.
> 
> ---
> v1: save the net_id of each TC action module to the tc_action_ops structure
> ---
> 
> Zhengchao Shao (22):
>   net: sched: act: move global static variable net_id to tc_action_ops
>   net: sched: act_api: implement generic walker and search for tc action
>   net: sched: act_bpf: get rid of tcf_bpf_walker and tcf_bpf_search
>   net: sched: act_connmark: get rid of tcf_connmark_walker and
>     tcf_connmark_search
>   net: sched: act_csum: get rid of tcf_csum_walker and tcf_csum_search
>   net: sched: act_ct: get rid of tcf_ct_walker and tcf_ct_search
>   net: sched: act_ctinfo: get rid of tcf_ctinfo_walker and
>     tcf_ctinfo_search
>   net: sched: act_gact: get rid of tcf_gact_walker and tcf_gact_search
>   net: sched: act_gate: get rid of tcf_gate_walker and tcf_gate_search
>   net: sched: act_ife: get rid of tcf_ife_walker and tcf_ife_search
>   net: sched: act_ipt: get rid of tcf_ipt_walker/tcf_xt_walker and
>     tcf_ipt_search/tcf_xt_search
>   net: sched: act_mirred: get rid of tcf_mirred_walker and
>     tcf_mirred_search
>   net: sched: act_mpls: get rid of tcf_mpls_walker and tcf_mpls_search
>   net: sched: act_nat: get rid of tcf_nat_walker and tcf_nat_search
>   net: sched: act_pedit: get rid of tcf_pedit_walker and
>     tcf_pedit_search
>   net: sched: act_police: get rid of tcf_police_walker and
>     tcf_police_search
>   net: sched: act_sample: get rid of tcf_sample_walker and
>     tcf_sample_search
>   net: sched: act_simple: get rid of tcf_simp_walker and tcf_simp_search
>   net: sched: act_skbedit: get rid of tcf_skbedit_walker and
>     tcf_skbedit_search
>   net: sched: act_skbmod: get rid of tcf_skbmod_walker and
>     tcf_skbmod_search
>   net: sched: act_tunnel_key: get rid of tunnel_key_walker and
>     tunnel_key_search
>   net: sched: act_vlan: get rid of tcf_vlan_walker and tcf_vlan_search
> 

I think it is easier to review if you can fold those removal patches
into one, pretty much like your 2nd patch. They are just cleanup's
following a same pattern.

Thanks.