[PATCH 00/18] SWN: Decouple witness registrations and tcons

[PATCH 00/18] SWN: Decouple witness registrations and tcons

[Samuel Cabrero]

This patches removes the cached tcon pointers from the witness
registrations.

First patch in the serie reverts the current approach based on snapshots
that fixes a race while processing a received notification. This
patchset implements a different approach based on reference counting and
fixes other races that could occur with concurrent
register/unregister/notification processing.

The witness registration stores now all required information to issue
register/unregister DCERPC calls (like auth info and target address),
and when a notification is received it applies it to all matching
witness-enabled tcons.

Each witness registration has its own task to enforce the registration
in case the userspace daemon is started after mounting the share,
instead of overload the echo task for this purpose.

Samuel Cabrero (18):
  Revert "smb: client: resolve SWN tcon from live registrations"
  smb:client: Adjust witness notification request
  smb:client: Rename variable to match the rest of the code
  smb:client: Split idr remove and release functions
  smb:client: Add a specific task to refresh witness registrations
  smb:client: Store the tcon dstaddr in the witness registration
  smb:client: Fix printf format string
  smb:client: Save memory allocations
  smb:client: Use the correct return code
  smb:client: Improve tcon matching logic
  smb:client: Pass the tcon to cifs_swn_send_register_message
  smb:client: Pass the tcon to notification handlers
  smb:client: Introduce swn_notification struct
  smb:client: Cache auth info
  smb:client: Do not store the tcon reference, find a matching one
  smb:client: Simplify registration creation
  smb:client: Improve registration network and share names handling
  smb:client: Avoid witness re-register if the address did not change

 fs/smb/client/cifs_swn.c | 1179 ++++++++++++++++++++++----------------
 fs/smb/client/cifs_swn.h |    3 -
 fs/smb/client/connect.c  |    3 -
 fs/smb/client/trace.h    |    4 +-
 4 files changed, 682 insertions(+), 507 deletions(-)

-- 
2.54.0

[PATCH 01/18] Revert "smb: client: resolve SWN tcon from live registrations"

[Samuel Cabrero]

This reverts commit ec457f9afe5ae9538bdcd58fd4cb442b9787e183.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 314 +++++++--------------------------------
 fs/smb/client/trace.h    |   2 -
 2 files changed, 54 insertions(+), 262 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 9951817d0d7f..9753a432d099 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -28,54 +28,10 @@ struct cifs_swn_reg {
 	bool net_name_notify;
 	bool share_name_notify;
 	bool ip_notify;
-};
 
-struct cifs_swn_reg_info {
-	int id;
-	unsigned int ref_count;
-	const char *net_name;
-	const char *share_name;
-	bool net_name_notify;
-	bool share_name_notify;
-	bool ip_notify;
+	struct cifs_tcon *tcon;
 };
 
-static void cifs_swn_snapshot_reg(struct cifs_swn_reg *swnreg,
-				  struct cifs_swn_reg_info *info)
-{
-	info->id = swnreg->id;
-	info->ref_count = kref_read(&swnreg->ref_count);
-	info->net_name = swnreg->net_name;
-	info->share_name = swnreg->share_name;
-	info->net_name_notify = swnreg->net_name_notify;
-	info->share_name_notify = swnreg->share_name_notify;
-	info->ip_notify = swnreg->ip_notify;
-}
-
-static int cifs_swn_dup_reg(struct cifs_swn_reg *swnreg,
-			    struct cifs_swn_reg_info *info)
-{
-	cifs_swn_snapshot_reg(swnreg, info);
-
-	info->net_name = kstrdup(swnreg->net_name, GFP_KERNEL);
-	if (!info->net_name)
-		return -ENOMEM;
-
-	info->share_name = kstrdup(swnreg->share_name, GFP_KERNEL);
-	if (!info->share_name) {
-		kfree(info->net_name);
-		return -ENOMEM;
-	}
-
-	return 0;
-}
-
-static void cifs_swn_free_reg_info(struct cifs_swn_reg_info *info)
-{
-	kfree(info->net_name);
-	kfree(info->share_name);
-}
-
 static int cifs_swn_auth_info_krb(struct cifs_tcon *tcon, struct sk_buff *skb)
 {
 	int ret;
@@ -117,8 +73,7 @@ static int cifs_swn_auth_info_ntlm(struct cifs_tcon *tcon, struct sk_buff *skb)
  * The authentication information to connect to the witness service is bundled
  * into the message.
  */
-static int cifs_swn_send_register_message(struct cifs_swn_reg_info *swnreg,
-					  struct cifs_tcon *tcon)
+static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 {
 	struct sk_buff *skb;
 	struct genlmsghdr *hdr;
@@ -154,10 +109,10 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg_info *swnreg,
 	 * told to switch to it (client move message). In these cases we unregister from the
 	 * server address and register to the new address when we receive the notification.
 	 */
-	if (tcon->ses->server->use_swn_dstaddr)
-		addr = &tcon->ses->server->swn_dstaddr;
+	if (swnreg->tcon->ses->server->use_swn_dstaddr)
+		addr = &swnreg->tcon->ses->server->swn_dstaddr;
 	else
-		addr = &tcon->ses->server->dstaddr;
+		addr = &swnreg->tcon->ses->server->dstaddr;
 
 	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP, sizeof(struct sockaddr_storage), addr);
 	if (ret < 0)
@@ -181,10 +136,10 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg_info *swnreg,
 			goto nlmsg_fail;
 	}
 
-	authtype = cifs_select_sectype(tcon->ses->server, tcon->ses->sectype);
+	authtype = cifs_select_sectype(swnreg->tcon->ses->server, swnreg->tcon->ses->sectype);
 	switch (authtype) {
 	case Kerberos:
-		ret = cifs_swn_auth_info_krb(tcon, skb);
+		ret = cifs_swn_auth_info_krb(swnreg->tcon, skb);
 		if (ret < 0) {
 			cifs_dbg(VFS, "%s: Failed to get kerberos auth info: %d\n", __func__, ret);
 			goto nlmsg_fail;
@@ -192,7 +147,7 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg_info *swnreg,
 		break;
 	case NTLMv2:
 	case RawNTLMSSP:
-		ret = cifs_swn_auth_info_ntlm(tcon, skb);
+		ret = cifs_swn_auth_info_ntlm(swnreg->tcon, skb);
 		if (ret < 0) {
 			cifs_dbg(VFS, "%s: Failed to get NTLM auth info: %d\n", __func__, ret);
 			goto nlmsg_fail;
@@ -221,8 +176,7 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg_info *swnreg,
 /*
  * Sends an uregister message to the userspace daemon based on the registration
  */
-static int cifs_swn_send_unregister_message(struct cifs_swn_reg_info *swnreg,
-					    struct cifs_tcon *tcon)
+static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 {
 	struct sk_buff *skb;
 	struct genlmsghdr *hdr;
@@ -251,7 +205,7 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg_info *swnreg,
 		goto nlmsg_fail;
 
 	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP, sizeof(struct sockaddr_storage),
-			&tcon->ses->server->dstaddr);
+			&swnreg->tcon->ses->server->dstaddr);
 	if (ret < 0)
 		goto nlmsg_fail;
 
@@ -287,88 +241,6 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg_info *swnreg,
 	return ret;
 }
 
-/*
- * Allocation-free mirror of extract_hostname() + extract_sharename() from
- * fs/smb/client/unc.c.  Those helpers kmalloc(GFP_KERNEL); this runs under
- * cifs_tcp_ses_lock and tcon->tc_lock, both spinlocks, so we mirror their
- * parsing in place against the caller's stable net_name/share_name strings.
- * Keep in sync with unc.c.
- */
-static bool cifs_swn_tcon_matches(struct cifs_tcon *tcon,
-				  const char *net_name,
-				  const char *share_name)
-{
-	const char *unc = tcon->tree_name;
-	const char *host, *share, *delim;
-	size_t host_len, share_len;
-
-	if (!tcon->use_witness)
-		return false;
-
-	/* extract_hostname: require strlen(unc) >= 3 */
-	if (strnlen(unc, 3) < 3)
-		return false;
-	/* extract_hostname: skip all leading '\' characters */
-	for (host = unc; *host == '\\'; host++)
-		;
-	if (!*host)
-		return false;
-	delim = strchr(host, '\\');
-	if (!delim)
-		return false;
-	host_len = delim - host;
-	if (strlen(net_name) != host_len ||
-	    strncasecmp(host, net_name, host_len))
-		return false;
-
-	/* extract_sharename: start at unc + 2, then first '\' onward */
-	share = unc + 2;
-	delim = strchr(share, '\\');
-	if (!delim)
-		return false;
-	share = delim + 1;
-	share_len = strlen(share);
-
-	return strlen(share_name) == share_len &&
-	       !strncasecmp(share, share_name, share_len);
-}
-
-/*
- * One SWN registration id represents one net/share name pair.  Multiple
- * mounted tcons can therefore share the id.  Pick a live representative at
- * use time instead of caching the first tcon pointer in the registration.
- */
-static struct cifs_tcon *cifs_swn_get_tcon(struct cifs_swn_reg_info *swnreg)
-{
-	struct TCP_Server_Info *server;
-	struct cifs_ses *ses;
-	struct cifs_tcon *tcon;
-
-	spin_lock(&cifs_tcp_ses_lock);
-	list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
-		list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
-			list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
-				spin_lock(&tcon->tc_lock);
-				if (tcon->status == TID_EXITING ||
-				    !cifs_swn_tcon_matches(tcon, swnreg->net_name,
-							   swnreg->share_name)) {
-					spin_unlock(&tcon->tc_lock);
-					continue;
-				}
-				++tcon->tc_count;
-				trace_smb3_tcon_ref(tcon->debug_id,
-						    tcon->tc_count,
-						    netfs_trace_tcon_ref_get_swn_notify);
-				spin_unlock(&tcon->tc_lock);
-				spin_unlock(&cifs_tcp_ses_lock);
-				return tcon;
-			}
-		}
-	}
-	spin_unlock(&cifs_tcp_ses_lock);
-	return NULL;
-}
-
 /*
  * Try to find a matching registration for the tcon's server name and share name.
  * Calls to this function must be protected by cifs_swnreg_idr_mutex.
@@ -475,6 +347,8 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	reg->net_name_notify = true;
 	reg->share_name_notify = true;
 	reg->ip_notify = (tcon->capabilities & SMB2_SHARE_CAP_SCALEOUT);
+
+	reg->tcon = tcon;
 unlock:
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
@@ -494,6 +368,11 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 static void cifs_swn_reg_release(struct kref *ref)
 {
 	struct cifs_swn_reg *swnreg = container_of(ref, struct cifs_swn_reg, ref_count);
+	int ret;
+
+	ret = cifs_swn_send_unregister_message(swnreg);
+	if (ret < 0)
+		cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", __func__, ret);
 
 	idr_remove(&cifs_swnreg_idr, swnreg->id);
 	kfree(swnreg->net_name);
@@ -501,33 +380,23 @@ static void cifs_swn_reg_release(struct kref *ref)
 	kfree(swnreg);
 }
 
-static void cifs_put_swn_reg_locked(struct cifs_swn_reg *swnreg,
-				    struct cifs_tcon *tcon)
+static void cifs_put_swn_reg(struct cifs_swn_reg *swnreg)
 {
-	if (kref_read(&swnreg->ref_count) == 1) {
-		struct cifs_swn_reg_info swnreg_info;
-		int ret;
-
-		cifs_swn_snapshot_reg(swnreg, &swnreg_info);
-		ret = cifs_swn_send_unregister_message(&swnreg_info, tcon);
-		if (ret < 0)
-			cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n",
-				 __func__, ret);
-	}
-
+	mutex_lock(&cifs_swnreg_idr_mutex);
 	kref_put(&swnreg->ref_count, cifs_swn_reg_release);
+	mutex_unlock(&cifs_swnreg_idr_mutex);
 }
 
-static int cifs_swn_resource_state_changed(struct cifs_tcon *tcon, const char *name, int state)
+static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const char *name, int state)
 {
 	switch (state) {
 	case CIFS_SWN_RESOURCE_STATE_UNAVAILABLE:
 		cifs_dbg(FYI, "%s: resource name '%s' become unavailable\n", __func__, name);
-		cifs_signal_cifsd_for_reconnect(tcon->ses->server, true);
+		cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true);
 		break;
 	case CIFS_SWN_RESOURCE_STATE_AVAILABLE:
 		cifs_dbg(FYI, "%s: resource name '%s' become available\n", __func__, name);
-		cifs_signal_cifsd_for_reconnect(tcon->ses->server, true);
+		cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true);
 		break;
 	case CIFS_SWN_RESOURCE_STATE_UNKNOWN:
 		cifs_dbg(FYI, "%s: resource name '%s' changed to unknown state\n", __func__, name);
@@ -633,7 +502,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
 	return ret;
 }
 
-static int cifs_swn_client_move(struct cifs_tcon *tcon, struct sockaddr_storage *addr)
+static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_storage *addr)
 {
 	struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr;
 	struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr;
@@ -643,17 +512,14 @@ static int cifs_swn_client_move(struct cifs_tcon *tcon, struct sockaddr_storage
 	else if (addr->ss_family == AF_INET6)
 		cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr);
 
-	return cifs_swn_reconnect(tcon, addr);
+	return cifs_swn_reconnect(swnreg->tcon, addr);
 }
 
 int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 {
 	struct cifs_swn_reg *swnreg;
-	struct cifs_swn_reg_info swnreg_info;
-	struct cifs_tcon *tcon;
 	char name[256];
 	int type;
-	int ret = 0;
 
 	if (info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]) {
 		int swnreg_id;
@@ -661,34 +527,21 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 		swnreg_id = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]);
 		mutex_lock(&cifs_swnreg_idr_mutex);
 		swnreg = idr_find(&cifs_swnreg_idr, swnreg_id);
+		mutex_unlock(&cifs_swnreg_idr_mutex);
 		if (swnreg == NULL) {
-			mutex_unlock(&cifs_swnreg_idr_mutex);
 			cifs_dbg(FYI, "%s: registration id %d not found\n", __func__, swnreg_id);
 			return -EINVAL;
 		}
-		ret = cifs_swn_dup_reg(swnreg, &swnreg_info);
-		mutex_unlock(&cifs_swnreg_idr_mutex);
-		if (ret)
-			return ret;
 	} else {
 		cifs_dbg(FYI, "%s: missing registration id attribute\n", __func__);
 		return -EINVAL;
 	}
 
-	tcon = cifs_swn_get_tcon(&swnreg_info);
-	if (!tcon) {
-		cifs_dbg(FYI, "%s: registration id %d has no live tcon\n",
-			 __func__, swnreg_info.id);
-		ret = -ENODEV;
-		goto free_info;
-	}
-
 	if (info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]) {
 		type = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]);
 	} else {
 		cifs_dbg(FYI, "%s: missing notification type attribute\n", __func__);
-		ret = -EINVAL;
-		goto out;
+		return -EINVAL;
 	}
 
 	switch (type) {
@@ -700,18 +553,15 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 					sizeof(name));
 		} else {
 			cifs_dbg(FYI, "%s: missing resource name attribute\n", __func__);
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
 		if (info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]) {
 			state = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]);
 		} else {
 			cifs_dbg(FYI, "%s: missing resource state attribute\n", __func__);
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
-		ret = cifs_swn_resource_state_changed(tcon, name, state);
-		break;
+		return cifs_swn_resource_state_changed(swnreg, name, state);
 	}
 	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE: {
 		struct sockaddr_storage addr;
@@ -720,36 +570,28 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 			nla_memcpy(&addr, info->attrs[CIFS_GENL_ATTR_SWN_IP], sizeof(addr));
 		} else {
 			cifs_dbg(FYI, "%s: missing IP address attribute\n", __func__);
-			ret = -EINVAL;
-			goto out;
+			return -EINVAL;
 		}
-		ret = cifs_swn_client_move(tcon, &addr);
-		break;
+		return cifs_swn_client_move(swnreg, &addr);
 	}
 	default:
 		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__, type);
 		break;
 	}
 
-out:
-	cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify);
-free_info:
-	cifs_swn_free_reg_info(&swnreg_info);
-	return ret;
+	return 0;
 }
 
 int cifs_swn_register(struct cifs_tcon *tcon)
 {
 	struct cifs_swn_reg *swnreg;
-	struct cifs_swn_reg_info swnreg_info;
 	int ret;
 
 	swnreg = cifs_get_swn_reg(tcon);
 	if (IS_ERR(swnreg))
 		return PTR_ERR(swnreg);
 
-	cifs_swn_snapshot_reg(swnreg, &swnreg_info);
-	ret = cifs_swn_send_register_message(&swnreg_info, tcon);
+	ret = cifs_swn_send_register_message(swnreg);
 	if (ret < 0) {
 		cifs_dbg(VFS, "%s: Failed to send swn register message: %d\n", __func__, ret);
 		/* Do not put the swnreg or return error, the echo task will retry */
