All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jamal Hadi Salim <jhs@mojatatu.com>
To: Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org
Cc: Cong Wang <xiyou.wangcong@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>,
	Eyal Birger <eyal.birger@gmail.com>,
	"David S. Miller" <davem@davemloft.net>
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	[thread overview]
Message-ID: <79b9d1a7-64ea-e385-1dcb-1f38955a01dd@mojatatu.com> (raw)
In-Reply-To: <b36ee3a23d828c304e0bbcb872499600ad9bf162.camel@redhat.com>

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

  reply	other threads:[~2018-07-31 15:33 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-30 12:30 [PATCH net-next v5 0/4] TC: refactor act_mirred packets re-injection Paolo Abeni
2018-07-30 12:30 ` [PATCH net-next v5 1/4] net/sched: user-space can't set unknown tcfa_action values Paolo Abeni
2018-07-30 12:36   ` Jiri Pirko
2018-07-30 14:03   ` Jamal Hadi Salim
2018-07-30 14:21     ` Jiri Pirko
2018-07-30 16:41     ` Paolo Abeni
2018-07-30 19:31       ` Jamal Hadi Salim
2018-07-31  9:41         ` Paolo Abeni
2018-07-31 13:53           ` Jamal Hadi Salim [this message]
2018-07-31 14:40             ` Paolo Abeni
2018-08-01 14:34               ` Jamal Hadi Salim
2018-07-30 12:30 ` [PATCH net-next v5 2/4] tc/act: remove unneeded RCU lock in action callback Paolo Abeni
2018-07-30 12:30 ` [PATCH net-next v5 3/4] net/tc: introduce TC_ACT_REINSERT Paolo Abeni
2018-07-30 12:40   ` Jiri Pirko
2018-07-30 16:31     ` David Miller
2018-07-30 16:49       ` Jiri Pirko
2018-07-30 16:56         ` David Miller
2018-07-30 12:30 ` [PATCH net-next v5 4/4] act_mirred: use TC_ACT_REINSERT when possible Paolo Abeni
2018-07-30 12:42   ` Jiri Pirko
2018-07-30 16:31 ` [PATCH net-next v5 0/4] TC: refactor act_mirred packets re-injection David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=79b9d1a7-64ea-e385-1dcb-1f38955a01dd@mojatatu.com \
    --to=jhs@mojatatu.com \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=eyal.birger@gmail.com \
    --cc=jiri@resnulli.us \
    --cc=marcelo.leitner@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.