From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamal Hadi Salim Subject: Re: [PATCH net-next v5 1/4] net/sched: user-space can't set unknown tcfa_action values Date: Tue, 31 Jul 2018 09:53:28 -0400 Message-ID: <79b9d1a7-64ea-e385-1dcb-1f38955a01dd@mojatatu.com> References: <951ad078ad4d5ee108abcd85189460aed7948634.1532934532.git.pabeni@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: Cong Wang , Jiri Pirko , Daniel Borkmann , Marcelo Ricardo Leitner , Eyal Birger , "David S. Miller" To: Paolo Abeni , netdev@vger.kernel.org Return-path: Received: from mail-io0-f194.google.com ([209.85.223.194]:35945 "EHLO mail-io0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1732160AbeGaPd5 (ORCPT ); Tue, 31 Jul 2018 11:33:57 -0400 Received: by mail-io0-f194.google.com with SMTP id r15-v6so13066351ioa.3 for ; Tue, 31 Jul 2018 06:53:31 -0700 (PDT) In-Reply-To: Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: On 31/07/18 05:41 AM, Paolo Abeni wrote: > Before this patch, the kernel exposed the same behaviour for negative > value of 'bar', while, for positive 'bar' values, the overall behaviour > was more complex (some classifier always stops with unknown positive > action value, others go to lower prio). > > > Overall, the kernel behavior should be more well-defined now, but yes, > there is a change of behavior under some circumstances. > > What about instead mapping undefined/unknown actions value to TC_ACT_OK > (still at initialization time)? > > this is what is already done by > tcf_action_exec() for faulty opcodes/graphs and by tcf_ipt() and would > handle the above example more conistently. > I think PIPE maybe more reasonable. You still continue the action graph even on such a mis-configuration. But let me see if i can make a convincing arguement for rejecting at init time: I would be _very suprised_ if someone is depending on a misconfiguration such as this in a deployment because they would get different results than what they are expecting and sooner or later fix it after a lot of debugging and cursing. Your patch helps them notice it sooner. And a rejection even much much sooner. With a rejection you dont get to execute a "fixup" the kernel assumes for you. BTW, I asked this earlier and Jiri said it was addressed in patch 2. I just looked again and i may be missing something basic: Lets say tomorrow in a new kernel we add new TC_ACT_XXX that then gets exposed to uapi - so user space tc is updated. You then use the new tc specifying TC_ACT_XXX policy on kernel with your changes. If i read correctly because TC_ACT_XXX is out of bounds for current kernel(which has your changes) you will fix it to be UNSPEC, no? cheers, jamal