Commit 7fd4b288 authored by Paolo Abeni's avatar Paolo Abeni Committed by David S. Miller
Browse files

tc/act: remove unneeded RCU lock in action callback 



Each lockless action currently does its own RCU locking in ->act().
This allows using plain RCU accessor, even if the context
is really RCU BH.

This change drops the per action RCU lock, replace the accessors
with the _bh variant, cleans up a bit the surrounding code and
documents the RCU status in the relevant header.
No functional nor performance change is intended.

The goal of this patch is clarifying that the RCU critical section
used by the tc actions extends up to the classifier's caller.

v1 -> v2:
 - preserve rcu lock in act_bpf: it's needed by eBPF helpers,
   as pointed out by Daniel

v3 -> v4:
 - fixed some typos in the commit message (JiriP)

Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
Acked-by: default avatarJiri Pirko <jiri@mellanox.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 802bfb19

include/net/act_api.h

+1 −1
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ struct tc_action_ops { 
	size_t	size;

	struct module		*owner;

	int     (*act)(struct sk_buff *, const struct tc_action *,

		       struct tcf_result *);

		       struct tcf_result *); /* called under RCU BH lock*/

	int     (*dump)(struct sk_buff *, struct tc_action *, int, int);

	void	(*cleanup)(struct tc_action *);

	int     (*lookup)(struct net *net, struct tc_action **a, u32 index,

include/net/sch_generic.h

+2 −0
Original line number Diff line number Diff line
@@ -285,6 +285,8 @@ struct tcf_proto { 
	/* Fast access part */

	struct tcf_proto __rcu	*next;

	void __rcu		*root;



	/* called under RCU BH lock*/

	int			(*classify)(struct sk_buff *,

					    const struct tcf_proto *,

					    struct tcf_result *);

net/sched/act_csum.c

+3 −9
Original line number Diff line number Diff line
@@ -561,15 +561,14 @@ static int tcf_csum(struct sk_buff *skb, const struct tc_action *a, 
	u32 update_flags;

	int action;



	rcu_read_lock();

	params = rcu_dereference(p->params);

	params = rcu_dereference_bh(p->params);



	tcf_lastuse_update(&p->tcf_tm);

	bstats_cpu_update(this_cpu_ptr(p->common.cpu_bstats), skb);



	action = READ_ONCE(p->tcf_action);

	if (unlikely(action == TC_ACT_SHOT))

		goto drop_stats;

		goto drop;



	update_flags = params->update_flags;

	switch (tc_skb_protocol(skb)) {
@@ -583,16 +582,11 @@ static int tcf_csum(struct sk_buff *skb, const struct tc_action *a, 
		break;

	}



unlock:

	rcu_read_unlock();

	return action;



drop:

	action = TC_ACT_SHOT;



drop_stats:

	qstats_drop_inc(this_cpu_ptr(p->common.cpu_qstats));

	goto unlock;

	return TC_ACT_SHOT;

}



static int tcf_csum_dump(struct sk_buff *skb, struct tc_action *a, int bind,

net/sched/act_ife.c

+1 −4
Original line number Diff line number Diff line
@@ -820,14 +820,11 @@ static int tcf_ife_act(struct sk_buff *skb, const struct tc_action *a, 
	struct tcf_ife_params *p;

	int ret;



	rcu_read_lock();

	p = rcu_dereference(ife->params);

	p = rcu_dereference_bh(ife->params);

	if (p->flags & IFE_ENCODE) {

		ret = tcf_ife_encode(skb, a, res, p);

		rcu_read_unlock();

		return ret;

	}

	rcu_read_unlock();



	return tcf_ife_decode(skb, a, res);

}

net/sched/act_mirred.c

+1 −3
Original line number Diff line number Diff line
@@ -181,11 +181,10 @@ static int tcf_mirred(struct sk_buff *skb, const struct tc_action *a, 
	tcf_lastuse_update(&m->tcf_tm);

	bstats_cpu_update(this_cpu_ptr(m->common.cpu_bstats), skb);



	rcu_read_lock();

	m_mac_header_xmit = READ_ONCE(m->tcfm_mac_header_xmit);

	m_eaction = READ_ONCE(m->tcfm_eaction);

	retval = READ_ONCE(m->tcf_action);

	dev = rcu_dereference(m->tcfm_dev);

	dev = rcu_dereference_bh(m->tcfm_dev);

	if (unlikely(!dev)) {

		pr_notice_once("tc mirred: target device is gone\n");

		goto out;
@@ -236,7 +235,6 @@ out: 
		if (tcf_mirred_is_act_redirect(m_eaction))

			retval = TC_ACT_SHOT;

	}

	rcu_read_unlock();



	return retval;

}

CRA Git | Maintained and supported by SUSTech CRA and CCSE