Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ido Schimmel <idosch@idosch.org>
To: Jamal Hadi Salim <jhs@mojatatu.com>
Cc: Boris Sukholitko <boris.sukholitko@broadcom.com>,
	netdev@vger.kernel.org, Jiri Pirko <jiri@resnulli.us>,
	Cong Wang <xiyou.wangcong@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Vladimir Oltean <olteanv@gmail.com>,
	Vadym Kochan <vadym.kochan@plvision.eu>,
	Ilya Lifshits <ilya.lifshits@broadcom.com>,
	tom Herbert <tom@sipanda.io>,
	Felipe Magno de Almeida <felipe@expertise.dev>,
	Pedro Tammela <pctammela@mojatatu.com>
Subject: Re: [PATCH net-next] net/sched: cls_flower: Add orig_ethtype
Date: Thu, 2 Sep 2021 09:48:51 +0300	[thread overview]
Message-ID: <YTBz0zitSUrd0Qd1@shredder> (raw)
In-Reply-To: <b400f8c6-8bd8-2617-0a4f-7c707809da7d@mojatatu.com>

On Tue, Aug 31, 2021 at 09:18:16AM -0400, Jamal Hadi Salim wrote:
> You have _not_ been unlucky - it is a design issue with flow dissector
> and the wrapping around flower. Just waiting to happen for more
> other use cases..

I agree. I think the fundamental problem is that flower does not set
'FLOW_DISSECTOR_F_STOP_AT_ENCAP' and simply lets the flow dissector
parse as deep as possible. For example, 'dst_ip' will match on the
inner most destination IP which is not intuitive and probably different
than what most hardware implementations do.

This behavior is also very error prone because it means that if the
kernel learns to dissect a new tunnel protocol, filters can be suddenly
broken (match on outer field now matches on inner field).

I don't think that changing the default behavior is a solution as it can
break user space. Maybe adding a 'stop_encap' flag to flower that user
space will have to set?

  parent reply	other threads:[~2021-09-02  6:49 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-30  8:08 Boris Sukholitko
2021-08-30  9:00 ` Vladimir Oltean
2021-08-30  9:04   ` Vladimir Oltean
2021-08-30  9:18   ` Boris Sukholitko
2021-08-30  9:21     ` Vladimir Oltean
2021-08-30  9:42       ` Boris Sukholitko
2021-08-30 10:13         ` Vladimir Oltean
2021-08-31  1:48 ` Jamal Hadi Salim
2021-08-31 12:04   ` Boris Sukholitko
2021-08-31 13:18     ` Jamal Hadi Salim
2021-08-31 14:03       ` Boris Sukholitko
2021-09-02  6:48       ` Ido Schimmel [this message]
2021-09-03 22:52         ` Jamal Hadi Salim
2021-09-04 14:08       ` Tom Herbert

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=YTBz0zitSUrd0Qd1@shredder \
    --to=idosch@idosch.org \
    --cc=boris.sukholitko@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=felipe@expertise.dev \
    --cc=ilya.lifshits@broadcom.com \
    --cc=jhs@mojatatu.com \
    --cc=jiri@resnulli.us \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pctammela@mojatatu.com \
    --cc=tom@sipanda.io \
    --cc=vadym.kochan@plvision.eu \
    --cc=xiyou.wangcong@gmail.com \
    --subject='Re: [PATCH net-next] net/sched: cls_flower: Add orig_ethtype' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).