On Tue, Jan 12, 2016 at 10:30:29AM +0100, Petr Cech wrote: > On 01/06/2016 02:19 PM, Jakub Hrozek wrote: > >On Wed, Jan 06, 2016 at 11:03:45AM +0100, Sumit Bose wrote: > >>On Wed, Jan 06, 2016 at 10:47:13AM +0100, Pavel Březina wrote: > >>>On 01/05/2016 05:33 PM, Jakub Hrozek wrote: > >>>>On Tue, Jan 05, 2016 at 02:12:51PM +0100, Pavel Březina wrote: > >>>>>On 12/14/2015 03:36 PM, Petr Cech wrote: > >>>>>>Hi all, > >>>>>> > >>>>>>there is patch for https://fedorahosted.org/sssd/ticket/2791 attached. > >>>>>> > >>>>>>Result of patch: > >>>>>> > >>>>>>The message: > >>>>>>Dec 14 14:16:11 vm-058-166 sssd[be[uma.dev]]: dereference processing > >>>>>>failed : Input/output error > >>>>>>is replaced by > >>>>>>Dec 14 15:29:26 vm-058-166 sssd[be[uma.dev]]: LDAP server claims to > >>>>>>support deref, but deref search failed. Disabling deref for further > >>>>>>requests. You can permanently disable deref by setting > >>>>>>ldap_deref_threshold to 0 in domain configuration. > >>>>>> > >>>>>>But I am little afraid of this patch. Why? I tested this work on RHEL 6 > >>>>>>how it is described in the ticket. > >>>>>>If I tried to set > >>>>>> ldap_deref_threshold = 0 > >>>>>>it was still falling into bad case and printed I/O error. So, if we > >>>>>>considered patch is right, there is question if advice in new message is > >>>>>>valid for IPA 3.x. > >>>>>> > >>>>>>Regards > >>>>>> > >>>>>>Petr > >>>>> > >>>>>This threshold parameter applies for groups membership, not for views > >>>>>where > >>>>>dereference is always used in current code. > >>>>> > >>>>>The question is, why does sdap_x_deref_search_send fail with EIO? > >>>>>Dereference should be supported with IPA... > >>>> > >>>>Because the dereferenced attribute doesn't exist on the server IIRC. > >>> > >>>Then the debug message is misleading since dereference is in fact working. > >>>We should return different error code if possible (ENOENT?) and this case > >>>should be handled in views code. > >> > >>Unfortunately there is no specific LDAP error code for this case, see > >>e.g. comments in a50b229c8ea1e22c9efa677760b94d8c48c3ec89. Different > >>versions of 389ds return LDAP_UNAVAILABLE_CRITICAL_EXTENSION or > >>LDAP_PROTOCOL_ERROR, not sure what OpenLDAP does. > >> > >>I think the error handling code should be moved to the callers because > >>only they know how to handle different error conditions. > > > >+1 > > > >we can also translate the LDAP-specific error codes into sssd private > >error codes instead of overloading ENOENT/EIO/ENOSUP (ENOSUP is IMO the > >only one that really fits the problem..) > > > >> > >>As an alternative now options/flags can be added to > >>sdap_deref_search_send() as it was done for sdap_get_generic_ext_send() > >>some time ago, but I would prefer the other. > > > >No, I also prefer to let the low-level function be dumb and just return > >an error which the caller deals with. > > Hi, > > thanks for all comments. > > I found processing of LDAP_UNAVAILABLE_CRITICAL_EXTENSION at > sdap_get_generic_op_finished() function. There is no processing of > LDAP_PROTOCOL_ERROR. So I added it. > > Now, the error codes for LDAP_RES_SEARCH_RESULT are ENOTSUP for our case and > EIO for the others. > > Next processing of those EIO and ENOTSUP is in sdap_deref_search_done(), > it is added by 4709ff46db0dbe073aef061b796d2fd7adeaf18f commit. I am not > sure, after Pavel Brezina comment, if the debug message for ENOTSUP case is > right here. > > Attached patch is applicable for master and sssd-1-12 too.
I'm sorry, but this is not the right thing to do. If the request errors out with ENOTSUP, we print a log message, too, just different, but on top of that we also disable dereference.. I don't think the problem can be solved at the lowest sdap level, but at the callers, similar to how Pavel solved the 'size exceeded' error in commit 1f1b41931d299d6356ac205b75b402adb2cc9234. We should add a flag to sdap_deref_search_state that would default to zero but would allow the caller to suppress the warnings. In sdap_deref_search_send() I think we can always leave the flag to zero, but in sdap_deref_search_with_filter_send() we should expose the flag so that the caller can set the suppress warnings flag. This would be evaluated in sdap_deref_search_done() and if the flag is set, then the sss_log() function calls would not be called. I hope this is doable in a couple of hours. Feel free to ping me if you need some help. _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/sssd-devel@lists.fedorahosted.org
