From: Florian Westphal <fw@strlen.de>
To: John Hurley <john.hurley@netronome.com>
Cc: netdev@vger.kernel.org, simon.horman@netronome.com,
jakub.kicinski@netronome.com, jhs@mojatatu.com, fw@strlen.de,
oss-drivers@netronome.com
Subject: Re: [RFC net-next v2 1/1] net: sched: protect against loops in TC filter hooks
Date: Thu, 6 Jun 2019 14:58:18 +0200 [thread overview]
Message-ID: <20190606125818.bvo5im2wqj365tai@breakpoint.cc> (raw)
In-Reply-To: <1559825374-32117-1-git-send-email-john.hurley@netronome.com>
John Hurley <john.hurley@netronome.com> wrote:
> TC hooks allow the application of filters and actions to packets at both
> ingress and egress of the network stack. It is possible, with poor
> configuration, that this can produce loops whereby an ingress hook calls
> a mirred egress action that has an egress hook that redirects back to
> the first ingress etc. The TC core classifier protects against loops when
> doing reclassifies but there is no protection against a packet looping
> between multiple hooks. This can lead to stack overflow panics among other
> things.
>
> Previous versions of the kernel (<4.2) had a TTL count in the tc_verd skb
> member that protected against loops. This was removed and the tc_verd
> variable replaced by bit fields.
>
> Extend the TC fields in the skb with an additional 2 bits to store the TC
> hop count. This should use existing allocated memory in the skb.
>
> Add the checking and setting of the new hop count to the act_mirred file
> given that it is the source of the loops. This means that the code
> additions are not in the main datapath.
>
> v1->v2
> - change from per cpu counter to per skb tracking (Jamal)
> - move check/update from fast path to act_mirred (Daniel)
>
> Signed-off-by: John Hurley <john.hurley@netronome.com>
> ---
> include/linux/skbuff.h | 2 ++
> net/sched/act_mirred.c | 9 +++++++++
> 2 files changed, 11 insertions(+)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 2ee5e63..f0dbc5b 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -645,6 +645,7 @@ typedef unsigned char *sk_buff_data_t;
> * @tc_at_ingress: used within tc_classify to distinguish in/egress
> * @tc_redirected: packet was redirected by a tc action
> * @tc_from_ingress: if tc_redirected, tc_at_ingress at time of redirect
> + * @tc_hop_count: hop counter to prevent packet loops
> * @peeked: this packet has been seen already, so stats have been
> * done for it, don't do them again
> * @nf_trace: netfilter packet trace flag
> @@ -827,6 +828,7 @@ struct sk_buff {
> __u8 tc_at_ingress:1;
> __u8 tc_redirected:1;
> __u8 tc_from_ingress:1;
> + __u8 tc_hop_count:2;
I dislike this, why can't we just use a pcpu counter?
The only problem is with recursion/nesting; whenever we
hit something that queues the skb for later we're safe.
We can't catch loops in real (physical) setups either,
e.g. bridge looping back on itself.
next prev parent reply other threads:[~2019-06-06 12:58 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-06 12:49 [RFC net-next v2 1/1] net: sched: protect against loops in TC filter hooks John Hurley
2019-06-06 12:58 ` Florian Westphal [this message]
2019-06-06 13:27 ` John Hurley
2019-06-06 14:04 ` Florian Westphal
2019-06-06 18:19 ` David Miller
2019-06-06 19:52 ` Florian Westphal
2019-06-11 14:58 ` John Hurley
2019-06-11 15:03 ` Florian Westphal
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=20190606125818.bvo5im2wqj365tai@breakpoint.cc \
--to=fw@strlen.de \
--cc=jakub.kicinski@netronome.com \
--cc=jhs@mojatatu.com \
--cc=john.hurley@netronome.com \
--cc=netdev@vger.kernel.org \
--cc=oss-drivers@netronome.com \
--cc=simon.horman@netronome.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 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).