Add handling for Flush to server, fix locking.

[robshakir]

  * (M) rib/rib.go
  * (M) rib/rib_test.go
    - Ensure that the DeleteXXX functions handle locking correctly, such
      that they always hold a lock when accessing the map.
    - Add lockless deleteXXX functions that can be called internally
      when the client has a lock on the entire RIB. This is needed in
      order to ensure that inter-client locks can be held (e.g.,
      when removing all entries with Flush).
  * (M) server/server.go
  * (M) server/server_test.go
    - Add support for Flush removing entries from the server RIB.
    - Add test coverage for Flush.

[sthesayi]

I would admit that I haven't done a deep review of the use of locks. If you want a deep review please let me know.

[robshakir]

Thanks @sthesayi -- I think this wasn't so much a bug in the locking, just an assumption that wasn't quite right.

Essentially, Flush says to remove all entries - and to do that we want some way to be able to make sure that someone else doesn't add something during the Flush being run. The DeleteXXX methods that we have today had fine-grained locks that were used to retrieve the current entry (added a read-lock which was missing) and then remove it (write-lock). This isn't the right behaviour for Flush.

To that end, I refactored Flush to use a delete-but-I-already-hold-the-lock method - this meant that we could take the RIB lock at the beginning of the flush call, and then use that lock throughout (avoiding the deadlock of running both).

I don't think there's anything different about the lock design here that sounds too concerning -- but I'm more than happy to talk it through. This RIB-level lock is the simplest implementation - and seems to be working fairly well thus far, but I'm sure at some point there might be a performance requirement to think through it more.

Let me know if you're concerned about anything above, and we can review it further/redesign.

[sthesayi]

This explanation is great and I gathered the same during the review. I agree that the RIB level lock is the simplest and safe one to think of and this can be revisited when a performance issue shows up. I would presume a Flush is a relatively rare operation, such as executing during startup, etc edge cases. In all other normal cases, individual deletes are more likely.

LGTM