[ovs-dev] [PATCH v2 3/9] uuid: Allow record/replay of generated UUIDs.

[Ilya Maximets]

On 5/10/21 12:13 PM, Dumitru Ceara wrote:
> On 4/13/21 12:00 AM, Ilya Maximets wrote:
>> This is required for the stream record/replay functionality of
>> ovsdb-server.  With record/replay of UUIDs we could record all
>> incoming transactions and replay them later while being sure
>> that ovsdb-server will generate exactly same UUIDs for all the
>> data updates.
>>
>> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
>> ---
> 
> I have a few minor nits below but regardless:
> 
> Acked-by: Dumitru Ceara <dceara at redhat.com>
> 
>>  lib/uuid.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 76 insertions(+)
>>
>> diff --git a/lib/uuid.c b/lib/uuid.c
>> index 13d20ac64..8a16606da 100644
>> --- a/lib/uuid.c
>> +++ b/lib/uuid.c
>> @@ -26,11 +26,16 @@
>>  
>>  #include "aes128.h"
>>  #include "entropy.h"
>> +#include "fatal-signal.h"
>> +#include "openvswitch/vlog.h"
>> +#include "ovs-replay.h"
>>  #include "ovs-thread.h"
>>  #include "sha1.h"
>>  #include "timeval.h"
>>  #include "util.h"
>>  
>> +VLOG_DEFINE_THIS_MODULE(uuid);
>> +
>>  static struct aes128 key;
>>  static uint64_t counter[2];
>>  BUILD_ASSERT_DECL(sizeof counter == 16);
>> @@ -54,6 +59,63 @@ uuid_init(void)
>>      pthread_once(&once, do_init);
>>  }
>>  
>> +/* Record/replay of uuid generation. */
>> +static replay_file_t uuid_replay_file;
>> +static int uuid_replay_seqno;
>> +
>> +static void
>> +uuid_replay_file_close(void *aux OVS_UNUSED)
>> +{
>> +    ovs_replay_file_close(uuid_replay_file);
>> +}
>> +
>> +static void
>> +uuid_replay_file_open(void)
>> +{
>> +    int error;
>> +
>> +    ovs_replay_lock();
>> +    error = ovs_replay_file_open("__uuid_generate", &uuid_replay_file,
>> +                                 &uuid_replay_seqno);
>> +    ovs_replay_unlock();
>> +    if (error) {
>> +        VLOG_FATAL("failed to open uuid replay file: %s.",
>> +                   ovs_strerror(error));
>> +    }
>> +    fatal_signal_add_hook(uuid_replay_file_close, NULL, NULL, true);
>> +}
>> +
>> +static void
>> +uuid_replay_file_read(struct uuid *uuid)
>> +{
>> +    int norm_seqno = ovs_replay_normalized_seqno(uuid_replay_seqno);
>> +    int retval, len;
>> +
>> +    ovs_replay_lock();
>> +    ovs_assert(norm_seqno == ovs_replay_seqno());
>> +    ovs_assert(ovs_replay_seqno_is_read(uuid_replay_seqno));
>> +
>> +    retval = ovs_replay_read(uuid_replay_file, uuid, sizeof *uuid,
>> +                             &len, &uuid_replay_seqno, true);
>> +    if (retval || len != sizeof *uuid) {
>> +        VLOG_FATAL("failed to read from replay file: %s.",
>> +                   ovs_strerror(retval));
>> +    }
>> +    ovs_replay_unlock();
>> +}
>> +
>> +static void
>> +uuid_replay_file_write(struct uuid *uuid)
>> +{
>> +    int retval;
>> +
>> +    retval = ovs_replay_write(uuid_replay_file, uuid, sizeof *uuid, true);
>> +    if (retval) {
>> +        VLOG_FATAL("failed to write uuid to replay file: %s.",
>> +                   ovs_strerror(retval));
>> +    }
>> +}
>> +
>>  /* Generates a new random UUID in 'uuid'.
>>   *
>>   * We go to some trouble to ensure as best we can that the generated UUID has
>> @@ -82,10 +144,16 @@ void
>>  uuid_generate(struct uuid *uuid)
>>  {
>>      static struct ovs_mutex mutex = OVS_MUTEX_INITIALIZER;
>> +    enum ovs_replay_state replay_state = ovs_replay_get_state();
>>      uint64_t copy[2];
>>  
>>      uuid_init();
>>  
>> +    if (replay_state == OVS_REPLAY_READ) {
> 
> Would OVS_UNLIKELY() make sense here?

I'd keep it for branch predictor to decide.  If replay is not
working, this is an unlikely branch, but if replay is in use,
this branch is likely.  So, as this is not always true, it's
better to not touch, I think.

> 
>> +        uuid_replay_file_read(uuid);
>> +        return;
>> +    }
>> +
>>      /* Copy out the counter's current value, then increment it. */
>>      ovs_mutex_lock(&mutex);
>>      copy[0] = counter[0];
>> @@ -99,6 +167,10 @@ uuid_generate(struct uuid *uuid)
>>      aes128_encrypt(&key, copy, uuid);
>>  
>>      uuid_set_bits_v4(uuid);
>> +
>> +    if (replay_state == OVS_REPLAY_WRITE) {
> 
> Same question here.
> 
>> +        uuid_replay_file_write(uuid);
>> +    }
>>  }
>>  
>>  struct uuid
>> @@ -276,6 +348,10 @@ do_init(void)
>>      uint8_t random_seed[16];
>>      struct timeval now;
>>  
>> +    if (ovs_replay_is_active()) {
>> +        uuid_replay_file_open();
>> +    }
>> +
>>      /* Get seed data. */
>>      get_entropy_or_die(random_seed, sizeof random_seed);
>>      xgettimeofday(&now);
>>
>