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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 23 Sep 2019 15:46:19 -0300 From: Marcelo Ricardo Leitner <marcelo.leitner@...il.com> To: Jakub Kicinski <jakub.kicinski@...ronome.com> Cc: Paul Blakey <paulb@...lanox.com>, Pravin Shelar <pshelar@....org>, Daniel Borkmann <daniel@...earbox.net>, Vlad Buslov <vladbu@...lanox.com>, David Miller <davem@...emloft.net>, "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>, Simon Horman <simon.horman@...ronome.com>, Or Gerlitz <gerlitz.or@...il.com> Subject: Re: CONFIG_NET_TC_SKB_EXT On Sun, Sep 22, 2019 at 02:47:15PM -0700, Jakub Kicinski wrote: > On Sun, 22 Sep 2019 14:51:44 +0300, Paul Blakey wrote: ... > > ------------------------------------------------------------ > > > > Subject: [PATCH net-next] net/sched: Set default of CONFIG_NET_TC_SKB_EXT to N > > > > This a new feature, it is prefered that it defaults to N. > > > > Fixes: 95a7233c452a ('net: openvswitch: Set OvS recirc_id from tc chain index') > > Signed-off-by: Paul Blakey <paulb@...lanox.com> > > --- > > net/sched/Kconfig | 1 - > > 1 file changed, 1 deletion(-) > > > > diff --git a/net/sched/Kconfig b/net/sched/Kconfig > > index b3faafe..4bb10b7 100644 > > --- a/net/sched/Kconfig > > +++ b/net/sched/Kconfig > > @@ -966,7 +966,6 @@ config NET_IFE_SKBTCINDEX > > config NET_TC_SKB_EXT > > bool "TC recirculation support" > > depends on NET_CLS_ACT > > - default y if NET_CLS_ACT > > select SKB_EXTENSIONS > > > > help > > - Linus suggested we hide this option from user and autoselect if > possible. > - As Daniel said all distros will enable this. > - If correctness is really our concern, giving users an option to > select whether they want something to behave correctly seems like > a hack of lowest kind. Perhaps it's time to add a CONFIG for TC > offload in general? That's a meaningful set of functionality. Now that we have the static branch in place on OvS code, maybe we can remove this option and rely just on NET_CLS_ACT instead (such as in cls_api.c and skbuff.h). Probing by OvS userspace is still somewhat possible, as the flag OVS_DP_F_TC_RECIRC_SHARING wouldn't be known and be rejected otherwise. 'somewhat because there is a limitation, as prior versions (prior to this commit) didn't validate if the flags being set were at least known. But this is not news. General OvS performance won't suffer, thanks to the static branch. Drivers won't allocate the skb extension just because. They will only do it when tc offloading is being done and, as already explained in the thread and other places, it's needed for correct operation. So general performance is also ok here. For tc sw datapath, maybe we can have another static branch in there, more specifically at tcf_classify(). Not sure how to control/toggle it, though.
Powered by blists - more mailing lists