From mboxrd@z Thu Jan  1 00:00:00 1970
From: Pieter Jansen van Vuuren <pieter.jansenvanvuuren@netronome.com>
Subject: Re: [PATCH net-next v6 3/3] act_vlan: VLAN action rewrite to use
 RCU lock/unlock and update
Date: Tue, 7 Nov 2017 17:08:16 +0000
Message-ID: <20171107170816.5e2da39e@pieter-Netronome>
References: <1509724247-5721-1-git-send-email-manish.kurup@verizon.com>
        <20171106183731.3e1e88fc@pieter-Netronome>
        <CAKgT0Udx73GNT39aHrmsaXbqGhiMxejygAQxUg3FM5Rm=ZpfKg@mail.gmail.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Cc: Manish Kurup <kurup.manish@gmail.com>,
        Jamal Hadi Salim <jhs@mojatatu.com>,
        Cong Wang <xiyou.wangcong@gmail.com>,
        Jiri Pirko <jiri@resnulli.us>,
        David Miller <davem@davemloft.net>,
        Jakub Kicinski <jakub.kicinski@netronome.com>,
        Simon Horman <simon.horman@netronome.com>,
        john.hurley@netronome.com, oss-drivers@netronome.com,
        Netdev <netdev@vger.kernel.org>, aring@mojatatu.com,
        Roman Mashak <mrv@mojatatu.com>,
        Manish Kurup <manish.kurup@verizon.com>
To: Alexander Duyck <alexander.duyck@gmail.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mail-wm0-f67.google.com ([74.125.82.67]:48022 "EHLO
        mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751954AbdKGRIV (ORCPT
        <rfc822;netdev@vger.kernel.org>); Tue, 7 Nov 2017 12:08:21 -0500
Received: by mail-wm0-f67.google.com with SMTP id r196so5253187wmf.2
        for <netdev@vger.kernel.org>; Tue, 07 Nov 2017 09:08:21 -0800 (PST)
In-Reply-To: <CAKgT0Udx73GNT39aHrmsaXbqGhiMxejygAQxUg3FM5Rm=ZpfKg@mail.gmail.com>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Tue, 7 Nov 2017 08:54:20 -0800
Alexander Duyck <alexander.duyck@gmail.com> wrote:

> On Mon, Nov 6, 2017 at 10:37 AM, Pieter Jansen van Vuuren
> <pieter.jansenvanvuuren@netronome.com> wrote:
> > On Fri,  3 Nov 2017 11:50:47 -0400
> > Manish Kurup <kurup.manish@gmail.com> wrote:
> >  
> >> Using a spinlock in the VLAN action causes performance issues when
> >> the VLAN action is used on multiple cores. Rewrote the VLAN action to
> >> use RCU read locking for reads and updates instead.
> >>
> >> Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
> >> Acked-by: Jiri Pirko <jiri@mellanox.com>
> >> Signed-off-by: Manish Kurup <manish.kurup@verizon.com>
> >> ---
> >>  include/net/tc_act/tc_vlan.h | 46 +++++++++++++++++++++------
> >>  net/sched/act_vlan.c         | 75
> >> ++++++++++++++++++++++++++++++-------------- 2 files changed, 88
> >> insertions(+), 33 deletions(-)  
> > ...  
> >>
> >> +static void tcf_vlan_cleanup(struct tc_action *a, int bind)
> >> +{
> >> +     struct tcf_vlan *v = to_vlan(a);
> >> +     struct tcf_vlan_params *p;
> >> +
> >> +     p = rcu_dereference_protected(v->vlan_p, 1);
> >> +     kfree_rcu(p, rcu);
> >> +}
> >> +
> >>  static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
> >>                        int bind, int ref)
> >>  {
> >>       unsigned char *b = skb_tail_pointer(skb);
> >>       struct tcf_vlan *v = to_vlan(a);
> >> +     struct tcf_vlan_params *p = rtnl_dereference(v->vlan_p);  
> > nack. This fails reverse xmas-tree.  
> 
> Are we really going to be so strict about the reverse xmas-tree that
> we won't allow for assignment w/ variable declaration because the
> dependency order won't fit into that format?
Okay, I think that is a fair point. I would be okay by making an exception
for this.
> 
> Last I knew this kind of setup was an exception to the reverse
> xmas-tree layout requirement because in this case 'p' relies on 'v' so
> we can't reorder these without having to kick the assignment of 'p'
> off onto a line by itself.
I was actually not aware of this, thank you for pointing it out. It does
make sense.
> 
> - Alex