All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Graf <tgraf@suug.ch>
To: Patrick McHardy <kaber@trash.net>
Cc: davem@davemloft.net, herbert@gondor.apana.org.au,
	paulmck@linux.vnet.ibm.com, ying.xue@windriver.com,
	netdev@vger.kernel.org, netfilter-devel@vger.kernel.org
Subject: Re: [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets
Date: Tue, 20 Jan 2015 14:55:51 +0000	[thread overview]
Message-ID: <20150120145551.GH20315@casper.infradead.org> (raw)
In-Reply-To: <20150120143154.GR14883@acer.localdomain>

On 01/20/15 at 02:31pm, Patrick McHardy wrote:
> On 20.01, Thomas Graf wrote:
> > Lock out table resizes while dumping Netlink sockets to user space.
> > This keeps disruptions to a minimum for readers which don't handle
> > the NLM_F_DUMP_INTR flag.
> 
> This doesn't lock them out for the duration of the entire dump of
> course, so the benefit seems rather small. Still with this patch,
> they will need to handle NLM_F_DUMP_INTR or will get unpredictable
> behaviour, in which case I'd think it makes more sense to not even
> try this, all it does is hide parts of the brokenness.

If it would lock out the resize for the entire dump I would not have
done patches 1 and 2 ;-)

I does provide better behaviour if the whole dump fits into a single
buffer or if it fits into 2 buffers and we are already dumping into
the 2nd buffer when the resize occurs. Otherwise we will see resizes
and thus tons of duplicates even in those scenarios even if no insert
or removal occurs in parallel.

In the case of Netlink diag that should be typical case. Most systems
will not have 1000s of Netlink sockets in parallel.

> An alternative would be to set a flag in ht when a dump begins that
> indicates to skip resizing operations and on the end of the dump
> perform any resizing operations that might be necessary. Herbert
> disagrees though and he might be right.

I don't like the flag as it prevents resizes (and possibly rehashes
further down the road) for a long period of time. The hashtable
becomes attackable.

  reply	other threads:[~2015-01-20 14:55 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-20 13:20 [PATCH 0/3 net-next] rhashtable: Notify on resize to allow signaling interrupted dumps Thomas Graf
2015-01-20 13:20 ` [PATCH 1/3] rhashtable: Provide notifier for deferred resizes Thomas Graf
2015-01-20 13:20 ` [PATCH 2/3] netlink: Mark dumps as inconsistent which have been interrupted by a resize Thomas Graf
2015-01-21  8:13   ` Ying Xue
2015-01-21 12:17     ` Thomas Graf
2015-01-22  8:49       ` Herbert Xu
2015-01-22  8:56         ` Patrick McHardy
2015-01-22  9:22           ` Herbert Xu
2015-01-22 10:07             ` Patrick McHardy
2015-01-25 23:20         ` [PATCH 0/2] rhashtable: Add walk iterator primitives and use them in netlink Herbert Xu
2015-01-25 23:21           ` [PATCH 1/2] rhashtable: Introduce rhashtable_walk_* Herbert Xu
2015-01-26  8:20             ` Thomas Graf
2015-01-26 22:21               ` Herbert Xu
2015-01-26 10:09             ` David Laight
2015-01-26 22:23               ` Herbert Xu
2015-01-26 22:36                 ` David Miller
2015-01-26 22:42                   ` Herbert Xu
2015-01-26 23:31                     ` Herbert Xu
2015-01-27  9:45                       ` Thomas Graf
2015-01-27  9:54                         ` Herbert Xu
2015-01-27 10:15                           ` Thomas Graf
2015-01-27 10:24                             ` Herbert Xu
2015-01-27 11:16                               ` Thomas Graf
2015-01-27 11:23                                 ` Herbert Xu
2015-01-27 11:40                                   ` Thomas Graf
2015-01-27 20:39                                     ` Herbert Xu
2015-01-27 22:10                                       ` David Miller
2015-01-27 23:16                                         ` Herbert Xu
2015-01-27 13:09                                   ` Patrick McHardy
2015-01-27 20:36                                     ` Herbert Xu
2015-01-28 19:07                                       ` Patrick McHardy
2015-01-30  5:58                                         ` Herbert Xu
2015-01-30  8:10                                           ` Patrick McHardy
2015-01-27 10:09                 ` David Laight
2015-01-27 10:12                   ` Herbert Xu
2015-01-25 23:21           ` [PATCH 2/2] netlink: Use rhashtable walk iterator Herbert Xu
2015-01-27 23:19           ` [PATCH 0/2] rhashtable: Add walk iterator primitives and use them in netlink Herbert Xu
2015-01-27 23:20             ` [PATCH 1/2] rhashtable: Introduce rhashtable_walk_* Herbert Xu
2015-01-29 22:26               ` Thomas Graf
2015-01-27 23:20             ` [PATCH 2/2] netlink: Use rhashtable walk iterator Herbert Xu
2015-01-29 22:27               ` Thomas Graf
2015-01-29 22:42             ` [PATCH 0/2] rhashtable: Add walk iterator primitives and use them in netlink David Miller
2015-01-31  3:13               ` Herbert Xu
2015-01-31  3:14                 ` [PATCH 1/2] rhashtable: Introduce rhashtable_walk_* Herbert Xu
2015-01-31  3:14                 ` [PATCH 2/2] netlink: Use rhashtable walk iterator Herbert Xu
2015-01-31  4:31                 ` netfilter: " Herbert Xu
2015-02-01  7:45                   ` Patrick McHardy
2015-02-03  3:19                   ` David Miller
2015-02-03  3:19                 ` [PATCH 0/2] rhashtable: Add walk iterator primitives and use them in netlink David Miller
2015-01-20 13:20 ` [PATCH 3/3] netlink: Lock out table resizes while dumping Netlink sockets Thomas Graf
2015-01-20 14:31   ` Patrick McHardy
2015-01-20 14:55     ` Thomas Graf [this message]
2015-01-20 15:21       ` Patrick McHardy
2015-01-20 15:35         ` Thomas Graf
2015-01-21  5:08           ` Herbert Xu
2015-01-21  5:15             ` Herbert Xu
2015-01-21  9:14               ` Herbert Xu
2015-01-21  9:56                 ` Thomas Graf
2015-01-21  9:59                   ` Herbert Xu
2015-01-21 10:00                   ` Patrick McHardy
2015-01-21  9:37             ` Thomas Graf
2015-01-21  9:38               ` Herbert Xu
2015-01-21  9:49                 ` Thomas Graf
2015-01-21  9:58                   ` Herbert Xu
2015-01-21 10:23                     ` Thomas Graf
2015-01-22  6:35                       ` Herbert Xu
2015-01-22  7:20                         ` Herbert Xu
2015-01-22  9:05                           ` Thomas Graf
2015-01-22  9:50                             ` Herbert Xu
2015-01-21 10:34                     ` Thomas Graf
2015-01-21 10:40                       ` Patrick McHardy
2015-01-21 11:37                         ` Thomas Graf
2015-01-21 11:59                           ` Patrick McHardy
2015-01-21 12:07                             ` Thomas Graf
2015-01-21 12:09                               ` Patrick McHardy
2015-01-21 10:36               ` David Laight
2015-01-20 15:00     ` David Laight
2015-01-20 15:05       ` Thomas Graf
2015-01-21  5:11     ` Herbert Xu

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=20150120145551.GH20315@casper.infradead.org \
    --to=tgraf@suug.ch \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=kaber@trash.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=ying.xue@windriver.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.