[cifs bindaddr v2] cifs: Allow binding to local IP address.

[cifs bindaddr v2] cifs: Allow binding to local IP address.

[Ben Greear]

When using multi-homed machines, it's nice to be able to specify
the local IP to use for outbound connections.  This patch gives
cifs the ability to bind to a particular IP address.

Usage:  mount -t cifs -o bindaddr=192.168.1.50,user=foo, ...

Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
---
:100644 100644 b7431af... 77293db... M	fs/cifs/cifsfs.c
:100644 100644 c9d0cfc... c0176d8... M	fs/cifs/cifsglob.h
:100644 100644 ec0ea4a... bacbf46... M	fs/cifs/connect.c
 fs/cifs/cifsfs.c   |    4 +++
 fs/cifs/cifsglob.h |    1 +
 fs/cifs/connect.c  |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index b7431af..77293db 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -374,6 +374,10 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
 	if (tcon->ses->domainName)
 		seq_printf(s, ",domain=%s", tcon->ses->domainName);
 
+	if (tcon->ses->server->ip4_local_ip)
+		seq_printf(s, ",bindaddr=%pI4",
+			   &tcon->ses->server->ip4_local_ip);
+
 	seq_printf(s, ",uid=%d", cifs_sb->mnt_uid);
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID)
 		seq_printf(s, ",forceuid");
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index c9d0cfc..c0176d8 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -157,6 +157,7 @@ struct TCP_Server_Info {
 		struct sockaddr_in sockAddr;
 		struct sockaddr_in6 sockAddr6;
 	} addr;
+	u32 ip4_local_ip;
 	wait_queue_head_t response_q;
 	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
 	struct list_head pending_mid_q;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ec0ea4a..bacbf46 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -105,6 +105,7 @@ struct smb_vol {
 	bool sockopt_tcp_nodelay:1;
 	unsigned short int port;
 	char *prepath;
+	u32 local_ip; /* allow binding to a local IP address if != 0 */
 	struct nls_table *local_nls;
 };
 
@@ -1064,6 +1065,32 @@ cifs_parse_mount_options(char *options, const char *devname,
 						    "long\n");
 				return 1;
 			}
