[PATCH REVIEW] s3-auth

[Andreas Schneider]

On Tuesday 21 June 2011 12:04:52 you wrote:
> On Mon, 2011-06-20 at 17:19 -0700, Jeremy Allison wrote:
> > On Thu, Jun 16, 2011 at 09:42:04PM +0200, Andreas Schneider wrote:
> > > Hello,
> > > 
> > > I have a patchset which touches s3-auth so it would be nice if
> > > someone of you could review it.
> > > 
> > > The patchset does the following things:
> > > 
> > > * Reduces the dependencies (lp_load instead of reload_services,
> > > 
> > >   smbd/globals.h).
> > > 
> > > * Introduces a new function get_remote_hostname().
> > > 
> > > * Removes the usage of the global variable smbd_server_conn from
> > > auth.
> > > 
> > > * Passes down the remote_address to the places where it is needed.
> > > 
> > > * Removes 'struct client_address' and replaces it with
> > > 
> > >   'struct tsocket_address'
> 
> The other goals seem very reasonable, but what is the particular benefit
> of this change?

Cause a lot of function use already tsocket_address. It is common code and 
more and more dcerpc code will use it.

> Andreas,
> 
> I reviewed this while it was under development (while browsing Andreas's
> repo), and forgot to mention then that it looked like a good change.
> 
> I also tried to send my positive review (at least as regards the auth
> changes) last week, but somehow I didn't sent that either!

Thanks Andrew.

> I do have some concerns however:
> 
> With these two patches (introducing and then using the
> get_remote_hostname() function), I'm concerned that we are doing a lot
> of DNS work for each packet.  I do see that there is some kind of cache
> involved, but what have we gained by doing from an explicit cache to an
> implicit one?
> 
> I'm of the view that we should avoid implicit and static caches where
> possible.  Why does the move to tsocket require this?
> 
> s3-smbd: Replace client_id in smbd process.
> http://git.samba.org/?p=asn/samba.git;a=commitdiff;h=614613680ae4fb9f3d50e96
> 3c7759866cadca6ce
> 
> s3-util: Add a get_remote_hostname() function.
> http://git.samba.org/?p=asn/samba.git;a=commitdiff;h=49b23dce25cdeacb3538e2e
> 0674d026b02e03cee

I've discussed this with metze how it should be done before I implemented it. 
I think we can cache it in smbd_server_connection. I will implement it and 
come back to you.

Thanks for the feedback!


	-- andreas

-- 
Andreas Schneider                   GPG-ID: F33E3FC6
Samba Team                             asn at samba.org
www.samba.org