From: Simon Horman <simon.horman@netronome•com>
To: Jamal Hadi Salim <jhs@mojatatu•com>
Cc: Jiri Pirko <jiri@mellanox•com>,
Cong Wang <xiyou.wangcong@gmail•com>,
Dinan Gunawardena <dinan.gunawardena@netronome•com>,
netdev@vger•kernel.org, oss-drivers@netronome•com
Subject: Re: [PATCH/RFC net-next 0/4] net/sched: cls_flower: avoid false matching of truncated packets
Date: Fri, 28 Apr 2017 16:14:15 +0200 [thread overview]
Message-ID: <20170428141414.GA11256@vergenet.net> (raw)
In-Reply-To: <2c14754a-f3f5-cece-2cd7-361948a00c22@mojatatu.com>
On Fri, Apr 28, 2017 at 09:41:00AM -0400, Jamal Hadi Salim wrote:
> On 17-04-28 09:11 AM, Simon Horman wrote:
> >On Fri, Apr 28, 2017 at 08:52:56AM -0400, Jamal Hadi Salim wrote:
> >>On 17-04-28 08:00 AM, Simon Horman wrote:
> >>>Hi,
> >>>
> >>>this series is intended to avoid false-positives which match
> >>>truncated packets against flower classifiers which match on:
> >>>* zero L4 ports or;
> >
> >How would you describe such a rule? The case that is being dealt with is
> >one where there is a parse error and thus nothing to match on from a flower
> >pov.
> >
>
> A default lower prio match all on udp or icmp?
I'm certainly not opposed to exploring ideas here.
The way that flower currently works is that a match on ip_proto ==
UDP/TCP/SCTP/ICMP but not fields in the L4 header itself would not result in
the dissector only dissecting the packet's L4 header and thus would not
discover (or as in currently the case, silently ignore) the absence of the
ports/ICMP type and code in the L4 header.
What my patch attempts to do is to describe a policy of what to do if
a given classifier invokes the dissector (to pull out the headers needed for
the match in question) and that dissection fails. Its basically describing
the error-path.
> >>Example what would offloading of
> >>header_parse_err_action mean?
> >
> >Why would it need to differ semantically to the implementation in this
> >patch? I feel that I am missing something.
> >
>
> Unless I misunderstood:
> Isnt the issue the dissector that confused something missing L4 ports
> and said "port is zero"?
>
> Unless the hardware has the same "bug" as the dissector seems like would
> be a different semantic in the h/ware.
There are two issues:
1. As things stand, without this patch-set, flower does not differentiate
between a packet truncated at the end of the IP header and a packet with
zero ports. Likewise for icmp type and code of zero.
The first three patches of this series address that so that a match for
port == zero only matches if ports are present in the packet. Again,
likewise for ICMP.
This is a bug-fix to my way of thinking.
2. The behaviour described above, prior to this patchset, might have been
utilised to f.e. drop packets that are either truncated or have port == 0
(because flower didn't differentiate between these cases).
So the question becomes if/how to provide such a feature.
The last patch is my attempt to answer that question.
next prev parent reply other threads:[~2017-04-28 14:14 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-28 12:00 [PATCH/RFC net-next 0/4] net/sched: cls_flower: avoid false matching of truncated packets Simon Horman
2017-04-28 12:00 ` [PATCH/RFC net-next 1/4] flow dissector: return error on port dissection under-run Simon Horman
2017-04-28 12:00 ` [PATCH/RFC net-next 2/4] flow dissector: return error on icmp " Simon Horman
2017-04-28 12:00 ` [PATCH/RFC net-next 3/4] net/sched: cls_flower: do not match if dissection fails Simon Horman
2017-04-28 12:00 ` [PATCH/RFC net-next 4/4] net/sched: cls_flower: allow control of tree traversal on packet parse errors Simon Horman
2017-04-28 12:52 ` [PATCH/RFC net-next 0/4] net/sched: cls_flower: avoid false matching of truncated packets Jamal Hadi Salim
2017-04-28 13:11 ` Simon Horman
2017-04-28 13:41 ` Jamal Hadi Salim
2017-04-28 14:14 ` Simon Horman [this message]
2017-04-30 13:51 ` Jamal Hadi Salim
2017-04-30 14:45 ` Jamal Hadi Salim
2017-05-01 10:36 ` Simon Horman
2017-05-02 1:35 ` Jamal Hadi Salim
2017-05-02 12:04 ` [oss-drivers] " Simon Horman
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=20170428141414.GA11256@vergenet.net \
--to=simon.horman@netronome$(echo .)com \
--cc=dinan.gunawardena@netronome$(echo .)com \
--cc=jhs@mojatatu$(echo .)com \
--cc=jiri@mellanox$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=oss-drivers@netronome$(echo .)com \
--cc=xiyou.wangcong@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox