All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Borkmann <daniel@iogearbox.net>
To: Cong Wang <xiyou.wangcong@gmail.com>
Cc: David Miller <davem@davemloft.net>,
	Shahar Klein <shahark@mellanox.com>,
	Or Gerlitz <gerlitz.or@gmail.com>, Roi Dayan <roid@mellanox.com>,
	Jiri Pirko <jiri@mellanox.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Linux Kernel Network Developers <netdev@vger.kernel.org>
Subject: Re: [PATCH net] net, sched: fix soft lockup in tc_classify
Date: Wed, 21 Dec 2016 20:16:19 +0100	[thread overview]
Message-ID: <585AD503.2070300@iogearbox.net> (raw)
In-Reply-To: <CAM_iQpXadrGWJYqizEkq+xmMKVV9krrcCWKur=ZCRZBS9Mx_8w@mail.gmail.com>

On 12/21/2016 07:51 PM, Cong Wang wrote:
> On Wed, Dec 21, 2016 at 9:04 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
>> What happens is that in tc_ctl_tfilter(), thread A allocates a new
>> tp, initializes it, sets tp_created to 1, and calls into tp->ops->change()
>> with it. In that classifier callback we had to unlock/lock the rtnl
>> mutex and returned with -EAGAIN. One reason why we need to drop there
>> is, for example, that we need to request an action module to be loaded.
>
> Excellent catch!
>
> But why do we have to replay the request here? Shouldn't we just return
> EAGAIN to user-space and let user-space decide to retry or not?
> Replaying is the root of the evil here.

Right, this behavior is already pretty old (2005), see history
tree 8d7c694553dc ("[PKT_SCHED]: act_api.c: drop rtnl for loading
modules") and 437293de63d8 ("[PKT_SCHED]: cls_api.c: drop rtnl
for loading modules"), some binaries could rely on that behavior
in one way or another I'd presume.

  parent reply	other threads:[~2016-12-21 19:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-21 17:04 [PATCH net] net, sched: fix soft lockup in tc_classify Daniel Borkmann
2016-12-21 17:37 ` Eric Dumazet
2016-12-21 18:51 ` Cong Wang
2016-12-21 19:10   ` Cong Wang
2016-12-21 20:02     ` Daniel Borkmann
2016-12-21 20:47       ` Cong Wang
2016-12-21 21:07         ` Daniel Borkmann
2016-12-22 16:53           ` David Miller
2016-12-22 17:50             ` John Fastabend
2016-12-22 23:21               ` Daniel Borkmann
2016-12-22 19:05           ` Cong Wang
2016-12-23  0:26             ` Daniel Borkmann
2016-12-24  7:34               ` Cong Wang
2016-12-24 21:03                 ` Daniel Borkmann
2016-12-21 19:16   ` Daniel Borkmann [this message]
2016-12-22 13:16 ` Shahar Klein
2016-12-22 23:20   ` Daniel Borkmann
2016-12-26 16:24 ` 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=585AD503.2070300@iogearbox.net \
    --to=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=gerlitz.or@gmail.com \
    --cc=jiri@mellanox.com \
    --cc=john.fastabend@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=roid@mellanox.com \
    --cc=shahark@mellanox.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.