Thread (103 messages) 103 messages, 10 authors, 2018-05-25

Re: [PATCH 14/14] net: sched: implement delete for all actions

From: Jiri Pirko <jiri@resnulli.us>
Date: 2018-05-16 09:49:12
Also in: lkml, netfilter-devel

Mon, May 14, 2018 at 04:27:15PM CEST, vladbu@mellanox.com wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
---
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 ++++++++
16 files changed, 136 insertions(+)
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 0bf4ecf..36f7f66 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -394,6 +394,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_find_delete(tn, index);
+}
+
static struct tc_action_ops act_bpf_ops __read_mostly = {
	.kind		=	"bpf",
	.type		=	TCA_ACT_BPF,
@@ -404,6 +411,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,
I wonder, right before this patch, how the idr index got removed?
delete op is NULL and I didn't find anyone else to do it.

Also, after this patch, does it make sense to have following check in
tcf_action_del_1()?

       if (ops->delete)
	       err = ops->delete(net, index);

Looks like ops->delete is non-null for all.

Seems to me that you need to introduce this patch filling up the delete
op in all acts and only after that introduce a code that actually calls
it.

[...]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help