@@ -770,68 +612,35 @@ int cifs_swn_unregister(struct cifs_tcon *tcon)
 		return PTR_ERR(swnreg);
 	}
 
-	cifs_put_swn_reg_locked(swnreg, tcon);
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
-	return 0;
-}
-
-/*
- * Snapshot one registration under cifs_swnreg_idr_mutex and return.  Callers
- * intentionally do the per-registration network/genlmsg work without the
- * mutex held, both to keep the critical section short and to avoid nesting
- * cifs_swnreg_idr_mutex inside the higher tc_lock when a live tcon is then
- * pinned for the send.
- */
-static int cifs_swn_get_next_reg_info(int *id, struct cifs_swn_reg_info *info)
-{
-	struct cifs_swn_reg *swnreg;
-	int ret = 0;
-
-	mutex_lock(&cifs_swnreg_idr_mutex);
-	swnreg = idr_get_next(&cifs_swnreg_idr, id);
-	if (swnreg) {
-		ret = cifs_swn_dup_reg(swnreg, info);
-		if (!ret) {
-			*id = swnreg->id + 1;
-			ret = 1;
-		}
-	}
-	mutex_unlock(&cifs_swnreg_idr_mutex);
+	cifs_put_swn_reg(swnreg);
 
-	return ret;
+	return 0;
 }
 
 void cifs_swn_dump(struct seq_file *m)
 {
-	struct cifs_swn_reg_info swnreg_info;
-	struct cifs_tcon *tcon;
+	struct cifs_swn_reg *swnreg;
 	struct sockaddr_in *sa;
 	struct sockaddr_in6 *sa6;
-	int id = 0;
-	int ret;
+	int id;
 
 	seq_puts(m, "Witness registrations:");
 
-	while ((ret = cifs_swn_get_next_reg_info(&id, &swnreg_info)) > 0) {
+	mutex_lock(&cifs_swnreg_idr_mutex);
+	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
 		seq_printf(m, "\nId: %u Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: ",
-			   swnreg_info.id, swnreg_info.ref_count,
-			   swnreg_info.net_name, swnreg_info.net_name_notify ? "(y)" : "(n)",
-			   swnreg_info.share_name, swnreg_info.share_name_notify ? "(y)" : "(n)");
-
-		tcon = cifs_swn_get_tcon(&swnreg_info);
-		if (!tcon) {
-			seq_puts(m, "(no live tcon)");
-			goto next;
-		}
-
-		switch (tcon->ses->server->dstaddr.ss_family) {
+				id, kref_read(&swnreg->ref_count),
+				swnreg->net_name, swnreg->net_name_notify ? "(y)" : "(n)",
+				swnreg->share_name, swnreg->share_name_notify ? "(y)" : "(n)");
+		switch (swnreg->tcon->ses->server->dstaddr.ss_family) {
 		case AF_INET:
-			sa = (struct sockaddr_in *)&tcon->ses->server->dstaddr;
+			sa = (struct sockaddr_in *) &swnreg->tcon->ses->server->dstaddr;
 			seq_printf(m, "%pI4", &sa->sin_addr.s_addr);
 			break;
 		case AF_INET6:
-			sa6 = (struct sockaddr_in6 *)&tcon->ses->server->dstaddr;
+			sa6 = (struct sockaddr_in6 *) &swnreg->tcon->ses->server->dstaddr;
 			seq_printf(m, "%pI6", &sa6->sin6_addr.s6_addr);
 			if (sa6->sin6_scope_id)
 				seq_printf(m, "%%%u", sa6->sin6_scope_id);
@@ -839,38 +648,23 @@ void cifs_swn_dump(struct seq_file *m)
 		default:
 			seq_puts(m, "(unknown)");
 		}
-		cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify);
-next:
-		seq_printf(m, "%s", swnreg_info.ip_notify ? "(y)" : "(n)");
-		cifs_swn_free_reg_info(&swnreg_info);
+		seq_printf(m, "%s", swnreg->ip_notify ? "(y)" : "(n)");
 	}
-	if (ret < 0)
-		seq_printf(m, "\nFailed to snapshot witness registration: %d", ret);
+	mutex_unlock(&cifs_swnreg_idr_mutex);
 	seq_puts(m, "\n");
 }
 
 void cifs_swn_check(void)
 {
-	struct cifs_swn_reg_info swnreg_info;
-	struct cifs_tcon *tcon;
-	int id = 0;
+	struct cifs_swn_reg *swnreg;
+	int id;
 	int ret;
 
-	while ((ret = cifs_swn_get_next_reg_info(&id, &swnreg_info)) > 0) {
-		tcon = cifs_swn_get_tcon(&swnreg_info);
-		if (!tcon) {
-			cifs_dbg(FYI, "%s: registration id %d has no live tcon\n",
-				 __func__, swnreg_info.id);
-			goto free_info;
-		}
-
-		ret = cifs_swn_send_register_message(&swnreg_info, tcon);
+	mutex_lock(&cifs_swnreg_idr_mutex);
+	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
+		ret = cifs_swn_send_register_message(swnreg);
 		if (ret < 0)
 			cifs_dbg(FYI, "%s: Failed to send register message: %d\n", __func__, ret);
-		cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify);
-free_info:
-		cifs_swn_free_reg_info(&swnreg_info);
 	}
-	if (ret < 0)
-		cifs_dbg(FYI, "%s: Failed to snapshot registration: %d\n", __func__, ret);
+	mutex_unlock(&cifs_swnreg_idr_mutex);
 }
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index 5b21ad3c15fb..b99ec5a417fa 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -181,7 +181,6 @@
 	EM(netfs_trace_tcon_ref_get_find,		"GET Find  ") \
 	EM(netfs_trace_tcon_ref_get_find_sess_tcon,	"GET FndSes") \
 	EM(netfs_trace_tcon_ref_get_reconnect_server,	"GET Reconn") \
-	EM(netfs_trace_tcon_ref_get_swn_notify,		"GET SwnNot") \
 	EM(netfs_trace_tcon_ref_new,			"NEW       ") \
 	EM(netfs_trace_tcon_ref_new_ipc,		"NEW Ipc   ") \
 	EM(netfs_trace_tcon_ref_new_reconnect_server,	"NEW Reconn") \
@@ -193,7 +192,6 @@
 	EM(netfs_trace_tcon_ref_put_mnt_ctx,		"PUT MntCtx") \
 	EM(netfs_trace_tcon_ref_put_dfs_refer,		"PUT DfsRfr") \
 	EM(netfs_trace_tcon_ref_put_reconnect_server,	"PUT Reconn") \
-	EM(netfs_trace_tcon_ref_put_swn_notify,		"PUT SwnNot") \
 	EM(netfs_trace_tcon_ref_put_tlink,		"PUT Tlink ") \
 	EM(netfs_trace_tcon_ref_see_cancelled_close,	"SEE Cn-Cls") \
 	EM(netfs_trace_tcon_ref_see_fscache_collision,	"SEE FV-CO!") \
-- 
2.54.0

[PATCH 02/18] smb:client: Adjust witness notification request

[Samuel Cabrero]

Request share notifications only if asymmetric capability is set.

Never request IP change notifications, handling is not implemented.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 9753a432d099..2f8af8a281e8 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -345,8 +345,9 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	}
 
 	reg->net_name_notify = true;
-	reg->share_name_notify = true;
-	reg->ip_notify = (tcon->capabilities & SMB2_SHARE_CAP_SCALEOUT);
+	reg->share_name_notify =
+		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
+	reg->ip_notify = false;
 
 	reg->tcon = tcon;
 unlock:
-- 
2.54.0

[PATCH 03/18] smb:client: Rename variable to match the rest of the code

[Samuel Cabrero]

struct cifs_swn_reg pointer is always named swnreg.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 50 ++++++++++++++++++++--------------------
 1 file changed, 25 insertions(+), 25 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 2f8af8a281e8..0afb69b2eed2 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -301,66 +301,66 @@ static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
  */
 static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 {
-	struct cifs_swn_reg *reg = NULL;
+	struct cifs_swn_reg *swnreg = NULL;
 	int ret;
 
 	mutex_lock(&cifs_swnreg_idr_mutex);
 
 	/* Check if we are already registered for this network and share names */
-	reg = cifs_find_swn_reg(tcon);
-	if (!IS_ERR(reg)) {
-		kref_get(&reg->ref_count);
+	swnreg = cifs_find_swn_reg(tcon);
+	if (!IS_ERR(swnreg)) {
+		kref_get(&swnreg->ref_count);
 		goto unlock;
-	} else if (PTR_ERR(reg) != -EEXIST) {
+	} else if (PTR_ERR(swnreg) != -EEXIST) {
 		goto unlock;
 	}
 
-	reg = kmalloc_obj(struct cifs_swn_reg, GFP_ATOMIC);
-	if (reg == NULL) {
+	swnreg = kmalloc_obj(struct cifs_swn_reg, GFP_ATOMIC);
+	if (swnreg == NULL) {
 		ret = -ENOMEM;
 		goto fail_unlock;
 	}
 
-	kref_init(&reg->ref_count);
+	kref_init(&swnreg->ref_count);
 
-	reg->id = idr_alloc(&cifs_swnreg_idr, reg, 1, 0, GFP_ATOMIC);
-	if (reg->id < 0) {
+	swnreg->id = idr_alloc(&cifs_swnreg_idr, swnreg, 1, 0, GFP_ATOMIC);
+	if (swnreg->id < 0) {
 		cifs_dbg(FYI, "%s: failed to allocate registration id\n", __func__);
-		ret = reg->id;
+		ret = swnreg->id;
 		goto fail;
 	}
 
-	reg->net_name = extract_hostname(tcon->tree_name);
-	if (IS_ERR(reg->net_name)) {
-		ret = PTR_ERR(reg->net_name);
+	swnreg->net_name = extract_hostname(tcon->tree_name);
+	if (IS_ERR(swnreg->net_name)) {
+		ret = PTR_ERR(swnreg->net_name);
 		cifs_dbg(VFS, "%s: failed to extract host name from target: %d\n", __func__, ret);
 		goto fail_idr;
 	}
 
-	reg->share_name = extract_sharename(tcon->tree_name);
-	if (IS_ERR(reg->share_name)) {
-		ret = PTR_ERR(reg->share_name);
+	swnreg->share_name = extract_sharename(tcon->tree_name);
+	if (IS_ERR(swnreg->share_name)) {
+		ret = PTR_ERR(swnreg->share_name);
 		cifs_dbg(VFS, "%s: failed to extract share name from target: %d\n", __func__, ret);
 		goto fail_net_name;
 	}
 
-	reg->net_name_notify = true;
-	reg->share_name_notify =
+	swnreg->net_name_notify = true;
+	swnreg->share_name_notify =
 		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
-	reg->ip_notify = false;
+	swnreg->ip_notify = false;
 
-	reg->tcon = tcon;
+	swnreg->tcon = tcon;
 unlock:
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
-	return reg;
+	return swnreg;
 
 fail_net_name:
-	kfree(reg->net_name);
+	kfree(swnreg->net_name);
 fail_idr:
-	idr_remove(&cifs_swnreg_idr, reg->id);
+	idr_remove(&cifs_swnreg_idr, swnreg->id);
 fail:
-	kfree(reg);
+	kfree(swnreg);
 fail_unlock:
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 	return ERR_PTR(ret);
-- 
2.54.0

[PATCH 04/18] smb:client: Split idr remove and release functions

[Samuel Cabrero]

Sending the unregister netlink message can sleep, send it after releasing
the mutex.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 0afb69b2eed2..fe032ae9c413 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -366,26 +366,24 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	return ERR_PTR(ret);
 }
 
-static void cifs_swn_reg_release(struct kref *ref)
+static void cifs_swn_reg_release(struct cifs_swn_reg *swnreg)
 {
-	struct cifs_swn_reg *swnreg = container_of(ref, struct cifs_swn_reg, ref_count);
 	int ret;
 
 	ret = cifs_swn_send_unregister_message(swnreg);
 	if (ret < 0)
 		cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", __func__, ret);
 
-	idr_remove(&cifs_swnreg_idr, swnreg->id);
 	kfree(swnreg->net_name);
 	kfree(swnreg->share_name);
 	kfree(swnreg);
 }
 
-static void cifs_put_swn_reg(struct cifs_swn_reg *swnreg)
+static void cifs_swn_reg_idr_remove(struct kref *ref)
 {
-	mutex_lock(&cifs_swnreg_idr_mutex);
-	kref_put(&swnreg->ref_count, cifs_swn_reg_release);
-	mutex_unlock(&cifs_swnreg_idr_mutex);
+	struct cifs_swn_reg *swnreg = container_of(ref, struct cifs_swn_reg, ref_count);
+
+	idr_remove(&cifs_swnreg_idr, swnreg->id);
 }
 
 static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const char *name, int state)
@@ -612,11 +610,14 @@ int cifs_swn_unregister(struct cifs_tcon *tcon)
 		mutex_unlock(&cifs_swnreg_idr_mutex);
 		return PTR_ERR(swnreg);
 	}
+	if (kref_put(&swnreg->ref_count, cifs_swn_reg_idr_remove)) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		cifs_swn_reg_release(swnreg);
+		return 0;
+	}
 
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
-	cifs_put_swn_reg(swnreg);
-
 	return 0;
 }
 
-- 
2.54.0

[PATCH 05/18] smb:client: Add a specific task to refresh witness registrations

[Samuel Cabrero]

Instead of overload the echo task, create a delayed work for each
registration.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 116 ++++++++++++++++++++++++++-------------
 fs/smb/client/cifs_swn.h |   3 -
 fs/smb/client/connect.c  |   3 -
 3 files changed, 79 insertions(+), 43 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index fe032ae9c413..36e199eb04c1 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -30,6 +30,9 @@ struct cifs_swn_reg {
 	bool ip_notify;
 
 	struct cifs_tcon *tcon;
+
+	unsigned long check_interval;
+	struct delayed_work check;
 };
 
 static int cifs_swn_auth_info_krb(struct cifs_tcon *tcon, struct sk_buff *skb)
@@ -241,6 +244,75 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 	return ret;
 }
 
