netdev - Re: CONFIG_NET_TC_SKB

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190920091647.0129e65f@cakuba.netronome.com>
Date:   Fri, 20 Sep 2019 09:16:47 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     Vlad Buslov <vladbu@...lanox.com>
Cc:     David Miller <davem@...emloft.net>,
        Paul Blakey <paulb@...lanox.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Jiri Pirko <jiri@...nulli.us>,
        Cong Wang <xiyou.wangcong@...il.com>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Pravin Shelar <pshelar@....org>,
        Simon Horman <simon.horman@...ronome.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        Or Gerlitz <gerlitz.or@...il.com>
Subject: Re: CONFIG_NET_TC_SKB_EXT

On Thu, 19 Sep 2019 15:13:55 +0000, Vlad Buslov wrote:
> On Thu 19 Sep 2019 at 14:21, David Miller <davem@...emloft.net> wrote:
> > As Linus pointed out, the Kconfig logic for CONFIG_NET_TC_SKB_EXT
> > is really not acceptable.
> >
> > It should not be enabled by default at all.
> >
> > Instead the actual users should turn it on or depend upon it, which in
> > this case seems to be OVS.
> >
> > Please fix this, thank you.  
> 
> Hi David,
> 
> We are working on it, but Paul is OoO today. Is it okay if we send the
> fix early next week?

Doesn't really seem like we have too many ways forward here, right?

How about this?

------>8-----------------------------------

net: hide NET_TC_SKB_EXT as a config option

Linus points out the NET_TC_SKB_EXT config option looks suspicious.
Indeed, it should really be selected to ensure correct OvS operation
if TC offload is used. Hopefully those who care about TC-only and
OvS-only performance disable the other one at compilation time.

Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
---
 net/openvswitch/Kconfig |  1 +
 net/sched/Kconfig       | 13 +++----------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig
index 22d7d5604b4c..bd407ea7c263 100644
--- a/net/openvswitch/Kconfig
+++ b/net/openvswitch/Kconfig
@@ -15,6 +15,7 @@ config OPENVSWITCH
 	select NET_MPLS_GSO
 	select DST_CACHE
 	select NET_NSH
+	select NET_TC_SKB_EXT if NET_CLS_ACT
 	---help---
 	  Open vSwitch is a multilayer Ethernet switch targeted at virtualized
 	  environments.  In addition to supporting a variety of features
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index b3faafeafab9..f1062ef55098 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -719,6 +719,7 @@ config NET_EMATCH_IPT
 config NET_CLS_ACT
 	bool "Actions"
 	select NET_CLS
+	select NET_TC_SKB_EXT if OPENVSWITCH
 	---help---
 	  Say Y here if you want to use traffic control actions. Actions
 	  get attached to classifiers and are invoked after a successful
@@ -964,18 +965,10 @@ config NET_IFE_SKBTCINDEX
         depends on NET_ACT_IFE
 
 config NET_TC_SKB_EXT
-	bool "TC recirculation support"
-	depends on NET_CLS_ACT
-	default y if NET_CLS_ACT
+	bool
+	depends on NET_CLS_ACT && OPENVSWITCH
 	select SKB_EXTENSIONS
 
-	help
-	  Say Y here to allow tc chain misses to continue in OvS datapath in
-	  the correct recirc_id, and hardware chain misses to continue in
-	  the correct chain in tc software datapath.
-
-	  Say N here if you won't be using tc<->ovs offload or tc chains offload.
-
 endif # NET_SCHED
 
 config NET_SCH_FIFO
-- 
2.21.0