[PATCH REVIEW] s3-auth

Andrew Bartlett abartlet at samba.org
Mon Jun 20 20:04:52 MDT 2011

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 ?


It's in his git repo:


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.

s3-util: Add a get_remote_hostname() function.


Andrew Bartlett

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

More information about the samba-technical mailing list