+static void cifs_swn_reg_release(struct cifs_swn_reg *swnreg)
+{
+	int ret;
+
+	ret = cifs_swn_send_unregister_message(swnreg);
+	if (ret < 0)
+		cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", __func__, ret);
+
+	kfree(swnreg->net_name);
+	kfree(swnreg->share_name);
+	kfree(swnreg);
+}
+
+static void cifs_swn_reg_idr_remove(struct kref *ref)
+{
+	struct cifs_swn_reg *swnreg = container_of(ref, struct cifs_swn_reg, ref_count);
+
+	idr_remove(&cifs_swnreg_idr, swnreg->id);
+}
+
+/*
+ * Periodic task to enforce registration even when the userspace daemon is
+ * started after mounting the share.
+ */
+static void cifs_swn_reg_check(struct work_struct *work)
+{
+	struct cifs_swn_reg *swnreg =
+		container_of(work, struct cifs_swn_reg, check.work);
+	int ret;
+
+	/*
+	 * First take a reference to avoid other thread releasing the swnreg
+	 * on concurrent cifs_swn_unregister().
+	 *
+	 * Do not resurrect dead registrations, a concurrent cifs_swn_unregister
+	 * can drop refcount to 0 and remove this swnreg from the IDR before
+	 * releasing the mutex, but it will then wait for this callback to end
+	 * before releasing the swnreg.
+	 */
+	mutex_lock(&cifs_swnreg_idr_mutex);
+	if (!kref_get_unless_zero(&swnreg->ref_count)) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return;
+	}
+	mutex_unlock(&cifs_swnreg_idr_mutex);
+
+
+	/*
+	 * It is safe to send the registration message multiple times.
+	 * The userspace client library tracks if registered or not
+	 * using the swnreg->id.
+	 */
+	ret = cifs_swn_send_register_message(swnreg);
+	if (ret < 0)
+		cifs_dbg(FYI, "%s: Failed to send register message: %d\n",
+			 __func__, ret);
+
+	/* Release our reference */
+	mutex_lock(&cifs_swnreg_idr_mutex);
+	if (kref_put(&swnreg->ref_count, cifs_swn_reg_idr_remove)) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		cifs_swn_reg_release(swnreg);
+		return;
+	}
+	mutex_unlock(&cifs_swnreg_idr_mutex);
+
+	queue_delayed_work(cifsiod_wq, &swnreg->check, swnreg->check_interval);
+}
+
 /*
  * Try to find a matching registration for the tcon's server name and share name.
  * Calls to this function must be protected by cifs_swnreg_idr_mutex.
@@ -350,6 +422,11 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	swnreg->ip_notify = false;
 
 	swnreg->tcon = tcon;
+
+	swnreg->check_interval = tcon->ses->server->echo_interval;
+	INIT_DELAYED_WORK(&swnreg->check, cifs_swn_reg_check);
+
+	queue_delayed_work(cifsiod_wq, &swnreg->check, swnreg->check_interval);
 unlock:
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
@@ -366,26 +443,6 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	return ERR_PTR(ret);
 }
 
-static void cifs_swn_reg_release(struct cifs_swn_reg *swnreg)
-{
-	int ret;
-
-	ret = cifs_swn_send_unregister_message(swnreg);
-	if (ret < 0)
-		cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", __func__, ret);
-
-	kfree(swnreg->net_name);
-	kfree(swnreg->share_name);
-	kfree(swnreg);
-}
-
-static void cifs_swn_reg_idr_remove(struct kref *ref)
-{
-	struct cifs_swn_reg *swnreg = container_of(ref, struct cifs_swn_reg, ref_count);
-
-	idr_remove(&cifs_swnreg_idr, swnreg->id);
-}
-
 static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const char *name, int state)
 {
 	switch (state) {
@@ -593,7 +650,7 @@ int cifs_swn_register(struct cifs_tcon *tcon)
 	ret = cifs_swn_send_register_message(swnreg);
 	if (ret < 0) {
 		cifs_dbg(VFS, "%s: Failed to send swn register message: %d\n", __func__, ret);
-		/* Do not put the swnreg or return error, the echo task will retry */
+		/* Do not put the swnreg or return error, the check task will retry */
 	}
 
 	return 0;
@@ -604,7 +661,6 @@ int cifs_swn_unregister(struct cifs_tcon *tcon)
 	struct cifs_swn_reg *swnreg;
 
 	mutex_lock(&cifs_swnreg_idr_mutex);
-
 	swnreg = cifs_find_swn_reg(tcon);
 	if (IS_ERR(swnreg)) {
 		mutex_unlock(&cifs_swnreg_idr_mutex);
@@ -612,6 +668,7 @@ int cifs_swn_unregister(struct cifs_tcon *tcon)
 	}
 	if (kref_put(&swnreg->ref_count, cifs_swn_reg_idr_remove)) {
 		mutex_unlock(&cifs_swnreg_idr_mutex);
+		cancel_delayed_work_sync(&swnreg->check);
 		cifs_swn_reg_release(swnreg);
 		return 0;
 	}
@@ -655,18 +712,3 @@ void cifs_swn_dump(struct seq_file *m)
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 	seq_puts(m, "\n");
 }
-
-void cifs_swn_check(void)
-{
-	struct cifs_swn_reg *swnreg;
-	int id;
-	int ret;
-
-	mutex_lock(&cifs_swnreg_idr_mutex);
-	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
-		ret = cifs_swn_send_register_message(swnreg);
-		if (ret < 0)
-			cifs_dbg(FYI, "%s: Failed to send register message: %d\n", __func__, ret);
-	}
-	mutex_unlock(&cifs_swnreg_idr_mutex);
-}
diff --git a/fs/smb/client/cifs_swn.h b/fs/smb/client/cifs_swn.h
index 955d07b69450..cdc5628a5459 100644
--- a/fs/smb/client/cifs_swn.h
+++ b/fs/smb/client/cifs_swn.h
@@ -22,8 +22,6 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info);
 
 void cifs_swn_dump(struct seq_file *m);
 
-void cifs_swn_check(void);
-
 static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server)
 {
 	if (server->use_swn_dstaddr) {
@@ -44,7 +42,6 @@ static inline int cifs_swn_register(struct cifs_tcon *tcon) { return 0; }
 static inline int cifs_swn_unregister(struct cifs_tcon *tcon) { return 0; }
 static inline int cifs_swn_notify(struct sk_buff *s, struct genl_info *i) { return 0; }
 static inline void cifs_swn_dump(struct seq_file *m) {}
-static inline void cifs_swn_check(void) {}
 static inline bool cifs_swn_set_server_dstaddr(struct TCP_Server_Info *server) { return false; }
 static inline void cifs_swn_reset_server_dstaddr(struct TCP_Server_Info *server) {}
 
diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c
index 85aec302c89e..3465566f8cef 100644
--- a/fs/smb/client/connect.c
+++ b/fs/smb/client/connect.c
@@ -632,9 +632,6 @@ cifs_echo_request(struct work_struct *work)
 	rc = server->ops->echo ? server->ops->echo(server) : -ENOSYS;
 	cifs_server_dbg(FYI, "send echo request: rc = %d\n", rc);
 
-	/* Check witness registrations */
-	cifs_swn_check();
-
 requeue_echo:
 	queue_delayed_work(cifsiod_wq, &server->echo, server->echo_interval);
 }
-- 
2.54.0

[PATCH 06/18] smb:client: Store the tcon dstaddr in the witness registration

[Samuel Cabrero]

Prepare to remove the tcon pointer from cifs_swn_reg struct.

Always use the address stored in the swnreg to send the register netlink
message.

To find the swnreg matching a tcon the stored address is compared to the
tcon dstaddr.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 125 ++++++++++++++++++---------------------
 1 file changed, 56 insertions(+), 69 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 36e199eb04c1..a33caaf4aaf0 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -25,6 +25,7 @@ struct cifs_swn_reg {
 
 	const char *net_name;
 	const char *share_name;
+	struct sockaddr_storage addr;
 	bool net_name_notify;
 	bool share_name_notify;
 	bool ip_notify;
@@ -81,7 +82,6 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 	struct sk_buff *skb;
 	struct genlmsghdr *hdr;
 	enum securityEnum authtype;
-	struct sockaddr_storage *addr;
 	int ret;
 
 	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -106,18 +106,8 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 	if (ret < 0)
 		goto nlmsg_fail;
 
-	/*
-	 * If there is an address stored use it instead of the server address, because we are
-	 * in the process of reconnecting to it after a share has been moved or we have been
-	 * told to switch to it (client move message). In these cases we unregister from the
-	 * server address and register to the new address when we receive the notification.
-	 */
-	if (swnreg->tcon->ses->server->use_swn_dstaddr)
-		addr = &swnreg->tcon->ses->server->swn_dstaddr;
-	else
-		addr = &swnreg->tcon->ses->server->dstaddr;
-
-	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP, sizeof(struct sockaddr_storage), addr);
+	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP,
+		      sizeof(struct sockaddr_storage), &swnreg->addr);
 	if (ret < 0)
 		goto nlmsg_fail;
 
@@ -207,8 +197,8 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 	if (ret < 0)
 		goto nlmsg_fail;
 
-	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP, sizeof(struct sockaddr_storage),
-			&swnreg->tcon->ses->server->dstaddr);
+	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP,
+		      sizeof(struct sockaddr_storage), &swnreg->addr);
 	if (ret < 0)
 		goto nlmsg_fail;
 
@@ -290,7 +280,6 @@ static void cifs_swn_reg_check(struct work_struct *work)
 	}
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
-
 	/*
 	 * It is safe to send the registration message multiple times.
 	 * The userspace client library tracks if registered or not
@@ -347,13 +336,23 @@ static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
 	}
 
 	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
-		if (strcasecmp(swnreg->net_name, net_name) != 0
-		    || strcasecmp(swnreg->share_name, share_name) != 0) {
+		struct sockaddr_storage *tcon_dstaddr;
+
+		if (tcon->ses->server->use_swn_dstaddr)
+			tcon_dstaddr = &tcon->ses->server->swn_dstaddr;
+		else
+			tcon_dstaddr = &tcon->ses->server->dstaddr;
+
+		if (strcasecmp(swnreg->net_name, net_name) != 0 ||
+		    strcasecmp(swnreg->share_name, share_name) != 0 ||
+		    !cifs_match_ipaddr((struct sockaddr *)&swnreg->addr,
+				       (struct sockaddr *)tcon_dstaddr)) {
 			continue;
 		}
 
-		cifs_dbg(FYI, "Existing swn registration for %s:%s found\n", swnreg->net_name,
-				swnreg->share_name);
+		cifs_dbg(FYI,
+			 "Existing swn registration for %pISc:%s:%s found\n",
+			 &swnreg->addr, swnreg->net_name, swnreg->share_name);
 
 		kfree(net_name);
 		kfree(share_name);
@@ -416,6 +415,17 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 		goto fail_net_name;
 	}
 
+	/*
+	 * If there is an address stored use it instead of the server address, because we are
+	 * in the process of reconnecting to it after a share has been moved or we have been
+	 * told to switch to it (client move message). In these cases we unregister from the
+	 * server address and register to the new address when we receive the notification.
+	 */
+	if (tcon->ses->server->use_swn_dstaddr)
+		swnreg->addr = tcon->ses->server->swn_dstaddr;
+	else
+		swnreg->addr = tcon->ses->server->dstaddr;
+
 	swnreg->net_name_notify = true;
 	swnreg->share_name_notify =
 		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
@@ -461,26 +471,6 @@ static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const ch
 	return 0;
 }
 
-static bool cifs_sockaddr_equal(struct sockaddr_storage *addr1, struct sockaddr_storage *addr2)
-{
-	if (addr1->ss_family != addr2->ss_family)
-		return false;
-
-	if (addr1->ss_family == AF_INET) {
-		return (memcmp(&((const struct sockaddr_in *)addr1)->sin_addr,
-				&((const struct sockaddr_in *)addr2)->sin_addr,
-				sizeof(struct in_addr)) == 0);
-	}
-
-	if (addr1->ss_family == AF_INET6) {
-		return (memcmp(&((const struct sockaddr_in6 *)addr1)->sin6_addr,
-				&((const struct sockaddr_in6 *)addr2)->sin6_addr,
-				sizeof(struct in6_addr)) == 0);
-	}
-
-	return false;
-}
-
 static int cifs_swn_store_swn_addr(const struct sockaddr_storage *new,
 				   const struct sockaddr_storage *old,
 				   struct sockaddr_storage *dst)
