[PATCH REVIEW] s3-auth
Andreas Schneider
asn at samba.org
Tue Jun 21 08:04:53 MDT 2011
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
More information about the samba-technical
mailing list