[ovs-dev] [PATCH v3 2/9] stream: Add record/replay functionality.

[Dumitru Ceara]

On 5/27/21 3:29 PM, Ilya Maximets wrote:
> For debugging purposes it is useful to be able to record all the
> incoming transactions and commands and replay them locally under
> debugger or with additional logging enabled.  This patch introduces
> ability to record all the incoming stream data and replay it via new
> stream provider named 'stream-replay'.  During the record phase all
> the incoming stream data written to special replay_* files in the
> application rundir.  On replay phase instead of opening real streams
> application will open replay_* files and read all the incoming data
> directly from them.
> 
> If enabled for ovsdb-server, for example, this allows to record all
> the connections and transactions from the big setup and replay them
> locally afterwards to debug the behaviour or test performance.
> 
> To start application in recording mode there is a --record cmdline
> option. --replay is to replay previously recorded streams.
> 
> Current version doesn't work well with time-based stream events like
> inactivity probes or any other events generated internally.  This is
> a point for further improvement.
> 
> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> ---

I have a couple tiny nits below.  Nevertheless:

Acked-by: Dumitru Ceara <dceara at redhat.com>

[...]

> diff --git a/lib/stream.c b/lib/stream.c
> index e246b3773..1e3c8a24e 100644
> --- a/lib/stream.c
> +++ b/lib/stream.c
> @@ -33,6 +33,7 @@
>  #include "openvswitch/ofp-print.h"
>  #include "openvswitch/ofpbuf.h"
>  #include "openvswitch/vlog.h"
> +#include "ovs-replay.h"
>  #include "ovs-thread.h"
>  #include "packets.h"
>  #include "openvswitch/poll-loop.h"
> @@ -185,6 +186,9 @@ stream_lookup_class(const char *name, const struct stream_class **classp)
>          if (strlen(class->name) == prefix_len
>              && !memcmp(class->name, name, prefix_len)) {
>              *classp = class;
> +            if (ovs_replay_get_state() == OVS_REPLAY_READ) {
> +                *classp = &replay_stream_class;
> +            }

Nit: I'd rewrite this as:

if (ovs_replay_get_state() == OVS_REPLAY_READ) {
    *classp = &replay_stream_class;
} else {
    *classp = class;
}

But it's not a "must" :)

>              return 0;
>          }
>      }
> @@ -227,6 +231,8 @@ stream_open(const char *name, struct stream **streamp, uint8_t dscp)
>      suffix_copy = xstrdup(strchr(name, ':') + 1);
>      error = class->open(name, suffix_copy, &stream, dscp);
>      free(suffix_copy);
> +
> +    stream_replay_open_wfd(stream, error, name);
>      if (error) {
>          goto error;
>      }
> @@ -295,6 +301,7 @@ stream_close(struct stream *stream)
>      if (stream != NULL) {
>          char *name = stream->name;
>          char *peer_id = stream->peer_id;
> +        stream_replay_close_wfd(stream);
>          (stream->class->close)(stream);
>          free(name);
>          free(peer_id);
> @@ -367,9 +374,13 @@ int
>  stream_recv(struct stream *stream, void *buffer, size_t n)
>  {
>      int retval = stream_connect(stream);
> -    return (retval ? -retval
> -            : n == 0 ? 0
> -            : (stream->class->recv)(stream, buffer, n));
> +
> +    retval = retval ? -retval
> +             : n == 0 ? 0
> +             : (stream->class->recv)(stream, buffer, n);
> +
> +    stream_replay_write(stream, buffer, retval, true);
> +    return retval;
>  }
>  
>  /* Tries to send up to 'n' bytes of 'buffer' on 'stream', and returns:
> @@ -385,9 +396,12 @@ int
>  stream_send(struct stream *stream, const void *buffer, size_t n)
>  {
>      int retval = stream_connect(stream);
> -    return (retval ? -retval
> -            : n == 0 ? 0
> -            : (stream->class->send)(stream, buffer, n));
> +    retval = retval ? -retval
> +             : n == 0 ? 0
> +             : (stream->class->send)(stream, buffer, n);
> +
> +    stream_replay_write(stream, buffer, retval, false);
> +    return retval;
>  }
>  
>  /* Allows 'stream' to perform maintenance activities, such as flushing
> @@ -483,6 +497,9 @@ pstream_lookup_class(const char *name, const struct pstream_class **classp)
>          if (strlen(class->name) == prefix_len
>              && !memcmp(class->name, name, prefix_len)) {
>              *classp = class;
> +            if (ovs_replay_get_state() == OVS_REPLAY_READ) {
> +                *classp = &preplay_pstream_class;
> +            }

Same nit here.

Thanks,
Dumitru