@@ -516,18 +506,13 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
 {
 	int ret = 0;
 
-	/* Store the reconnect address */
 	cifs_server_lock(tcon->ses->server);
-	if (cifs_sockaddr_equal(&tcon->ses->server->dstaddr, addr))
-		goto unlock;
 
-	ret = cifs_swn_store_swn_addr(addr, &tcon->ses->server->dstaddr,
-				      &tcon->ses->server->swn_dstaddr);
-	if (ret < 0) {
-		cifs_dbg(VFS, "%s: failed to store address: %d\n", __func__, ret);
+	if (cifs_match_ipaddr((struct sockaddr *)&tcon->ses->server->dstaddr,
+			      (struct sockaddr *)addr)) {
+		/* no-op */
 		goto unlock;
 	}
-	tcon->ses->server->use_swn_dstaddr = true;
 
 	/*
 	 * Unregister to stop receiving notifications for the old IP address.
@@ -536,8 +521,23 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
 	if (ret < 0) {
 		cifs_dbg(VFS, "%s: Failed to unregister for witness notifications: %d\n",
 			 __func__, ret);
+		/*
+		 * Do not jump return on error, continue storing and registering for
+		 * notifications for the new address. There will be a stale registration
+		 * around running its periodic check task, which should cancel itself
+		 * if no matching any tcon.
+		 */
+	}
+
+	/* Store the reconnect address */
+	ret = cifs_swn_store_swn_addr(addr, &tcon->ses->server->dstaddr,
+				      &tcon->ses->server->swn_dstaddr);
+	if (ret < 0) {
+		cifs_dbg(VFS, "%s: failed to store address: %d\n", __func__,
+			 ret);
 		goto unlock;
 	}
+	tcon->ses->server->use_swn_dstaddr = true;
 
 	/*
 	 * And register to receive notifications for the new IP address now that we have
@@ -681,33 +681,20 @@ int cifs_swn_unregister(struct cifs_tcon *tcon)
 void cifs_swn_dump(struct seq_file *m)
 {
 	struct cifs_swn_reg *swnreg;
-	struct sockaddr_in *sa;
-	struct sockaddr_in6 *sa6;
 	int id;
 
 	seq_puts(m, "Witness registrations:");
 
 	mutex_lock(&cifs_swnreg_idr_mutex);
 	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
-		seq_printf(m, "\nId: %u Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: ",
-				id, kref_read(&swnreg->ref_count),
-				swnreg->net_name, swnreg->net_name_notify ? "(y)" : "(n)",
-				swnreg->share_name, swnreg->share_name_notify ? "(y)" : "(n)");
-		switch (swnreg->tcon->ses->server->dstaddr.ss_family) {
-		case AF_INET:
-			sa = (struct sockaddr_in *) &swnreg->tcon->ses->server->dstaddr;
-			seq_printf(m, "%pI4", &sa->sin_addr.s_addr);
-			break;
-		case AF_INET6:
-			sa6 = (struct sockaddr_in6 *) &swnreg->tcon->ses->server->dstaddr;
-			seq_printf(m, "%pI6", &sa6->sin6_addr.s6_addr);
-			if (sa6->sin6_scope_id)
-				seq_printf(m, "%%%u", sa6->sin6_scope_id);
-			break;
-		default:
-			seq_puts(m, "(unknown)");
-		}
-		seq_printf(m, "%s", swnreg->ip_notify ? "(y)" : "(n)");
+		seq_printf(
+			m,
+			"\nId: %u Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: '%pISc'%s",
+			id, kref_read(&swnreg->ref_count), swnreg->net_name,
+			swnreg->net_name_notify ? "(y)" : "(n)",
+			swnreg->share_name,
+			swnreg->share_name_notify ? "(y)" : "(n)",
+			&swnreg->addr, swnreg->ip_notify ? "(y)" : "(n)");
 	}
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 	seq_puts(m, "\n");
-- 
2.54.0

[PATCH 07/18] smb:client: Fix printf format string

[Samuel Cabrero]

It was printing a signed integer as unsigned.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index a33caaf4aaf0..877272788399 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -689,7 +689,7 @@ void cifs_swn_dump(struct seq_file *m)
 	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
 		seq_printf(
 			m,
-			"\nId: %u Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: '%pISc'%s",
+			"\nId: %d Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: '%pISc'%s",
 			id, kref_read(&swnreg->ref_count), swnreg->net_name,
 			swnreg->net_name_notify ? "(y)" : "(n)",
 			swnreg->share_name,
-- 
2.54.0

[PATCH 08/18] smb:client: Save memory allocations

[Samuel Cabrero]

Avoid memory allocations, parse the unc string directly and use
string lengths when comparing parts of it.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 117 +++++++++++++++++++++++----------------
 1 file changed, 68 insertions(+), 49 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 877272788399..b823a496cba7 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -72,6 +72,72 @@ static int cifs_swn_auth_info_ntlm(struct cifs_tcon *tcon, struct sk_buff *skb)
 	return 0;
 }
 
+static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
+				      const struct cifs_tcon *tcon)
+{
+	const char *unc = tcon->tree_name;
+	struct sockaddr_storage *tcon_dstaddr;
+	const char *host, *share, *delim;
+	size_t host_len, share_len;
+
+	if (!tcon->use_witness)
+		return false;
+
+	if (tcon->ses->server->use_swn_dstaddr)
+		tcon_dstaddr = &tcon->ses->server->swn_dstaddr;
+	else
+		tcon_dstaddr = &tcon->ses->server->dstaddr;
+
+	if (!cifs_match_ipaddr((struct sockaddr *)&swnreg->addr,
+			       (struct sockaddr *)tcon_dstaddr))
+		return false;
+
+	if (swnreg->net_name_notify) {
+		/* extract hostname, requires strlen(unc) >= 3 (\\a)*/
+		if (strnlen(unc, 3) < 3)
+			return false;
+
+		/* extract_hostname: skip all leading '\' characters */
+		for (host = unc; *host && *host == '\\'; host++)
+			;
+
+		if (!*host)
+			return false;
+
+		delim = strchr(host, '\\');
+		if (!delim)
+			return false;
+
+		host_len = delim - host;
+		if (strlen(swnreg->net_name) == host_len &&
+		    !strncasecmp(swnreg->net_name, host, host_len)) {
+			return true;
+		}
+	}
+
+	if (swnreg->share_name_notify) {
+		/* extract share name, requires strlen(unc) >= 5 (\\a\b) */
+		if (strnlen(unc, 5) < 5)
+			return false;
+
+		/* extract share name, start at unc + 2, then first '\' onward */
+		share = unc + 2;
+		delim = strchr(share, '\\');
+		if (!delim)
+			return false;
+
+		share = delim + 1;
+		share_len = strlen(share);
+
+		if (strlen(swnreg->share_name) == share_len &&
+		    !strncasecmp(swnreg->share_name, share, share_len)) {
+			return true;
+		}
+	}
+
+	return false;
+}
+
 /*
  * Sends a register message to the userspace daemon based on the registration.
  * The authentication information to connect to the witness service is bundled
@@ -305,64 +371,17 @@ static void cifs_swn_reg_check(struct work_struct *work)
 /*
  * Try to find a matching registration for the tcon's server name and share name.
  * Calls to this function must be protected by cifs_swnreg_idr_mutex.
- * TODO Try to avoid memory allocations
  */
 static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
 {
 	struct cifs_swn_reg *swnreg;
 	int id;
-	const char *share_name;
-	const char *net_name;
-
-	net_name = extract_hostname(tcon->tree_name);
-	if (IS_ERR(net_name)) {
-		int ret;
-
-		ret = PTR_ERR(net_name);
-		cifs_dbg(VFS, "%s: failed to extract host name from target '%s': %d\n",
-				__func__, tcon->tree_name, ret);
-		return ERR_PTR(-EINVAL);
-	}
-
-	share_name = extract_sharename(tcon->tree_name);
-	if (IS_ERR(share_name)) {
-		int ret;
-
-		ret = PTR_ERR(share_name);
-		cifs_dbg(VFS, "%s: failed to extract share name from target '%s': %d\n",
-				__func__, tcon->tree_name, ret);
-		kfree(net_name);
-		return ERR_PTR(-EINVAL);
-	}
 
 	idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) {
-		struct sockaddr_storage *tcon_dstaddr;
-
-		if (tcon->ses->server->use_swn_dstaddr)
-			tcon_dstaddr = &tcon->ses->server->swn_dstaddr;
-		else
-			tcon_dstaddr = &tcon->ses->server->dstaddr;
-
-		if (strcasecmp(swnreg->net_name, net_name) != 0 ||
-		    strcasecmp(swnreg->share_name, share_name) != 0 ||
-		    !cifs_match_ipaddr((struct sockaddr *)&swnreg->addr,
-				       (struct sockaddr *)tcon_dstaddr)) {
-			continue;
-		}
-
-		cifs_dbg(FYI,
-			 "Existing swn registration for %pISc:%s:%s found\n",
-			 &swnreg->addr, swnreg->net_name, swnreg->share_name);
-
-		kfree(net_name);
-		kfree(share_name);
-
-		return swnreg;
+		if (cifs_swn_reg_tcon_matches(swnreg, tcon))
+			return swnreg;
 	}
 
-	kfree(net_name);
-	kfree(share_name);
-
 	return ERR_PTR(-EEXIST);
 }
 
-- 
2.54.0

[PATCH 09/18] smb:client: Use the correct return code

[Samuel Cabrero]

Return ENOENT instead of EEXIST when a registration does not exist.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index b823a496cba7..0d7e99203281 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -382,7 +382,7 @@ static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
 			return swnreg;
 	}
 
-	return ERR_PTR(-EEXIST);
+	return ERR_PTR(-ENOENT);
 }
 
 /*
@@ -401,7 +401,7 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	if (!IS_ERR(swnreg)) {
 		kref_get(&swnreg->ref_count);
 		goto unlock;
-	} else if (PTR_ERR(swnreg) != -EEXIST) {
+	} else if (PTR_ERR(swnreg) != -ENOENT) {
 		goto unlock;
 	}
 
-- 
2.54.0

[PATCH 10/18] smb:client: Improve tcon matching logic

[Samuel Cabrero]

Split the network name and share name checking into their own functions,
and check if these names match only if its associated flag is enabled.
The registration address is always compared.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 117 ++++++++++++++++++++++++---------------
 1 file changed, 72 insertions(+), 45 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 0d7e99203281..5728647a1e5d 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -72,13 +72,67 @@ static int cifs_swn_auth_info_ntlm(struct cifs_tcon *tcon, struct sk_buff *skb)
 	return 0;
 }
 
+static bool cifs_swn_reg_net_name_matches(const struct cifs_swn_reg *swnreg,
+					  const struct cifs_tcon *tcon)
+{
+	const char *unc = tcon->tree_name;
+	const char *host, *delim;
+	size_t host_len;
+
+	/* extract hostname, requires strlen(unc) >= 3 (\\a)*/
+	if (strnlen(unc, 3) < 3)
+		return false;
+
+	/* extract_hostname: skip all leading '\' characters */
+	for (host = unc; *host && *host == '\\'; host++)
+		;
+
+	if (!*host)
+		return false;
+
+	delim = strchr(host, '\\');
+	if (!delim)
+		return false;
+
+	host_len = delim - host;
+	if (strlen(swnreg->net_name) == host_len &&
+	    !strncasecmp(swnreg->net_name, host, host_len)) {
+		return true;
+	}
+	return false;
+}
+
+static bool cifs_swn_reg_share_name_matches(const struct cifs_swn_reg *swnreg,
+					    const struct cifs_tcon *tcon)
+{
+	const char *unc = tcon->tree_name;
+	const char *share, *delim;
+	size_t share_len;
+
+	/* extract share name, requires strlen(unc) >= 5 (\\a\b) */
+	if (strnlen(unc, 5) < 5)
+		return false;
+
+	/* extract share name, start at unc + 2, then first '\' onward */
+	share = unc + 2;
+	delim = strchr(share, '\\');
+	if (!delim)
+		return false;
+
+	share = delim + 1;
+	share_len = strlen(share);
+
+	if (strlen(swnreg->share_name) == share_len &&
+	    !strncasecmp(swnreg->share_name, share, share_len)) {
+		return true;
+	}
+	return false;
+}
+
 static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
 				      const struct cifs_tcon *tcon)
 {
-	const char *unc = tcon->tree_name;
 	struct sockaddr_storage *tcon_dstaddr;
-	const char *host, *share, *delim;
-	size_t host_len, share_len;
 
 	if (!tcon->use_witness)
 		return false;
@@ -88,54 +142,27 @@ static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
 	else
 		tcon_dstaddr = &tcon->ses->server->dstaddr;
 
+	/* Address must always match */
 	if (!cifs_match_ipaddr((struct sockaddr *)&swnreg->addr,
 			       (struct sockaddr *)tcon_dstaddr))
 		return false;
 
-	if (swnreg->net_name_notify) {
-		/* extract hostname, requires strlen(unc) >= 3 (\\a)*/
-		if (strnlen(unc, 3) < 3)
-			return false;
-
-		/* extract_hostname: skip all leading '\' characters */
-		for (host = unc; *host && *host == '\\'; host++)
-			;
-
-		if (!*host)
-			return false;
-
-		delim = strchr(host, '\\');
-		if (!delim)
-			return false;
-
-		host_len = delim - host;
-		if (strlen(swnreg->net_name) == host_len &&
-		    !strncasecmp(swnreg->net_name, host, host_len)) {
-			return true;
-		}
-	}
+	/* The network name notification is always enabled */
+	if (!cifs_swn_reg_net_name_matches(swnreg, tcon))
+		return false;
 
-	if (swnreg->share_name_notify) {
-		/* extract share name, requires strlen(unc) >= 5 (\\a\b) */
-		if (strnlen(unc, 5) < 5)
-			return false;
-
-		/* extract share name, start at unc + 2, then first '\' onward */
-		share = unc + 2;
-		delim = strchr(share, '\\');
-		if (!delim)
-			return false;
-
-		share = delim + 1;
-		share_len = strlen(share);
-
-		if (strlen(swnreg->share_name) == share_len &&
-		    !strncasecmp(swnreg->share_name, share, share_len)) {
-			return true;
-		}
-	}
+	/*
+	 * Share name notifications is enabled only if asymmetric
+	 * capability enabled otherwise ignored
+	 */
+	if (swnreg->share_name_notify !=
+	    (tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC))
+		return false;
+	else if (swnreg->share_name_notify &&
+		 !cifs_swn_reg_share_name_matches(swnreg, tcon))
+		return false;
 
-	return false;
+	return true;
 }
 
 /*
-- 
2.54.0

[PATCH 11/18] smb:client: Pass the tcon to cifs_swn_send_register_message

[Samuel Cabrero]

Prepare to remove the cached tcon from the registration.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 5728647a1e5d..be275a99e8c4 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -170,7 +170,8 @@ static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
  * The authentication information to connect to the witness service is bundled
  * into the message.
  */