+		} else if (strnicmp(data, "bindaddr", 8) == 0) {
+			struct sockaddr_storage laddr;
+			memset(&laddr, 0, sizeof(laddr));
+
+			if (!value || !*value) {
+				printk(KERN_WARNING "CIFS: bindaddr value"
+				       " not specified.\n");
+				return 1;	/* needs_arg; */
+			}
+			i = cifs_convert_address((struct sockaddr *)(&laddr),
+						 value, strlen(value));
+			if (i < 0) {
+				vol->local_ip = 0;
+				printk(KERN_WARNING "CIFS:  Could not parse"
+				       " bindaddr: %s\n",
+				       value);
+				return 1;
+			} else {
+				struct sockaddr_in *s4;
+				s4 = (struct sockaddr_in *)&laddr;
+				if (s4->sin_family == AF_INET)
+					vol->local_ip = s4->sin_addr.s_addr;
+				else
+					printk(KERN_WARNING "WARNING:  IPv6 "
+					       "bindaddr not supported yet.\n");
+			}
 		} else if (strnicmp(data, "prefixpath", 10) == 0) {
 			if (!value || !*value) {
 				printk(KERN_WARNING
@@ -1393,7 +1420,8 @@ cifs_parse_mount_options(char *options, const char *devname,
 }
 
 static bool
-match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
+match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
+	      u32 local_ip4)
 {
 	struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
 	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
@@ -1406,6 +1434,8 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
 		if (addr4->sin_port &&
 		    addr4->sin_port != server->addr.sockAddr.sin_port)
 			return false;
+		if (local_ip4 && (local_ip4 != server->ip4_local_ip))
+			return false;
 		break;
 	case AF_INET6:
 		if (!ipv6_addr_equal(&addr6->sin6_addr,
@@ -1487,7 +1517,7 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
 		if (server->tcpStatus == CifsNew)
 			continue;
 
-		if (!match_address(server, addr))
+		if (!match_address(server, addr, vol->local_ip))
 			continue;
 
 		if (!match_security(server, vol))
@@ -1602,6 +1632,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	 * no need to spinlock this init of tcpStatus or srv_count
 	 */
 	tcp_ses->tcpStatus = CifsNew;
+	tcp_ses->ip4_local_ip = volume_info->local_ip;
 	++tcp_ses->srv_count;
 
 	if (addr.ss_family == AF_INET6) {
@@ -1678,6 +1709,10 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
 				    vol->password ? vol->password : "",
 				    MAX_PASSWORD_SIZE))
 				continue;
+			if (server->ip4_local_ip &&
+			    (server->ip4_local_ip != ses->server->ip4_local_ip))
+				continue;
+			/* TODO:  Deal with IPv6 local addr matching? --Ben */
 		}
 		++ses->ses_count;
 		write_unlock(&cifs_tcp_ses_lock);
@@ -2051,6 +2086,20 @@ ipv4_connect(struct TCP_Server_Info *server)
 		cifs_reclassify_socket4(socket);
 	}
 
+	/* Bind to the local IP address if specified */
+	if (server->ip4_local_ip) {
+		struct sockaddr_in myaddr = {
+			.sin_family = AF_INET,
+		};
+		myaddr.sin_addr.s_addr = server->ip4_local_ip;
+		myaddr.sin_port = 0; /* any */
+		rc = socket->ops->bind(socket, (struct sockaddr *) &myaddr,
+				       sizeof(myaddr));
+		if (rc < 0)
+			cERROR(1, "Failed to bind to: %pI4, error: %d\n",
+			       &server->ip4_local_ip, rc);
+	}
+
 	/* user overrode default port */
 	if (server->addr.sockAddr.sin_port) {
 		rc = socket->ops->connect(socket, (struct sockaddr *)
-- 
1.6.2.5

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Jeff Layton]

On Thu, 26 Aug 2010 10:34:56 -0700
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:

> When using multi-homed machines, it's nice to be able to specify
> the local IP to use for outbound connections.  This patch gives
> cifs the ability to bind to a particular IP address.
> 
> Usage:  mount -t cifs -o bindaddr=192.168.1.50,user=foo, ...
> 
> Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
> ---
> :100644 100644 b7431af... 77293db... M	fs/cifs/cifsfs.c
> :100644 100644 c9d0cfc... c0176d8... M	fs/cifs/cifsglob.h
> :100644 100644 ec0ea4a... bacbf46... M	fs/cifs/connect.c
>  fs/cifs/cifsfs.c   |    4 +++
>  fs/cifs/cifsglob.h |    1 +
>  fs/cifs/connect.c  |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 56 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index b7431af..77293db 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -374,6 +374,10 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
>  	if (tcon->ses->domainName)
>  		seq_printf(s, ",domain=%s", tcon->ses->domainName);
>  
> +	if (tcon->ses->server->ip4_local_ip)
> +		seq_printf(s, ",bindaddr=%pI4",
> +			   &tcon->ses->server->ip4_local_ip);
> +
>  	seq_printf(s, ",uid=%d", cifs_sb->mnt_uid);
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID)
>  		seq_printf(s, ",forceuid");
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index c9d0cfc..c0176d8 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -157,6 +157,7 @@ struct TCP_Server_Info {
>  		struct sockaddr_in sockAddr;
>  		struct sockaddr_in6 sockAddr6;
>  	} addr;
> +	u32 ip4_local_ip;
>  	wait_queue_head_t response_q;
>  	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
>  	struct list_head pending_mid_q;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ec0ea4a..bacbf46 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -105,6 +105,7 @@ struct smb_vol {
>  	bool sockopt_tcp_nodelay:1;
>  	unsigned short int port;
>  	char *prepath;
> +	u32 local_ip; /* allow binding to a local IP address if != 0 */
>  	struct nls_table *local_nls;
>  };
>  
> @@ -1064,6 +1065,32 @@ cifs_parse_mount_options(char *options, const char *devname,
>  						    "long\n");
>  				return 1;
>  			}
> +		} else if (strnicmp(data, "bindaddr", 8) == 0) {
> +			struct sockaddr_storage laddr;
> +			memset(&laddr, 0, sizeof(laddr));
> +
> +			if (!value || !*value) {
> +				printk(KERN_WARNING "CIFS: bindaddr value"
> +				       " not specified.\n");
> +				return 1;	/* needs_arg; */
> +			}
> +			i = cifs_convert_address((struct sockaddr *)(&laddr),
> +						 value, strlen(value));
> +			if (i < 0) {
> +				vol->local_ip = 0;
> +				printk(KERN_WARNING "CIFS:  Could not parse"
> +				       " bindaddr: %s\n",
> +				       value);
> +				return 1;
> +			} else {
> +				struct sockaddr_in *s4;
> +				s4 = (struct sockaddr_in *)&laddr;
> +				if (s4->sin_family == AF_INET)
> +					vol->local_ip = s4->sin_addr.s_addr;
> +				else
> +					printk(KERN_WARNING "WARNING:  IPv6 "
> +					       "bindaddr not supported yet.\n");
> +			}
>  		} else if (strnicmp(data, "prefixpath", 10) == 0) {
>  			if (!value || !*value) {
>  				printk(KERN_WARNING
> @@ -1393,7 +1420,8 @@ cifs_parse_mount_options(char *options, const char *devname,
>  }
>  
>  static bool
> -match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
> +match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
> +	      u32 local_ip4)
>  {
>  	struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
>  	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
> @@ -1406,6 +1434,8 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
>  		if (addr4->sin_port &&
>  		    addr4->sin_port != server->addr.sockAddr.sin_port)
>  			return false;
> +		if (local_ip4 && (local_ip4 != server->ip4_local_ip))
> +			return false;
>  		break;
>  	case AF_INET6:
>  		if (!ipv6_addr_equal(&addr6->sin6_addr,
> @@ -1487,7 +1517,7 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
>  		if (server->tcpStatus == CifsNew)
>  			continue;
>  
> -		if (!match_address(server, addr))
> +		if (!match_address(server, addr, vol->local_ip))
>  			continue;
>  
>  		if (!match_security(server, vol))
> @@ -1602,6 +1632,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  	 * no need to spinlock this init of tcpStatus or srv_count
>  	 */
>  	tcp_ses->tcpStatus = CifsNew;
> +	tcp_ses->ip4_local_ip = volume_info->local_ip;
>  	++tcp_ses->srv_count;
>  
>  	if (addr.ss_family == AF_INET6) {
> @@ -1678,6 +1709,10 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
>  				    vol->password ? vol->password : "",
>  				    MAX_PASSWORD_SIZE))
>  				continue;
> +			if (server->ip4_local_ip &&
> +			    (server->ip4_local_ip != ses->server->ip4_local_ip))
> +				continue;
> +			/* TODO:  Deal with IPv6 local addr matching? --Ben */
>  		}
>  		++ses->ses_count;
>  		write_unlock(&cifs_tcp_ses_lock);
> @@ -2051,6 +2086,20 @@ ipv4_connect(struct TCP_Server_Info *server)
>  		cifs_reclassify_socket4(socket);
>  	}
>  
> +	/* Bind to the local IP address if specified */
> +	if (server->ip4_local_ip) {
> +		struct sockaddr_in myaddr = {
> +			.sin_family = AF_INET,
> +		};
> +		myaddr.sin_addr.s_addr = server->ip4_local_ip;
> +		myaddr.sin_port = 0; /* any */
> +		rc = socket->ops->bind(socket, (struct sockaddr *) &myaddr,
> +				       sizeof(myaddr));
> +		if (rc < 0)
> +			cERROR(1, "Failed to bind to: %pI4, error: %d\n",
> +			       &server->ip4_local_ip, rc);
> +	}
> +
>  	/* user overrode default port */
>  	if (server->addr.sockAddr.sin_port) {
>  		rc = socket->ops->connect(socket, (struct sockaddr *)


I'm not crazy about this patch. It seems like this ought to be the
purview of the routing table. I know however that there are situations
where that's not really possible (clusters with floating IP addrs, for
instance).

That said, even if we were to do something along these lines, it seems
like it ought to be more transport neutral (i.e. not IPv4 only like
this patch is).

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Steve French]

There are clearly valid reasons for being able to select outgoing
interface - I wouldn't mind doing this using IPv6 address format if
that is more natural (since that can include ipv4 addresses) - or
distinct ipv4/ipv6

On Thu, Aug 26, 2010 at 2:57 PM, Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org> wrote:
> On Thu, 26 Aug 2010 10:34:56 -0700
> Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:
>
>> When using multi-homed machines, it's nice to be able to specify
>> the local IP to use for outbound connections.  This patch gives
>> cifs the ability to bind to a particular IP address.
>>
>> Usage:  mount -t cifs -o bindaddr=192.168.1.50,user=foo, ...
>>
>> Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
>> ---
>> :100644 100644 b7431af... 77293db... M        fs/cifs/cifsfs.c
>> :100644 100644 c9d0cfc... c0176d8... M        fs/cifs/cifsglob.h
>> :100644 100644 ec0ea4a... bacbf46... M        fs/cifs/connect.c
>>  fs/cifs/cifsfs.c   |    4 +++
>>  fs/cifs/cifsglob.h |    1 +
>>  fs/cifs/connect.c  |   53 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  3 files changed, 56 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
>> index b7431af..77293db 100644
>> --- a/fs/cifs/cifsfs.c
>> +++ b/fs/cifs/cifsfs.c
>> @@ -374,6 +374,10 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
>>       if (tcon->ses->domainName)
>>               seq_printf(s, ",domain=%s", tcon->ses->domainName);
>>
>> +     if (tcon->ses->server->ip4_local_ip)
>> +             seq_printf(s, ",bindaddr=%pI4",
>> +                        &tcon->ses->server->ip4_local_ip);
>> +
>>       seq_printf(s, ",uid=%d", cifs_sb->mnt_uid);
>>       if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID)
>>               seq_printf(s, ",forceuid");
>> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
>> index c9d0cfc..c0176d8 100644
>> --- a/fs/cifs/cifsglob.h
>> +++ b/fs/cifs/cifsglob.h
>> @@ -157,6 +157,7 @@ struct TCP_Server_Info {
>>               struct sockaddr_in sockAddr;
>>               struct sockaddr_in6 sockAddr6;
>>       } addr;
>> +     u32 ip4_local_ip;
>>       wait_queue_head_t response_q;
>>       wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
>>       struct list_head pending_mid_q;
>> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
>> index ec0ea4a..bacbf46 100644
>> --- a/fs/cifs/connect.c
>> +++ b/fs/cifs/connect.c
>> @@ -105,6 +105,7 @@ struct smb_vol {
>>       bool sockopt_tcp_nodelay:1;
>>       unsigned short int port;
>>       char *prepath;
>> +     u32 local_ip; /* allow binding to a local IP address if != 0 */
>>       struct nls_table *local_nls;
>>  };
>>
>> @@ -1064,6 +1065,32 @@ cifs_parse_mount_options(char *options, const char *devname,
>>                                                   "long\n");
>>                               return 1;
>>                       }
>> +             } else if (strnicmp(data, "bindaddr", 8) == 0) {
>> +                     struct sockaddr_storage laddr;
>> +                     memset(&laddr, 0, sizeof(laddr));
>> +
>> +                     if (!value || !*value) {
>> +                             printk(KERN_WARNING "CIFS: bindaddr value"
>> +                                    " not specified.\n");
>> +                             return 1;       /* needs_arg; */
>> +                     }
>> +                     i = cifs_convert_address((struct sockaddr *)(&laddr),
>> +                                              value, strlen(value));
>> +                     if (i < 0) {
>> +                             vol->local_ip = 0;
>> +                             printk(KERN_WARNING "CIFS:  Could not parse"
>> +                                    " bindaddr: %s\n",
>> +                                    value);
>> +                             return 1;
>> +                     } else {
>> +                             struct sockaddr_in *s4;
>> +                             s4 = (struct sockaddr_in *)&laddr;
>> +                             if (s4->sin_family == AF_INET)
>> +                                     vol->local_ip = s4->sin_addr.s_addr;
>> +                             else
>> +                                     printk(KERN_WARNING "WARNING:  IPv6 "
>> +                                            "bindaddr not supported yet.\n");
>> +                     }
>>               } else if (strnicmp(data, "prefixpath", 10) == 0) {
>>                       if (!value || !*value) {
>>                               printk(KERN_WARNING
>> @@ -1393,7 +1420,8 @@ cifs_parse_mount_options(char *options, const char *devname,
>>  }
>>
>>  static bool
>> -match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
>> +match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
>> +           u32 local_ip4)
>>  {
>>       struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
>>       struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
>> @@ -1406,6 +1434,8 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
>>               if (addr4->sin_port &&
>>                   addr4->sin_port != server->addr.sockAddr.sin_port)
>>                       return false;
>> +             if (local_ip4 && (local_ip4 != server->ip4_local_ip))
>> +                     return false;
>>               break;
>>       case AF_INET6:
>>               if (!ipv6_addr_equal(&addr6->sin6_addr,
>> @@ -1487,7 +1517,7 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
>>               if (server->tcpStatus == CifsNew)
>>                       continue;
>>
>> -             if (!match_address(server, addr))
>> +             if (!match_address(server, addr, vol->local_ip))
>>                       continue;
>>
>>               if (!match_security(server, vol))
>> @@ -1602,6 +1632,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>>        * no need to spinlock this init of tcpStatus or srv_count
>>        */
>>       tcp_ses->tcpStatus = CifsNew;
>> +     tcp_ses->ip4_local_ip = volume_info->local_ip;
>>       ++tcp_ses->srv_count;
>>
>>       if (addr.ss_family == AF_INET6) {
>> @@ -1678,6 +1709,10 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
>>                                   vol->password ? vol->password : "",
>>                                   MAX_PASSWORD_SIZE))
>>                               continue;
>> +                     if (server->ip4_local_ip &&
>> +                         (server->ip4_local_ip != ses->server->ip4_local_ip))
>> +                             continue;
>> +                     /* TODO:  Deal with IPv6 local addr matching? --Ben */
>>               }
>>               ++ses->ses_count;
>>               write_unlock(&cifs_tcp_ses_lock);
>> @@ -2051,6 +2086,20 @@ ipv4_connect(struct TCP_Server_Info *server)
>>               cifs_reclassify_socket4(socket);
>>       }
>>
>> +     /* Bind to the local IP address if specified */
>> +     if (server->ip4_local_ip) {
>> +             struct sockaddr_in myaddr = {
>> +                     .sin_family = AF_INET,
>> +             };
>> +             myaddr.sin_addr.s_addr = server->ip4_local_ip;
>> +             myaddr.sin_port = 0; /* any */
>> +             rc = socket->ops->bind(socket, (struct sockaddr *) &myaddr,
>> +                                    sizeof(myaddr));
>> +             if (rc < 0)
>> +                     cERROR(1, "Failed to bind to: %pI4, error: %d\n",
>> +                            &server->ip4_local_ip, rc);
>> +     }
>> +
>>       /* user overrode default port */
>>       if (server->addr.sockAddr.sin_port) {
>>               rc = socket->ops->connect(socket, (struct sockaddr *)
>
>
> I'm not crazy about this patch. It seems like this ought to be the
> purview of the routing table. I know however that there are situations
> where that's not really possible (clusters with floating IP addrs, for
> instance).
>
> That said, even if we were to do something along these lines, it seems
> like it ought to be more transport neutral (i.e. not IPv4 only like
> this patch is).
>
> --
> Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-cifs" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Thanks,

Steve

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Jeff Layton]

On Thu, 26 Aug 2010 18:36:54 -0500
Steve French <smfrench-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> There are clearly valid reasons for being able to select outgoing
> interface - I wouldn't mind doing this using IPv6 address format if
> that is more natural (since that can include ipv4 addresses) - or
> distinct ipv4/ipv6
> 

The patch already uses cifs_convert_address which handles IPv4 or IPv6
addresses just fine. I don't think it would be that difficult to
optionally store a v4 or v6 address to bind socket.

You'll also need to think about what this means for ip address
selection in the mount helper. It'll need to be modified to make sure
that the address families between the two match. What happens if
someone specifies and IPv4 bindaddr, and the host in the UNC resolves
to an IPv6 address?

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Ben Greear]

On 08/27/2010 06:54 AM, Jeff Layton wrote:

>> Also, I think bindaddr might not be the best name for the option. How
>> about sourceaddr or srcaddr?
>>
>
> Sounds reasonable.

It doesn't matter to me.  Please tell me which one you want me to use
and I'll work on re-spinning this patch with ipv6 support.

Thanks,
Ben

-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Jeff Layton]

