[ovs-dev] [PATCH v2 2/9] stream: Add record/replay functionality.
Ilya Maximets
i.maximets at ovn.org
Thu May 27 13:01:29 UTC 2021
On 5/10/21 12:13 PM, Dumitru Ceara wrote:
> On 4/13/21 12:00 AM, 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 --replay-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>
>> ---
>> lib/automake.mk | 1 +
>> lib/stream-provider.h | 5 +
>> lib/stream-replay.c | 459 ++++++++++++++++++++++++++++++++++++++++++
>> lib/stream.c | 35 +++-
>> lib/stream.h | 12 ++
>> 5 files changed, 506 insertions(+), 6 deletions(-)
>> create mode 100644 lib/stream-replay.c
>>
>> diff --git a/lib/automake.mk b/lib/automake.mk
>> index b558692c6..db9017591 100644
>> --- a/lib/automake.mk
>> +++ b/lib/automake.mk
>> @@ -312,6 +312,7 @@ lib_libopenvswitch_la_SOURCES = \
>> lib/stream-fd.c \
>> lib/stream-fd.h \
>> lib/stream-provider.h \
>> + lib/stream-replay.c \
>> lib/stream-ssl.h \
>> lib/stream-tcp.c \
>> lib/stream.c \
>> diff --git a/lib/stream-provider.h b/lib/stream-provider.h
>> index 75f4f059b..44e3c6431 100644
>> --- a/lib/stream-provider.h
>> +++ b/lib/stream-provider.h
>> @@ -18,6 +18,7 @@
>> #define STREAM_PROVIDER_H 1
>>
>> #include <sys/types.h>
>> +#include "ovs-replay.h"
>> #include "stream.h"
>>
>> /* Active stream connection. */
>> @@ -29,6 +30,7 @@ struct stream {
>> const struct stream_class *class;
>> int state;
>> int error;
>> + replay_file_t replay_wfd;
>> char *name;
>> char *peer_id;
>> };
>> @@ -133,6 +135,7 @@ struct pstream {
>> const struct pstream_class *class;
>> char *name;
>> ovs_be16 bound_port;
>> + replay_file_t replay_wfd;
>> };
>>
>> void pstream_init(struct pstream *, const struct pstream_class *, char *name);
>> @@ -200,5 +203,7 @@ extern const struct pstream_class pwindows_pstream_class;
>> extern const struct stream_class ssl_stream_class;
>> extern const struct pstream_class pssl_pstream_class;
>> #endif
>> +extern const struct stream_class replay_stream_class;
>> +extern const struct pstream_class preplay_pstream_class;
>>
>> #endif /* stream-provider.h */
>> diff --git a/lib/stream-replay.c b/lib/stream-replay.c
>> new file mode 100644
>> index 000000000..ef591b920
>> --- /dev/null
>> +++ b/lib/stream-replay.c
>> @@ -0,0 +1,459 @@
>> +/*
>> + * Copyright (c) 2021, Red Hat, Inc.
>> + *
>> + * Licensed under the Apache License, Version 2.0 (the "License");
>> + * you may not use this file except in compliance with the License.
>> + * You may obtain a copy of the License at:
>> + *
>> + * http://www.apache.org/licenses/LICENSE-2.0
>> + *
>> + * Unless required by applicable law or agreed to in writing, software
>> + * distributed under the License is distributed on an "AS IS" BASIS,
>> + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
>> + * See the License for the specific language governing permissions and
>> + * limitations under the License.
>> + */
>> +
>> +#include <config.h>
>> +#include <ctype.h>
>> +#include <errno.h>
>> +#include <poll.h>
>> +#include <stdlib.h>
>> +#include <string.h>
>> +#include <sys/socket.h>
>> +#include <sys/types.h>
>> +#include <unistd.h>
>> +#include "ovs-atomic.h"
>> +#include "ovs-replay.h"
>> +#include "util.h"
>> +#include "stream-provider.h"
>> +#include "stream.h"
>> +#include "openvswitch/poll-loop.h"
>> +#include "openvswitch/vlog.h"
>> +
>> +VLOG_DEFINE_THIS_MODULE(stream_replay);
>> +
>> +/* Active replay stream. */
>> +
>> +struct stream_replay
>> +{
>
> Nit: Curly brace on the line above, please.
>
Fixed.
>> + struct stream stream;
>> + replay_file_t f;
>> + int seqno;
>> +};
>> +
>> +const struct stream_class replay_stream_class;
>> +
>> +/* Creates a new stream named 'name' that will emulate sending and receiving
>> + * data using replay file and stores a pointer to the stream in '*streamp'.
>> + *
>> + * Takes ownership of 'name'.
>> + *
>> + * Returns 0 if successful, otherwise a positive errno value. */
>> +static int
>> +new_replay_stream(char *name, struct stream **streamp)
>> +{
>> + struct stream_replay *s;
>> + int seqno = 0, error = 0, open_result;
>> + replay_file_t f;
>> +
>> + ovs_replay_lock();
>> + error = ovs_replay_file_open(name, &f, &seqno);
>> + if (error) {
>> + VLOG_ERR("%s: failed to open stream.", name);
>> + goto unlock;
>
> I didn't try this out but don't we leak 'name' here?
Thanks! I removed the "ownership" comments and made this function
to accept a const string just like the similar pstream function.
>
>> + }
>> +
>> + error = ovs_replay_read(f, NULL, 0, &open_result, &seqno, true);
>> + if (error) {
>> + VLOG_ERR("%s: failed to read 'open' record.", name);
>> + ovs_replay_file_close(f);
>> + goto unlock;
>
> Same here.
>
>> + }
>> +
>> + if (open_result) {
>> + error = -open_result;
>> + ovs_replay_file_close(f);
>> + goto unlock;
>
> Here too.
>
>> + }
>> +
>> + s = xmalloc(sizeof *s);
>> + stream_init(&s->stream, &replay_stream_class, 0, name);
And added xstrdup() here.
>> + s->f = f;
>> + s->seqno = seqno;
>> + *streamp = &s->stream;
>> +unlock:
>> + ovs_replay_unlock();
>> + return error;
>> +}
>> +
>> +static struct stream_replay *
>> +stream_replay_cast(struct stream *stream)
>> +{
>> + stream_assert_class(stream, &replay_stream_class);
>> + return CONTAINER_OF(stream, struct stream_replay, stream);
>> +}
>> +
>> +void
>> +stream_replay_open_wfd(struct stream *s, int open_result, const char *name)
>> +{
>> + int state = ovs_replay_get_state();
>> + int error = 0;
>> + replay_file_t f;
>> +
>> + if (OVS_LIKELY(state != OVS_REPLAY_WRITE)) {
>> + return;
>> + }
>> +
>> + ovs_replay_lock();
>> + error = ovs_replay_file_open(name, &f, NULL);
>> + if (error) {
>> + VLOG_ERR("%s: failed to open replay file for stream.", name);
>> + ovs_replay_unlock();
>> + return;
>> + }
>> + ovs_replay_unlock();
>> +
>> + if (ovs_replay_write(f, NULL, -open_result, true)) {
>> + VLOG_ERR("%s: failed to write 'open' failure: %d",
>> + s->name, open_result);
>> + }
>> + if (open_result) {
>> + /* We recorded failure to open the stream. */
>> + ovs_replay_file_close(f);
>> + } else {
>> + s->replay_wfd = f;
>> + }
>> +}
>> +
>> +void
>> +stream_replay_write(struct stream *s, const void *buffer, int n, bool is_read)
>> +{
>> + int state = ovs_replay_get_state();
>> +
>> + if (OVS_LIKELY(state != OVS_REPLAY_WRITE)) {
>> + return;
>> + }
>> +
>> + if (ovs_replay_write(s->replay_wfd, buffer, n, is_read)) {
>> + VLOG_ERR("%s: failed to write buffer.", s->name);
>
> ovs_replay_write() rate limits error logs, should we do that here too?
> AFAICT this applies to all VLOG_ERR() calls in this file.
Sure. I added rate-limits to all replay errors here and in ovs-replay.
>
>> + }
>> +}
>> +
>> +void
>> +stream_replay_close_wfd(struct stream *s)
>> +{
>> + if (s->replay_wfd) {
>> + ovs_replay_file_close(s->replay_wfd);
>> + }
>> +}
>> +
>> +static int
>> +stream_replay_open(const char *name, char *suffix OVS_UNUSED,
>> + struct stream **streamp, uint8_t dscp OVS_UNUSED)
>> +{
>> + return new_replay_stream(xstrdup(name), streamp);
>> +}
>> +
>> +static void
>> +stream_replay_close(struct stream *stream)
>> +{
>> + struct stream_replay *s = stream_replay_cast(stream);
>> + ovs_replay_file_close(s->f);
>> + free(s);
>> +}
>> +
>> +static ssize_t
>> +stream_replay_recv(struct stream *stream, void *buffer, size_t n)
>> +{
>> + struct stream_replay *s = stream_replay_cast(stream);
>> + int norm_seqno = ovs_replay_normalized_seqno(s->seqno);
>> + int error, len;
>> +
>> + ovs_replay_lock();
>> + ovs_assert(norm_seqno >= ovs_replay_seqno());
>> +
>> + if (norm_seqno != ovs_replay_seqno()
>> + || !ovs_replay_seqno_is_read(s->seqno)) {
>> + error = EAGAIN;
>> + goto unlock;
>> + }
>> +
>> + error = ovs_replay_read(s->f, buffer, n, &len, &s->seqno, true);
>> + if (error) {
>> + VLOG_ERR("%s: failed to read from replay file.", stream->name);
>> + goto unlock;
>> + }
>> +
>> +unlock:
>> + ovs_replay_unlock();
>> + return error ? -error : len;
>> +}
>> +
>> +static ssize_t
>> +stream_replay_send(struct stream *stream OVS_UNUSED,
>> + const void *buffer OVS_UNUSED, size_t n)
>> +{
>> + struct stream_replay *s = stream_replay_cast(stream);
>> + int norm_seqno = ovs_replay_normalized_seqno(s->seqno);
>> + int error, len;
>> +
>> + ovs_replay_lock();
>> + ovs_assert(norm_seqno >= ovs_replay_seqno());
>> +
>> + if (norm_seqno != ovs_replay_seqno()
>> + || ovs_replay_seqno_is_read(s->seqno)) {
>> + error = EAGAIN;
>> + goto unlock;
>> + }
>> +
>> + error = ovs_replay_read(s->f, NULL, 0, &len, &s->seqno, false);
>> + if (error) {
>> + VLOG_ERR("%s: failed to read from replay file.", stream->name);
>> + goto unlock;
>> + }
>> + ovs_assert(len < 0 || len <= n);
>> +
>> +unlock:
>> + ovs_replay_unlock();
>> + return error ? -error : len;
>> +}
>> +
>> +static void
>> +stream_replay_wait(struct stream *stream, enum stream_wait_type wait)
>> +{
>> + struct stream_replay *s = stream_replay_cast(stream);
>> + switch (wait) {
>> + case STREAM_CONNECT:
>> + /* Connect does nothing and always available. */
>> + poll_immediate_wake();
>> + break;
>> +
>> + case STREAM_SEND:
>> + if (s->seqno != INT_MAX && !ovs_replay_seqno_is_read(s->seqno)) {
>> + /* Stream waits for write. */
>> + poll_immediate_wake();
>> + }
>> + break;
>> +
>> + case STREAM_RECV:
>> + if (s->seqno != INT_MAX && ovs_replay_seqno_is_read(s->seqno)) {
>> + /* We still have something to read. */
>> + poll_immediate_wake();
>> + }
>> + break;
>> +
>> + default:
>> + OVS_NOT_REACHED();
>> + }
>> +}
>> +
>> +const struct stream_class replay_stream_class = {
>> + "replay", /* name */
>> + false, /* needs_probes */
>> + stream_replay_open, /* open */
>> + stream_replay_close, /* close */
>> + NULL, /* connect */
>> + stream_replay_recv, /* recv */
>> + stream_replay_send, /* send */
>> + NULL, /* run */
>> + NULL, /* run_wait */
>> + stream_replay_wait, /* wait */
>> +};
>> +
>> +/* Passive replay stream. */
>> +
>> +struct replay_pstream
>> +{
>> + struct pstream pstream;
>> + replay_file_t f;
>> + int seqno;
>> +};
>> +
>> +const struct pstream_class preplay_pstream_class;
>> +
>> +static struct replay_pstream *
>> +replay_pstream_cast(struct pstream *pstream)
>> +{
>> + pstream_assert_class(pstream, &preplay_pstream_class);
>> + return CONTAINER_OF(pstream, struct replay_pstream, pstream);
>> +}
>> +
>> +/* Creates a new pstream named 'name' that will accept new replay connections
>> + * reading them from the replay file and stores a pointer to the stream in
>> + * '*pstreamp'.
>> + *
>> + * Takes ownership of 'name'.
>> + *
>> + * Returns 0 if successful, otherwise a positive errno value. */
>> +static int
>> +pstream_replay_listen(const char *name, char *suffix OVS_UNUSED,
>> + struct pstream **pstreamp, uint8_t dscp OVS_UNUSED)
>> +{
>> + int seqno = 0, error = 0, listen_result;
>> + replay_file_t f;
>> +
>> + ovs_replay_lock();
>> + error = ovs_replay_file_open(name, &f, &seqno);
>> + if (error) {
>> + VLOG_ERR("%s: failed to open pstream.", name);> + goto unlock;
>
> 'name' is leaked here.
>
>> + }
>> +
>> + error = ovs_replay_read(f, NULL, 0, &listen_result, &seqno, true);
>> + if (error) {
>> + VLOG_ERR("%s: failed to read 'listen' record.", name);
>> + ovs_replay_file_close(f);
>> + goto unlock;
>
> Same here.
>
>> + }
>> +
>> + if (listen_result) {
>> + error = -listen_result;
>> + ovs_replay_file_close(f);
>> + goto unlock;
>
> Here too.
>
>> + }
>> +
>> + struct replay_pstream *ps = xmalloc(sizeof *ps);
>> + pstream_init(&ps->pstream, &preplay_pstream_class, xstrdup(name));
>
> I guess the xstrdup(name) is not needed, otherwise we don't really take
> ownership of 'name' and we actually leak it.
>
>> + ps->f = f;
>> + ps->seqno = seqno;
>> + *pstreamp = &ps->pstream;
>> +unlock:
>> + ovs_replay_unlock();
>> + return error;
>> +}
>> +
>> +void
>> +pstream_replay_open_wfd(struct pstream *ps, int listen_result,
>> + const char *name)
>> +{
>> + int state = ovs_replay_get_state();
>> + int error = 0;
>> + replay_file_t f;
>> +
>> + if (OVS_LIKELY(state != OVS_REPLAY_WRITE)) {
>> + return;
>> + }
>> +
>> + ovs_replay_lock();
>> + error = ovs_replay_file_open(name, &f, NULL);
>> + if (error) {
>> + VLOG_ERR("%s: failed to open replay file for pstream.", name);
>> + ovs_replay_unlock();
>> + return;
>> + }
>> + ovs_replay_unlock();
>> +
>> + if (ovs_replay_write(f, NULL, -listen_result, true)) {
>> + VLOG_ERR("%s: failed to write 'listen' result: %d",
>> + ps->name, listen_result);
>> + }
>> +
>> + if (listen_result) {
>> + /* We recorded failure to open the stream. */
>> + ovs_replay_file_close(f);
>> + } else {
>> + ps->replay_wfd = f;
>> + }
>> +}
>> +
>> +
>> +void
>> +pstream_replay_write_accept(struct pstream *ps, const struct stream *s,
>> + int accept_result)
>> +{
>> + int state = ovs_replay_get_state();
>> + int len;
>> +
>> + if (OVS_LIKELY(state != OVS_REPLAY_WRITE)) {
>> + return;
>> + }
>> +
>> + if (!accept_result) {
>> + len = strlen(s->name);
>> + if (ovs_replay_write(ps->replay_wfd, s->name, len, true)) {
>> + VLOG_ERR("%s: failed to write accept name: %s", ps->name, s->name);
>> + }
>> + } else if (ovs_replay_write(ps->replay_wfd, NULL, -accept_result, true)) {
>> + VLOG_ERR("%s: failed to write 'accept' failure: %d",
>> + ps->name, accept_result);
>> + }
>> +}
>> +
>> +void
>> +pstream_replay_close_wfd(struct pstream *ps)
>> +{
>> + if (ps->replay_wfd) {
>> + ovs_replay_file_close(ps->replay_wfd);
>> + }
>> +}
>> +
>> +
>
> Nit: I'd remove the second newline.
Done.
>
> Thanks,
> Dumitru
>
More information about the dev
mailing list