-static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
+static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg,
+					  struct cifs_tcon *tcon)
 {
 	struct sk_buff *skb;
 	struct genlmsghdr *hdr;
@@ -222,10 +223,10 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 			goto nlmsg_fail;
 	}
 
-	authtype = cifs_select_sectype(swnreg->tcon->ses->server, swnreg->tcon->ses->sectype);
+	authtype = cifs_select_sectype(tcon->ses->server, tcon->ses->sectype);
 	switch (authtype) {
 	case Kerberos:
-		ret = cifs_swn_auth_info_krb(swnreg->tcon, skb);
+		ret = cifs_swn_auth_info_krb(tcon, skb);
 		if (ret < 0) {
 			cifs_dbg(VFS, "%s: Failed to get kerberos auth info: %d\n", __func__, ret);
 			goto nlmsg_fail;
@@ -233,7 +234,7 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 		break;
 	case NTLMv2:
 	case RawNTLMSSP:
-		ret = cifs_swn_auth_info_ntlm(swnreg->tcon, skb);
+		ret = cifs_swn_auth_info_ntlm(tcon, skb);
 		if (ret < 0) {
 			cifs_dbg(VFS, "%s: Failed to get NTLM auth info: %d\n", __func__, ret);
 			goto nlmsg_fail;
@@ -378,7 +379,7 @@ static void cifs_swn_reg_check(struct work_struct *work)
 	 * The userspace client library tracks if registered or not
 	 * using the swnreg->id.
 	 */
-	ret = cifs_swn_send_register_message(swnreg);
+	ret = cifs_swn_send_register_message(swnreg, swnreg->tcon);
 	if (ret < 0)
 		cifs_dbg(FYI, "%s: Failed to send register message: %d\n",
 			 __func__, ret);
@@ -693,7 +694,7 @@ int cifs_swn_register(struct cifs_tcon *tcon)
 	if (IS_ERR(swnreg))
 		return PTR_ERR(swnreg);
 
-	ret = cifs_swn_send_register_message(swnreg);
+	ret = cifs_swn_send_register_message(swnreg, tcon);
 	if (ret < 0) {
 		cifs_dbg(VFS, "%s: Failed to send swn register message: %d\n", __func__, ret);
 		/* Do not put the swnreg or return error, the check task will retry */
-- 
2.54.0

[PATCH 12/18] smb:client: Pass the tcon to notification handlers

[Samuel Cabrero]

Prepare to remove the cached tcon from the registration.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index be275a99e8c4..0a45be8f36d7 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -500,16 +500,17 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	return ERR_PTR(ret);
 }
 
-static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const char *name, int state)
+static int cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
+					   const char *name, int state)
 {
 	switch (state) {
 	case CIFS_SWN_RESOURCE_STATE_UNAVAILABLE:
 		cifs_dbg(FYI, "%s: resource name '%s' become unavailable\n", __func__, name);
-		cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true);
+		cifs_signal_cifsd_for_reconnect(tcon->ses->server, true);
 		break;
 	case CIFS_SWN_RESOURCE_STATE_AVAILABLE:
 		cifs_dbg(FYI, "%s: resource name '%s' become available\n", __func__, name);
-		cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true);
+		cifs_signal_cifsd_for_reconnect(tcon->ses->server, true);
 		break;
 	case CIFS_SWN_RESOURCE_STATE_UNKNOWN:
 		cifs_dbg(FYI, "%s: resource name '%s' changed to unknown state\n", __func__, name);
@@ -605,7 +606,8 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
 	return ret;
 }
 
-static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_storage *addr)
+static int cifs_swn_client_move(struct cifs_tcon *tcon,
+				struct sockaddr_storage *addr)
 {
 	struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr;
 	struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr;
@@ -615,7 +617,7 @@ static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_sto
 	else if (addr->ss_family == AF_INET6)
 		cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr);
 
-	return cifs_swn_reconnect(swnreg->tcon, addr);
+	return cifs_swn_reconnect(tcon, addr);
 }
 
 int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
@@ -664,7 +666,8 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 			cifs_dbg(FYI, "%s: missing resource state attribute\n", __func__);
 			return -EINVAL;
 		}
-		return cifs_swn_resource_state_changed(swnreg, name, state);
+		return cifs_swn_resource_state_changed(swnreg->tcon, name,
+						       state);
 	}
 	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE: {
 		struct sockaddr_storage addr;
@@ -675,7 +678,7 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 			cifs_dbg(FYI, "%s: missing IP address attribute\n", __func__);
 			return -EINVAL;
 		}
-		return cifs_swn_client_move(swnreg, &addr);
+		return cifs_swn_client_move(swnreg->tcon, &addr);
 	}
 	default:
 		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__, type);
-- 
2.54.0

[PATCH 13/18] smb:client: Introduce swn_notification struct

[Samuel Cabrero]

This is a helper struct to contain all necessary data to process a
notification.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 129 ++++++++++++++++++++++++++++++---------
 1 file changed, 99 insertions(+), 30 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 0a45be8f36d7..7e665b657f8d 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -36,6 +36,24 @@ struct cifs_swn_reg {
 	struct delayed_work check;
 };
 
+struct cifs_swn_notification {
+	int type;
+	union {
+		struct {
+			const char *name;
+			int state;
+		} resource_name_changed;
+		struct {
+			struct sockaddr_storage *addr;
+		} client_move;
+		struct {
+			struct sockaddr_storage *addr;
+		} share_move;
+	} data;
+
+	struct cifs_swn_reg *swnreg;
+};
+
 static int cifs_swn_auth_info_krb(struct cifs_tcon *tcon, struct sk_buff *skb)
 {
 	int ret;
@@ -620,71 +638,122 @@ static int cifs_swn_client_move(struct cifs_tcon *tcon,
 	return cifs_swn_reconnect(tcon, addr);
 }
 
+static int cifs_swn_handle_notification(const struct cifs_swn_notification *not)
+{
+	switch (not->type) {
+	case CIFS_SWN_NOTIFICATION_RESOURCE_CHANGE:
+		return cifs_swn_resource_state_changed(
+			not->swnreg->tcon, not->data.resource_name_changed.name,
+			not->data.resource_name_changed.state);
+	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE:
+		return cifs_swn_client_move(not->swnreg->tcon,
+					    not->data.client_move.addr);
+	default:
+		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__,
+			 not->type);
+		break;
+	}
+	return 0;
+}
+
 int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info)
 {
-	struct cifs_swn_reg *swnreg;
-	char name[256];
-	int type;
+	int swnreg_id;
+	struct cifs_swn_notification not;
+	int ret;
 
+	/* Get the registration ID */
 	if (info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]) {
-		int swnreg_id;
-
-		swnreg_id = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]);
-		mutex_lock(&cifs_swnreg_idr_mutex);
-		swnreg = idr_find(&cifs_swnreg_idr, swnreg_id);
-		mutex_unlock(&cifs_swnreg_idr_mutex);
-		if (swnreg == NULL) {
-			cifs_dbg(FYI, "%s: registration id %d not found\n", __func__, swnreg_id);
-			return -EINVAL;
-		}
+		swnreg_id = nla_get_u32(
+			info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]);
 	} else {
 		cifs_dbg(FYI, "%s: missing registration id attribute\n", __func__);
 		return -EINVAL;
 	}
 
+	/* Fill the notification struct */
 	if (info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]) {
-		type = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]);
+		not.type = nla_get_u32(
+			info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]);
 	} else {
 		cifs_dbg(FYI, "%s: missing notification type attribute\n", __func__);
 		return -EINVAL;
 	}
 
-	switch (type) {
+	switch (not.type) {
 	case CIFS_SWN_NOTIFICATION_RESOURCE_CHANGE: {
-		int state;
-
 		if (info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_NAME]) {
-			nla_strscpy(name, info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_NAME],
-					sizeof(name));
+			not.data.resource_name_changed
+				.name = (const char *)nla_data(
+				info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_NAME]);
 		} else {
 			cifs_dbg(FYI, "%s: missing resource name attribute\n", __func__);
 			return -EINVAL;
 		}
+
 		if (info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]) {
-			state = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]);
+			not.data.resource_name_changed.state = nla_get_u32(
+				info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]);
 		} else {
 			cifs_dbg(FYI, "%s: missing resource state attribute\n", __func__);
 			return -EINVAL;
 		}
-		return cifs_swn_resource_state_changed(swnreg->tcon, name,
-						       state);
-	}
+	} break;
 	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE: {
-		struct sockaddr_storage addr;
-
 		if (info->attrs[CIFS_GENL_ATTR_SWN_IP]) {
-			nla_memcpy(&addr, info->attrs[CIFS_GENL_ATTR_SWN_IP], sizeof(addr));
+			not.data.client_move.addr =
+				nla_data(info->attrs[CIFS_GENL_ATTR_SWN_IP]);
 		} else {
 			cifs_dbg(FYI, "%s: missing IP address attribute\n", __func__);
 			return -EINVAL;
 		}
-		return cifs_swn_client_move(swnreg->tcon, &addr);
-	}
+	} break;
 	default:
-		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__, type);
-		break;
+		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__,
+			 not.type);
+		return 0;
+	}
+
+	/*
+	 * Get the registration.
+	 */
+	mutex_lock(&cifs_swnreg_idr_mutex);
+	not.swnreg = idr_find(&cifs_swnreg_idr, swnreg_id);
+	if (not.swnreg == NULL) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		cifs_dbg(FYI, "%s: registration id %d not found\n", __func__,
+			 swnreg_id);
+		return 0;
+	}
+
+	/*
+	 * Increment the refcount under the lock while processing the notification
+	 * and release the swnreg_idr_mutex because processing will take
+	 * cifs_tcp_ses_lock.
+	 */
+	if (!kref_get_unless_zero(&not.swnreg->ref_count)) {
+		/* The registration is being released, ignore the notificaiton */
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return 0;
+	}
+	mutex_unlock(&cifs_swnreg_idr_mutex);
+
+	ret = cifs_swn_handle_notification(&not);
+	if (ret) {
+		cifs_dbg(FYI,
+			"%s: Failed to process notification for registration id %d: %d\n",
+			__func__, swnreg_id, ret);
 	}
 
+	mutex_lock(&cifs_swnreg_idr_mutex);
+	if (kref_put(&not.swnreg->ref_count, cifs_swn_reg_idr_remove)) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		cancel_delayed_work_sync(&not.swnreg->check);
+		cifs_swn_reg_release(not.swnreg);
+		return 0;
+	}
+	mutex_unlock(&cifs_swnreg_idr_mutex);
+
 	return 0;
 }
 
-- 
2.54.0

[PATCH 14/18] smb:client: Cache auth info

[Samuel Cabrero]

Continue reducing dependency from tcon. Now the registration have all
required information to operate on its own and register/unregister as
necessary.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 153 +++++++++++++++++++++++++++++++++++----
 1 file changed, 138 insertions(+), 15 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 7e665b657f8d..6d8e3b94a432 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -30,6 +30,18 @@ struct cifs_swn_reg {
 	bool share_name_notify;
 	bool ip_notify;
 
+	/* Need to keep this information to re-register when reconnecting */
+	enum securityEnum auth_type;
+	union {
+		struct {
+		} kerberos;
+		struct {
+			const char *domain_name;
+			const char *user_name;
+			const char *password;
+		} ntlm;
+	} authinfo;
+
 	struct cifs_tcon *tcon;
 
 	unsigned long check_interval;
@@ -54,7 +66,92 @@ struct cifs_swn_notification {
 	struct cifs_swn_reg *swnreg;
 };
 
-static int cifs_swn_auth_info_krb(struct cifs_tcon *tcon, struct sk_buff *skb)
+static int cifs_swn_reg_set_auth(struct cifs_swn_reg *swnreg,
+				 const struct cifs_tcon *tcon)
+{
+	swnreg->auth_type =
+		cifs_select_sectype(tcon->ses->server, tcon->ses->sectype);
+	switch (swnreg->auth_type) {
+	case Kerberos:
+		break;
+	case NTLMv2:
+	case RawNTLMSSP:
+		if (tcon->ses->user_name != NULL) {
+			swnreg->authinfo.ntlm.user_name =
+				kstrdup(tcon->ses->user_name, GFP_KERNEL);
+			if (swnreg->authinfo.ntlm.user_name == NULL)
+				return -ENOMEM;
+		}
+		if (tcon->ses->domainName != NULL) {
+			swnreg->authinfo.ntlm.domain_name =
+				kstrdup(tcon->ses->domainName, GFP_KERNEL);
+			if (swnreg->authinfo.ntlm.domain_name == NULL) {
+				kfree(swnreg->authinfo.ntlm.user_name);
+				return -ENOMEM;
+			}
+		}
+		if (tcon->ses->password != NULL) {
+			swnreg->authinfo.ntlm.password =
+				kstrdup(tcon->ses->password, GFP_KERNEL);
+			if (swnreg->authinfo.ntlm.password == NULL) {
+				kfree(swnreg->authinfo.ntlm.user_name);
+				kfree(swnreg->authinfo.ntlm.domain_name);
+				return -ENOMEM;
+			}
+		}
+		break;
+	default:
+		cifs_dbg(VFS, "%s: secType %d not supported!\n", __func__,
+			 swnreg->auth_type);
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static bool cifs_swn_str_equal(const char *a, const char *b)
+{
+	if (a == b)
+		return true;
+
+	if (a == NULL || b == NULL)
+		return false;
+
+	return strcmp(a, b) == 0;
+}
+
+static bool cifs_swn_auth_info_equal(const struct cifs_swn_reg *swnreg,
+				     const struct cifs_tcon *tcon)
+{
+	enum securityEnum auth_type =
+		cifs_select_sectype(tcon->ses->server, tcon->ses->sectype);
+	if (swnreg->auth_type != auth_type)
+		return false;
+
+	switch (auth_type) {
+	case Kerberos:
+		break;
+	case NTLMv2:
+	case RawNTLMSSP:
+		if (!cifs_swn_str_equal(tcon->ses->user_name,
+					swnreg->authinfo.ntlm.user_name))
+			return false;
+		if (!cifs_swn_str_equal(tcon->ses->domainName,
+					swnreg->authinfo.ntlm.domain_name))
+			return false;
+		if (!cifs_swn_str_equal(tcon->ses->password,
+					swnreg->authinfo.ntlm.password))
+			return false;
+		break;
+	default:
+		cifs_dbg(VFS, "%s: secType %d not supported!\n", __func__,
+			 auth_type);
+		return false;
+	}
+
+	return true;
+}
+
+static int cifs_swn_auth_info_krb(struct sk_buff *skb)
 {
 	int ret;
 
@@ -65,24 +162,28 @@ static int cifs_swn_auth_info_krb(struct cifs_tcon *tcon, struct sk_buff *skb)
 	return 0;
 }
 
-static int cifs_swn_auth_info_ntlm(struct cifs_tcon *tcon, struct sk_buff *skb)
+static int cifs_swn_auth_info_ntlm(struct cifs_swn_reg *swnreg,
+				   struct sk_buff *skb)
 {
 	int ret;
 
-	if (tcon->ses->user_name != NULL) {
-		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_USER_NAME, tcon->ses->user_name);
+	if (swnreg->authinfo.ntlm.user_name != NULL) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_USER_NAME,
+				     swnreg->authinfo.ntlm.user_name);
 		if (ret < 0)
 			return ret;
 	}
 
-	if (tcon->ses->password != NULL) {
-		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_PASSWORD, tcon->ses->password);
+	if (swnreg->authinfo.ntlm.password != NULL) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_PASSWORD,
+				     swnreg->authinfo.ntlm.password);
 		if (ret < 0)
 			return ret;
 	}
 
