netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Luciano Coelho <luciano.coelho@nokia.com>
To: ext Jan Engelhardt <jengelh@medozas.de>
Cc: ext Patrick McHardy <kaber@trash.net>,
	"netfilter-devel@vger.kernel.org"
	<netfilter-devel@vger.kernel.org>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Timo Teras <timo.teras@iki.fi>
Subject: Re: [PATCH v3] netfilter: Xtables: idletimer target implementation
Date: Thu, 10 Jun 2010 00:28:40 +0300	[thread overview]
Message-ID: <1276118920.11199.53.camel@powerslave> (raw)
In-Reply-To: <alpine.LSU.2.01.1006092304270.16916@obet.zrqbmnf.qr>

On Wed, 2010-06-09 at 23:05 +0200, ext Jan Engelhardt wrote:
> On Wednesday 2010-06-09 20:42, Luciano Coelho wrote:
> >
> >> I'll move the sysfs file creation to outside that function so I can keep
> >> the lock until after the timer is added to the list.  Thanks for
> >> clarifying!
> >
> >Hmmm... after struggling with this for a while, I think it's not really
> >possible to simply create the sysfs file outside of the lock, because if
> >the sysfs creation fails, we will again risk a race condition.
> 
> Well if sysfs_add can return an error code when a file already
> exists (instead of adding it again), it's much easier. Try checking.

Unfortunately sysfs_add and sysfs_create will throw a kernel warning if
we try to create a file that already exists.  But this was not really
the problem.

Now I just throw an WARN_ON if the sysfs file creation fails, the other
sysfs functions I'm calling can handle this situation.  This shouldn't
happen in normal cases and, if it does, the timer will run normally, but
there won't be a sysfs file associated with it (and there won't be any
notification).

As I just wrote in another email, I thought I had figured out a way to
do it without a workqueue.  But now that I looked into the code again, I
think there might still be some race conditions... For example: If
someone deletes the timer immediately after I released the lock and
before I created the sysfs entry... The deletion won't cause problems if
the file is not there, it will just nop.  But the creation of the file
after the timer has been deleted will cause the file to be dangling in
sysfs without any timer associated with it... :(


-- 
Cheers,
Luca.


  reply	other threads:[~2010-06-09 21:50 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-03 19:14 [PATCH v3] netfilter: Xtables: idletimer target implementation luciano.coelho
2010-06-09 13:00 ` Luciano Coelho
2010-06-09 13:45 ` Patrick McHardy
2010-06-09 15:11   ` Luciano Coelho
2010-06-09 15:18     ` Jan Engelhardt
2010-06-09 17:48       ` Luciano Coelho
2010-06-09 18:42         ` Luciano Coelho
2010-06-09 21:00           ` Luciano Coelho
2010-06-09 21:05           ` Jan Engelhardt
2010-06-09 21:28             ` Luciano Coelho [this message]
2010-06-10 10:07           ` Patrick McHardy
2010-06-10 12:42             ` Luciano Coelho
2010-06-10 13:32               ` Luciano Coelho
2010-06-10 15:55                 ` Jan Engelhardt

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=1276118920.11199.53.camel@powerslave \
    --to=luciano.coelho@nokia.com \
    --cc=jengelh@medozas.de \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=timo.teras@iki.fi \
    /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).