Re: [PATCH v3 06/11] net: sched: add 'delete' function to action ops

Marcelo Ricardo Leitner
2018-05-28T21:38:31Z

On Mon, May 28, 2018 at 12:17:24AM +0300, Vlad Buslov wrote:
> Extend action ops with 'delete' function. Each action type to implements
> its own delete function that doesn't depend on rtnl lock.
>
> Implement delete function that is required to delete actions without
> holding rtnl lock. Use action API function that atomically deletes action
> only if it is still in action idr. This implementation prevents concurrent
> threads from deleting same action twice.
>
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
> Changes from V1 to V2:
> - Merge action ops delete definition and implementation.
>
> include/net/act_api.h | 1 +
> net/sched/act_bpf.c | 8 ++++++++
> net/sched/act_connmark.c | 8 ++++++++
> net/sched/act_csum.c | 8 ++++++++
> net/sched/act_gact.c | 8 ++++++++
> net/sched/act_ife.c | 8 ++++++++
> net/sched/act_ipt.c | 16 ++++++++++++++++
> net/sched/act_mirred.c | 8 ++++++++
> net/sched/act_nat.c | 8 ++++++++
> net/sched/act_pedit.c | 8 ++++++++
> net/sched/act_police.c | 8 ++++++++
> net/sched/act_sample.c | 8 ++++++++
> net/sched/act_simple.c | 8 ++++++++
> net/sched/act_skbedit.c | 8 ++++++++
> net/sched/act_skbmod.c | 8 ++++++++
> net/sched/act_tunnel_key.c | 8 ++++++++
> net/sched/act_vlan.c | 8 ++++++++
> 17 files changed, 137 insertions(+)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index d94ec6400673..d256e20507b9 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -101,6 +101,7 @@ struct tc_action_ops {
> void (*stats_update)(struct tc_action *, u64, u32, u64);
> size_t (*get_fill_size)(const struct tc_action *act);
> struct net_device *(*get_dev)(const struct tc_action *a);
> + int (*delete)(struct net *net, u32 index);
> };
>
> struct tc_action_net {
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index 8ebf40a3506c..7941dd66ff83 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -388,6 +388,13 @@ static int tcf_bpf_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_bpf_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, bpf_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_bpf_ops __read_mostly = {
> .kind = "bpf",
> .type = TCA_ACT_BPF,
> @@ -398,6 +405,7 @@ static struct tc_action_ops act_bpf_ops __read_mostly = {
> .init = tcf_bpf_init,
> .walk = tcf_bpf_walker,
> .lookup = tcf_bpf_search,
> + .delete = tcf_bpf_delete,
> .size = sizeof(struct tcf_bpf),
> };
>
> diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
> index e3787aa0025a..143c2d3de723 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -193,6 +193,13 @@ static int tcf_connmark_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_connmark_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, connmark_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_connmark_ops = {
> .kind = "connmark",
> .type = TCA_ACT_CONNMARK,
> @@ -202,6 +209,7 @@ static struct tc_action_ops act_connmark_ops = {
> .init = tcf_connmark_init,
> .walk = tcf_connmark_walker,
> .lookup = tcf_connmark_search,
> + .delete = tcf_connmark_delete,
> .size = sizeof(struct tcf_connmark_info),
> };
>
> diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
> index 334261943f9f..3768539340e0 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -654,6 +654,13 @@ static size_t tcf_csum_get_fill_size(const struct tc_action *act)
> return nla_total_size(sizeof(struct tc_csum));
> }
>
> +static int tcf_csum_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, csum_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_csum_ops = {
> .kind = "csum",
> .type = TCA_ACT_CSUM,
> @@ -665,6 +672,7 @@ static struct tc_action_ops act_csum_ops = {
> .walk = tcf_csum_walker,
> .lookup = tcf_csum_search,
> .get_fill_size = tcf_csum_get_fill_size,
> + .delete = tcf_csum_delete,
> .size = sizeof(struct tcf_csum),
> };
>
> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
> index b4dfb2b4addc..a431a711f0dd 100644
> --- a/net/sched/act_gact.c
> +++ b/net/sched/act_gact.c
> @@ -231,6 +231,13 @@ static size_t tcf_gact_get_fill_size(const struct tc_action *act)
> return sz;
> }
>
> +static int tcf_gact_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, gact_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_gact_ops = {
> .kind = "gact",
> .type = TCA_ACT_GACT,
> @@ -242,6 +249,7 @@ static struct tc_action_ops act_gact_ops = {
> .walk = tcf_gact_walker,
> .lookup = tcf_gact_search,
> .get_fill_size = tcf_gact_get_fill_size,
> + .delete = tcf_gact_delete,
> .size = sizeof(struct tcf_gact),
> };
>
> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> index 3dccc4e1d378..027c305dcb37 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
> @@ -846,6 +846,13 @@ static int tcf_ife_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_ife_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, ife_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_ife_ops = {
> .kind = "ife",
> .type = TCA_ACT_IFE,
> @@ -856,6 +863,7 @@ static struct tc_action_ops act_ife_ops = {
> .init = tcf_ife_init,
> .walk = tcf_ife_walker,
> .lookup = tcf_ife_search,
> + .delete = tcf_ife_delete,
> .size = sizeof(struct tcf_ife_info),
> };
>
> diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
> index 9c21663a86a6..6c234411c771 100644
> --- a/net/sched/act_ipt.c
> +++ b/net/sched/act_ipt.c
> @@ -324,6 +324,13 @@ static int tcf_ipt_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_ipt_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, ipt_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_ipt_ops = {
> .kind = "ipt",
> .type = TCA_ACT_IPT,
> @@ -334,6 +341,7 @@ static struct tc_action_ops act_ipt_ops = {
> .init = tcf_ipt_init,
> .walk = tcf_ipt_walker,
> .lookup = tcf_ipt_search,
> + .delete = tcf_ipt_delete,
> .size = sizeof(struct tcf_ipt),
> };
>
> @@ -374,6 +382,13 @@ static int tcf_xt_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_xt_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, xt_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_xt_ops = {
> .kind = "xt",
> .type = TCA_ACT_XT,
> @@ -384,6 +399,7 @@ static struct tc_action_ops act_xt_ops = {
> .init = tcf_xt_init,
> .walk = tcf_xt_walker,
> .lookup = tcf_xt_search,
> + .delete = tcf_xt_delete,
> .size = sizeof(struct tcf_ipt),
> };
>
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 5434f08f2eb7..3d8300bce7e4 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -322,6 +322,13 @@ static struct net_device *tcf_mirred_get_dev(const struct tc_action *a)
> return rtnl_dereference(m->tcfm_dev);
> }
>
> +static int tcf_mirred_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, mirred_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_mirred_ops = {
> .kind = "mirred",
> .type = TCA_ACT_MIRRED,
> @@ -335,6 +342,7 @@ static struct tc_action_ops act_mirred_ops = {
> .lookup = tcf_mirred_search,
> .size = sizeof(struct tcf_mirred),
> .get_dev = tcf_mirred_get_dev,
> + .delete = tcf_mirred_delete,
> };
>
> static __net_init int mirred_init_net(struct net *net)
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index e6487ad1e4a8..9eb27c89dc46 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -294,6 +294,13 @@ static int tcf_nat_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_nat_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, nat_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_nat_ops = {
> .kind = "nat",
> .type = TCA_ACT_NAT,
> @@ -303,6 +310,7 @@ static struct tc_action_ops act_nat_ops = {
> .init = tcf_nat_init,
> .walk = tcf_nat_walker,
> .lookup = tcf_nat_search,
> + .delete = tcf_nat_delete,
> .size = sizeof(struct tcf_nat),
> };
>
> diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> index 7c9a3f24edba..b8857035e3f8 100644
> --- a/net/sched/act_pedit.c
> +++ b/net/sched/act_pedit.c
> @@ -436,6 +436,13 @@ static int tcf_pedit_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_pedit_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, pedit_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_pedit_ops = {
> .kind = "pedit",
> .type = TCA_ACT_PEDIT,
> @@ -446,6 +453,7 @@ static struct tc_action_ops act_pedit_ops = {
> .init = tcf_pedit_init,
> .walk = tcf_pedit_walker,
> .lookup = tcf_pedit_search,
> + .delete = tcf_pedit_delete,
> .size = sizeof(struct tcf_pedit),
> };
>
> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index 0e1c2fb0ebea..c955fb0d4f3f 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -314,6 +314,13 @@ static int tcf_police_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_police_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, police_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> MODULE_AUTHOR("Alexey Kuznetsov");
> MODULE_DESCRIPTION("Policing actions");
> MODULE_LICENSE("GPL");
> @@ -327,6 +334,7 @@ static struct tc_action_ops act_police_ops = {
> .init = tcf_act_police_init,
> .walk = tcf_act_police_walker,
> .lookup = tcf_police_search,
> + .delete = tcf_police_delete,
> .size = sizeof(struct tcf_police),
> };
>
> diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
> index 316fc645595d..6f79d2afcba2 100644
> --- a/net/sched/act_sample.c
> +++ b/net/sched/act_sample.c
> @@ -220,6 +220,13 @@ static int tcf_sample_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_sample_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, sample_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_sample_ops = {
> .kind = "sample",
> .type = TCA_ACT_SAMPLE,
> @@ -230,6 +237,7 @@ static struct tc_action_ops act_sample_ops = {
> .cleanup = tcf_sample_cleanup,
> .walk = tcf_sample_walker,
> .lookup = tcf_sample_search,
> + .delete = tcf_sample_delete,
> .size = sizeof(struct tcf_sample),
> };
>
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index 23fa893ea092..b570e7ca7e33 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -187,6 +187,13 @@ static int tcf_simp_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_simp_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, simp_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_simp_ops = {
> .kind = "simple",
> .type = TCA_ACT_SIMP,
> @@ -197,6 +204,7 @@ static struct tc_action_ops act_simp_ops = {
> .init = tcf_simp_init,
> .walk = tcf_simp_walker,
> .lookup = tcf_simp_search,
> + .delete = tcf_simp_delete,
> .size = sizeof(struct tcf_defact),
> };
>
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index 85ed9d603dc1..dc0cb350aa45 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -226,6 +226,13 @@ static int tcf_skbedit_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_skbedit_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, skbedit_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_skbedit_ops = {
> .kind = "skbedit",
> .type = TCA_ACT_SKBEDIT,
> @@ -235,6 +242,7 @@ static struct tc_action_ops act_skbedit_ops = {
> .init = tcf_skbedit_init,
> .walk = tcf_skbedit_walker,
> .lookup = tcf_skbedit_search,
> + .delete = tcf_skbedit_delete,
> .size = sizeof(struct tcf_skbedit),
> };
>
> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
> index 026d6f58eda1..30be3f767495 100644
> --- a/net/sched/act_skbmod.c
> +++ b/net/sched/act_skbmod.c
> @@ -253,6 +253,13 @@ static int tcf_skbmod_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_skbmod_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, skbmod_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_skbmod_ops = {
> .kind = "skbmod",
> .type = TCA_ACT_SKBMOD,
> @@ -263,6 +270,7 @@ static struct tc_action_ops act_skbmod_ops = {
> .cleanup = tcf_skbmod_cleanup,
> .walk = tcf_skbmod_walker,
> .lookup = tcf_skbmod_search,
> + .delete = tcf_skbmod_delete,
> .size = sizeof(struct tcf_skbmod),
> };
>
> diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
> index ed698fcb0e5a..4b7f9a3b47d7 100644
> --- a/net/sched/act_tunnel_key.c
> +++ b/net/sched/act_tunnel_key.c
> @@ -310,6 +310,13 @@ static int tunnel_key_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tunnel_key_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_tunnel_key_ops = {
> .kind = "tunnel_key",
> .type = TCA_ACT_TUNNEL_KEY,
> @@ -320,6 +327,7 @@ static struct tc_action_ops act_tunnel_key_ops = {
> .cleanup = tunnel_key_release,
> .walk = tunnel_key_walker,
> .lookup = tunnel_key_search,
> + .delete = tunnel_key_delete,
> .size = sizeof(struct tcf_tunnel_key),
> };
>
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 72d2d78fb942..b44377c951b6 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -285,6 +285,13 @@ static int tcf_vlan_search(struct net *net, struct tc_action **a, u32 index,
> return tcf_idr_search(tn, a, index);
> }
>
> +static int tcf_vlan_delete(struct net *net, u32 index)
> +{
> + struct tc_action_net *tn = net_generic(net, vlan_net_id);
> +
> + return tcf_idr_delete_index(tn, index);
> +}
> +
> static struct tc_action_ops act_vlan_ops = {
> .kind = "vlan",
> .type = TCA_ACT_VLAN,
> @@ -295,6 +302,7 @@ static struct tc_action_ops act_vlan_ops = {
> .cleanup = tcf_vlan_cleanup,
> .walk = tcf_vlan_walker,
> .lookup = tcf_vlan_search,
> + .delete = tcf_vlan_delete,
> .size = sizeof(struct tcf_vlan),
> };
>
> --
> 2.7.5
>