On Fri, 27 Aug 2010 08:29:49 -0700
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:

> On 08/27/2010 06:54 AM, Jeff Layton wrote:
> 
> >> Also, I think bindaddr might not be the best name for the option. How
> >> about sourceaddr or srcaddr?
> >>
> >
> > Sounds reasonable.
> 
> It doesn't matter to me.  Please tell me which one you want me to use
> and I'll work on re-spinning this patch with ipv6 support.
> 
> Thanks,
> Ben
> 

My preference would be "srcaddr=".

Thanks,
-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Steve French]

On Fri, Aug 27, 2010 at 11:52 AM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 27 Aug 2010 08:29:49 -0700
> Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:
>
>> On 08/27/2010 06:54 AM, Jeff Layton wrote:
>>
>> >> Also, I think bindaddr might not be the best name for the option. How
>> >> about sourceaddr or srcaddr?
>> >>
>> >
>> > Sounds reasonable.
>>
>> It doesn't matter to me.  Please tell me which one you want me to use
>> and I'll work on re-spinning this patch with ipv6 support.
>>
>> Thanks,
>> Ben
>>
>
> My preference would be "srcaddr=".

Shorter is good ...
srcaddr wins unless we have a particular compatibility objective with
other fs mounts (ie an existing mount option - which so far I have not
been able to find).

For completeness we should consider, at least briefly, one alternative name.
Note that smb.conf (and thus smbclient indirectly uses this) can
override the local interface using the optional smb.conf parm
"interfaces" - we could shorten this to "interface"   If anyone feels
strongly about this being better ...


-- 
Thanks,

Steve

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Ben Greear]

On 08/27/2010 10:08 AM, Steve French wrote:
> On Fri, Aug 27, 2010 at 11:52 AM, Jeff Layton<jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>  wrote:
>> On Fri, 27 Aug 2010 08:29:49 -0700
>> Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>  wrote:
>>
>>> On 08/27/2010 06:54 AM, Jeff Layton wrote:
>>>
>>>>> Also, I think bindaddr might not be the best name for the option. How
>>>>> about sourceaddr or srcaddr?
>>>>>
>>>>
>>>> Sounds reasonable.
>>>
>>> It doesn't matter to me.  Please tell me which one you want me to use
>>> and I'll work on re-spinning this patch with ipv6 support.
>>>
>>> Thanks,
>>> Ben
>>>
>>
>> My preference would be "srcaddr=".
>
> Shorter is good ...
> srcaddr wins unless we have a particular compatibility objective with
> other fs mounts (ie an existing mount option - which so far I have not
> been able to find).
>
> For completeness we should consider, at least briefly, one alternative name.
> Note that smb.conf (and thus smbclient indirectly uses this) can
> override the local interface using the optional smb.conf parm
> "interfaces" - we could shorten this to "interface"   If anyone feels
> strongly about this being better ...

I don't like that, as we're binding to IPs, not interfaces.

If we ever need to support SO_BINDTODEV logic, then could
use interface= for that.

srcaddr= sounds fine to me.

Thanks,
Ben

>
>


-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Jeff Layton]

On Fri, 27 Aug 2010 10:12:16 -0700
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:

> On 08/27/2010 10:08 AM, Steve French wrote:
> > On Fri, Aug 27, 2010 at 11:52 AM, Jeff Layton<jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>  wrote:
> >> On Fri, 27 Aug 2010 08:29:49 -0700
> >> Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>  wrote:
> >>
> >>> On 08/27/2010 06:54 AM, Jeff Layton wrote:
> >>>
> >>>>> Also, I think bindaddr might not be the best name for the option. How
> >>>>> about sourceaddr or srcaddr?
> >>>>>
> >>>>
> >>>> Sounds reasonable.
> >>>
> >>> It doesn't matter to me.  Please tell me which one you want me to use
> >>> and I'll work on re-spinning this patch with ipv6 support.
> >>>
> >>> Thanks,
> >>> Ben
> >>>
> >>
> >> My preference would be "srcaddr=".
> >
> > Shorter is good ...
> > srcaddr wins unless we have a particular compatibility objective with
> > other fs mounts (ie an existing mount option - which so far I have not
> > been able to find).
> >
> > For completeness we should consider, at least briefly, one alternative name.
> > Note that smb.conf (and thus smbclient indirectly uses this) can
> > override the local interface using the optional smb.conf parm
> > "interfaces" - we could shorten this to "interface"   If anyone feels
> > strongly about this being better ...
> 
> I don't like that, as we're binding to IPs, not interfaces.
> 
> If we ever need to support SO_BINDTODEV logic, then could
> use interface= for that.
> 
> srcaddr= sounds fine to me.
> 


Agreed. The samba config option seems poorly named in that regard.
They're sort of stuck with it though for legacy reasons, but there's no
reason to saddle cifs with it.

-- 
Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Re: [cifs bindaddr v2] cifs: Allow binding to local IP address.

[Steve French]

On Fri, Aug 27, 2010 at 12:34 PM, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> On Fri, 27 Aug 2010 10:12:16 -0700
> Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:
>
>> On 08/27/2010 10:08 AM, Steve French wrote:
>> > On Fri, Aug 27, 2010 at 11:52 AM, Jeff Layton<jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>  wrote:
>> >> On Fri, 27 Aug 2010 08:29:49 -0700
>> >> Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>  wrote:
>> >>
>> >>> On 08/27/2010 06:54 AM, Jeff Layton wrote:
>> >>>
>> >>>>> Also, I think bindaddr might not be the best name for the option. How
>> >>>>> about sourceaddr or srcaddr?
>> >>>>>
>> >>>>
>> >>>> Sounds reasonable.
>> >>>
>> >>> It doesn't matter to me.  Please tell me which one you want me to use
>> >>> and I'll work on re-spinning this patch with ipv6 support.
>> >>>
>> >>> Thanks,
>> >>> Ben
>> >>>
>> >>
>> >> My preference would be "srcaddr=".
>> >
>> > Shorter is good ...
>> > srcaddr wins unless we have a particular compatibility objective with
>> > other fs mounts (ie an existing mount option - which so far I have not
>> > been able to find).
>> >
>> > For completeness we should consider, at least briefly, one alternative name.
>> > Note that smb.conf (and thus smbclient indirectly uses this) can
>> > override the local interface using the optional smb.conf parm
>> > "interfaces" - we could shorten this to "interface"   If anyone feels
>> > strongly about this being better ...
>>
>> I don't like that, as we're binding to IPs, not interfaces.
>>
>> If we ever need to support SO_BINDTODEV logic, then could
>> use interface= for that.
>>
>> srcaddr= sounds fine to me.
>>
>
>
> Agreed.

