[PATCH REVIEW] s3-auth

[Andrew Bartlett]

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?

> > Every commit compiles for me and it passes make test here.
> > 
> > 
> > Thanks for your time,
> 
> Andreas, where is the patchset hosted ?

Jeremy,

It's in his git repo:
http://git.samba.org/?p=asn/samba.git;a=shortlog;h=refs/heads/s3-auth

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!

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=614613680ae4fb9f3d50e963c7759866cadca6ce

s3-util: Add a get_remote_hostname() function.
http://git.samba.org/?p=asn/samba.git;a=commitdiff;h=49b23dce25cdeacb3538e2e0674d026b02e03cee

Thanks,

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org