netdev - Re: [PATCH net-next 1/4] net: mpls: Convert number of nexthops to u8

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <78820aeb-6c41-5f71-90b6-ff41a91ca088@cumulusnetworks.com>
Date:   Tue, 28 Mar 2017 09:25:54 -0600
From:   David Ahern <dsa@...ulusnetworks.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     netdev@...r.kernel.org, roopa@...ulusnetworks.com,
        rshearma@...cade.com
Subject: Re: [PATCH net-next 1/4] net: mpls: Convert number of nexthops to u8

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.