On 09/01/2010 05:41 AM, Jeff Layton wrote:
On Tue, 31 Aug 2010 12:55:14 -0700 Ben Greear<greearb@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
+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@xxxxxxxxxxxxxxx> Candela Technologies Inc http://www.candelatech.com -- To unsubscribe from this list: send the line "unsubscribe linux-cifs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