OK - srcaddr it is ...

-- 
Thanks,

Steve

Re: [cifs bindaddr v3] cifs: Allow binding to local IP address.

[Ben Greear]

[-- Attachment #1: Type: text/plain, Size: 328 bytes --]

Updated patch is attached.  It *should* support IPv4 & IPv6, but I've only
tested IPv4 to date.  I need to work on my test rig before I can
test v6, hopefully will get that done in a day or two.

Thanks,
Ben

-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com


[-- Attachment #2: cifs_bind_v4_v6.patch --]
[-- Type: text/plain, Size: 7780 bytes --]

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index b7431af..fb4ecbe 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -357,6 +357,24 @@ cifs_show_address(struct seq_file *s, struct TCP_Server_Info *server)
 	}
 }
 
+bool
+cifs_addr_is_specified(struct sockaddr* srcaddr) {
+	struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
+	struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
+	switch (srcaddr->sa_family) {
+	case AF_INET:
+		return saddr4->sin_addr.s_addr != 0;
+		
+#if defined(CONFIG_IPV6) || defined(CONFIG_IPV6_MODULE)
+	case AF_INET6:
+		/* in6addr_any isn't available if IPv6 isn't compiled in */
+		return (memcmp(&in6addr_any, &saddr6->sin6_addr,
+			       sizeof(in6addr_any)) == 0);
+#endif
+	}
+	return false;
+}
+
 /*
  * cifs_show_options() is for displaying mount options in /proc/mounts.
  * Not all settable options are displayed but most of the important
@@ -374,6 +392,19 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
 	if (tcon->ses->domainName)
 		seq_printf(s, ",domain=%s", tcon->ses->domainName);
 
+	if (cifs_addr_is_specified((struct sockaddr*)(&tcon->ses->server->srcaddr))) {
+		struct sockaddr_in *saddr4;
+		struct sockaddr_in6 *saddr6;
+		saddr4 = (struct sockaddr_in *)&tcon->ses->server->srcaddr;
+		saddr6 = (struct sockaddr_in6 *)&tcon->ses->server->srcaddr;
+		if (saddr6->sin6_family == AF_INET6)
+			seq_printf(s, ",srcaddr=%pI6",
+			   &saddr6->sin6_addr);
+		else
+			seq_printf(s, ",srcaddr=%pI4",
+			   &saddr4->sin_addr.s_addr);	
+	}
+
 	seq_printf(s, ",uid=%d", cifs_sb->mnt_uid);
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID)
 		seq_printf(s, ",forceuid");
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index d82f5fb..0f0d786 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -110,6 +110,9 @@ extern ssize_t	cifs_getxattr(struct dentry *, const char *, void *, size_t);
 extern ssize_t	cifs_listxattr(struct dentry *, char *, size_t);
 extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
 
+struct sockaddr;
+extern bool cifs_addr_is_specified(struct sockaddr* srcaddr);
+
 #ifdef CONFIG_CIFS_EXPERIMENTAL
 extern const struct export_operations cifs_export_ops;
 #endif /* EXPERIMENTAL */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index c9d0cfc..784fd4a 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -157,6 +157,7 @@ struct TCP_Server_Info {
 		struct sockaddr_in sockAddr;
 		struct sockaddr_in6 sockAddr6;
 	} addr;
+	struct sockaddr_storage srcaddr; /* locally bind to this IP */
 	wait_queue_head_t response_q;
 	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
 	struct list_head pending_mid_q;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ec0ea4a..f88af63 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -49,6 +49,7 @@
 #include "rfc1002pdu.h"
 #include "cn_cifs.h"
 #include "fscache.h"
+#include "cifsfs.h"
 
 #define CIFS_PORT 445
 #define RFC1001_PORT 139
@@ -105,6 +106,7 @@ struct smb_vol {
 	bool sockopt_tcp_nodelay:1;
 	unsigned short int port;
 	char *prepath;
+	struct sockaddr_storage srcaddr; /* allow binding to a local IP address */
 	struct nls_table *local_nls;
 };
 
@@ -1064,6 +1066,22 @@ cifs_parse_mount_options(char *options, const char *devname,
 						    "long\n");
 				return 1;
 			}
+		} else if (strnicmp(data, "srcaddr", 8) == 0) {
+			memset(&vol->srcaddr, 0, sizeof(vol->srcaddr));
+
+			if (!value || !*value) {
+				printk(KERN_WARNING "CIFS: srcaddr value"
+				       " not specified.\n");
+				return 1;	/* needs_arg; */
+			}
+			i = cifs_convert_address((struct sockaddr *)(&vol->srcaddr),
+						 value, strlen(value));
+			if (i < 0) {
+				printk(KERN_WARNING "CIFS:  Could not parse"
+				       " srcaddr: %s\n",
+				       value);
+				return 1;
+			}
 		} else if (strnicmp(data, "prefixpath", 10) == 0) {
 			if (!value || !*value) {
 				printk(KERN_WARNING
@@ -1392,8 +1410,37 @@ cifs_parse_mount_options(char *options, const char *devname,
 	return 0;
 }
 
+/** Returns true if srcaddr isn't specified or if it matches
+ * the IP address of the rhs argument.
+ */
 static bool
-match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
+srcip_matches(struct sockaddr* srcaddr, struct sockaddr* rhs)
+{
+	if (cifs_addr_is_specified(srcaddr)) {
+		struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
+		struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
+		struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
+		struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)&rhs;
+
+		switch (srcaddr->sa_family) {
+		case AF_INET:
+			if (saddr4->sin_addr.s_addr != vaddr4->sin_addr.s_addr)
+				return false;
+			break;
+		case AF_INET6:
+			if (memcmp(&saddr6->sin6_addr, &vaddr6->sin6_addr,
+				   sizeof(saddr6->sin6_addr)) != 0)
+				return false;
+			break;
+		}
+	}
+	return true;
+}
+
+
+static bool
+match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
+	      struct sockaddr* srcaddr)
 {
 	struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
 	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
@@ -1420,6 +1467,9 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
 		break;
 	}
 
+	if (!srcip_matches(srcaddr, (struct sockaddr*)&server->srcaddr))
+		return false;
+	
 	return true;
 }
 
@@ -1487,7 +1537,7 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
 		if (server->tcpStatus == CifsNew)
 			continue;
 
-		if (!match_address(server, addr))
+		if (!match_address(server, addr, (struct sockaddr*)&vol->srcaddr))
 			continue;
 
 		if (!match_security(server, vol))
@@ -1602,6 +1652,7 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	 * no need to spinlock this init of tcpStatus or srv_count
 	 */
 	tcp_ses->tcpStatus = CifsNew;
+	memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr, sizeof(tcp_ses->srcaddr));
 	++tcp_ses->srv_count;
 
 	if (addr.ss_family == AF_INET6) {
@@ -1678,6 +1729,9 @@ cifs_find_smb_ses(struct TCP_Server_Info *server, struct smb_vol *vol)
 				    vol->password ? vol->password : "",
 				    MAX_PASSWORD_SIZE))
 				continue;
+			if (!srcip_matches((struct sockaddr*)&server->srcaddr,
+					   (struct sockaddr*)&ses->server->srcaddr))
+				continue;
 		}
 		++ses->ses_count;
 		write_unlock(&cifs_tcp_ses_lock);
@@ -2051,6 +2105,24 @@ ipv4_connect(struct TCP_Server_Info *server)
 		cifs_reclassify_socket4(socket);
 	}
 
