[PATCH] Add remote arch caching
Jeremy Allison
jra at samba.org
Mon May 2 21:31:36 UTC 2016
On Wed, Apr 27, 2016 at 05:49:33PM +0200, Ralph Boehme wrote:
> Hi Volker!
>
> Attached is an updated patchset with the discussed changes:
>
> o move variable declaration out of the for loop
>
> o fix blob checking in the gencache_parse() parser ra_parse()
>
> Thanks!
> -Ralph
LGTM. Pushed with a couple of const additions and
making the timeout check consistent with other uses !
> From 87068a4ad74c65127790e7bcce6d87ff63776e0c Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 13 Apr 2016 17:39:26 +0200
> Subject: [PATCH 1/4] s3/lib: rework get_remote_arch_str() to use an array
>
> By using C99 designated array initializers we can simplify the code and
> remove the dependency on initializers appearing in a particular order.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/include/smb.h | 2 +-
> source3/lib/util.c | 80 ++++++++++++++++++++++-----------------------------
> 2 files changed, 35 insertions(+), 47 deletions(-)
>
> diff --git a/source3/include/smb.h b/source3/include/smb.h
> index 7eeef88..dec3189 100644
> --- a/source3/include/smb.h
> +++ b/source3/include/smb.h
> @@ -529,7 +529,7 @@ http://msdn.microsoft.com/en-us/library/cc246334(PROT.13).aspx
> #define NO_SUBSTREAMS 0x2
> #define NO_REPARSETAG 0x4
>
> -/* Remote architectures we know about. */
> +/* Remote architectures we know about, keep in sync with remote_arch_strings */
> enum remote_arch_types {RA_UNKNOWN, RA_WFWG, RA_OS2, RA_WIN95, RA_WINNT,
> RA_WIN2K, RA_WINXP, RA_WIN2K3, RA_VISTA,
> RA_SAMBA, RA_CIFSFS, RA_WINXP64, RA_OSX};
> diff --git a/source3/lib/util.c b/source3/lib/util.c
> index 2895c14..a02e0cf 100644
> --- a/source3/lib/util.c
> +++ b/source3/lib/util.c
> @@ -1230,14 +1230,34 @@ void ra_lanman_string( const char *native_lanman )
> set_remote_arch( RA_WIN2K3 );
> }
>
> -static const char *remote_arch_str;
> +static const char *remote_arch_strings[] = {
> + [RA_UNKNOWN] = "UNKNOWN",
> + [RA_WFWG] = "WfWg",
> + [RA_OS2] = "OS2",
> + [RA_WIN95] = "Win95",
> + [RA_WINNT] = "WinNT",
> + [RA_WIN2K] = "Win2K",
> + [RA_WINXP] = "WinXP",
> + [RA_WIN2K3] = "Win2K3",
> + [RA_VISTA] = "Vista",
> + [RA_SAMBA] = "Samba",
> + [RA_CIFSFS] = "CIFSFS",
> + [RA_WINXP64] = "WinXP64",
> + [RA_OSX] = "OSX",
> +};
>
> const char *get_remote_arch_str(void)
> {
> - if (!remote_arch_str) {
> - return "UNKNOWN";
> + if (ra_type >= ARRAY_SIZE(remote_arch_strings)) {
> + /*
> + * set_remote_arch() already checks this so ra_type
> + * should be in the allowed range, but anyway, let's
> + * do another bound check here.
> + */
> + DBG_ERR("Remote arch info out of sync [%d] missing\n", ra_type);
> + ra_type = RA_UNKNOWN;
> }
> - return remote_arch_str;
> + return remote_arch_strings[ra_type];
> }
>
> /*******************************************************************
> @@ -1246,52 +1266,20 @@ const char *get_remote_arch_str(void)
>
> void set_remote_arch(enum remote_arch_types type)
> {
> - ra_type = type;
> - switch( type ) {
> - case RA_WFWG:
> - remote_arch_str = "WfWg";
> - break;
> - case RA_OS2:
> - remote_arch_str = "OS2";
> - break;
> - case RA_WIN95:
> - remote_arch_str = "Win95";
> - break;
> - case RA_WINNT:
> - remote_arch_str = "WinNT";
> - break;
> - case RA_WIN2K:
> - remote_arch_str = "Win2K";
> - break;
> - case RA_WINXP:
> - remote_arch_str = "WinXP";
> - break;
> - case RA_WINXP64:
> - remote_arch_str = "WinXP64";
> - break;
> - case RA_WIN2K3:
> - remote_arch_str = "Win2K3";
> - break;
> - case RA_VISTA:
> - remote_arch_str = "Vista";
> - break;
> - case RA_SAMBA:
> - remote_arch_str = "Samba";
> - break;
> - case RA_CIFSFS:
> - remote_arch_str = "CIFSFS";
> - break;
> - case RA_OSX:
> - remote_arch_str = "OSX";
> - break;
> - default:
> + if (ra_type >= ARRAY_SIZE(remote_arch_strings)) {
> + /*
> + * This protects against someone adding values to enum
> + * remote_arch_types without updating
> + * remote_arch_strings array.
> + */
> + DBG_ERR("Remote arch info out of sync [%d] missing\n", ra_type);
> ra_type = RA_UNKNOWN;
> - remote_arch_str = "UNKNOWN";
> - break;
> + return;
> }
>
> + ra_type = type;
> DEBUG(10,("set_remote_arch: Client arch is \'%s\'\n",
> - remote_arch_str));
> + get_remote_arch_str()));
> }
>
> /*******************************************************************
> --
> 2.5.0
>
>
> From fb44e3ac48d784a9d044b05ea915da1945fe1b19 Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 13 Apr 2016 17:55:11 +0200
> Subject: [PATCH 2/4] s3/lib: add get_remote_arch_from_str()
>
> This will be used when fetching remote arch from gencache.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/include/proto.h | 1 +
> source3/lib/util.c | 12 ++++++++++++
> 2 files changed, 13 insertions(+)
>
> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index afbbff0..8baf9d9 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -389,6 +389,7 @@ bool fcntl_getlock(int fd, off_t *poffset, off_t *pcount, int *ptype, pid_t *ppi
> bool is_myname(const char *s);
> void ra_lanman_string( const char *native_lanman );
> const char *get_remote_arch_str(void);
> +enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string);
> void set_remote_arch(enum remote_arch_types type);
> enum remote_arch_types get_remote_arch(void);
> const char *tab_depth(int level, int depth);
> diff --git a/source3/lib/util.c b/source3/lib/util.c
> index a02e0cf..62ea50f 100644
> --- a/source3/lib/util.c
> +++ b/source3/lib/util.c
> @@ -1260,6 +1260,18 @@ const char *get_remote_arch_str(void)
> return remote_arch_strings[ra_type];
> }
>
> +enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(remote_arch_strings); i++) {
> + if (strcmp(remote_arch_string, remote_arch_strings[i]) == 0) {
> + return i;
> + }
> + }
> + return RA_UNKNOWN;
> +}
> +
> /*******************************************************************
> Set the horrid remote_arch string based on an enum.
> ********************************************************************/
> --
> 2.5.0
>
>
> From 2ba8f3111f348bbf3ee7bd7a7e1820c0bd7526bd Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 13 Apr 2016 17:42:55 +0200
> Subject: [PATCH 3/4] s3/lib: add remote arch caching
>
> This allows caching the remote arch string in gencache. A subsequent
> commit will use this in SMB2 negprot.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/include/proto.h | 2 +
> source3/lib/util.c | 134 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 136 insertions(+)
>
> diff --git a/source3/include/proto.h b/source3/include/proto.h
> index 8baf9d9..c032b9e 100644
> --- a/source3/include/proto.h
> +++ b/source3/include/proto.h
> @@ -392,6 +392,8 @@ const char *get_remote_arch_str(void);
> enum remote_arch_types get_remote_arch_from_str(const char *remote_arch_string);
> void set_remote_arch(enum remote_arch_types type);
> enum remote_arch_types get_remote_arch(void);
> +bool remote_arch_cache_update(const struct GUID *client_guid);
> +bool remote_arch_cache_delete(const struct GUID *client_guid);
> const char *tab_depth(int level, int depth);
> int str_checksum(const char *s);
> void zero_free(void *p, size_t size);
> diff --git a/source3/lib/util.c b/source3/lib/util.c
> index 62ea50f..d794b93 100644
> --- a/source3/lib/util.c
> +++ b/source3/lib/util.c
> @@ -1303,6 +1303,140 @@ enum remote_arch_types get_remote_arch(void)
> return ra_type;
> }
>
> +#define RA_CACHE_TTL 7*24*3600
> +
> +static bool remote_arch_cache_key(const struct GUID *client_guid,
> + fstring key)
> +{
> + struct GUID_txt_buf guid_buf;
> + char *guid_string = NULL;
> +
> + guid_string = GUID_buf_string(client_guid, &guid_buf);
> + if (guid_string == NULL) {
> + return false;
> + }
> +
> + fstr_sprintf(key, "RA/%s", guid_string);
> + return true;
> +}
> +
> +struct ra_parser_state {
> + bool ok;
> + bool expired;
> + enum remote_arch_types ra;
> +};
> +
> +static void ra_parser(time_t timeout, DATA_BLOB blob, void *priv_data)
> +{
> + struct ra_parser_state *state = (struct ra_parser_state *)priv_data;
> + char *ra_str = NULL;
> +
> + if (time(NULL) > timeout) {
> + state->expired = true;
> + state->ok = true;
> + return;
> + }
> +
> + if ((blob.length == 0) || (blob.data[blob.length-1] != '\0')) {
> + DBG_ERR("Remote arch cache key not a string\n");
> + return;
> + }
> +
> + ra_str = (char *)blob.data;
> + DBG_INFO("Got remote arch [%s] from cache\n", ra_str);
> +
> + state->ra = get_remote_arch_from_str(ra_str);
> + state->ok = true;
> + return;
> +}
> +
> +static bool remote_arch_cache_get(const struct GUID *client_guid)
> +{
> + bool ok;
> + fstring ra_key;
> + struct ra_parser_state state = (struct ra_parser_state) {
> + .ok = false,
> + .expired = false,
> + .ra = RA_UNKNOWN,
> + };
> +
> + ok = remote_arch_cache_key(client_guid, ra_key);
> + if (!ok) {
> + return false;
> + }
> +
> + ok = gencache_parse(ra_key, ra_parser, &state);
> + if (!ok || !state.ok) {
> + return false;
> + }
> +
> + if (state.expired) {
> + return gencache_del(ra_key);
> + }
> +
> + if (state.ra == RA_UNKNOWN) {
> + return true;
> + }
> +
> + set_remote_arch(state.ra);
> + return true;
> +}
> +
> +static bool remote_arch_cache_set(const struct GUID *client_guid)
> +{
> + bool ok;
> + fstring ra_key;
> + const char *ra_str = NULL;
> +
> + if (get_remote_arch() == RA_UNKNOWN) {
> + return true;
> + }
> +
> + ok = remote_arch_cache_key(client_guid, ra_key);
> + if (!ok) {
> + return false;
> + }
> +
> + ra_str = get_remote_arch_str();
> + if (ra_str == NULL) {
> + return false;
> + }
> +
> + ok = gencache_set(ra_key, ra_str, time(NULL) + RA_CACHE_TTL);
> + if (!ok) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> +bool remote_arch_cache_update(const struct GUID *client_guid)
> +{
> + if (get_remote_arch() == RA_UNKNOWN) {
> + return remote_arch_cache_get(client_guid);
> + }
> +
> + return remote_arch_cache_set(client_guid);
> +}
> +
> +bool remote_arch_cache_delete(const struct GUID *client_guid)
> +{
> + bool ok;
> + fstring ra_key;
> +
> + ok = remote_arch_cache_key(client_guid, ra_key);
> + if (!ok) {
> + return false;
> + }
> +
> + ok = gencache_del(ra_key);
> + if (!ok) {
> + return false;
> + }
> +
> + return true;
> +}
> +
> const char *tab_depth(int level, int depth)
> {
> if( CHECK_DEBUGLVL(level) ) {
> --
> 2.5.0
>
>
> From 5714b60e9a6932db9d69af5be4ea72bb2b9c420f Mon Sep 17 00:00:00 2001
> From: Ralph Boehme <slow at samba.org>
> Date: Wed, 13 Apr 2016 17:44:26 +0200
> Subject: [PATCH 4/4] smbd: use remote arch caching
>
> We're using the client guid as gencache db key, so this can only be used
> with SMB 2_10 or higher.
>
> The idea is that whenever we get a direct SMB2 negprot, we can then try
> to see if a value is cached for the client's guid.
>
> When a user logs off the cache entry is deleted.
>
> Signed-off-by: Ralph Boehme <slow at samba.org>
> ---
> source3/smbd/smb2_negprot.c | 13 ++++++++++++-
> source3/smbd/smb2_sesssetup.c | 11 +++++++++++
> 2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/source3/smbd/smb2_negprot.c b/source3/smbd/smb2_negprot.c
> index 9c03b2c..6cfa64f 100644
> --- a/source3/smbd/smb2_negprot.c
> +++ b/source3/smbd/smb2_negprot.c
> @@ -162,6 +162,7 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
> uint32_t max_write = lp_smb2_max_write();
> NTTIME now = timeval_to_nttime(&req->request_time);
> bool signing_required = true;
> + bool ok;
>
> status = smbd_smb2_request_verify_sizes(req, 0x24);
> if (!NT_STATUS_IS_OK(status)) {
> @@ -260,6 +261,17 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
> }
> }
>
> + if ((dialect != SMB2_DIALECT_REVISION_2FF) &&
> + (protocol >= PROTOCOL_SMB2_10) &&
> + !GUID_all_zero(&in_guid))
> + {
> + ok = remote_arch_cache_update(&in_guid);
> + if (!ok) {
> + return smbd_smb2_request_error(
> + req, NT_STATUS_UNSUCCESSFUL);
> + }
> + }
> +
> switch (get_remote_arch()) {
> case RA_VISTA:
> case RA_SAMBA:
> @@ -532,7 +544,6 @@ NTSTATUS smbd_smb2_request_process_negprot(struct smbd_smb2_request *req)
> static const uint8_t zeros[8];
> size_t pad = 0;
> size_t ofs;
> - bool ok;
>
> outdyn = data_blob_dup_talloc(req, security_buffer);
> if (outdyn.length != security_buffer.length) {
> diff --git a/source3/smbd/smb2_sesssetup.c b/source3/smbd/smb2_sesssetup.c
> index 78bda7b..873caed 100644
> --- a/source3/smbd/smb2_sesssetup.c
> +++ b/source3/smbd/smb2_sesssetup.c
> @@ -1305,6 +1305,9 @@ static void smbd_smb2_logoff_shutdown_done(struct tevent_req *subreq)
> struct smbd_smb2_logoff_state *state = tevent_req_data(
> req, struct smbd_smb2_logoff_state);
> NTSTATUS status;
> + bool ok;
> + const struct GUID *client_guid =
> + &state->smb2req->session->client->connections->smb2.client.guid;
>
> status = smb2srv_session_shutdown_recv(subreq);
> if (tevent_req_nterror(req, status)) {
> @@ -1312,6 +1315,14 @@ static void smbd_smb2_logoff_shutdown_done(struct tevent_req *subreq)
> }
> TALLOC_FREE(subreq);
>
> + if (!GUID_all_zero(client_guid)) {
> + ok = remote_arch_cache_delete(client_guid);
> + if (!ok) {
> + /* Most likely not an error, but not in cache */
> + DBG_DEBUG("Deletion from remote arch cache failed\n");
> + }
> + }
> +
> /*
> * As we've been awoken, we may have changed
> * uid in the meantime. Ensure we're still
> --
> 2.5.0
>
More information about the samba-technical
mailing list