On Thu, May 6, 2021, at 01:19, Ben Pfaff wrote: > On Thu, May 06, 2021 at 12:37:36AM +0200, Gaëtan Rivet wrote: > > On Wed, May 5, 2021, at 21:36, Ben Pfaff wrote: > > > On Wed, Apr 28, 2021 at 01:03:24AM +0200, Gaetan Rivet wrote: > > > > This series adds a compilation option that changes the behavior of the > > > > RCU > > > > module. Once enabled, RCU reclamation by user threads becomes blocking > > > > until > > > > the RCU threads has executed the scheduled callbacks. > > > > > > It's a great idea to improve the quality of the tests, and RCU can be a > > > sticking point. > > > > > > I don't quite understand the description. I think that's for one > > > particular reason: I don't understand yet what it causes to block. > > > Would you mind giving an example? I haven't yet read the patches (I > > > might not; I don't plan to review this series in detail), so maybe > > > there is an example in one of the patch. If so, then please just point > > > me to that one. > > > > Hello Ben, > > > > The feature unit-test in patch 4 gives an example: > > https://patchwork.ozlabs.org/project/openvswitch/patch/6de208dfd9d2fdea5999140c88632d42bdfe428b.1619564350.git.gr...@u256.net/ > > > > In short: > > > > char *x = xmalloc(1); > > x[0] = 'a'; > > ovsrcu_postpone(free, x); > > ovsrcu_quiesce(); > > x[0] = 'b'; /* potential UAF. */ > > > > If the RCU is made blocking, the user thread will wait at the end of > > 'ovsrcu_quiesce()' > > for 'free(x)' to be executed. Only threads having scheduled callbacks would > > wait. > > This resolves the above race condition. As it executes deterministically, > > if ASAN is > > enabled as well it will detect the UAF. > > This is a good example, thank you. I think that it would be useful to > put this or a similar example into the documentation. > > > In development, it could help verify that RCU is properly used. > > As it makes ASAN more effective, the RCU could be made blocking in > > conjunction in CI jobs. > > > > > This message is a good introduction to the series. Usually, > > > introductory messages don't make it into the repository, so I hope that > > > this material is also included in commit messages or in documentation or > > > comments. > > > > Most of the information here is explained in patch 8 commit log: > > https://patchwork.ozlabs.org/project/openvswitch/patch/54575393370fa6076ba6ce7201f36bbfecbe9039.1619564350.git.gr...@u256.net/ > > > > It also gives the configure option and how to test it. > > However, this feature is not otherwise documented. > > I could add a description in lib/ovs-rcu.h? > > I think that would be a good place for it. You could add a new > "Debugging" section at the end of the big comment, for example. > > Thanks! >
Hello Ben, all, Although you don't plan on reviewing this series, for people reading this thread: A v2 has been sent: https://mail.openvswitch.org/pipermail/ovs-dev/2021-May/383188.html The only change is the new 'Debugging' section in lib/ovs-rcu.h. Kind regards, -- Gaetan Rivet _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