+	if (cifs_addr_is_specified((struct sockaddr*)&server->srcaddr)) {
+		/* Bind to the local IP address if specified */
+		rc = socket->ops->bind(socket, (struct sockaddr *) &server->srcaddr,
+				       sizeof(server->srcaddr));
+		if (rc < 0) {
+			struct sockaddr_in *saddr4;
+			struct sockaddr_in6 *saddr6;
+			saddr4 = (struct sockaddr_in *)&server->srcaddr;
+			saddr6 = (struct sockaddr_in6 *)&server->srcaddr;
+			if (saddr6->sin6_family == AF_INET6)
+				cERROR(1, "Failed to bind to: %pI6, error: %d\n",
+				       &saddr6->sin6_addr, rc);
+			else
+				cERROR(1, "Failed to bind to: %pI4, error: %d\n",
+				       &saddr4->sin_addr.s_addr, rc);
+		}
+	}
+	
 	/* user overrode default port */
 	if (server->addr.sockAddr.sin_port) {
 		rc = socket->ops->connect(socket, (struct sockaddr *)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index ab70a3f..4c0e04b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -234,6 +234,8 @@ const struct in6_addr in6addr_loopback = IN6ADDR_LOOPBACK_INIT;
 const struct in6_addr in6addr_linklocal_allnodes = IN6ADDR_LINKLOCAL_ALLNODES_INIT;
 const struct in6_addr in6addr_linklocal_allrouters = IN6ADDR_LINKLOCAL_ALLROUTERS_INIT;
 
+EXPORT_SYMBOL(in6addr_any);
+
 /* Check if a valid qdisc is available */
 static inline bool addrconf_qdisc_ok(const struct net_device *dev)
 {

[cifs bindaddr v3] cifs: Allow binding to local IP address.

[Ben Greear]

When using multi-homed machines, it's nice to be able to specify
the local IP to use for outbound connections.  This patch gives
cifs the ability to bind to a particular IP address.

   Usage:  mount -t cifs -o srcaddr=192.168.1.50,user=foo, ...
   Usage:  mount -t cifs -o srcaddr=2002::100:1,user=foo, ...

Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
---
:100644 100644 b7431af... ac69602... M	fs/cifs/cifsfs.c
:100644 100644 d82f5fb... d6f0a7d... M	fs/cifs/cifsfs.h
:100644 100644 c9d0cfc... 784fd4a... M	fs/cifs/cifsglob.h
:100644 100644 ec0ea4a... c70f6f8... M	fs/cifs/connect.c
 fs/cifs/cifsfs.c   |   30 +++++++++++++++++
 fs/cifs/cifsfs.h   |    3 ++
 fs/cifs/cifsglob.h |    1 +
 fs/cifs/connect.c  |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 125 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
index b7431af..ac69602 100644
--- a/fs/cifs/cifsfs.c
+++ b/fs/cifs/cifsfs.c
@@ -36,6 +36,7 @@
 #include <linux/kthread.h>
 #include <linux/freezer.h>
 #include <linux/smp_lock.h>
+#include <net/ipv6.h>
 #include "cifsfs.h"
 #include "cifspdu.h"
 #define DECLARE_GLOBALS_HERE
@@ -357,6 +358,20 @@ cifs_show_address(struct seq_file *s, struct TCP_Server_Info *server)
 	}
 }
 
+bool
+cifs_addr_is_specified(struct sockaddr *srcaddr) {
+	struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
+	struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
+	static const struct in6_addr c_in6addr_any = IN6ADDR_ANY_INIT;
+	switch (srcaddr->sa_family) {
+	case AF_INET:
+		return saddr4->sin_addr.s_addr != 0;
+	case AF_INET6:
+		return (!ipv6_addr_equal(&c_in6addr_any, &saddr6->sin6_addr));
+	}
+	return false;
+}
+
 /*
  * cifs_show_options() is for displaying mount options in /proc/mounts.
  * Not all settable options are displayed but most of the important
@@ -367,6 +382,8 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
 {
 	struct cifs_sb_info *cifs_sb = CIFS_SB(m->mnt_sb);
 	struct cifsTconInfo *tcon = cifs_sb->tcon;
+	struct sockaddr *srcaddr;
+	srcaddr = (struct sockaddr *)(&tcon->ses->server->srcaddr);
 
 	seq_printf(s, ",unc=%s", tcon->treeName);
 	if (tcon->ses->userName)
@@ -374,6 +391,19 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
 	if (tcon->ses->domainName)
 		seq_printf(s, ",domain=%s", tcon->ses->domainName);
 
+	if (cifs_addr_is_specified(srcaddr)) {
+		struct sockaddr_in *saddr4;
+		struct sockaddr_in6 *saddr6;
+		saddr4 = (struct sockaddr_in *)srcaddr;
+		saddr6 = (struct sockaddr_in6 *)srcaddr;
+		if (saddr6->sin6_family == AF_INET6)
+			seq_printf(s, ",srcaddr=%pI6c",
+				   &saddr6->sin6_addr);
+		else
+			seq_printf(s, ",srcaddr=%pI4",
+				   &saddr4->sin_addr.s_addr);
+	}
+
 	seq_printf(s, ",uid=%d", cifs_sb->mnt_uid);
 	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID)
 		seq_printf(s, ",forceuid");
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index d82f5fb..d6f0a7d 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -110,6 +110,9 @@ extern ssize_t	cifs_getxattr(struct dentry *, const char *, void *, size_t);
 extern ssize_t	cifs_listxattr(struct dentry *, char *, size_t);
 extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
 
+struct sockaddr;
+extern bool cifs_addr_is_specified(struct sockaddr *srcaddr);
+
 #ifdef CONFIG_CIFS_EXPERIMENTAL
 extern const struct export_operations cifs_export_ops;
 #endif /* EXPERIMENTAL */
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index c9d0cfc..784fd4a 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -157,6 +157,7 @@ struct TCP_Server_Info {
 		struct sockaddr_in sockAddr;
 		struct sockaddr_in6 sockAddr6;
 	} addr;
+	struct sockaddr_storage srcaddr; /* locally bind to this IP */
 	wait_queue_head_t response_q;
 	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
 	struct list_head pending_mid_q;
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index ec0ea4a..c70f6f8 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -49,6 +49,7 @@
 #include "rfc1002pdu.h"
 #include "cn_cifs.h"
 #include "fscache.h"
+#include "cifsfs.h"
 
 #define CIFS_PORT 445
 #define RFC1001_PORT 139
@@ -105,6 +106,7 @@ struct smb_vol {
 	bool sockopt_tcp_nodelay:1;
 	unsigned short int port;
 	char *prepath;
+	struct sockaddr_storage srcaddr; /* allow binding to a local IP */
 	struct nls_table *local_nls;
 };
 
@@ -1064,6 +1066,22 @@ cifs_parse_mount_options(char *options, const char *devname,
 						    "long\n");
 				return 1;
 			}
+		} else if (strnicmp(data, "srcaddr", 8) == 0) {
+			vol->srcaddr.ss_family = AF_UNSPEC;
+
+			if (!value || !*value) {
+				printk(KERN_WARNING "CIFS: srcaddr value"
+				       " not specified.\n");
+				return 1;	/* needs_arg; */
+			}
+			i = cifs_convert_address((struct sockaddr *)&vol->srcaddr,
+						 value, strlen(value));
+			if (i < 0) {
+				printk(KERN_WARNING "CIFS:  Could not parse"
+				       " srcaddr: %s\n",
+				       value);
+				return 1;
+			}
 		} else if (strnicmp(data, "prefixpath", 10) == 0) {
 			if (!value || !*value) {
 				printk(KERN_WARNING
@@ -1392,8 +1410,42 @@ cifs_parse_mount_options(char *options, const char *devname,
 	return 0;
 }
 
+/** Returns true if srcaddr isn't specified and rhs isn't
+ * specified, or if srcaddr is specified and
+ * matches the IP address of the rhs argument.
+ */
+static bool
+srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
+{
+	if (cifs_addr_is_specified(srcaddr)) {
+		struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
+		struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
+		struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
+		struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)&rhs;
+
+		switch (srcaddr->sa_family) {
+		case AF_INET:
+			if (saddr4->sin_addr.s_addr != vaddr4->sin_addr.s_addr)
+				return false;
+			break;
+		case AF_INET6:
+			if (!ipv6_addr_equal(&saddr6->sin6_addr,
+					     &vaddr6->sin6_addr))
+				return false;
+			break;
+		default:
+			return false;
+		}
+		return true;
+	}
+	else
+		return !cifs_addr_is_specified(rhs);
+}
+
+
 static bool
-match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
+match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
+	      struct sockaddr *srcaddr)
 {
 	struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
 	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
@@ -1420,6 +1472,9 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
 		break;
 	}
 
+	if (!srcip_matches(srcaddr, (struct sockaddr *)&server->srcaddr))
+		return false;
+
 	return true;
 }
 
@@ -1487,7 +1542,8 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
 		if (server->tcpStatus == CifsNew)
 			continue;
 
-		if (!match_address(server, addr))
+		if (!match_address(server, addr,
+				   (struct sockaddr *)&vol->srcaddr))
 			continue;
 
 		if (!match_security(server, vol))
@@ -1602,6 +1658,8 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
 	 * no need to spinlock this init of tcpStatus or srv_count
 	 */
 	tcp_ses->tcpStatus = CifsNew;
+	memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr,
+	       sizeof(tcp_ses->srcaddr));
 	++tcp_ses->srv_count;
 
 	if (addr.ss_family == AF_INET6) {
@@ -2026,6 +2084,33 @@ static void rfc1002mangle(char *target, char *source, unsigned int length)
 
 }
 