-	if (tcon->ses->domainName != NULL) {
-		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_DOMAIN_NAME, tcon->ses->domainName);
+	if (swnreg->authinfo.ntlm.domain_name != NULL) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_DOMAIN_NAME,
+				     swnreg->authinfo.ntlm.domain_name);
 		if (ret < 0)
 			return ret;
 	}
@@ -160,6 +261,10 @@ static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
 	else
 		tcon_dstaddr = &tcon->ses->server->dstaddr;
 
+	/* Auth info must match */
+	if (!cifs_swn_auth_info_equal(swnreg, tcon))
+		return false;
+
 	/* Address must always match */
 	if (!cifs_match_ipaddr((struct sockaddr *)&swnreg->addr,
 			       (struct sockaddr *)tcon_dstaddr))
@@ -193,7 +298,6 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg,
 {
 	struct sk_buff *skb;
 	struct genlmsghdr *hdr;
-	enum securityEnum authtype;
 	int ret;
 
 	skb = genlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
@@ -241,10 +345,9 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg,
 			goto nlmsg_fail;
 	}
 
-	authtype = cifs_select_sectype(tcon->ses->server, tcon->ses->sectype);
-	switch (authtype) {
+	switch (swnreg->auth_type) {
 	case Kerberos:
-		ret = cifs_swn_auth_info_krb(tcon, skb);
+		ret = cifs_swn_auth_info_krb(skb);
 		if (ret < 0) {
 			cifs_dbg(VFS, "%s: Failed to get kerberos auth info: %d\n", __func__, ret);
 			goto nlmsg_fail;
@@ -252,14 +355,15 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg,
 		break;
 	case NTLMv2:
 	case RawNTLMSSP:
-		ret = cifs_swn_auth_info_ntlm(tcon, skb);
+		ret = cifs_swn_auth_info_ntlm(swnreg, skb);
 		if (ret < 0) {
 			cifs_dbg(VFS, "%s: Failed to get NTLM auth info: %d\n", __func__, ret);
 			goto nlmsg_fail;
 		}
 		break;
 	default:
-		cifs_dbg(VFS, "%s: secType %d not supported!\n", __func__, authtype);
+		cifs_dbg(VFS, "%s: secType %d not supported!\n", __func__,
+			 swnreg->auth_type);
 		ret = -EINVAL;
 		goto nlmsg_fail;
 	}
@@ -354,6 +458,17 @@ static void cifs_swn_reg_release(struct cifs_swn_reg *swnreg)
 	if (ret < 0)
 		cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", __func__, ret);
 
+	switch (swnreg->auth_type) {
+	case NTLMv2:
+	case RawNTLMSSP:
+		kfree(swnreg->authinfo.ntlm.user_name);
+		kfree(swnreg->authinfo.ntlm.domain_name);
+		kfree(swnreg->authinfo.ntlm.password);
+		break;
+	default:
+		break;
+	}
+
 	kfree(swnreg->net_name);
 	kfree(swnreg->share_name);
 	kfree(swnreg);
@@ -480,6 +595,13 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 		goto fail_net_name;
 	}
 
+	ret = cifs_swn_reg_set_auth(swnreg, tcon);
+	if (ret != 0) {
+		cifs_dbg(VFS, "%s: failed to set auth info: %d\n", __func__,
+			 ret);
+		goto fail_share_name;
+	}
+
 	/*
 	 * If there is an address stored use it instead of the server address, because we are
 	 * in the process of reconnecting to it after a share has been moved or we have been
@@ -506,7 +628,8 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
 	return swnreg;
-
+fail_share_name:
+	kfree(swnreg->share_name);
 fail_net_name:
 	kfree(swnreg->net_name);
 fail_idr:
-- 
2.54.0

[PATCH 15/18] smb:client: Do not store the tcon reference, find a matching one

[Samuel Cabrero]

Finally remove the cached tcon pointer from the registration.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 227 ++++++++++++++++++++++++++++-----------
 fs/smb/client/trace.h    |   2 +
 2 files changed, 166 insertions(+), 63 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 6d8e3b94a432..97e415e441e4 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -42,8 +42,6 @@ struct cifs_swn_reg {
 		} ntlm;
 	} authinfo;
 
-	struct cifs_tcon *tcon;
-
 	unsigned long check_interval;
 	struct delayed_work check;
 };
@@ -288,13 +286,46 @@ static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
 	return true;
 }
 
+static struct cifs_tcon *
+cifs_swn_reg_get_tcon(const struct cifs_swn_reg *swnreg)
+{
+	struct TCP_Server_Info *server;
+	struct cifs_ses *ses;
+	struct cifs_tcon *tcon;
+
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
+		if (SERVER_IS_CHAN(server))
+			continue;
+
+		list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+			list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
+				spin_lock(&tcon->tc_lock);
+				if (tcon->status == TID_EXITING ||
+				    !cifs_swn_reg_tcon_matches(swnreg, tcon)) {
+					spin_unlock(&tcon->tc_lock);
+					continue;
+				}
+				++tcon->tc_count;
+				trace_smb3_tcon_ref(
+					tcon->debug_id, tcon->tc_count,
+					netfs_trace_tcon_ref_get_swn);
+				spin_unlock(&tcon->tc_lock);
+				spin_unlock(&cifs_tcp_ses_lock);
+				return tcon;
+			}
+		}
+	}
+	spin_unlock(&cifs_tcp_ses_lock);
+	return ERR_PTR(-ENOENT);
+}
+
 /*
  * Sends a register message to the userspace daemon based on the registration.
  * The authentication information to connect to the witness service is bundled
  * into the message.
  */
-static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg,
-					  struct cifs_tcon *tcon)
+static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 {
 	struct sk_buff *skb;
 	struct genlmsghdr *hdr;
@@ -489,6 +520,7 @@ static void cifs_swn_reg_check(struct work_struct *work)
 {
 	struct cifs_swn_reg *swnreg =
 		container_of(work, struct cifs_swn_reg, check.work);
+	struct cifs_tcon *tcon;
 	int ret;
 
 	/*
@@ -507,12 +539,48 @@ static void cifs_swn_reg_check(struct work_struct *work)
 	}
 	mutex_unlock(&cifs_swnreg_idr_mutex);
 
+	tcon = cifs_swn_reg_get_tcon(swnreg);
+	if (IS_ERR(tcon)) {
+		ret = PTR_ERR(tcon);
+		cifs_dbg(FYI, "No matching tcon for registration id %d: %d\n",
+			 swnreg->id, ret);
+
+		/*
+		 * There is no point in keeping a live registration if there
+		 * are no matching tcons. Drop the reference that this fn
+		 * took at the top.
+		 */
+		mutex_lock(&cifs_swnreg_idr_mutex);
+		if (kref_put(&swnreg->ref_count, cifs_swn_reg_idr_remove)) {
+			mutex_unlock(&cifs_swnreg_idr_mutex);
+			cifs_swn_reg_release(swnreg);
+			return;
+		}
+
+		/*
+		 * Again, to drop the birth reference created via kref_init().
+		 * If there are other live references, next check run might release it.
+		 */
+		if (kref_put(&swnreg->ref_count, cifs_swn_reg_idr_remove)) {
+			mutex_unlock(&cifs_swnreg_idr_mutex);
+			cifs_swn_reg_release(swnreg);
+			return;
+		}
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+
+		/* References remain: retry later. */
+		queue_delayed_work(cifsiod_wq, &swnreg->check,
+				   swnreg->check_interval);
+		return;
+	}
+	cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn);
+
 	/*
 	 * It is safe to send the registration message multiple times.
 	 * The userspace client library tracks if registered or not
 	 * using the swnreg->id.
 	 */
-	ret = cifs_swn_send_register_message(swnreg, swnreg->tcon);
+	ret = cifs_swn_send_register_message(swnreg);
 	if (ret < 0)
 		cifs_dbg(FYI, "%s: Failed to send register message: %d\n",
 			 __func__, ret);
@@ -618,8 +686,6 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
 	swnreg->ip_notify = false;
 
-	swnreg->tcon = tcon;
-
 	swnreg->check_interval = tcon->ses->server->echo_interval;
 	INIT_DELAYED_WORK(&swnreg->check, cifs_swn_reg_check);
 
@@ -641,8 +707,8 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
 	return ERR_PTR(ret);
 }
 
-static int cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
-					   const char *name, int state)
+static void cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
+					    const char *name, int state)
 {
 	switch (state) {
 	case CIFS_SWN_RESOURCE_STATE_UNAVAILABLE:
@@ -657,7 +723,6 @@ static int cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
 		cifs_dbg(FYI, "%s: resource name '%s' changed to unknown state\n", __func__, name);
 		break;
 	}
-	return 0;
 }
 
 static int cifs_swn_store_swn_addr(const struct sockaddr_storage *new,
@@ -691,31 +756,22 @@ static int cifs_swn_store_swn_addr(const struct sockaddr_storage *new,
 	return 0;
 }
 
-static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *addr)
+static bool cifs_swn_client_move(struct cifs_tcon *tcon,
+				 struct sockaddr_storage *addr)
 {
-	int ret = 0;
+	struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr;
+	struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr;
+	int ret;
 
-	cifs_server_lock(tcon->ses->server);
+	if (addr->ss_family == AF_INET)
+		cifs_dbg(FYI, "%s: move to %pI4\n", __func__, &ipv4->sin_addr);
+	else if (addr->ss_family == AF_INET6)
+		cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr);
 
 	if (cifs_match_ipaddr((struct sockaddr *)&tcon->ses->server->dstaddr,
 			      (struct sockaddr *)addr)) {
 		/* no-op */
-		goto unlock;
-	}
-
-	/*
-	 * Unregister to stop receiving notifications for the old IP address.
-	 */
-	ret = cifs_swn_unregister(tcon);
-	if (ret < 0) {
-		cifs_dbg(VFS, "%s: Failed to unregister for witness notifications: %d\n",
-			 __func__, ret);
-		/*
-		 * Do not jump return on error, continue storing and registering for
-		 * notifications for the new address. There will be a stale registration
-		 * around running its periodic check task, which should cancel itself
-		 * if no matching any tcon.
-		 */
+		return false;
 	}
 
 	/* Store the reconnect address */
@@ -724,58 +780,103 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a
 	if (ret < 0) {
 		cifs_dbg(VFS, "%s: failed to store address: %d\n", __func__,
 			 ret);
-		goto unlock;
+		return false;
 	}
 	tcon->ses->server->use_swn_dstaddr = true;
 
-	/*
-	 * And register to receive notifications for the new IP address now that we have
-	 * stored the new address.
-	 */
-	ret = cifs_swn_register(tcon);
-	if (ret < 0) {
-		cifs_dbg(VFS, "%s: Failed to register for witness notifications: %d\n",
-			 __func__, ret);
-		goto unlock;
-	}
-
 	cifs_signal_cifsd_for_reconnect(tcon->ses->server, false);
 
-unlock:
-	cifs_server_unlock(tcon->ses->server);
-
-	return ret;
+	return true;
 }
 
-static int cifs_swn_client_move(struct cifs_tcon *tcon,
-				struct sockaddr_storage *addr)
+/*
+ * This function applies the notification to a matching tcon.
+ * It is called holding a spinlock. Just sets the reconnect flag and
+ * store the reconnect address if necessary.
+ */
+static bool
+cifs_swn_handle_notification_tcon(const struct cifs_swn_notification *not,
+				  struct cifs_tcon *tcon)
 {
-	struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr;
-	struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr;
-
-	if (addr->ss_family == AF_INET)
-		cifs_dbg(FYI, "%s: move to %pI4\n", __func__, &ipv4->sin_addr);
-	else if (addr->ss_family == AF_INET6)
-		cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr);
-
-	return cifs_swn_reconnect(tcon, addr);
+	switch (not->type) {
+	case CIFS_SWN_NOTIFICATION_RESOURCE_CHANGE:
+		cifs_swn_resource_state_changed(
+			tcon, not->data.resource_name_changed.name,
+			not->data.resource_name_changed.state);
+		return false;
+	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE:
+		return cifs_swn_client_move(tcon, not->data.client_move.addr);
+	default:
+		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__,
+			 not->type);
+		break;
+	}
+	return false;
 }
 
+/*
+ * This function process a notification received for a registration. It
+ * searches all matching tcons to deliver it and then unregisters/registers
+ * as necessary if the address has changed.
+ */
 static int cifs_swn_handle_notification(const struct cifs_swn_notification *not)
 {
+	struct TCP_Server_Info *server;
+	struct cifs_ses *ses;
+	struct cifs_tcon *tcon;
+	int ret;
+
+	/* Walk the tcons and apply the notification to matching ones */
+	spin_lock(&cifs_tcp_ses_lock);
+	list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
+		if (SERVER_IS_CHAN(server))
+			continue;
+
+		cifs_server_lock(server);
+		list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
+			list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
+				spin_lock(&tcon->tc_lock);
+				if (tcon->status == TID_EXITING ||
+				    !cifs_swn_reg_tcon_matches(not->swnreg,
+							       tcon)) {
+					spin_unlock(&tcon->tc_lock);
+					continue;
+				}
+				cifs_swn_handle_notification_tcon(not, tcon);
+				spin_unlock(&tcon->tc_lock);
+			}
+		}
+		cifs_server_unlock(server);
+	}
+	spin_unlock(&cifs_tcp_ses_lock);
+
 	switch (not->type) {
-	case CIFS_SWN_NOTIFICATION_RESOURCE_CHANGE:
-		return cifs_swn_resource_state_changed(
-			not->swnreg->tcon, not->data.resource_name_changed.name,
-			not->data.resource_name_changed.state);
 	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE:
-		return cifs_swn_client_move(not->swnreg->tcon,
-					    not->data.client_move.addr);
+		/* Unregister from the current address */
+		ret = cifs_swn_send_unregister_message(not->swnreg);
+		if (ret < 0) {
+			cifs_dbg(VFS,
+				 "%s: Failed to unregister for witness notifications: %d\n",
+				 __func__, ret);
+		}
+
+		/* Store the new address */
+		not->swnreg->addr = *not->data.client_move.addr;
+
+		/* Register for this new address */
+		ret = cifs_swn_send_register_message(not->swnreg);
+		if (ret < 0) {
+			cifs_dbg(VFS,
+				 "%s: Failed to register for witness notifications: %d\n",
+				 __func__, ret);
+		}
+		break;
 	default:
 		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__,
 			 not->type);
 		break;
 	}
