drepl: Support GET_TGT on periodic replication client
authorTim Beale <timbeale@catalyst.net.nz>
Thu, 15 Jun 2017 04:52:33 +0000 (16:52 +1200)
committerAndrew Bartlett <abartlet@samba.org>
Fri, 18 Aug 2017 04:07:12 +0000 (06:07 +0200)
- Update IDL comments to include Microsoft reference doc
- Add support for sending v10 GetNCChanges request (needed for the
  GET_TGT flag, which is in the new 'more_flags' field)
- Update to also set the GET_TGT flag in the same place we were setting
  GET_ANC (I split this logic out into a separate function).
- The state struct now needs to hold a 'more_flags' field as well (this
  flag is different to the GET_ANC replica flag)

Note that using the GET_TGT when replicating from a Windows DC could be
highly inefficient. Because Samba keeps the GET_TGT flag set throughout
the replication cycle, it will basically receive a repeated object from
Windows for every single linked attribute that it receives.

I believe Windows behaviour only expects the client to set the GET_TGT
flag when it actually needs to (i.e. when it receives a target object it
doesn't know about), rather than throughout the replication cycle.
However, this approach won't work with Samba-to-Samba replication,
because when the server receives the GET_TGT flag it restarts the
replication cycle from scratch. So if we only set the GET_TGT flag when
the client encountered an unknown target then Samba-to-Samba could
potentially get into an endless replication loop.

Signed-off-by: Tim Beale <timbeale@catalyst.net.nz>
Reviewed-by: Garming Sam <garming@samba.org>
Reviewed-by: Andrew Bartlett <abartlet@samba.org>
BUG: https://bugzilla.samba.org/show_bug.cgi?id=12972

librpc/idl/drsuapi.idl
source4/dsdb/repl/drepl_out_helpers.c
source4/dsdb/repl/drepl_out_pull.c
source4/dsdb/repl/drepl_service.h

index 2eb1d655f89e05d788176c96d75509db30cd815b..51ef567c807a671a7d42acaeec6fb3acf330526c 100644 (file)
@@ -70,7 +70,9 @@ interface drsuapi
         } drsuapi_DrsUpdate;
 
        /*****************/
-        /* Function 0x00 */
+        /* Function 0x00 drsuapi_DsBind() */
+
+        /* MS-DRSR 5.39 DRS_EXTENSIONS_INT */
         typedef [bitmap32bit] bitmap {
                DRSUAPI_SUPPORTED_EXTENSION_BASE                        = 0x00000001,
                DRSUAPI_SUPPORTED_EXTENSION_ASYNC_REPLICATION           = 0x00000002,
index ad4801adec0bc135a1e68d4ea8511cb21651bd0e..6e24e7f8de32846f89bad3c7f2a4ce0a85babff5 100644 (file)
@@ -555,7 +555,28 @@ static void dreplsrv_op_pull_source_get_changes_trigger(struct tevent_req *req)
        }
 
        r->in.bind_handle       = &drsuapi->bind_handle;
