netdev - Re: [PATCH net-next 0/5] bonding: groundwork and initial conversion to RCU

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <1375284479.10515.106.camel@edumazet-glaptop>
Date:	Wed, 31 Jul 2013 08:27:59 -0700
From:	Eric Dumazet <eric.dumazet@...il.com>
To:	Nikolay Aleksandrov <nikolay@...hat.com>
Cc:	netdev@...r.kernel.org, andy@...yhouse.net, davem@...emloft.net,
	fubar@...ibm.com
Subject: Re: [PATCH net-next 0/5] bonding: groundwork and initial conversion
 to RCU

On Wed, 2013-07-31 at 17:12 +0200, Nikolay Aleksandrov wrote:
> From: Nikolay Aleksandrov <Nikolay Aleksandrov nikolay@...hat.com>
> 
> Hello,
>  This patchset aims to lay the groundwork, and do the initial conversion to
> RCUism. I decided that it'll be much better to make the bonding RCU
> conversion gradual, so patches can be reviewed and tested better rather
> than having one huge patch (which I did in the beginning, before this).
> The first patch is straightforward and it converts the bonding to the
> standard list API, simplifying a lot of code, removing unnecessary local
> variables and allowing to use the nice rculist API later. It also takes
> care of some minor styling issues (re-arranging local variables longest ->
> shortest, removing brackets for single statement if/else, leaving new line
> before return statement etc.).
>  The second patch simplifies the conversion by removing unnecessary
> read_lock(&bond->curr_slave_lock) in xmit paths that are to be converted
> later, because we only care if the pointer is NULL or a slave there, since
> we already have bond->lock the slave can't go away.
>  The third patch simplifies the broadcast xmit function by removing
> the use of curr_active_slave and converting to standard list API. Also this
> design of the broadcast xmit function avoids a subtle double packet tx race
> when converted to RCU.
>  The fourth patch factors out the code that transmits skb through a slave
> with given id (i.e. rr_tx_counter in rr mode, hashed value in xor mode) and
> simplifies the active-backup xmit path because bond_dev_queue_xmit always
> consumes the skb. The new bond_xmit_slave_id function is used in rr and xor
> modes currently, but the plans are to use it in 3ad mode as well thus it's
> made global. I've left the function prototype to be 81 chars so I wouldn't
> break it, if this is an issue I can always break it in more lines.
>  The fifth patch introduces RCU by converting attach/detach and release to
> RCU. It also converts dereferencing of curr_active_slave to rcu_dereference
> although it's not fully converted to RCU, that is needed for the converted
> xmit paths. And it converts roundrobin, broadcast, xor and active-backup
> xmit paths to RCU. The 3ad and ALB/TLB modes acquire read_lock(&bond->lock)
> to make sure that no slave will be removed and to sync properly with
> enslave and release as before.
>  This way for the price of a little complexity, we'll be able to convert
> individual parts of the bonding to RCU, and test them easier in the
> process. If this patchset is accepted in some form, I'll post followups
> in the next weeks that gradually convert the bonding to RCU and remove the
> need for the rwlocks.
>  For performance notes please refer to patch 5 (RCU conversion one).
> 
> Best regards,
>  Nikolay Aleksandrov
> 

This is awesome work, thanks Nikolay.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html