+
 	return 0;
 }
 
@@ -889,7 +990,7 @@ int cifs_swn_register(struct cifs_tcon *tcon)
 	if (IS_ERR(swnreg))
 		return PTR_ERR(swnreg);
 
-	ret = cifs_swn_send_register_message(swnreg, tcon);
+	ret = cifs_swn_send_register_message(swnreg);
 	if (ret < 0) {
 		cifs_dbg(VFS, "%s: Failed to send swn register message: %d\n", __func__, ret);
 		/* Do not put the swnreg or return error, the check task will retry */
diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h
index b99ec5a417fa..2b4cbffd2931 100644
--- a/fs/smb/client/trace.h
+++ b/fs/smb/client/trace.h
@@ -181,6 +181,7 @@
 	EM(netfs_trace_tcon_ref_get_find,		"GET Find  ") \
 	EM(netfs_trace_tcon_ref_get_find_sess_tcon,	"GET FndSes") \
 	EM(netfs_trace_tcon_ref_get_reconnect_server,	"GET Reconn") \
+	EM(netfs_trace_tcon_ref_get_swn,		"GET Swn   ") \
 	EM(netfs_trace_tcon_ref_new,			"NEW       ") \
 	EM(netfs_trace_tcon_ref_new_ipc,		"NEW Ipc   ") \
 	EM(netfs_trace_tcon_ref_new_reconnect_server,	"NEW Reconn") \
@@ -193,6 +194,7 @@
 	EM(netfs_trace_tcon_ref_put_dfs_refer,		"PUT DfsRfr") \
 	EM(netfs_trace_tcon_ref_put_reconnect_server,	"PUT Reconn") \
 	EM(netfs_trace_tcon_ref_put_tlink,		"PUT Tlink ") \
+	EM(netfs_trace_tcon_ref_put_swn,		"PUT Swn   ") \
 	EM(netfs_trace_tcon_ref_see_cancelled_close,	"SEE Cn-Cls") \
 	EM(netfs_trace_tcon_ref_see_fscache_collision,	"SEE FV-CO!") \
 	EM(netfs_trace_tcon_ref_see_fscache_okay,	"SEE FV-Ok ") \
-- 
2.54.0

[PATCH 16/18] smb:client: Simplify registration creation

[Samuel Cabrero]

And switch to GFP_KERNEL since the code path can sleep.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 209 ++++++++++++++++++++-------------------
 1 file changed, 106 insertions(+), 103 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 97e415e441e4..3375a65ec32e 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -323,7 +323,8 @@ cifs_swn_reg_get_tcon(const struct cifs_swn_reg *swnreg)
 /*
  * Sends a register message to the userspace daemon based on the registration.
  * The authentication information to connect to the witness service is bundled
- * into the message.
+ * into the message. This function can sleep while allocating the genlmsg so
+ * it must be called after taking a swnreg reference and release the lock.
  */
 static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 {
@@ -414,7 +415,9 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 }
 
 /*
- * Sends an uregister message to the userspace daemon based on the registration
+ * Sends an unregister message to the userspace daemon based on the registration.
+ * This function can sleep while allocating the genlmsg so it must be called after
+ * taking a swnreg reference and release the lock.
  */
 static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 {
@@ -481,6 +484,11 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 	return ret;
 }
 
+/*
+ * Release a registration. Must be called with the last reference dropped (the
+ * refcount has reached zero) and with the registration already removed from the
+ * IDR under cifs_swnreg_idr_mutex, so it is no longer discoverable.
+ */
 static void cifs_swn_reg_release(struct cifs_swn_reg *swnreg)
 {
 	int ret;
@@ -614,99 +622,6 @@ static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
 	return ERR_PTR(-ENOENT);
 }
 
-/*
- * Get a registration for the tcon's server and share name, allocating a new one if it does not
- * exists
- */
-static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon)
-{
-	struct cifs_swn_reg *swnreg = NULL;
-	int ret;
-
-	mutex_lock(&cifs_swnreg_idr_mutex);
-
-	/* Check if we are already registered for this network and share names */
-	swnreg = cifs_find_swn_reg(tcon);
-	if (!IS_ERR(swnreg)) {
-		kref_get(&swnreg->ref_count);
-		goto unlock;
-	} else if (PTR_ERR(swnreg) != -ENOENT) {
-		goto unlock;
-	}
-
-	swnreg = kmalloc_obj(struct cifs_swn_reg, GFP_ATOMIC);
-	if (swnreg == NULL) {
-		ret = -ENOMEM;
-		goto fail_unlock;
-	}
-
-	kref_init(&swnreg->ref_count);
-
-	swnreg->id = idr_alloc(&cifs_swnreg_idr, swnreg, 1, 0, GFP_ATOMIC);
-	if (swnreg->id < 0) {
-		cifs_dbg(FYI, "%s: failed to allocate registration id\n", __func__);
-		ret = swnreg->id;
-		goto fail;
-	}
-
-	swnreg->net_name = extract_hostname(tcon->tree_name);
-	if (IS_ERR(swnreg->net_name)) {
-		ret = PTR_ERR(swnreg->net_name);
-		cifs_dbg(VFS, "%s: failed to extract host name from target: %d\n", __func__, ret);
-		goto fail_idr;
-	}
-
-	swnreg->share_name = extract_sharename(tcon->tree_name);
-	if (IS_ERR(swnreg->share_name)) {
-		ret = PTR_ERR(swnreg->share_name);
-		cifs_dbg(VFS, "%s: failed to extract share name from target: %d\n", __func__, ret);
-		goto fail_net_name;
-	}
-
-	ret = cifs_swn_reg_set_auth(swnreg, tcon);
-	if (ret != 0) {
-		cifs_dbg(VFS, "%s: failed to set auth info: %d\n", __func__,
-			 ret);
-		goto fail_share_name;
-	}
-
-	/*
-	 * If there is an address stored use it instead of the server address, because we are
-	 * in the process of reconnecting to it after a share has been moved or we have been
-	 * told to switch to it (client move message). In these cases we unregister from the
-	 * server address and register to the new address when we receive the notification.
-	 */
-	if (tcon->ses->server->use_swn_dstaddr)
-		swnreg->addr = tcon->ses->server->swn_dstaddr;
-	else
-		swnreg->addr = tcon->ses->server->dstaddr;
-
-	swnreg->net_name_notify = true;
-	swnreg->share_name_notify =
-		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
-	swnreg->ip_notify = false;
-
-	swnreg->check_interval = tcon->ses->server->echo_interval;
-	INIT_DELAYED_WORK(&swnreg->check, cifs_swn_reg_check);
-
-	queue_delayed_work(cifsiod_wq, &swnreg->check, swnreg->check_interval);
-unlock:
-	mutex_unlock(&cifs_swnreg_idr_mutex);
-
-	return swnreg;
-fail_share_name:
-	kfree(swnreg->share_name);
-fail_net_name:
-	kfree(swnreg->net_name);
-fail_idr:
-	idr_remove(&cifs_swnreg_idr, swnreg->id);
-fail:
-	kfree(swnreg);
-fail_unlock:
-	mutex_unlock(&cifs_swnreg_idr_mutex);
-	return ERR_PTR(ret);
-}
-
 static void cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
 					    const char *name, int state)
 {
@@ -816,8 +731,8 @@ cifs_swn_handle_notification_tcon(const struct cifs_swn_notification *not,
 
 /*
  * This function process a notification received for a registration. It
- * searches all matching tcons to deliver it and then unregisters/registers
- * as necessary if the address has changed.
+ * unregisters/registers as necessary and applies the notification to all
+ * matching tcons.
  */
 static int cifs_swn_handle_notification(const struct cifs_swn_notification *not)
 {
@@ -986,16 +901,104 @@ int cifs_swn_register(struct cifs_tcon *tcon)
 	struct cifs_swn_reg *swnreg;
 	int ret;
 
-	swnreg = cifs_get_swn_reg(tcon);
-	if (IS_ERR(swnreg))
+	mutex_lock(&cifs_swnreg_idr_mutex);
+
+	swnreg = cifs_find_swn_reg(tcon);
+	if (!IS_ERR(swnreg)) {
+		/*
+		 * There is a registration matching this tcon, could be a second mount of
+		 * the same share, increment the refcount.
+		 */
+		if (kref_get_unless_zero(&swnreg->ref_count)) {
+			mutex_unlock(&cifs_swnreg_idr_mutex);
+			return 0;
+		}
+		/* Else it is being released, allocate new one */
+	} else if (PTR_ERR(swnreg) != -ENOENT) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
 		return PTR_ERR(swnreg);
+	}
 
-	ret = cifs_swn_send_register_message(swnreg);
-	if (ret < 0) {
-		cifs_dbg(VFS, "%s: Failed to send swn register message: %d\n", __func__, ret);
-		/* Do not put the swnreg or return error, the check task will retry */
+	/* Allocate new registration */
+	swnreg = kzalloc_obj(struct cifs_swn_reg, GFP_KERNEL);
+	if (swnreg == NULL) {
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return -ENOMEM;
 	}
 
+	kref_init(&swnreg->ref_count);
+
+	swnreg->id = idr_alloc(&cifs_swnreg_idr, swnreg, 1, 0, GFP_KERNEL);
+	if (swnreg->id < 0) {
+		ret = swnreg->id;
+		cifs_dbg(FYI, "%s: failed to allocate registration id\n",
+			 __func__);
+		kfree(swnreg);
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return ret;
+	}
+
+	swnreg->net_name = extract_hostname(tcon->tree_name);
+	if (IS_ERR(swnreg->net_name)) {
+		ret = PTR_ERR(swnreg->net_name);
+		cifs_dbg(VFS,
+			 "%s: failed to extract host name from target: %d\n",
+			 __func__, ret);
+		idr_remove(&cifs_swnreg_idr, swnreg->id);
+		kfree(swnreg);
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return ret;
+	}
+
+	swnreg->share_name = extract_sharename(tcon->tree_name);
+	if (IS_ERR(swnreg->share_name)) {
+		ret = PTR_ERR(swnreg->share_name);
+		cifs_dbg(VFS,
+			 "%s: failed to extract share name from target: %d\n",
+			 __func__, ret);
+		kfree(swnreg->net_name);
+		idr_remove(&cifs_swnreg_idr, swnreg->id);
+		kfree(swnreg);
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return ret;
+	}
+
+	ret = cifs_swn_reg_set_auth(swnreg, tcon);
+	if (ret != 0) {
+		cifs_dbg(VFS, "%s: failed to set auth info: %d\n", __func__,
+			 ret);
+		kfree(swnreg->net_name);
+		kfree(swnreg->share_name);
+		idr_remove(&cifs_swnreg_idr, swnreg->id);
+		kfree(swnreg);
+		mutex_unlock(&cifs_swnreg_idr_mutex);
+		return ret;
+	}
+
+	/*
+	 * If there is an address stored use it instead of the server address, because we are
+	 * in the process of reconnecting to it after a share has been moved or we have been
+	 * told to switch to it (client move message). In these cases we unregister from the
+	 * server address and register to the new address when we receive the notification.
+	 */
+	if (tcon->ses->server->use_swn_dstaddr)
+		swnreg->addr = tcon->ses->server->swn_dstaddr;
+	else
+		swnreg->addr = tcon->ses->server->dstaddr;
+
+	swnreg->net_name_notify = true;
+	swnreg->share_name_notify =
+		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
+	swnreg->ip_notify = false;
+
+	swnreg->check_interval = tcon->ses->server->echo_interval;
+	INIT_DELAYED_WORK(&swnreg->check, cifs_swn_reg_check);
+
+	/* Queue a immediate run to send the netlink message */
+	queue_delayed_work(cifsiod_wq, &swnreg->check, 0);
+
+	mutex_unlock(&cifs_swnreg_idr_mutex);
+
 	return 0;
 }
 
-- 
2.54.0

[PATCH 17/18] smb:client: Improve registration network and share names handling

[Samuel Cabrero]

Store the names and add them to the netlink messages only if required.

The network name notification is always enabled. The share name
notification is only enabled for asymmetric shares.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 111 +++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 50 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index 3375a65ec32e..a196b06c4b8c 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -212,7 +212,7 @@ static bool cifs_swn_reg_net_name_matches(const struct cifs_swn_reg *swnreg,
 		return false;
 
 	host_len = delim - host;
-	if (strlen(swnreg->net_name) == host_len &&
+	if (swnreg->net_name != NULL && strlen(swnreg->net_name) == host_len &&
 	    !strncasecmp(swnreg->net_name, host, host_len)) {
 		return true;
 	}
@@ -269,7 +269,7 @@ static bool cifs_swn_reg_tcon_matches(const struct cifs_swn_reg *swnreg,
 		return false;
 
 	/* The network name notification is always enabled */
-	if (!cifs_swn_reg_net_name_matches(swnreg, tcon))
+	if (swnreg->net_name_notify && !cifs_swn_reg_net_name_matches(swnreg, tcon))
 		return false;
 
 	/*
@@ -346,26 +346,28 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 	if (ret < 0)
 		goto nlmsg_fail;
 
-	ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_NET_NAME, swnreg->net_name);
-	if (ret < 0)
-		goto nlmsg_fail;
-
-	ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_SHARE_NAME, swnreg->share_name);
-	if (ret < 0)
-		goto nlmsg_fail;
-
 	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP,
 		      sizeof(struct sockaddr_storage), &swnreg->addr);
 	if (ret < 0)
 		goto nlmsg_fail;
 
 	if (swnreg->net_name_notify) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_NET_NAME,
+				     swnreg->net_name);
+		if (ret < 0)
+			goto nlmsg_fail;
+
 		ret = nla_put_flag(skb, CIFS_GENL_ATTR_SWN_NET_NAME_NOTIFY);
 		if (ret < 0)
 			goto nlmsg_fail;
 	}
 
 	if (swnreg->share_name_notify) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_SHARE_NAME,
+				     swnreg->share_name);
+		if (ret < 0)
+			goto nlmsg_fail;
+
 		ret = nla_put_flag(skb, CIFS_GENL_ATTR_SWN_SHARE_NAME_NOTIFY);
 		if (ret < 0)
 			goto nlmsg_fail;
@@ -403,8 +405,8 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg)
 	genlmsg_end(skb, hdr);
 	genlmsg_multicast(&cifs_genl_family, skb, 0, CIFS_GENL_MCGRP_SWN, GFP_ATOMIC);
 
-	cifs_dbg(FYI, "%s: Message to register for network name %s with id %d sent\n", __func__,
-			swnreg->net_name, swnreg->id);
+	cifs_dbg(FYI, "%s: Message to register for network name '%s' with id %d sent\n", __func__,
+			swnreg->net_name ? swnreg->net_name : "", swnreg->id);
 
 	return 0;
 
@@ -439,26 +441,28 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 	if (ret < 0)
 		goto nlmsg_fail;
 
-	ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_NET_NAME, swnreg->net_name);
-	if (ret < 0)
-		goto nlmsg_fail;
-
-	ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_SHARE_NAME, swnreg->share_name);
-	if (ret < 0)
-		goto nlmsg_fail;
-
 	ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP,
 		      sizeof(struct sockaddr_storage), &swnreg->addr);
 	if (ret < 0)
 		goto nlmsg_fail;
 
 	if (swnreg->net_name_notify) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_NET_NAME,
+				     swnreg->net_name);
+		if (ret < 0)
+			goto nlmsg_fail;
+
 		ret = nla_put_flag(skb, CIFS_GENL_ATTR_SWN_NET_NAME_NOTIFY);
 		if (ret < 0)
 			goto nlmsg_fail;
 	}
 
 	if (swnreg->share_name_notify) {
+		ret = nla_put_string(skb, CIFS_GENL_ATTR_SWN_SHARE_NAME,
+				     swnreg->share_name);
+		if (ret < 0)
+			goto nlmsg_fail;
+
 		ret = nla_put_flag(skb, CIFS_GENL_ATTR_SWN_SHARE_NAME_NOTIFY);
 		if (ret < 0)
 			goto nlmsg_fail;
@@ -473,8 +477,8 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg)
 	genlmsg_end(skb, hdr);
 	genlmsg_multicast(&cifs_genl_family, skb, 0, CIFS_GENL_MCGRP_SWN, GFP_ATOMIC);
 
-	cifs_dbg(FYI, "%s: Message to unregister for network name %s with id %d sent\n", __func__,
-			swnreg->net_name, swnreg->id);
+	cifs_dbg(FYI, "%s: Message to unregister for network name '%s' with id %d sent\n", __func__,
+			swnreg->net_name ? swnreg->net_name : "", swnreg->id);
 
 	return 0;
 
@@ -938,29 +942,40 @@ int cifs_swn_register(struct cifs_tcon *tcon)
 		return ret;
 	}
 
-	swnreg->net_name = extract_hostname(tcon->tree_name);
-	if (IS_ERR(swnreg->net_name)) {
-		ret = PTR_ERR(swnreg->net_name);
-		cifs_dbg(VFS,
-			 "%s: failed to extract host name from target: %d\n",
-			 __func__, ret);
-		idr_remove(&cifs_swnreg_idr, swnreg->id);
-		kfree(swnreg);
-		mutex_unlock(&cifs_swnreg_idr_mutex);
-		return ret;
+	swnreg->net_name_notify = true;
+	swnreg->share_name_notify =
+		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
+	swnreg->ip_notify = false;
+
+	if (swnreg->net_name_notify) {
+		swnreg->net_name = extract_hostname(tcon->tree_name);
+		if (IS_ERR(swnreg->net_name)) {
+			ret = PTR_ERR(swnreg->net_name);
+			cifs_dbg(
+				VFS,
+				"%s: failed to extract host name from target: %d\n",
+				__func__, ret);
+			idr_remove(&cifs_swnreg_idr, swnreg->id);
+			kfree(swnreg);
+			mutex_unlock(&cifs_swnreg_idr_mutex);
+			return ret;
+		}
 	}
 
-	swnreg->share_name = extract_sharename(tcon->tree_name);
-	if (IS_ERR(swnreg->share_name)) {
-		ret = PTR_ERR(swnreg->share_name);
-		cifs_dbg(VFS,
-			 "%s: failed to extract share name from target: %d\n",
-			 __func__, ret);
-		kfree(swnreg->net_name);
-		idr_remove(&cifs_swnreg_idr, swnreg->id);
-		kfree(swnreg);
-		mutex_unlock(&cifs_swnreg_idr_mutex);
-		return ret;
+	if (swnreg->share_name_notify) {
+		swnreg->share_name = extract_sharename(tcon->tree_name);
+		if (IS_ERR(swnreg->share_name)) {
+			ret = PTR_ERR(swnreg->share_name);
+			cifs_dbg(
+				VFS,
+				"%s: failed to extract share name from target: %d\n",
+				__func__, ret);
+			kfree(swnreg->net_name);
+			idr_remove(&cifs_swnreg_idr, swnreg->id);
+			kfree(swnreg);
+			mutex_unlock(&cifs_swnreg_idr_mutex);
+			return ret;
+		}
 	}
 
 	ret = cifs_swn_reg_set_auth(swnreg, tcon);
@@ -986,11 +1001,6 @@ int cifs_swn_register(struct cifs_tcon *tcon)
 	else
 		swnreg->addr = tcon->ses->server->dstaddr;
 
-	swnreg->net_name_notify = true;
-	swnreg->share_name_notify =
-		(tcon->capabilities & SMB2_SHARE_CAP_ASYMMETRIC);
-	swnreg->ip_notify = false;
-
 	swnreg->check_interval = tcon->ses->server->echo_interval;
 	INIT_DELAYED_WORK(&swnreg->check, cifs_swn_reg_check);
 
@@ -1036,9 +1046,10 @@ void cifs_swn_dump(struct seq_file *m)
 		seq_printf(
 			m,
 			"\nId: %d Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: '%pISc'%s",
-			id, kref_read(&swnreg->ref_count), swnreg->net_name,
+			id, kref_read(&swnreg->ref_count),
+			swnreg->net_name ? swnreg->net_name : "",
 			swnreg->net_name_notify ? "(y)" : "(n)",
-			swnreg->share_name,
+			swnreg->share_name ? swnreg->share_name : "",
 			swnreg->share_name_notify ? "(y)" : "(n)",
 			&swnreg->addr, swnreg->ip_notify ? "(y)" : "(n)");
 	}
-- 
2.54.0

[PATCH 18/18] smb:client: Avoid witness re-register if the address did not change

[Samuel Cabrero]

If received a client-move notification with a destination address
matching the current one, skip unregister and register to the same
address.

Signed-off-by: Samuel Cabrero <scabrero@suse.com>
---
 fs/smb/client/cifs_swn.c | 67 ++++++++++++++++++++--------------------
 1 file changed, 33 insertions(+), 34 deletions(-)

diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c
index a196b06c4b8c..fabde01d998a 100644
--- a/fs/smb/client/cifs_swn.c
+++ b/fs/smb/client/cifs_swn.c
@@ -626,7 +626,7 @@ static struct cifs_swn_reg *cifs_find_swn_reg(struct cifs_tcon *tcon)
 	return ERR_PTR(-ENOENT);
 }
 
-static void cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
+static bool cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
 					    const char *name, int state)
 {
 	switch (state) {
@@ -642,6 +642,9 @@ static void cifs_swn_resource_state_changed(struct cifs_tcon *tcon,
 		cifs_dbg(FYI, "%s: resource name '%s' changed to unknown state\n", __func__, name);
 		break;
 	}
+
+	/* Address has not changed */
+	return false;
 }
 
 static int cifs_swn_store_swn_addr(const struct sockaddr_storage *new,
@@ -678,21 +681,16 @@ static int cifs_swn_store_swn_addr(const struct sockaddr_storage *new,
 static bool cifs_swn_client_move(struct cifs_tcon *tcon,
 				 struct sockaddr_storage *addr)
 {
-	struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr;
-	struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr;
 	int ret;
 
-	if (addr->ss_family == AF_INET)
-		cifs_dbg(FYI, "%s: move to %pI4\n", __func__, &ipv4->sin_addr);
-	else if (addr->ss_family == AF_INET6)
-		cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr);
-
 	if (cifs_match_ipaddr((struct sockaddr *)&tcon->ses->server->dstaddr,
 			      (struct sockaddr *)addr)) {
 		/* no-op */
 		return false;
 	}
 
+	cifs_dbg(FYI, "%s: move to %pISc\n", __func__, addr);
+
 	/* Store the reconnect address */
 	ret = cifs_swn_store_swn_addr(addr, &tcon->ses->server->dstaddr,
 				      &tcon->ses->server->swn_dstaddr);
@@ -705,6 +703,7 @@ static bool cifs_swn_client_move(struct cifs_tcon *tcon,
 
 	cifs_signal_cifsd_for_reconnect(tcon->ses->server, false);
 
+	/* Address changed */
 	return true;
 }
 
@@ -719,10 +718,9 @@ cifs_swn_handle_notification_tcon(const struct cifs_swn_notification *not,
 {
 	switch (not->type) {
 	case CIFS_SWN_NOTIFICATION_RESOURCE_CHANGE:
-		cifs_swn_resource_state_changed(
+		return cifs_swn_resource_state_changed(
 			tcon, not->data.resource_name_changed.name,
 			not->data.resource_name_changed.state);
-		return false;
 	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE:
 		return cifs_swn_client_move(tcon, not->data.client_move.addr);
 	default:
@@ -743,6 +741,7 @@ static int cifs_swn_handle_notification(const struct cifs_swn_notification *not)
 	struct TCP_Server_Info *server;
 	struct cifs_ses *ses;
 	struct cifs_tcon *tcon;
+	bool addr_changed = false;
 	int ret;
 
 	/* Walk the tcons and apply the notification to matching ones */
@@ -761,7 +760,9 @@ static int cifs_swn_handle_notification(const struct cifs_swn_notification *not)
 					spin_unlock(&tcon->tc_lock);
 					continue;
 				}
-				cifs_swn_handle_notification_tcon(not, tcon);
+				addr_changed |=
+					cifs_swn_handle_notification_tcon(not,
+									  tcon);
 				spin_unlock(&tcon->tc_lock);
 			}
 		}
@@ -769,31 +770,29 @@ static int cifs_swn_handle_notification(const struct cifs_swn_notification *not)
 	}
 	spin_unlock(&cifs_tcp_ses_lock);
 
-	switch (not->type) {
-	case CIFS_SWN_NOTIFICATION_CLIENT_MOVE:
-		/* Unregister from the current address */
-		ret = cifs_swn_send_unregister_message(not->swnreg);
-		if (ret < 0) {
-			cifs_dbg(VFS,
-				 "%s: Failed to unregister for witness notifications: %d\n",
-				 __func__, ret);
-		}
+	if (!addr_changed)
+		return 0;
 
-		/* Store the new address */
-		not->swnreg->addr = *not->data.client_move.addr;
+	/*
+	 * If the address has changed, unregister from the previous and
+	 * register for the new one
+	 */
+	ret = cifs_swn_send_unregister_message(not->swnreg);
+	if (ret < 0) {
+		cifs_dbg(VFS,
+			"%s: Failed to unregister for witness notifications: %d\n",
+			__func__, ret);
+	}
 
-		/* Register for this new address */
-		ret = cifs_swn_send_register_message(not->swnreg);
-		if (ret < 0) {
-			cifs_dbg(VFS,
-				 "%s: Failed to register for witness notifications: %d\n",
-				 __func__, ret);
-		}
-		break;
-	default:
-		cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__,
-			 not->type);
-		break;
+	/* Store the new address */
+	not->swnreg->addr = *not->data.client_move.addr;
+
+	/* Register for this new address */
+	ret = cifs_swn_send_register_message(not->swnreg);
+	if (ret < 0) {
+		cifs_dbg(VFS,
+			"%s: Failed to register for witness notifications: %d\n",
+			__func__, ret);
 	}
 
 	return 0;
-- 
2.54.0

Re: [PATCH 00/18] SWN: Decouple witness registrations and tcons

[Enzo Matsumiya]

On 07/10, Samuel Cabrero wrote:
>This patches removes the cached tcon pointers from the witness
>registrations.
>
>First patch in the serie reverts the current approach based on snapshots
>that fixes a race while processing a received notification. This
>patchset implements a different approach based on reference counting and
>fixes other races that could occur with concurrent
>register/unregister/notification processing.
>
>The witness registration stores now all required information to issue
>register/unregister DCERPC calls (like auth info and target address),
>and when a notification is received it applies it to all matching
>witness-enabled tcons.
>
>Each witness registration has its own task to enforce the registration
>in case the userspace daemon is started after mounting the share,
>instead of overload the echo task for this purpose.

@Steve you may add my Rb to the whole series.

>Samuel Cabrero (18):
>  Revert "smb: client: resolve SWN tcon from live registrations"
>  smb:client: Adjust witness notification request
>  smb:client: Rename variable to match the rest of the code
>  smb:client: Split idr remove and release functions
>  smb:client: Add a specific task to refresh witness registrations
>  smb:client: Store the tcon dstaddr in the witness registration
>  smb:client: Fix printf format string
>  smb:client: Save memory allocations
>  smb:client: Use the correct return code
>  smb:client: Improve tcon matching logic
>  smb:client: Pass the tcon to cifs_swn_send_register_message
>  smb:client: Pass the tcon to notification handlers
>  smb:client: Introduce swn_notification struct
>  smb:client: Cache auth info

@Samuel just a small observation in this patch (14/18):

It's better practice to use kfree_sensitive() when freeing
passwords (swnreg->authinfo.ntlm.password).

IMHO this can be done in a follow up patch instead of a v2.

>  smb:client: Do not store the tcon reference, find a matching one
>  smb:client: Simplify registration creation
>  smb:client: Improve registration network and share names handling
>  smb:client: Avoid witness re-register if the address did not change
>
> fs/smb/client/cifs_swn.c | 1179 ++++++++++++++++++++++----------------
> fs/smb/client/cifs_swn.h |    3 -
> fs/smb/client/connect.c  |    3 -
> fs/smb/client/trace.h    |    4 +-
> 4 files changed, 682 insertions(+), 507 deletions(-)


Cheers,

Enzo