Commit 288864ef authored by Pedro Tammela's avatar Pedro Tammela Committed by Paolo Abeni
Browse files

net/sched: act_connmark: transition to percpu stats and rcu 



The tc action act_connmark was using shared stats and taking the per
action lock in the datapath. Improve it by using percpu stats and rcu.

perf before:
- 13.55% tcf_connmark_act
   - 81.18% _raw_spin_lock
       80.46% native_queued_spin_lock_slowpath

perf after:
- 2.85% tcf_connmark_act

tdc results:
1..15
ok 1 2002 - Add valid connmark action with defaults
ok 2 56a5 - Add valid connmark action with control pass
ok 3 7c66 - Add valid connmark action with control drop
ok 4 a913 - Add valid connmark action with control pipe
ok 5 bdd8 - Add valid connmark action with control reclassify
ok 6 b8be - Add valid connmark action with control continue
ok 7 d8a6 - Add valid connmark action with control jump
ok 8 aae8 - Add valid connmark action with zone argument
ok 9 2f0b - Add valid connmark action with invalid zone argument
ok 10 9305 - Add connmark action with unsupported argument
ok 11 71ca - Add valid connmark action and replace it
ok 12 5f8f - Add valid connmark action with cookie
ok 13 c506 - Replace connmark with invalid goto chain control
ok 14 6571 - Delete connmark action with valid index
ok 15 3426 - Delete connmark action with invalid index

Reviewed-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: default avatarPedro Tammela <pctammela@mojatatu.com>
Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
parent 7d12057b

include/net/tc_act/tc_connmark.h

+7 −2
Original line number Diff line number Diff line
@@ -4,10 +4,15 @@ 


#include <net/act_api.h>



struct tcf_connmark_info {

	struct tc_action common;

struct tcf_connmark_parms {

	struct net *net;

	u16 zone;

	struct rcu_head rcu;

};



struct tcf_connmark_info {

	struct tc_action common;

	struct tcf_connmark_parms __rcu *parms;

};



#define to_connmark(a) ((struct tcf_connmark_info *)a)

net/sched/act_connmark.c

+68 −39
Original line number Diff line number Diff line
@@ -36,13 +36,15 @@ TC_INDIRECT_SCOPE int tcf_connmark_act(struct sk_buff *skb, 
	struct nf_conntrack_tuple tuple;

	enum ip_conntrack_info ctinfo;

	struct tcf_connmark_info *ca = to_connmark(a);

	struct tcf_connmark_parms *parms;

	struct nf_conntrack_zone zone;

	struct nf_conn *c;

	int proto;



	spin_lock(&ca->tcf_lock);

	tcf_lastuse_update(&ca->tcf_tm);

	bstats_update(&ca->tcf_bstats, skb);

	tcf_action_update_bstats(&ca->common, skb);



	parms = rcu_dereference_bh(ca->parms);



	switch (skb_protocol(skb, true)) {

	case htons(ETH_P_IP):
@@ -64,31 +66,29 @@ TC_INDIRECT_SCOPE int tcf_connmark_act(struct sk_buff *skb, 
	c = nf_ct_get(skb, &ctinfo);

	if (c) {

		skb->mark = READ_ONCE(c->mark);

		/* using overlimits stats to count how many packets marked */

		ca->tcf_qstats.overlimits++;

		goto out;

		goto count;

	}



	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),

			       proto, ca->net, &tuple))

	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), proto, parms->net,

			       &tuple))

		goto out;



	zone.id = ca->zone;

	zone.id = parms->zone;

	zone.dir = NF_CT_DEFAULT_ZONE_DIR;



	thash = nf_conntrack_find_get(ca->net, &zone, &tuple);

	thash = nf_conntrack_find_get(parms->net, &zone, &tuple);

	if (!thash)

		goto out;



	c = nf_ct_tuplehash_to_ctrack(thash);

	/* using overlimits stats to count how many packets marked */

	ca->tcf_qstats.overlimits++;

	skb->mark = READ_ONCE(c->mark);

	nf_ct_put(c);



count:

	/* using overlimits stats to count how many packets marked */

	tcf_action_inc_overlimit_qstats(&ca->common);

out:

	spin_unlock(&ca->tcf_lock);

	return ca->tcf_action;

	return READ_ONCE(ca->tcf_action);

}



static const struct nla_policy connmark_policy[TCA_CONNMARK_MAX + 1] = {
@@ -101,6 +101,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla, 
			     struct netlink_ext_ack *extack)

