Wed, Oct 11, 2017 at 02:35:50PM CEST, manish.ku...@verizon.com wrote: >Hi Jamal, Yes, that's an indentation mismatch - I put in one tab too many. >Shall fix before commit.
No top-posting please! > >Thanks, > >-Manish > >-----Original Message----- >From: Jamal Hadi Salim [mailto:j...@mojatatu.com] >Sent: Wednesday, October 11, 2017 8:28 AM >To: Manish Kurup; xiyou.wangc...@gmail.com; j...@resnulli.us; >da...@davemloft.net; netdev@vger.kernel.org >Cc: ar...@mojatatu.com; m...@mojatatu.com; Kurup, Manish B >Subject: [E] Re: [PATCH net-next 1/2] net sched act_vlan: Change stats update >to use per-core stats > >minus lk > >On 17-10-10 10:32 PM, Manish Kurup wrote: >> The VLAN action maintains one set of stats across all cores, and uses >> a spinlock to synchronize updates to it from the same. Changed this to >> use a per-CPU stats context instead. >> This change will result in better performance. >> >> Signed-off-by: Manish Kurup <manish.ku...@verizon.com> >> --- >> net/sched/act_vlan.c | 10 ++++++---- >> 1 file changed, 6 insertions(+), 4 deletions(-) >> >> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c index >> 16eb067..14c262c 100644 >> --- a/net/sched/act_vlan.c >> +++ b/net/sched/act_vlan.c >> @@ -30,9 +30,10 @@ static int tcf_vlan(struct sk_buff *skb, const struct >> tc_action *a, >> int err; >> u16 tci; >> >> - spin_lock(&v->tcf_lock); >> tcf_lastuse_update(&v->tcf_tm); >> - bstats_update(&v->tcf_bstats, skb); >> + bstats_cpu_update(this_cpu_ptr(v->common.cpu_bstats), skb); >> + >> + spin_lock(&v->tcf_lock); >> action = v->tcf_action; >> >> /* Ensure 'data' points at mac_header prior calling vlan >> manipulating @@ -85,7 +86,8 @@ static int tcf_vlan(struct sk_buff >> *skb, const struct tc_action *a, >> >> drop: >> action = TC_ACT_SHOT; >> - v->tcf_qstats.drops++; >> + qstats_drop_inc(this_cpu_ptr(v->common.cpu_qstats)); >> + >> unlock: >> if (skb_at_tc_ingress(skb)) >> skb_pull_rcsum(skb, skb->mac_len); @@ -172,7 +174,7 @@ static >> int >> tcf_vlan_init(struct net *net, struct nlattr *nla, >> >> if (!exists) { >> ret = tcf_idr_create(tn, parm->index, est, a, >> - &act_vlan_ops, bind, false); >> + &act_vlan_ops, bind, true); >> > >Indentation mismatch here? > >Otherwise looks good to me. > >Acked-by: Jamal Hadi Salim <j...@mojatatu.com> > >cheers, >jamal
