From: ebiederm@xmission•com (Eric W. Biederman)
To: David Ahern <dsa@cumulusnetworks•com>
Cc: netdev@vger•kernel.org, roopa@cumulusnetworks•com, rshearma@brocade•com
Subject: Re: [PATCH net-next 1/4] net: mpls: Convert number of nexthops to u8
Date: Tue, 28 Mar 2017 13:39:57 -0500 [thread overview]
Message-ID: <87k2799req.fsf@xmission.com> (raw)
In-Reply-To: <78820aeb-6c41-5f71-90b6-ff41a91ca088@cumulusnetworks.com> (David Ahern's message of "Tue, 28 Mar 2017 09:25:54 -0600")
David Ahern <dsa@cumulusnetworks•com> writes:
> On 3/27/17 4:54 PM, Eric W. Biederman wrote:
>> It is absolutely a no-brainer to change rt_nhn to a u8. And I very much
>> appreciate all work to keep mpls_route into a single cache line. As in
>> practices that is one of the most important parts to performance.
>>
>> Which leads to the functions mpls_ifup, mpls_ifdown, and
>> mpls_select_multipath.
>>
>> To make this all work correctly we need a couple of things.
>> - A big fat comment on struct mpls_route and mpls_nh about how
>> and why these structures are modified and not replaced during
>> nexthop processing. Including the fact that it all modifications
>> may only happen with rntl_lock held.
>>
>> - The use of READ_ONCE and WRITE_ONCE on all rt->rt_nhn_alive accesses,
>> that happen after the route is installed (and is thus rcu reachable).
>>
>> - The use of READ_ONCE and WRITE_ONCE on all nh->nh_flags accesses,
>> that happen after the route is installed (and is thus rcu reachable).
>
> For both of these, mpls_select_multipath does need to use READ_ONCE to
> read the nh_flags and rt_nhn_alive. In this case it is reading a value
> that could change behind its back.
>
> The READ_ONCE is not necessary for mpls_ifdown or mpls_ifup as these are
> the functions that change the values. These 2 functions only need a
> WRITE_ONCE for both struct members.
True. We don't need READ_ONCE under rtnl_lock which we use to protect writes.
Eric
next prev parent reply other threads:[~2017-03-28 18:45 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-25 17:03 [PATCH net-next 0/4] net: mpls: Allow users to configure more labels per route David Ahern
2017-03-25 17:03 ` [PATCH net-next 1/4] net: mpls: Convert number of nexthops to u8 David Ahern
2017-03-27 3:11 ` Eric W. Biederman
2017-03-27 14:43 ` David Ahern
2017-03-27 22:54 ` Eric W. Biederman
2017-03-28 15:25 ` David Ahern
2017-03-28 18:39 ` Eric W. Biederman [this message]
2017-03-25 17:03 ` [PATCH net-next 2/4] net: mpls: change mpls_route layout David Ahern
2017-03-28 0:04 ` Eric W. Biederman
2017-03-25 17:03 ` [PATCH net-next 3/4] net: mpls: bump maximum number of labels David Ahern
2017-03-25 17:03 ` [PATCH net-next 4/4] net: mpls: Increase max number of labels for lwt encap David Ahern
2017-03-25 19:15 ` [PATCH net-next 0/4] net: mpls: Allow users to configure more labels per route Eric W. Biederman
2017-03-27 10:39 ` Robert Shearman
2017-03-27 14:21 ` David Ahern
2017-03-28 3:08 ` Eric W. Biederman
2017-03-28 9:52 ` Robert Shearman
2017-03-28 14:39 ` David Ahern
2017-03-29 21:20 ` David Ahern
2017-03-27 22:52 ` David Miller
2017-03-28 9:59 ` Robert Shearman
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=87k2799req.fsf@xmission.com \
--to=ebiederm@xmission$(echo .)com \
--cc=dsa@cumulusnetworks$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=roopa@cumulusnetworks$(echo .)com \
--cc=rshearma@brocade$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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