[ovs-git] [openvswitch/ovs] b70e69: cmap: New macro CMAP_INITIALIZER, for initializing...

GitHub noreply at github.com
Mon May 9 23:47:07 UTC 2016


  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: b70e697679bd0b8f248348be6985c996f85643ab
      https://github.com/openvswitch/ovs/commit/b70e697679bd0b8f248348be6985c996f85643ab
  Author: Ben Pfaff <blp at ovn.org>
  Date:   2016-05-09 (Mon, 09 May 2016)

  Changed paths:
    M include/openvswitch/compiler.h
    M lib/cmap.c
    M lib/cmap.h
    M lib/tnl-neigh-cache.c
    M ofproto/ofproto-dpif-rid.c
    M ofproto/ofproto-dpif-rid.h
    M ofproto/ofproto-dpif.c
    M tests/test-cmap.c

  Log Message:
  -----------
  cmap: New macro CMAP_INITIALIZER, for initializing an empty cmap.

Sometimes code is much simpler if we can statically initialize data
structures.  Until now, this has not been possible for cmap-based data
structures, so this commit introduces a CMAP_INITIALIZER macro.

This works by adding a singleton empty cmap_impl that simply forces the
first insertion into any cmap that points to it to allocate a real
cmap_impl.  There could be some risk that rogue code modifies the
singleton, so for safety it is also marked 'const' to allow the linker to
put it into a read-only page.

This adds a new OVS_ALIGNED_VAR macro with GCC and MSVC implementations.
The latter is based on Microsoft webpages, so developers who know Windows
might want to scrutinize it.

As examples of the kind of simplification this can make possible, this
commit removes an initialization function from ofproto-dpif-rid.c and a
call to cmap_init() from tnl-neigh-cache.c.  An upcoming commit will add
another user.

CC: Jarno Rajahalme <jarno at ovn.org>
CC: Gurucharan Shetty <guru at ovn.org>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Acked-by: Ryan Moats <rmoats at us.ibm.com>


  Commit: 0692257923fed2ecd56906a5c06c9426f7e62463
      https://github.com/openvswitch/ovs/commit/0692257923fed2ecd56906a5c06c9426f7e62463
  Author: Ben Pfaff <blp at ovn.org>
  Date:   2016-05-09 (Mon, 09 May 2016)

  Changed paths:
    M lib/netdev.c

  Log Message:
  -----------
  netdev: Fix potential deadlock.

Until now, netdev_class_mutex and route_table_mutex could be taken in
either order:

    * netdev_run() takes netdev_class_mutex, then netdev_vport_run() calls
      route_table_run(), which takes route_table_mutex.

    * route_table_init() takes route_table_mutex and then eventually calls
      netdev_open(), which takes netdev_class_mutex.

This commit fixes the problem by converting the netdev_classes hmap,
protected by netdev_class_mutex, into a cmap protected on the read
side by RCU.  Only a very small amount of code actually writes to the
cmap in question, so it's a lot easier to understand the locking rules
at that point.  In particular, there's no need to take netdev_class_mutex
from either netdev_run() or netdev_open(), so neither of the code paths
above determines a lock ordering any longer.

Reported-by: William Tu <u9012063 at gmail.com>
Reported-at: http://openvswitch.org/pipermail/discuss/2016-February/020216.html
Signed-off-by: Ben Pfaff <blp at ovn.org>
Acked-by: Ryan Moats <rmoats at us.ibm.com>
Tested-by: William Tu <u9012063 at gmail.com>


Compare: https://github.com/openvswitch/ovs/compare/790c5d2694bb...0692257923fe


More information about the git mailing list