+static int
+bind_socket(struct TCP_Server_Info *server)
+{
+	int rc = 0;
+	if (cifs_addr_is_specified((struct sockaddr *)&server->srcaddr)) {
+		/* Bind to the local IP address if specified */
+		struct socket *socket = server->ssocket;
+		rc = socket->ops->bind(socket,
+				       (struct sockaddr *) &server->srcaddr,
+				       sizeof(server->srcaddr));
+		if (rc < 0) {
+			struct sockaddr_in *saddr4;
+			struct sockaddr_in6 *saddr6;
+			saddr4 = (struct sockaddr_in *)&server->srcaddr;
+			saddr6 = (struct sockaddr_in6 *)&server->srcaddr;
+			if (saddr6->sin6_family == AF_INET6)
+				printk(KERN_WARNING "cifs: "
+				       "Failed to bind to: %pI6c, error: %d\n",
+				       &saddr6->sin6_addr, rc);
+			else
+				printk(KERN_WARNING "cifs: "
+				       "Failed to bind to: %pI4, error: %d\n",
+				       &saddr4->sin_addr.s_addr, rc);
+		}
+	}
+	return rc;
+}
 
 static int
 ipv4_connect(struct TCP_Server_Info *server)
@@ -2051,6 +2136,8 @@ ipv4_connect(struct TCP_Server_Info *server)
 		cifs_reclassify_socket4(socket);
 	}
 
