messaging3: Avoid self-send complexity

With the notify code I've hit another case where self-sends caused
a problem.  This time messages were lost because we tried to do
multiple dispatch_rec calls from within a single inotify callback.
Only the first one was being taken care of, the others did not find
receivers.

This patch makes self-sends go through the kernel as well, the
kernel queues everything nicely for us. With dgram messaging this
should be pretty fast. If it turns out to be a performance problem,
we can solve it later by doing proper queueing in user space. We
need to completely decouple any processing from callbacks.

Signed-off-by: Volker Lendecke <vl@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>

diff --git a/source3/lib/messages.c b/source3/lib/messages.c
index 5b4daa2932679434a6572d989f7e2f1388c76ff3..1fd7601f829fe0bbcc5999d17e3200ff0a670b8d 100644 (file)
--- a/source3/lib/messages.c
+++ b/source3/lib/messages.c
@@ -507,36 +507,6 @@ NTSTATUS messaging_send_iov(struct messaging_context *msg_ctx,
                return NT_STATUS_OK;
        }
 
-       if (server_id_same_process(&msg_ctx->id, &server)) {
-               struct messaging_rec rec;
-               uint8_t *buf;
-
-               /*
-                * Self-send, directly dispatch
-                */
-
-               if (num_fds > 0) {
-                       return NT_STATUS_NOT_SUPPORTED;
-               }
-
-               buf = iov_buf(talloc_tos(), iov, iovlen);
-               if (buf == NULL) {
-                       return NT_STATUS_NO_MEMORY;
-               }
-
-               rec = (struct messaging_rec) {
-                       .msg_version = MESSAGE_VERSION,
-                       .msg_type = msg_type & MSG_TYPE_MASK,
-                       .dest = server,
-                       .src = msg_ctx->id,
-                       .buf = data_blob_const(buf, talloc_get_size(buf)),
-               };
-
-               messaging_dispatch_rec(msg_ctx, &rec);
-               TALLOC_FREE(buf);
-               return NT_STATUS_OK;
-       }
-
        ZERO_STRUCT(hdr);
        hdr = (struct messaging_hdr) {
                .msg_type = msg_type,
@@ -826,68 +796,6 @@ static bool messaging_append_new_waiters(struct messaging_context *msg_ctx)
        return true;
 }
 
-struct messaging_defer_callback_state {
-       struct messaging_context *msg_ctx;
-       struct messaging_rec *rec;
-       void (*fn)(struct messaging_context *msg, void *private_data,
-                  uint32_t msg_type, struct server_id server_id,
-                  DATA_BLOB *data);
-       void *private_data;
-};
-
-static void messaging_defer_callback_trigger(struct tevent_context *ev,
-                                            struct tevent_immediate *im,
-                                            void *private_data);
-
-static void messaging_defer_callback(
-       struct messaging_context *msg_ctx, struct messaging_rec *rec,
-       void (*fn)(struct messaging_context *msg, void *private_data,
-                  uint32_t msg_type, struct server_id server_id,
-                  DATA_BLOB *data),
-       void *private_data)
-{
-       struct messaging_defer_callback_state *state;
-       struct tevent_immediate *im;
-
-       state = talloc(msg_ctx, struct messaging_defer_callback_state);
-       if (state == NULL) {
-               DEBUG(1, ("talloc failed\n"));
-               return;
-       }
-       state->msg_ctx = msg_ctx;
-       state->fn = fn;
-       state->private_data = private_data;
-
-       state->rec = messaging_rec_dup(state, rec);
-       if (state->rec == NULL) {
-               DEBUG(1, ("talloc failed\n"));
-               TALLOC_FREE(state);
-               return;
-       }
-
-       im = tevent_create_immediate(state);
-       if (im == NULL) {
-               DEBUG(1, ("tevent_create_immediate failed\n"));
-               TALLOC_FREE(state);
-               return;
-       }
-       tevent_schedule_immediate(im, msg_ctx->event_ctx,
-                                 messaging_defer_callback_trigger, state);
-}
-
-static void messaging_defer_callback_trigger(struct tevent_context *ev,
-                                            struct tevent_immediate *im,
-                                            void *private_data)
-{
-       struct messaging_defer_callback_state *state = talloc_get_type_abort(
-               private_data, struct messaging_defer_callback_state);
-       struct messaging_rec *rec = state->rec;
-
-       state->fn(state->msg_ctx, state->private_data, rec->msg_type, rec->src,
-                 &rec->buf);
-       TALLOC_FREE(state);
-}
-
 /*
   Dispatch one messaging_rec
 */
@@ -914,24 +822,9 @@ void messaging_dispatch_rec(struct messaging_context *msg_ctx,
                rec->num_fds = 0;
                rec->fds = NULL;
 
-               if (server_id_same_process(&rec->src, &rec->dest)) {
-                       /*
-                        * This is a self-send. We are called here from
-                        * messaging_send(), and we don't want to directly
-                        * recurse into the callback but go via a
-                        * tevent_loop_once
-                        */
-                       messaging_defer_callback(msg_ctx, rec, cb->fn,
-                                                cb->private_data);
-               } else {
-                       /*
-                        * This comes from a different process. we are called
-                        * from the event loop, so we should call back
-                        * directly.
-                        */
-                       cb->fn(msg_ctx, cb->private_data, rec->msg_type,
-                              rec->src, &rec->buf);
-               }
+               cb->fn(msg_ctx, cb->private_data, rec->msg_type,
+                      rec->src, &rec->buf);
+
                /*
                 * we continue looking for matching messages after finding
                 * one. This matters for subsystems like the internal notify

diff --git a/source3/torture/test_messaging_fd_passing.c b/source3/torture/test_messaging_fd_passing.c
index abd142fdd378e48e5f834114863ef70aed934ecf..7bee41b5986719a81981088d657730ab38871616 100644 (file)
--- a/source3/torture/test_messaging_fd_passing.c
+++ b/source3/torture/test_messaging_fd_passing.c
@@ -63,7 +63,7 @@ bool run_messaging_fdpass1(int dummy)
 
        status = messaging_send_iov(msg_ctx, dst, MSG_PING, NULL, 0,
                                    pass_fds, 1);
-       if (!NT_STATUS_EQUAL(status, NT_STATUS_NOT_SUPPORTED)) {
+       if (!NT_STATUS_EQUAL(status, NT_STATUS_OK)) {
                fprintf(stderr,
                        "messaging_send_iov gave: %s\n", nt_errstr(status));
                goto fail;

diff --git a/source3/torture/test_messaging_read.c b/source3/torture/test_messaging_read.c
index 9c4017c36f2b2b6906de89fbb865a540af52157e..802b4fe734ff01a14c6cf7e928b6761848cc208e 100644 (file)
--- a/source3/torture/test_messaging_read.c
+++ b/source3/torture/test_messaging_read.c
@@ -122,7 +122,7 @@ bool run_messaging_read1(int dummy)
                goto fail;
        }
 
-       for (i=0; i<2; i++) {
+       for (i=0; i<3; i++) {
                if (tevent_loop_once(ev) != 0) {
                        fprintf(stderr, "tevent_loop_once failed\n");
                        goto fail;