Eugene Indenbom wrote:
Thank you very much for your contribution!
It is really great and we appreciate your effort.
However I just wanted to mention that Simo is on vacation this week and
I know for sure that he would want to look at these patches before they
The patches look well structured now so it would be easier to review and
discuss them but it will most likely not happen this week.
Sorry for delay.
I have finally finished my work on the patch refactoring. All the
changes made are structured into 4 separate patches:
All of them are made by git format-patch with all the patience git has
managed to find. :)
I have taken into all the notes and suggestion from your previous
e-mails, but now I want to start from a blank sheet. The work has been
substantially redone and old quotations take to much room.
This patch adds expire_time member to sdap_handle. expire_time is the
time when connection is expected to expire. Currently the time is set
only for GSSAPI SASL mech. For other currently supported
authentication types connection never expires and expire_time is set
Having this time at hand saves from complicated query of kerberos
ticket cache (see sdap_check_gssapi_reconnect) and is more reliable as
kerberos ticket cache could be modified since time of connection.
This patch simply adds an interface to query number of configured
failover servers. The interface will be really useful when DNS SRV
record service discovery will be supported. I have added it as a
provision for forthcoming dynamic failover configuration.
This patch fixes a critical bug of accessing freed memory in
sdap_handle_release. The problem can be reproduced as follows:
1. Break LDAP connection when LDAP query is in progress (e.g. by
restarting directory server)
2. sdap_process_result detects communication error and calls
3. sdap_handle_release calls active sdap_op callback
4. callback deletes sdap_handle
5. sdap_release_handle segfaults or asserts on access to deallocated
I have solved the problem by splitting private data from sdap_handle
into separate structure sdap_handle_data. The deallocation of
sdap_handle_data is delayed until all enumeration of active ops chain
are finished (currently only sdap_handle_release enumerates
operations). The destruction protected region is scoped by
sdap_handle_release contains also other assorted changes to prevent
double data deallocation.
This patch contains refactoring of reconnect retry logic for LDAP
operations. The ldap_common.c/.h contain reconnect retry API and
implementation, while changes in ipa_access.c, ldap_id.c and
ldap_id_enum.c convert existing LDAP queries to new API.
The current reconnect logic does not address the following issues:
- There is no failover retries after connection breakdown during
- It is possible that 2 connections would be opened in parallel when
2 operations are executed concurrently
The primary entry points of new reconnect retry framework are:
sdap_id_op_connect and sdap_id_op_done. The reconnect retry usage can
be outlined as follows:
1. Operation creates an op handle with sdap_id_op_create
2. When LDAP connection is needed operation call sdap_id_op_connect
- attaches cached connection to sdap_id_op if available
- or starts a new connection and returns tevent_req for
4. When asynchronous connection request is completed operation calls
sdap_id_op_connect_recv to get connection result
5. When operation is done with connection it calls sdap_id_op_done to:
- notify that LDAP connection is no longer in use
- check whether reconnect retry is allowed
6. If reconnect retry is allowed operation restarts at step 2.
Therefore sdap_id_op_connect and sdap_id_op_done define a scope, when
LDAP connection is in use by operation. This allows to organize
effective and transparent connection caching. More over further
changes to caching strategy would not affect operation code.
The connection caching strategy employed by patch is simplistic:
1. The connection is established on demand
2. A single connection is cached and discarded from cache when:
- it is broken or
- it is about to expire
3. Cached connection is reused (concurrently if needed) by all other
operations while it is valid
4. The connection is closed when:
- it is not cached and
- it is not in use
So typically connection is established on first LDAP query and kept
until kerberos ticket is expired. Then the connection is reestablished
with next LDAP query. During normal operation there is strictly one
The number of reconnect retries for each operation is determined as
1. Reconnect retries are stopped and backend put offline when there is
no more servers to try.
2. There is an absolute limit on retries performed per operation. This
is required to handle the case when during each retry the connection
is successfully established, but breaks before any results are
returned. In such a scenario failover logic will try indefinitely one
and the same server.
3. The absolute retry limit is calculated as 2 x <number of servers>
- one established and broken midway connection to each server
(e.g. reused cached connection)
- one failed connection attempt to this server, after which
failover logic moves to next server in the list
And finally it is possible that blackout time for first failed server
to expire before last failover server is tried. This is also a
possibility for creation of infinite retry loop.
All changes together has been tested by me as follows:
1. I have tested each operation separately:
- sdap_account_info: user info, group info, user groups (see
- enum_users and enum_groups (see ldap_id_enum.c)
- hbac_get_host_info and hbac_get_rules (see ipa_access.c)
2. In the following scenarios:
- normal operation
- 1 retry on server failure
- switch to offline during operation after all trying all servers
Currently patches are being tested during overnight operation.
sssd-devel mailing list
Engineering Manager IPA project,
Red Hat Inc.
Looking to carve out IT costs?