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

Jakub Hrozek jhrozek at redhat.com
Fri Oct 25 13:45:41 UTC 2013


On Fri, Oct 25, 2013 at 03:31:58PM +0200, Sumit Bose wrote:
> 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

Pushed to master and sssd-1-11



More information about the sssd-devel mailing list