+	bind_socket(server);
+
 	/* user overrode default port */
 	if (server->addr.sockAddr.sin_port) {
 		rc = socket->ops->connect(socket, (struct sockaddr *)
@@ -2213,6 +2300,8 @@ ipv6_connect(struct TCP_Server_Info *server)
 		cifs_reclassify_socket6(socket);
 	}
 
+	bind_socket(server);
+
 	/* user overrode default port */
 	if (server->addr.sockAddr6.sin6_port) {
 		rc = socket->ops->connect(socket,
-- 
1.6.2.5

Re: [cifs bindaddr v3] cifs: Allow binding to local IP address.

[Jeff Layton]

On Tue, 31 Aug 2010 12:55:14 -0700
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:

> When using multi-homed machines, it's nice to be able to specify
> the local IP to use for outbound connections.  This patch gives
> cifs the ability to bind to a particular IP address.
> 
>    Usage:  mount -t cifs -o srcaddr=192.168.1.50,user=foo, ...
>    Usage:  mount -t cifs -o srcaddr=2002::100:1,user=foo, ...
> 
> Signed-off-by: Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
> ---
> :100644 100644 b7431af... ac69602... M	fs/cifs/cifsfs.c
> :100644 100644 d82f5fb... d6f0a7d... M	fs/cifs/cifsfs.h
> :100644 100644 c9d0cfc... 784fd4a... M	fs/cifs/cifsglob.h
> :100644 100644 ec0ea4a... c70f6f8... M	fs/cifs/connect.c
>  fs/cifs/cifsfs.c   |   30 +++++++++++++++++
>  fs/cifs/cifsfs.h   |    3 ++
>  fs/cifs/cifsglob.h |    1 +
>  fs/cifs/connect.c  |   93 ++++++++++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 125 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c
> index b7431af..ac69602 100644
> --- a/fs/cifs/cifsfs.c
> +++ b/fs/cifs/cifsfs.c
> @@ -36,6 +36,7 @@
>  #include <linux/kthread.h>
>  #include <linux/freezer.h>
>  #include <linux/smp_lock.h>
> +#include <net/ipv6.h>
>  #include "cifsfs.h"
>  #include "cifspdu.h"
>  #define DECLARE_GLOBALS_HERE
> @@ -357,6 +358,20 @@ cifs_show_address(struct seq_file *s, struct TCP_Server_Info *server)
>  	}
>  }
>  
> +bool
> +cifs_addr_is_specified(struct sockaddr *srcaddr) {
> +	struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
> +	struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
> +	static const struct in6_addr c_in6addr_any = IN6ADDR_ANY_INIT;
> +	switch (srcaddr->sa_family) {
> +	case AF_INET:
> +		return saddr4->sin_addr.s_addr != 0;
> +	case AF_INET6:
> +		return (!ipv6_addr_equal(&c_in6addr_any, &saddr6->sin6_addr));
> +	}
> +	return false;
> +}
> +

I don't think you need all of this. cifs_addr_is_specified ought to
just be srcaddr->sa_family != AF_UNSPEC. That could be a static inline
or macro, even.

>  /*
>   * cifs_show_options() is for displaying mount options in /proc/mounts.
>   * Not all settable options are displayed but most of the important
> @@ -367,6 +382,8 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
>  {
>  	struct cifs_sb_info *cifs_sb = CIFS_SB(m->mnt_sb);
>  	struct cifsTconInfo *tcon = cifs_sb->tcon;
> +	struct sockaddr *srcaddr;
> +	srcaddr = (struct sockaddr *)(&tcon->ses->server->srcaddr);
>  
>  	seq_printf(s, ",unc=%s", tcon->treeName);
>  	if (tcon->ses->userName)
> @@ -374,6 +391,19 @@ cifs_show_options(struct seq_file *s, struct vfsmount *m)
>  	if (tcon->ses->domainName)
>  		seq_printf(s, ",domain=%s", tcon->ses->domainName);
>  
> +	if (cifs_addr_is_specified(srcaddr)) {
> +		struct sockaddr_in *saddr4;
> +		struct sockaddr_in6 *saddr6;
> +		saddr4 = (struct sockaddr_in *)srcaddr;
> +		saddr6 = (struct sockaddr_in6 *)srcaddr;
> +		if (saddr6->sin6_family == AF_INET6)
> +			seq_printf(s, ",srcaddr=%pI6c",
> +				   &saddr6->sin6_addr);
> +		else
> +			seq_printf(s, ",srcaddr=%pI4",
> +				   &saddr4->sin_addr.s_addr);
> +	}
> +
>  	seq_printf(s, ",uid=%d", cifs_sb->mnt_uid);
>  	if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_OVERR_UID)
>  		seq_printf(s, ",forceuid");
> diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
> index d82f5fb..d6f0a7d 100644
> --- a/fs/cifs/cifsfs.h
> +++ b/fs/cifs/cifsfs.h
> @@ -110,6 +110,9 @@ extern ssize_t	cifs_getxattr(struct dentry *, const char *, void *, size_t);
>  extern ssize_t	cifs_listxattr(struct dentry *, char *, size_t);
>  extern long cifs_ioctl(struct file *filep, unsigned int cmd, unsigned long arg);
>  
> +struct sockaddr;
> +extern bool cifs_addr_is_specified(struct sockaddr *srcaddr);
> +
>  #ifdef CONFIG_CIFS_EXPERIMENTAL
>  extern const struct export_operations cifs_export_ops;
>  #endif /* EXPERIMENTAL */
> diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
> index c9d0cfc..784fd4a 100644
> --- a/fs/cifs/cifsglob.h
> +++ b/fs/cifs/cifsglob.h
> @@ -157,6 +157,7 @@ struct TCP_Server_Info {
>  		struct sockaddr_in sockAddr;
>  		struct sockaddr_in6 sockAddr6;
>  	} addr;
> +	struct sockaddr_storage srcaddr; /* locally bind to this IP */
>  	wait_queue_head_t response_q;
>  	wait_queue_head_t request_q; /* if more than maxmpx to srvr must block*/
>  	struct list_head pending_mid_q;
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index ec0ea4a..c70f6f8 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -49,6 +49,7 @@
>  #include "rfc1002pdu.h"
>  #include "cn_cifs.h"
>  #include "fscache.h"
> +#include "cifsfs.h"
>  
>  #define CIFS_PORT 445
>  #define RFC1001_PORT 139
> @@ -105,6 +106,7 @@ struct smb_vol {
>  	bool sockopt_tcp_nodelay:1;
>  	unsigned short int port;
>  	char *prepath;
> +	struct sockaddr_storage srcaddr; /* allow binding to a local IP */
>  	struct nls_table *local_nls;
>  };
>  
> @@ -1064,6 +1066,22 @@ cifs_parse_mount_options(char *options, const char *devname,
>  						    "long\n");
>  				return 1;
>  			}
> +		} else if (strnicmp(data, "srcaddr", 8) == 0) {
> +			vol->srcaddr.ss_family = AF_UNSPEC;
> +
> +			if (!value || !*value) {
> +				printk(KERN_WARNING "CIFS: srcaddr value"
> +				       " not specified.\n");
> +				return 1;	/* needs_arg; */
> +			}
> +			i = cifs_convert_address((struct sockaddr *)&vol->srcaddr,
> +						 value, strlen(value));
> +			if (i < 0) {
> +				printk(KERN_WARNING "CIFS:  Could not parse"
> +				       " srcaddr: %s\n",
> +				       value);
> +				return 1;
> +			}
>  		} else if (strnicmp(data, "prefixpath", 10) == 0) {
>  			if (!value || !*value) {
>  				printk(KERN_WARNING
> @@ -1392,8 +1410,42 @@ cifs_parse_mount_options(char *options, const char *devname,
>  	return 0;
>  }
>  
> +/** Returns true if srcaddr isn't specified and rhs isn't
> + * specified, or if srcaddr is specified and
> + * matches the IP address of the rhs argument.
> + */
> +static bool
> +srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
> +{
> +	if (cifs_addr_is_specified(srcaddr)) {
> +		struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
> +		struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
> +		struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
> +		struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)&rhs;
> +
> +		switch (srcaddr->sa_family) {
> +		case AF_INET:
> +			if (saddr4->sin_addr.s_addr != vaddr4->sin_addr.s_addr)
> +				return false;
> +			break;
> +		case AF_INET6:
> +			if (!ipv6_addr_equal(&saddr6->sin6_addr,
> +					     &vaddr6->sin6_addr))
> +				return false;
> +			break;
> +		default:
> +			return false;
> +		}
> +		return true;
> +	}
> +	else
> +		return !cifs_addr_is_specified(rhs);
> +}

This is more complicated than it really needs to be I think. I think
all what you really need to do here is check to see if the address
families match. If they do and they're either AF_INET flavor, then check
to see if the addresses match. You might even be able to reuse some of
the code in match_address here.

> +
> +
>  static bool
> -match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
> +match_address(struct TCP_Server_Info *server, struct sockaddr *addr,
> +	      struct sockaddr *srcaddr)
>  {
>  	struct sockaddr_in *addr4 = (struct sockaddr_in *)addr;
>  	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
> @@ -1420,6 +1472,9 @@ match_address(struct TCP_Server_Info *server, struct sockaddr *addr)
>  		break;
>  	}
>  
> +	if (!srcip_matches(srcaddr, (struct sockaddr *)&server->srcaddr))
> +		return false;
> +
>  	return true;
>  }
>  
> @@ -1487,7 +1542,8 @@ cifs_find_tcp_session(struct sockaddr *addr, struct smb_vol *vol)
>  		if (server->tcpStatus == CifsNew)
>  			continue;
>  
> -		if (!match_address(server, addr))
> +		if (!match_address(server, addr,
> +				   (struct sockaddr *)&vol->srcaddr))
>  			continue;
>  
>  		if (!match_security(server, vol))
> @@ -1602,6 +1658,8 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
>  	 * no need to spinlock this init of tcpStatus or srv_count
>  	 */
>  	tcp_ses->tcpStatus = CifsNew;
> +	memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr,
> +	       sizeof(tcp_ses->srcaddr));
>  	++tcp_ses->srv_count;
>  
>  	if (addr.ss_family == AF_INET6) {
> @@ -2026,6 +2084,33 @@ static void rfc1002mangle(char *target, char *source, unsigned int length)
>  
>  }
>  
> +static int
> +bind_socket(struct TCP_Server_Info *server)
> +{
> +	int rc = 0;
> +	if (cifs_addr_is_specified((struct sockaddr *)&server->srcaddr)) {
> +		/* Bind to the local IP address if specified */
> +		struct socket *socket = server->ssocket;
> +		rc = socket->ops->bind(socket,
> +				       (struct sockaddr *) &server->srcaddr,
> +				       sizeof(server->srcaddr));
> +		if (rc < 0) {
> +			struct sockaddr_in *saddr4;
> +			struct sockaddr_in6 *saddr6;
> +			saddr4 = (struct sockaddr_in *)&server->srcaddr;
> +			saddr6 = (struct sockaddr_in6 *)&server->srcaddr;
> +			if (saddr6->sin6_family == AF_INET6)
> +				printk(KERN_WARNING "cifs: "
> +				       "Failed to bind to: %pI6c, error: %d\n",
> +				       &saddr6->sin6_addr, rc);
> +			else
> +				printk(KERN_WARNING "cifs: "
> +				       "Failed to bind to: %pI4, error: %d\n",
> +				       &saddr4->sin_addr.s_addr, rc);
> +		}
			^^^^^^^^^^^
For better or worse, the CIFS code uses the cFYI and cERROR macros for
printk's. You should probably do the same here.

> +	}
> +	return rc;
> +}
>  
>  static int
>  ipv4_connect(struct TCP_Server_Info *server)
> @@ -2051,6 +2136,8 @@ ipv4_connect(struct TCP_Server_Info *server)
>  		cifs_reclassify_socket4(socket);
>  	}
>  
> +	bind_socket(server);
> +
>  	/* user overrode default port */
>  	if (server->addr.sockAddr.sin_port) {
>  		rc = socket->ops->connect(socket, (struct sockaddr *)
> @@ -2213,6 +2300,8 @@ ipv6_connect(struct TCP_Server_Info *server)
>  		cifs_reclassify_socket6(socket);
>  	}
>  
> +	bind_socket(server);
> +

^^^^
The printk's are nice and all, but shouldn't you fail the
ipv[4,6]_connect if the socket can't be bound?

>  	/* user overrode default port */
>  	if (server->addr.sockAddr6.sin6_port) {
>  		rc = socket->ops->connect(socket,


-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [cifs bindaddr v3] cifs: Allow binding to local IP address.

[Ben Greear]

On 09/01/2010 05:41 AM, Jeff Layton wrote:
> On Tue, 31 Aug 2010 12:55:14 -0700
> Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>  wrote:
>
>> When using multi-homed machines, it's nice to be able to specify
>> the local IP to use for outbound connections.  This patch gives
>> cifs the ability to bind to a particular IP address.
>>
>>     Usage:  mount -t cifs -o srcaddr=192.168.1.50,user=foo, ...
>>     Usage:  mount -t cifs -o srcaddr=2002::100:1,user=foo, ...
>>
>> Signed-off-by: Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>

>> +bool
>> +cifs_addr_is_specified(struct sockaddr *srcaddr) {
>> +	struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
>> +	struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
>> +	static const struct in6_addr c_in6addr_any = IN6ADDR_ANY_INIT;
>> +	switch (srcaddr->sa_family) {
>> +	case AF_INET:
>> +		return saddr4->sin_addr.s_addr != 0;
>> +	case AF_INET6:
>> +		return (!ipv6_addr_equal(&c_in6addr_any,&saddr6->sin6_addr));
>> +	}
>> +	return false;
>> +}
>> +
>
> I don't think you need all of this. cifs_addr_is_specified ought to
> just be srcaddr->sa_family != AF_UNSPEC. That could be a static inline
> or macro, even.

Yeah, I'll fix that next patch.

>> +/** Returns true if srcaddr isn't specified and rhs isn't
>> + * specified, or if srcaddr is specified and
>> + * matches the IP address of the rhs argument.
>> + */
>> +static bool
>> +srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
>> +{
>> +	if (cifs_addr_is_specified(srcaddr)) {
>> +		struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
>> +		struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
>> +		struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
>> +		struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)&rhs;
>> +
>> +		switch (srcaddr->sa_family) {
>> +		case AF_INET:
>> +			if (saddr4->sin_addr.s_addr != vaddr4->sin_addr.s_addr)
>> +				return false;
>> +			break;
>> +		case AF_INET6:
>> +			if (!ipv6_addr_equal(&saddr6->sin6_addr,
>> +					&vaddr6->sin6_addr))
>> +				return false;
>> +			break;
>> +		default:
>> +			return false;
>> +		}
>> +		return true;
>> +	}
>> +	else
>> +		return !cifs_addr_is_specified(rhs);
>> +}
>
> This is more complicated than it really needs to be I think. I think
> all what you really need to do here is check to see if the address
> families match. If they do and they're either AF_INET flavor, then check
> to see if the addresses match. You might even be able to reuse some of
> the code in match_address here.

I think I'm basically doing what you suggest, with the only trick that
any un-bound (non-specified) addresses must match only other non-specified
connections.

Please post an improved version of this method if you have one to
suggest, but I don't see any way to significantly simplify this.

>> +static int
>> +bind_socket(struct TCP_Server_Info *server)
>> +{
>> +	int rc = 0;
>> +	if (cifs_addr_is_specified((struct sockaddr *)&server->srcaddr)) {
>> +		/* Bind to the local IP address if specified */
>> +		struct socket *socket = server->ssocket;
>> +		rc = socket->ops->bind(socket,
>> +				       (struct sockaddr *)&server->srcaddr,
>> +				       sizeof(server->srcaddr));
>> +		if (rc<  0) {
>> +			struct sockaddr_in *saddr4;
>> +			struct sockaddr_in6 *saddr6;
>> +			saddr4 = (struct sockaddr_in *)&server->srcaddr;
>> +			saddr6 = (struct sockaddr_in6 *)&server->srcaddr;
>> +			if (saddr6->sin6_family == AF_INET6)
>> +				printk(KERN_WARNING "cifs: "
>> +				       "Failed to bind to: %pI6c, error: %d\n",
>> +				&saddr6->sin6_addr, rc);
>> +			else
>> +				printk(KERN_WARNING "cifs: "
>> +				       "Failed to bind to: %pI4, error: %d\n",
>> +				&saddr4->sin_addr.s_addr, rc);
>> +		}
> 			^^^^^^^^^^^
> For better or worse, the CIFS code uses the cFYI and cERROR macros for
> printk's. You should probably do the same here.

If I make these cERROR, will they be printed to /var/log/messages
and/or dmesg by default if the error case hits?  I definately want this
visible in the logs by default so users have a chance of figuring out why
the bind failed.

> ^^^^
> The printk's are nice and all, but shouldn't you fail the
> ipv[4,6]_connect if the socket can't be bound?

Either way is fine with me.  It would be possible for the actual
connection to still work in the bind-failure case, but it might not
be what the user would expect.

Thanks,
Ben

-- 
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
Candela Technologies Inc  http://www.candelatech.com

Re: [cifs bindaddr v3] cifs: Allow binding to local IP address.

[Jeff Layton]

On Wed, 01 Sep 2010 09:31:24 -0700
Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:

> On 09/01/2010 05:41 AM, Jeff Layton wrote:
> > On Tue, 31 Aug 2010 12:55:14 -0700
> > Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>  wrote:
> >
> >> When using multi-homed machines, it's nice to be able to specify
> >> the local IP to use for outbound connections.  This patch gives
> >> cifs the ability to bind to a particular IP address.
> >>
> >>     Usage:  mount -t cifs -o srcaddr=192.168.1.50,user=foo, ...
> >>     Usage:  mount -t cifs -o srcaddr=2002::100:1,user=foo, ...
> >>
> >> Signed-off-by: Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>
> 
> >> +bool
> >> +cifs_addr_is_specified(struct sockaddr *srcaddr) {
> >> +	struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
> >> +	struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
> >> +	static const struct in6_addr c_in6addr_any = IN6ADDR_ANY_INIT;
> >> +	switch (srcaddr->sa_family) {
> >> +	case AF_INET:
> >> +		return saddr4->sin_addr.s_addr != 0;
> >> +	case AF_INET6:
> >> +		return (!ipv6_addr_equal(&c_in6addr_any,&saddr6->sin6_addr));
> >> +	}
> >> +	return false;
> >> +}
> >> +
> >
> > I don't think you need all of this. cifs_addr_is_specified ought to
> > just be srcaddr->sa_family != AF_UNSPEC. That could be a static inline
> > or macro, even.
> 
> Yeah, I'll fix that next patch.
> 
> >> +/** Returns true if srcaddr isn't specified and rhs isn't
> >> + * specified, or if srcaddr is specified and
> >> + * matches the IP address of the rhs argument.
> >> + */
> >> +static bool
> >> +srcip_matches(struct sockaddr *srcaddr, struct sockaddr *rhs)
> >> +{
> >> +	if (cifs_addr_is_specified(srcaddr)) {
> >> +		struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
> >> +		struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
> >> +		struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
> >> +		struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)&rhs;
> >> +
> >> +		switch (srcaddr->sa_family) {
> >> +		case AF_INET:
> >> +			if (saddr4->sin_addr.s_addr != vaddr4->sin_addr.s_addr)
> >> +				return false;
> >> +			break;
> >> +		case AF_INET6:
> >> +			if (!ipv6_addr_equal(&saddr6->sin6_addr,
> >> +					&vaddr6->sin6_addr))
> >> +				return false;
> >> +			break;
> >> +		default:
> >> +			return false;
> >> +		}
> >> +		return true;
> >> +	}
> >> +	else
> >> +		return !cifs_addr_is_specified(rhs);
> >> +}
> >
> > This is more complicated than it really needs to be I think. I think
> > all what you really need to do here is check to see if the address
> > families match. If they do and they're either AF_INET flavor, then check
> > to see if the addresses match. You might even be able to reuse some of
> > the code in match_address here.
> 
> I think I'm basically doing what you suggest, with the only trick that
> any un-bound (non-specified) addresses must match only other non-specified
> connections.
> 
> Please post an improved version of this method if you have one to
> suggest, but I don't see any way to significantly simplify this.
> 

In pseudocode...

switch(srcaddr->sa_family) {
AF_UNSPEC:
	if (rhs->sa_family == AF_UNSPEC)
		return true;
AF_INET:
	/* compare v4 address, return true if same */
AF_INET6:
	/* compare v6 address, return true if same */
default:
	/* fall through */
}
return false;

> >> +static int
> >> +bind_socket(struct TCP_Server_Info *server)
> >> +{
> >> +	int rc = 0;
> >> +	if (cifs_addr_is_specified((struct sockaddr *)&server->srcaddr)) {
> >> +		/* Bind to the local IP address if specified */
> >> +		struct socket *socket = server->ssocket;
> >> +		rc = socket->ops->bind(socket,
> >> +				       (struct sockaddr *)&server->srcaddr,
> >> +				       sizeof(server->srcaddr));
> >> +		if (rc<  0) {
> >> +			struct sockaddr_in *saddr4;
> >> +			struct sockaddr_in6 *saddr6;
> >> +			saddr4 = (struct sockaddr_in *)&server->srcaddr;
> >> +			saddr6 = (struct sockaddr_in6 *)&server->srcaddr;
> >> +			if (saddr6->sin6_family == AF_INET6)
> >> +				printk(KERN_WARNING "cifs: "
> >> +				       "Failed to bind to: %pI6c, error: %d\n",
> >> +				&saddr6->sin6_addr, rc);
> >> +			else
> >> +				printk(KERN_WARNING "cifs: "
> >> +				       "Failed to bind to: %pI4, error: %d\n",
> >> +				&saddr4->sin_addr.s_addr, rc);
> >> +		}
> > 			^^^^^^^^^^^
> > For better or worse, the CIFS code uses the cFYI and cERROR macros for
> > printk's. You should probably do the same here.
> 
> If I make these cERROR, will they be printed to /var/log/messages
> and/or dmesg by default if the error case hits?  I definately want this
> visible in the logs by default so users have a chance of figuring out why
> the bind failed.
> 

They should -- those are both macros around printk.

I'd make it a cFYI macro and fail the mount if the bind fails. They can
then crank up the debugging if they need to dig further.

> > ^^^^
> > The printk's are nice and all, but shouldn't you fail the
> > ipv[4,6]_connect if the socket can't be bound?
> 
> Either way is fine with me.  It would be possible for the actual
> connection to still work in the bind-failure case, but it might not
> be what the user would expect.
> 

Sounds like reason enough to fail to mount. If they don't really care
then they can remount without srcaddr=.

-- 
Jeff Layton <jlayton-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>

Re: [cifs bindaddr v3] cifs: Allow binding to local IP address.

[Steve French]

On 9/1/10, Ben Greear <greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org> wrote:
> On 09/01/2010 05:41 AM, Jeff Layton wrote:
>> On Tue, 31 Aug 2010 12:55:14 -0700
>> Ben Greear<greearb-my8/4N5VtI7c+919tysfdA@public.gmane.org>  wrote:
>>
>>> When using multi-homed machines, it's nice to be able to specify
>>> the local IP to use for outbound connections.  This patch gives
>>> cifs the ability to bind to a particular IP address.
>>>
>>>     Usage:  mount -t cifs -o srcaddr=192.168.1.50,user=foo, ...
>>>     Usage:  mount -t cifs -o srcaddr=2002::100:1,user=foo, ...
<snip>
>>> +			if (saddr6->sin6_family == AF_INET6)
>>> +				printk(KERN_WARNING "cifs: "
>>> +				       "Failed to bind to: %pI6c, error: %d\n",
>>> +				&saddr6->sin6_addr, rc);
>>> +			else
>>> +				printk(KERN_WARNING "cifs: "
>>> +				       "Failed to bind to: %pI4, error: %d\n",
>>> +				&saddr4->sin_addr.s_addr, rc);
>>> +		}
>> 			^^^^^^^^^^^
>> For better or worse, the CIFS code uses the cFYI and cERROR macros for
>> printk's. You should probably do the same here.
>
> If I make these cERROR, will they be printed to /var/log/messages
> and/or dmesg by default if the error case hits?  I definately want this
> visible in the logs by default so users have a chance of figuring out why
> the bind failed.

Use cERROR if you want it to be logged always, cFYI if you only want
it logged when cifs debugging is enabled at run time.

cFYI messages will not be logged unless /proc/fs/cifs/cifsFYI is set
to 1 (it is a flag, CIFS_INFO, so setting cifsFYI to 3, 5, or 7 will
also turn it on these log messages on.  The other flags 0x02 is used
to log entry/exit from cifs functions, and 0x04 for timing related
logging of slow responses).

cERRORs will always be logged (although it could be changed via a
debugger to disable it, users would not disable it)


>> ^^^^
>> The printk's are nice and all, but shouldn't you fail the
>> ipv[4,6]_connect if the socket can't be bound?
>
> Either way is fine with me.  It would be possible for the actual
> connection to still work in the bind-failure case, but it might not
> be what the user would expect.

-- 
Thanks,

Steve