All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ismael Luceno <iluceno@suse.de>
To: David Ahern <dsahern@gmail.com>
Cc: Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Paolo Abeni <pabeni@redhat.com>,
	"netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Florian Westphal <fw@strlen.de>
Subject: Re: Netlink NLM_F_DUMP_INTR flag lost
Date: Wed, 24 Aug 2022 12:59:01 +0200	[thread overview]
Message-ID: <20220824125901.21a28927@pirotess> (raw)
In-Reply-To: <b0246015-7fe6-7f30-c5ae-5531c126366f@gmail.com>

On 17/Jun/2022 10:28, David Ahern wrote:
> On 6/17/22 9:22 AM, Jakub Kicinski wrote:
<...>
> > FWIW what I think is strange is that we record the gen id before the
> > dump and then check if the recorded version was old. Like.. what's
> > the point of that? Nothing updates cb->seq while dumping AFAICS, so
> > the code is functionally equivalent to this right?
<...> 
> FWIW, net/ipv4/nexthop.c sets cb->seq at the end of the loop and the
> nl_dump_check_consistent follows that.

[CCing Florian Westphal because he gave a related talk at netdev 2.2,
 maybe he can add something]

It seems to me now that most of the calls to nl_dump_check_consistent
are redundant.

Either the rtnl lock is explicitly taken:
- ethnl_tunnel_info_dumpit

Or are implicitly called with the rtnl lock held:
- rtnl_dump_ifinfo
- rtnl_dump_all
- in_dev_dump_addr
- inet_netconf_dump_devconf
- rtm_dump_nexthop
- rtm_dump_nexthop_bucket
- mpls_netconf_dump_devconf

Except:
- inet6_netconf_dump_devconf
- inet6_dump_addr

I assume the ones that rely on rcu_read_lock are safe too.

Also, the following ones set cb->seq just before calling it:
- rtm_dump_nh_ctx
- rtm_dump_res_bucket_ctx

Does it make sense to remove these calls or is anyone looking forward
to convert the functions to run without the rtnl lock?

Am I missing something here?

-- 
Ismael Luceno
SUSE - Support Level 3

  reply	other threads:[~2022-08-24 10:59 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20220615171113.7d93af3e@pirotess>
2022-06-15 16:00 ` Netlink NLM_F_DUMP_INTR flag lost Jakub Kicinski
2022-06-16 15:10   ` Ismael Luceno
2022-06-17  0:16     ` Jakub Kicinski
2022-06-17 13:01       ` Ismael Luceno
2022-06-17 14:55         ` David Ahern
2022-06-17 15:22           ` Jakub Kicinski
2022-06-17 16:17             ` David Ahern
2022-06-17 16:28             ` David Ahern
2022-08-24 10:59               ` Ismael Luceno [this message]
2022-08-24 11:46                 ` Florian Westphal
2022-06-22 11:12         ` Ismael Luceno
2022-06-22 23:55           ` Jakub Kicinski
2022-06-23  4:01             ` David Ahern
2022-06-23 16:03               ` Jakub Kicinski
2022-06-23 16:17                 ` David Ahern
2022-06-23 16:36                   ` Jakub Kicinski
2022-06-23 17:31                     ` David Ahern
2022-06-23 19:03                       ` Jakub Kicinski
2022-06-28 19:38                         ` Ismael Luceno

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=20220824125901.21a28927@pirotess \
    --to=iluceno@suse.de \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=fw@strlen.de \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.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.