[SSSD] [PATCH] AD: fall back to LDAP if GC is not available.

Sumit Bose sbose at redhat.com
Fri Oct 25 13:31:58 UTC 2013


On Fri, Oct 25, 2013 at 02:46:31PM +0200, Lukas Slebodnik wrote:
> On (25/10/13 11:54), Sumit Bose wrote:
> >On Thu, Oct 24, 2013 at 11:54:30AM +0200, Lukas Slebodnik wrote:
> >> ehlo,
> >> 
> >> Global catalog port was ignored from SRV record, hard coded value was used
> >> every time.
> >> 1st patch should fix it.
> >
> >good catch, but see my comment below.
> >
> >> 
> >> 2nd patch add fall back to LDAP if GC is not available.
> >> with this patch SSSD will not go offline if GC is not available,
> >> but it may happen that SSSD will try to connect to GC often.
> >> I am not sure, but we can add new option do disable GC lookup completely.
> >
> >The patch is working as expected, so ACK. I still can see some debug
> >messages saying that SSD will go offline before the fallback to LDAP
> >happens, but I think this is aceptable.
> >
> I forgot to move this debug message inside the if statement.
> and debug message has high debug level SSSDBG_CRIT_FAILURE.
> It make sense.
> 
> >I don't think that we need an option to disable the GC lookup
> >completely. Since we are doing SRV lookups to find the GC server there
> >should be always one wokring GC. If not, there is either a firewall
> >issue or the AD domain has other problems.
> >
> Thank you for review.
> 
> >bye,
> >Sumit
> >
> >> 
> >> LS
> >
> >> From abf7cfeaa9826fb52e9a18d3bd5812003c38f118 Mon Sep 17 00:00:00 2001
> >> From: Lukas Slebodnik <lslebodn at redhat.com>
> >> Date: Thu, 24 Oct 2013 11:03:32 +0200
> >> Subject: [PATCH 1/3] AD: Prefer GC port from SRV record
> >> 
> >> We had a hard coded value of Global Catalog port (3268).
> >> Informations from SRV record was ignored.
> >> This patch prefer port number from SRV record and hard coded value is used only
> >> as a fall back if port number was not initialized.
> >> ---
> >>  src/providers/ad/ad_common.c | 7 +++++--
> >>  1 file changed, 5 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/src/providers/ad/ad_common.c b/src/providers/ad/ad_common.c
> >> index ab62d64a943a726724698d1b7eb9f15bc40b389e..1c4bd088b30eed4f71a787ce4c3bd60366b72def 100644
> >> --- a/src/providers/ad/ad_common.c
> >> +++ b/src/providers/ad/ad_common.c
> >> @@ -729,12 +729,15 @@ ad_resolve_callback(void *private_data, struct fo_server *server)
> >>      talloc_zfree(service->gc->uri);
> >>      talloc_zfree(service->gc->sockaddr);
> >>      if (sdata && sdata->gc) {
> >> +        int new_port = fo_get_server_port(server);
> >
> >Please move the declaration to the top of the function (see
> >http://www.freeipa.org/page/Coding_Style)
> >
> Fixed.
> (I thought that declaration at the beginning of block is allowed.)
> 
> >> +        new_port = (new_port == 0) ? AD_GC_PORT : new_port;
> >> +
> >>          service->gc->uri = talloc_asprintf(service->gc, "%s:%d",
> >> -                                           new_uri, AD_GC_PORT);
> >> +                                           new_uri, new_port);
> >>  
> >>          service->gc->sockaddr = resolv_get_sockaddr_address(service->gc,
> >>                                                              srvaddr,
> >> -                                                            AD_GC_PORT);
> >> +                                                            new_port);
> >>      } else {
> >>          /* Make sure there always is an URI even if we know that this
> >>           * server doesn't support GC. That way the lookup would go through
> >> -- 
> >> 1.8.3.1
> >> 
> 
> Updated patches are attached.

ACK

bye,
Sumit

> 
> LS



More information about the sssd-devel mailing list