{

	struct tc_action_net *tn = net_generic(net, act_connmark_ops.net_id);

	struct tcf_connmark_parms *nparms, *oparms;

	struct nlattr *tb[TCA_CONNMARK_MAX + 1];

	bool bind = flags & TCA_ACT_FLAGS_BIND;

	struct tcf_chain *goto_ch = NULL;
@@ -120,52 +121,66 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla, 
	if (!tb[TCA_CONNMARK_PARMS])

		return -EINVAL;



	nparms = kzalloc(sizeof(*nparms), GFP_KERNEL);

	if (!nparms)

		return -ENOMEM;



	parm = nla_data(tb[TCA_CONNMARK_PARMS]);

	index = parm->index;

	ret = tcf_idr_check_alloc(tn, &index, a, bind);

	if (!ret) {

		ret = tcf_idr_create(tn, index, est, a,

				     &act_connmark_ops, bind, false, flags);

		ret = tcf_idr_create_from_flags(tn, index, est, a,

						&act_connmark_ops, bind, flags);

		if (ret) {

			tcf_idr_cleanup(tn, index);

			return ret;

			err = ret;

			goto out_free;

		}



		ci = to_connmark(*a);

		err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch,

					       extack);

		if (err < 0)

			goto release_idr;

		tcf_action_set_ctrlact(*a, parm->action, goto_ch);

		ci->net = net;

		ci->zone = parm->zone;



		nparms->net = net;

		nparms->zone = parm->zone;



		ret = ACT_P_CREATED;

	} else if (ret > 0) {

		ci = to_connmark(*a);

		if (bind)

			return 0;

		if (bind) {

			err = 0;

			goto out_free;

		}

		if (!(flags & TCA_ACT_FLAGS_REPLACE)) {

			tcf_idr_release(*a, bind);

			return -EEXIST;

			err = -EEXIST;

			goto release_idr;

		}



		nparms->net = rtnl_dereference(ci->parms)->net;

		nparms->zone = parm->zone;



		ret = 0;

	}

		err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch,

					       extack);



	err = tcf_action_check_ctrlact(parm->action, tp, &goto_ch, extack);

	if (err < 0)

		goto release_idr;

		/* replacing action and zone */



	spin_lock_bh(&ci->tcf_lock);

	goto_ch = tcf_action_set_ctrlact(*a, parm->action, goto_ch);

		ci->zone = parm->zone;

	oparms = rcu_replace_pointer(ci->parms, nparms, lockdep_is_held(&ci->tcf_lock));

	spin_unlock_bh(&ci->tcf_lock);



	if (goto_ch)

		tcf_chain_put_by_act(goto_ch);

		ret = 0;

	}



	if (oparms)

		kfree_rcu(oparms, rcu);



	return ret;



release_idr:

	tcf_idr_release(*a, bind);

out_free:

	kfree(nparms);

	return err;

}
@@ -179,11 +194,14 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a, 
		.refcnt  = refcount_read(&ci->tcf_refcnt) - ref,

		.bindcnt = atomic_read(&ci->tcf_bindcnt) - bind,

	};

	struct tcf_connmark_parms *parms;

	struct tcf_t t;



	spin_lock_bh(&ci->tcf_lock);

	parms = rcu_dereference_protected(ci->parms, lockdep_is_held(&ci->tcf_lock));



	opt.action = ci->tcf_action;

	opt.zone = ci->zone;

	opt.zone = parms->zone;

	if (nla_put(skb, TCA_CONNMARK_PARMS, sizeof(opt), &opt))

		goto nla_put_failure;
@@ -201,6 +219,16 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a, 
	return -1;

}



static void tcf_connmark_cleanup(struct tc_action *a)

{

	struct tcf_connmark_info *ci = to_connmark(a);

	struct tcf_connmark_parms *parms;



	parms = rcu_dereference_protected(ci->parms, 1);

	if (parms)

		kfree_rcu(parms, rcu);

}



static struct tc_action_ops act_connmark_ops = {

	.kind		=	"connmark",

	.id		=	TCA_ID_CONNMARK,
@@ -208,6 +236,7 @@ static struct tc_action_ops act_connmark_ops = { 
	.act		=	tcf_connmark_act,

	.dump		=	tcf_connmark_dump,

	.init		=	tcf_connmark_init,

	.cleanup	=	tcf_connmark_cleanup,

	.size		=	sizeof(struct tcf_connmark_info),

};