From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jiri Pirko Subject: Re: [Patch net-next] act_mirred: use tcfm_dev in tcf_mirred_get_dev() Date: Sat, 2 Dec 2017 09:57:39 +0100 Message-ID: <20171202085739.GE1821@nanopsycho> References: <20171130225335.6957-1-xiyou.wangcong@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: netdev@vger.kernel.org, Jiri Pirko , Jamal Hadi Salim To: Cong Wang Return-path: Received: from mail-wm0-f68.google.com ([74.125.82.68]:44000 "EHLO mail-wm0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751667AbdLBI5n (ORCPT ); Sat, 2 Dec 2017 03:57:43 -0500 Received: by mail-wm0-f68.google.com with SMTP id n138so7181093wmg.2 for ; Sat, 02 Dec 2017 00:57:42 -0800 (PST) Content-Disposition: inline In-Reply-To: <20171130225335.6957-1-xiyou.wangcong@gmail.com> Sender: netdev-owner@vger.kernel.org List-ID: Thu, Nov 30, 2017 at 11:53:32PM CET, xiyou.wangcong@gmail.com wrote: >tcfm_dev always points to the correct netdev and we already >hold a refcnt, so no need to use ifindex to lookup again. > >If we would support moving target netdev across netns, using >pointer would be better than ifindex. > >Cc: Jiri Pirko >Cc: Jamal Hadi Salim >Signed-off-by: Cong Wang >--- > include/net/tc_act/tc_mirred.h | 1 - > net/sched/act_mirred.c | 3 +-- > 2 files changed, 1 insertion(+), 3 deletions(-) > >diff --git a/include/net/tc_act/tc_mirred.h b/include/net/tc_act/tc_mirred.h >index 21d253c9a8c6..b2dbbfaefd22 100644 >--- a/include/net/tc_act/tc_mirred.h >+++ b/include/net/tc_act/tc_mirred.h >@@ -11,7 +11,6 @@ struct tcf_mirred { > int tcfm_ifindex; > bool tcfm_mac_header_xmit; > struct net_device __rcu *tcfm_dev; >- struct net *net; > struct list_head tcfm_list; > }; > #define to_mirred(a) ((struct tcf_mirred *)a) >diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c >index 8b3e59388480..fe6489f9c3cf 100644 >--- a/net/sched/act_mirred.c >+++ b/net/sched/act_mirred.c >@@ -140,7 +140,6 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla, > m->tcfm_eaction = parm->eaction; > if (dev != NULL) { > m->tcfm_ifindex = parm->ifindex; >- m->net = net; > if (ret != ACT_P_CREATED) > dev_put(rcu_dereference_protected(m->tcfm_dev, 1)); > dev_hold(dev); >@@ -318,7 +317,7 @@ static struct net_device *tcf_mirred_get_dev(const struct tc_action *a) > { > struct tcf_mirred *m = to_mirred(a); > >- return __dev_get_by_index(m->net, m->tcfm_ifindex); >+ return rtnl_dereference(m->tcfm_dev); Good. Please also use m->tcfm_dev->ifindex in tcf_mirred_dump and tcf_mirred_ifindex. Then you can remove tcfm_ifindex completely. > } > > static struct tc_action_ops act_mirred_ops = { >-- >2.13.0 >