Re: (ITS#8719) slapd_crypt() become slow when many ldap cliant connections occur.

[hyc@symas.com]

Yoshinori Nishino wrote:
> Dear Howard-san,
>=20
>=20
> Thanks to your advice, I could create the attached tentative patches.

Thanks, this looks much better.
>=20
> They modifies servers/slapd/passwd.c and configure.in
> so that running "./configure" can check whether or not crypt_r() exists
> and use the function in slapd_crypt() if possible.
>=20
> I am going to check whether or not "calloc()/free()" causes
> unacceptable=E3=80=80memory fragmentation.

Never use calloc/malloc/free directly in slapd code. Always use=20
ch_calloc/ch_malloc/ch_free.

In this particular case, there's no reason to allocate at all. Just defin=
e=20
"struct crypt_data data" as a local variable.
>=20
>=20
> Best Regards,
> ------------------------------------------------
> On 2017/09/01 20:23, Yoshinori Nishino wrote:
>>
>>
>> On 2017/08/30 20:46, Yoshinori Nishino wrote:
>>> Dear Howard-san,
>>>
>>>
>>> Thank you so much for your prompt advice.
>>>
>>> As you told, I need to check whether or not crypt_r() exists.
>>> I am going to consider including the check.
>>>
>>> Would it be possible to let me know whether there are any other
>>> concerns about the fix if we can use crypt_r()?
>>>
>>> The implementation that "both calloc() and free() occur every time
>>> slapd_crypt() is called" does not seem to look good.
>>> The callers of slapd_crypt() might be able to get the memory for
>>> "struct crypt_data" before they call it.
>>> However, I think it causes the changes of other source codes.
>>> So, I think the aforementioned implementation is a workaround for the
>>> slowdown if the risk of memory fragmentation can be acceptable.
>>>
>>> On the other hands, I think we do not need to care about strcmp()
>>> because it is thread-safe.
>>>
>>>
>>> Best Regards,
>>> ----------------------------------------
>>> On 2017/08/30 19:26, Howard Chu wrote:
>>>> yos-nishino@ys.jp.nec.com wrote:
>>>>> Full_Name: Yoshinori Nishino
>>>>> Version: 2.4.45
>>>>> OS: CentOS 7
>>>>> URL: ftp://ftp.openldap.org/incoming/
>>>>> Submission from: (NULL) (210.143.35.20)
>>>>>
>>>>>
>>>>> Dear sir,
>>>>>
>>>>> The function slapd_crypt() in servers/slapd/passwd.c seems to becom=
e
>>>>> slow when
>>>>> many ldap client connections occur.
>>>>> It seems it is because the function uses crypt()(non thread-safe
>>>>> function) and
>>>>> pthread_mutex_lock(), which results in the slowdown.
>>>>> #Besides, we need to use {CRYPT} hash as users' password hash.
>>>>>
>>>>> So, I modified servers/slapd/passwd.c like the following.
>>>>> As a result, slapd_crypt() becomes much faster under the same
>>>>> condition.
>>>>> Would you let me know whether or not the fix is appropriate for sla=
pd?
>>>>
>>>> No it is not an appropriate fix.
>>>>
>>>> You should add an autoconf test to check for the existence of the
>>>> crypt_r function, and use an #ifdef here based on the result of that
>>>> test, since crypt_r is a non-standard function.
>>>>>
>=20
>=20


--=20
   -- Howard Chu
   CTO, Symas Corp.           http://www.symas.com
   Director, Highland Sun     http://highlandsun.com/hyc/
   Chief Architect, OpenLDAP  http://www.openldap.org/project/