netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Florian Westphal <fw@strlen.de>
To: John Hurley <john.hurley@netronome.com>
Cc: Florian Westphal <fw@strlen.de>,
	Linux Netdev List <netdev@vger.kernel.org>,
	Simon Horman <simon.horman@netronome.com>,
	Jakub Kicinski <jakub.kicinski@netronome.com>,
	Jamal Hadi Salim <jhs@mojatatu.com>,
	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 16:04:10 +0200	[thread overview]
Message-ID: <20190606140410.4rp3eudxamdtfme7@breakpoint.cc> (raw)
In-Reply-To: <CAK+XE=kQyq-ZW=DOaQq92zSmwcEi3BBwma1MydrdpnZ6F3Gp+A@mail.gmail.com>

John Hurley <john.hurley@netronome.com> wrote:
> On Thu, Jun 6, 2019 at 1:58 PM Florian Westphal <fw@strlen.de> wrote:
> > 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.
> >
> 
> Hi Florian,
> The per cpu counter (initial proposal) should protect against
> recursion through loops and the potential stack overflows.
> It will not protect against a packet infinitely looping through a poor
> configuration if (as you say) the packet is queued at some point and
> the cpu counter reset.

Yes, it won't help, but thats not harmful, such cycle will be
broken/resolved as soon as the configuration is fixed.

> The per skb tracking seems to accommodate both issues.

Yes, but I do not see the 'looping with queueing' as a problem,
it can also occur for different reasons.

> Do you see the how the cpu counter could stop infinite loops in the
> case of queuing?

No, but I don't think it has to.

> Or perhaps these are 2 different issues and should be treated differently?

The recursion is a problem, so, yes, I think these are different issues.

> > We can't catch loops in real (physical) setups either,
> > e.g. bridge looping back on itself.
> 
> yes, this is only targeted at 'internal' loops.

Right, however, I'm not sure we should bother with those, we can't
prevent this (looping packets) from happening for other reasons.

I'm sure you can make packets go in circles without tc, e.g. via
veth+bridge+netns.

  reply	other threads:[~2019-06-06 14:04 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
2019-06-06 13:27   ` John Hurley
2019-06-06 14:04     ` Florian Westphal [this message]
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=20190606140410.4rp3eudxamdtfme7@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).