-       if (drsuapi->remote_info28.supported_extensions & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8) {
+
+       if (drsuapi->remote_info28.supported_extensions & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V10) {
+               r->in.level                             = 10;
+               r->in.req->req10.destination_dsa_guid   = service->ntds_guid;
+               r->in.req->req10.source_dsa_invocation_id= rf1->source_dsa_invocation_id;
+               r->in.req->req10.naming_context         = &partition->nc;
+               r->in.req->req10.highwatermark          = highwatermark;
+               r->in.req->req10.uptodateness_vector    = uptodateness_vector;
+               r->in.req->req10.replica_flags          = replica_flags;
+               r->in.req->req10.max_object_count       = 133;
+               r->in.req->req10.max_ndr_size           = 1336811;
+               r->in.req->req10.extended_op            = state->op->extended_op;
+               r->in.req->req10.fsmo_info              = state->op->fsmo_info;
+               r->in.req->req10.partial_attribute_set  = pas;
+               r->in.req->req10.partial_attribute_set_ex= NULL;
+               r->in.req->req10.mapping_ctr.num_mappings= mappings == NULL ? 0 : mappings->num_mappings;
+               r->in.req->req10.mapping_ctr.mappings   = mappings == NULL ? NULL : mappings->mappings;
+
+               /* the only difference to v8 is the more_flags */
+               r->in.req->req10.more_flags = state->op->more_flags;
+
+       } else if (drsuapi->remote_info28.supported_extensions & DRSUAPI_SUPPORTED_EXTENSION_GETCHGREQ_V8) {
                r->in.level                             = 8;
                r->in.req->req8.destination_dsa_guid    = service->ntds_guid;
                r->in.req->req8.source_dsa_invocation_id= rf1->source_dsa_invocation_id;
@@ -693,6 +714,53 @@ static void dreplsrv_op_pull_source_get_changes_done(struct tevent_req *subreq)
        dreplsrv_op_pull_source_apply_changes_trigger(req, r, ctr_level, ctr1, ctr6);
 }
 
+/**
+ * If processing a chunk of replication data fails, check if it is due to a
+ * problem that can be fixed by setting extra flags in the GetNCChanges request,
+ * i.e. GET_ANC or GET_TGT.
+ * @returns NT_STATUS_OK if the request was retried, and an error code if not
+ */
+static NTSTATUS dreplsrv_op_pull_retry_with_flags(struct tevent_req *req,
+                                                 WERROR error_code)
+{
+       struct dreplsrv_op_pull_source_state *state;
+       NTSTATUS nt_status = NT_STATUS_OK;
+
+       state = tevent_req_data(req, struct dreplsrv_op_pull_source_state);
+
+       /*
+        * Check if we failed to apply the records due to a missing parent or
+        * target object. If so, try again and ask for any mising parent/target
+        * objects to be included this time.
+        */
+       if (W_ERROR_EQUAL(error_code, WERR_DS_DRA_RECYCLED_TARGET)) {
+
+               if (state->op->more_flags & DRSUAPI_DRS_GET_TGT) {
+                       DEBUG(1,("Missing target object despite setting DRSUAPI_DRS_GET_TGT flag\n"));
+                       nt_status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+               } else {
+                       state->op->more_flags |= DRSUAPI_DRS_GET_TGT;
+                       DEBUG(1,("Missing target object when we didn't set the DRSUAPI_DRS_GET_TGT flag, retrying\n"));
+                       dreplsrv_op_pull_source_get_changes_trigger(req);
+               }
+       } else if (W_ERROR_EQUAL(error_code, WERR_DS_DRA_MISSING_PARENT)) {
+
+               if (state->op->options & DRSUAPI_DRS_GET_ANC) {
+                       DEBUG(1,("Missing parent object despite setting DRSUAPI_DRS_GET_ANC flag\n"));
+                       nt_status = NT_STATUS_INVALID_NETWORK_RESPONSE;
+               } else {
+                       state->op->options |= DRSUAPI_DRS_GET_ANC;
+                       DEBUG(4,("Missing parent object when we didn't set the DRSUAPI_DRS_GET_ANC flag, retrying\n"));
+                       dreplsrv_op_pull_source_get_changes_trigger(req);
+               }
+       } else {
+               nt_status = werror_to_ntstatus(WERR_BAD_NET_RESP);
+       }
+
+       return nt_status;
+}
+
+
 static void dreplsrv_update_refs_trigger(struct tevent_req *req);
 
 static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req,
@@ -937,25 +1005,20 @@ static void dreplsrv_op_pull_source_apply_changes_trigger(struct tevent_req *req
        if (!W_ERROR_IS_OK(status)) {
 
                /*
-                * If we failed to apply the records due to a missing
-                * parent, try again after asking for the parent
-                * records first.  Because we don't update the
-                * highwatermark, we start this part of the cycle
-                * again.
+                * Check if this error can be fixed by resending the GetNCChanges
+                * request with extra flags set (i.e. GET_ANC/GET_TGT)
                 */
-               if (((state->op->options & DRSUAPI_DRS_GET_ANC) == 0)
-                   && W_ERROR_EQUAL(status, WERR_DS_DRA_MISSING_PARENT)) {
-                       state->op->options |= DRSUAPI_DRS_GET_ANC;
-                       DEBUG(4,("Missing parent object when we didn't set the DRSUAPI_DRS_GET_ANC flag, retrying\n"));
-                       dreplsrv_op_pull_source_get_changes_trigger(req);
+               nt_status = dreplsrv_op_pull_retry_with_flags(req, status);
+
+               if (NT_STATUS_IS_OK(nt_status)) {
+
+                       /*
+                        * We resent the request. Don't update the highwatermark,
+                        * we'll start this part of the cycle again.
+                        */
                        return;
-               } else if (((state->op->options & DRSUAPI_DRS_GET_ANC))
-                          && W_ERROR_EQUAL(status, WERR_DS_DRA_MISSING_PARENT)) {
-                       DEBUG(1,("Missing parent object despite setting DRSUAPI_DRS_GET_ANC flag\n"));
-                       nt_status = NT_STATUS_INVALID_NETWORK_RESPONSE;
-               } else {
-                       nt_status = werror_to_ntstatus(WERR_BAD_NET_RESP);
                }
+
                DEBUG(0,("Failed to commit objects: %s/%s\n",
                          win_errstr(status), nt_errstr(nt_status)));
                tevent_req_nterror(req, nt_status);
index 8b8ecd90d8937792ce12457ddca92e714402b356..62a403c91c94545b780d698b79bd9b3db29d2426 100644 (file)
@@ -126,6 +126,7 @@ WERROR dreplsrv_schedule_partition_pull_source(struct dreplsrv_service *s,
        op->callback    = callback;
        op->cb_data     = cb_data;
        op->schedule_time = time(NULL);
+       op->more_flags  = 0;
 
        DLIST_ADD_END(s->ops.pending, op);
 
index 4b0e43feaa78b0621c43160d3d33e13d82f1dd49..8c44e190a74aab5726f32bb89b7dd099763f8568 100644 (file)
@@ -130,6 +130,8 @@ struct dreplsrv_out_operation {
        enum drsuapi_DsExtendedError extended_ret;
        dreplsrv_extended_callback_t callback;
        void *cb_data;
+       /* more replication flags - used by DsReplicaSync GET_TGT */
+       uint32_t more_flags;
 };
 
 struct dreplsrv_notify_operation {