[PATCH] Use conn->session_info->security_token in posix_acls.c to make sysvolreset faster (was: Re: [PATCH] improve performance for samba-tool ntacl sysvolreset)

[Gary Lockyer]

Looks good to me RB+

I'll defer pushing this until tomorrow morning, to get wider feedback.

Gary.

On 10/07/18 12:53, Andrew Bartlett via samba-technical wrote:
> On Fri, 2018-07-06 at 15:12 +1200, joeg--- via samba-technical wrote:
>> Hi all:
>>
>> I created a patch to improve the performance for samba-tool ntacl
>> sysvolreset cmd.
>>
>> The `sysvolreset` cmd was slow with large amount of files.
>>
>> With perf tool, we figure out the bottle neck is in
>> `user_sid_in_group_sid` which is trying to create user token from sid
>> all the time while we call `setntacl`.
>>
>> To avoid that, we pass a `session_info` parameter all the way down to
>> the related functions, which has the `security_token` with it. And then
>> in `smbd/posix_acls.c:uid_entry_in_group`, we check with the
>> security_token if exist.
>>
>> In our testenv, with 2k files, time for sysvolreset cmd decrease from
>> about 155s to 80s.
>>
>> A Merge Request is also created on GitLab:
>>
>> https://gitlab.com/samba-team/samba/merge_requests/17
> 
> This looks great. 
> 
> I've pushed it back to that merge request with my review and fixes to a
> couple of comments/whitespace.  The previous CI (successful) is here:
> 
> https://gitlab.com/catalyst-samba/samba/pipelines/25235335
> 
> Reviewed-by: Andrew Bartlett <abartlet at samba.org>
> 
> sysvolreset time is a major pain point for users of the AD DC, so
> getting this in for 4.9 would be great.
> 
> Can I get a second team reviewer?
> 
> Thanks,
> 
> Andrew Bartlett
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20180710/8cbef02b/signature.sig>