On 9/20/19 6:16 PM, Jakub Kicinski wrote:
On Thu, 19 Sep 2019 15:13:55 +0000, Vlad Buslov wrote:
On Thu 19 Sep 2019 at 14:21, David Miller <da...@davemloft.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.kicin...@netronome.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
But how would that make much of a difference :( Distros are still going to
enable all of this blindlessly. Given discussion in [0], could we just get
rid of this tasteless hack altogether which is for such a narrow use case
anyway?
I thought idea of stuffing things into skb extensions are only justified if
it's not enabled by default for everyone. :(
[0]
https://lore.kernel.org/netdev/cahc9vhsz1_ka1tcjtnjwk26bokghkgbpt7v1o82mwpduvww...@mail.gmail.com/T/#u
---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
