Resending patch to a slightly broader list for last minute check if
anyone objects. Although setting this particular socket option
(TCP_NODELAY) may not be as useful when corking/uncorking explicitly,
I want to doublecheck before removing them because there has been some
utility to the server allowing override of various sockopt options.
Samba server has long supported at least the following set of settable
socket options (although I don't know if the defaults are frequently
overridden now, by setting sockopts in smb.conf as used to be common
for the server).
SO_KEEPALIVE
SO_REUSEADDR
SO_BROADCAST
TCP_NODELAY
IPTOS_LOWDELAY
IPTOS_THROUGHPUT
SO_SNDBUF *
SO_RCVBUF *
SO_SNDLOWAT *
SO_RCVLOWAT *
* takes an integer argument rather than a boolean on/off
Any objections to removing the ability to set socket options
explicitly for the cifs network file system client?
On Thu, Feb 21, 2013 at 5:32 AM, Jeff Layton <[email protected]> wrote:
>
> ...as promised for 3.9.
>
> Signed-off-by: Jeff Layton <[email protected]>
> ---
> fs/cifs/connect.c | 16 +---------------
> 1 file changed, 1 insertion(+), 15 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index d997737..8609c42 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -97,7 +97,7 @@ enum {
> Opt_user, Opt_pass, Opt_ip,
> Opt_unc, Opt_domain,
> Opt_srcaddr, Opt_prefixpath,
> - Opt_iocharset, Opt_sockopt,
> + Opt_iocharset,
> Opt_netbiosname, Opt_servern,
> Opt_ver, Opt_vers, Opt_sec, Opt_cache,
>
> @@ -202,7 +202,6 @@ static const match_table_t cifs_mount_option_tokens =
> {
> { Opt_srcaddr, "srcaddr=%s" },
> { Opt_prefixpath, "prefixpath=%s" },
> { Opt_iocharset, "iocharset=%s" },
> - { Opt_sockopt, "sockopt=%s" },
> { Opt_netbiosname, "netbiosname=%s" },
> { Opt_servern, "servern=%s" },
> { Opt_ver, "ver=%s" },
> @@ -1722,19 +1721,6 @@ cifs_parse_mount_options(const char *mountdata,
> const char *devname,
> */
> cFYI(1, "iocharset set to %s", string);
> break;
> - case Opt_sockopt:
> - string = match_strdup(args);
> - if (string == NULL)
> - goto out_nomem;
> -
> - if (strnicmp(string, "TCP_NODELAY", 11) == 0) {
> - printk(KERN_WARNING "CIFS: the "
> - "sockopt=TCP_NODELAY option has
> been "
> - "deprecated and will be removed "
> - "in 3.9\n");
> - vol->sockopt_tcp_nodelay = 1;
> - }
> - break;
> case Opt_netbiosname:
> string = match_strdup(args);
> if (string == NULL)
> --
> 1.7.11.7
>
--
Thanks,
Steve
On Mon, 4 Mar 2013 16:08:30 -0600
Steve French <[email protected]> wrote:
> Resending patch to a slightly broader list for last minute check if
> anyone objects. Although setting this particular socket option
> (TCP_NODELAY) may not be as useful when corking/uncorking explicitly,
> I want to doublecheck before removing them because there has been some
> utility to the server allowing override of various sockopt options.
> Samba server has long supported at least the following set of settable
> socket options (although I don't know if the defaults are frequently
> overridden now, by setting sockopts in smb.conf as used to be common
> for the server).
>
> SO_KEEPALIVE
> SO_REUSEADDR
> SO_BROADCAST
> TCP_NODELAY
> IPTOS_LOWDELAY
> IPTOS_THROUGHPUT
> SO_SNDBUF *
> SO_RCVBUF *
> SO_SNDLOWAT *
> SO_RCVLOWAT *
>
> * takes an integer argument rather than a boolean on/off
>
> Any objections to removing the ability to set socket options
> explicitly for the cifs network file system client?
>
A couple of points...
The sockopt= option was never documented in the mount.cifs manpage and
the only value it ever accepted was TCP_NODELAY. Now that we're
explicitly corking the socket, TCP_NODELAY has no effect. I don't think
there's any value in leaving in a "placeholder" socket= option.
--
Jeff Layton <[email protected]>
On Wed, Mar 6, 2013 at 8:40 AM, Jeff Layton <[email protected]> wrote:
> On Mon, 4 Mar 2013 16:08:30 -0600
> Steve French <[email protected]> wrote:
>
>> Resending patch to a slightly broader list for last minute check if
>> anyone objects. Although setting this particular socket option
>> (TCP_NODELAY) may not be as useful when corking/uncorking explicitly,
>> I want to doublecheck before removing them because there has been some
>> utility to the server allowing override of various sockopt options.
>> Samba server has long supported at least the following set of settable
>> socket options (although I don't know if the defaults are frequently
>> overridden now, by setting sockopts in smb.conf as used to be common
>> for the server).
>>
>> SO_KEEPALIVE
>> SO_REUSEADDR
>> SO_BROADCAST
>> TCP_NODELAY
>> IPTOS_LOWDELAY
>> IPTOS_THROUGHPUT
>> SO_SNDBUF *
>> SO_RCVBUF *
>> SO_SNDLOWAT *
>> SO_RCVLOWAT *
>>
>> * takes an integer argument rather than a boolean on/off
>>
>> Any objections to removing the ability to set socket options
>> explicitly for the cifs network file system client?
>>
>
> A couple of points...
>
> The sockopt= option was never documented in the mount.cifs manpage and
> the only value it ever accepted was TCP_NODELAY. Now that we're
> explicitly corking the socket, TCP_NODELAY has no effect. I don't think
> there's any value in leaving in a "placeholder" socket= option.
>
After doing some research, I agree. I've been updating the socket
options section of the Samba smb.conf man page and the performance
section of the Samba HOWTO. The more research I do the more convinced
that there is almost no reason to set most socket options on a modern
Linux kernel or in Samba (server or client side).
One thing pointed out to me in another thread is that the smbd server
sets TCP_NODELAY by default. AIUI this was because the TCP_NODELAY
option used to almost double the speed of SMB in some cases due to the
way Microsoft's old TCP/IP stack sent ACKs.
This is all to say that I think the "sockopt" option can go altogether
since it's unlikely that any socket options will be needed in the
future and the only one that was supported is now useless.
--
Peace and Blessings,
-Scott.